July 2016 - Wine-Devel - list.winehq.org

Re: [PATCH 2/6] d3d9/tests: Add visual tests for locking vertexbuffers

by Patrick Rudolph

Am Thu, 28 Jul 2016 14:04:56 +0200 schrieb Henri Verbeet <hverbeet(a)gmail.com>: > On 26 July 2016 at 19:09, Patrick Rudolph <siro(a)das-labor.org> wrote: > > + static const struct > > + { > > + const char *flags_s; > > + unsigned int flags; > > + DWORD usage; > > + BOOL unsyncronized; > > + BOOL todo; > > + BOOL broken; > > + } > > + /* Nvidia ignores D3DLOCK_NOOVERWRITE on non dynamic buffers, > > + * while AMD always accepts D3DLOCK_NOOVERWRITE on all > > buffers. */ > > + tests[] = > > + { > > + {"0", 0, 0, FALSE, FALSE, FALSE}, > > + {"0", 0, D3DUSAGE_DYNAMIC, FALSE, FALSE, FALSE}, > > + {"D3DLOCK_NOOVERWRITE", D3DLOCK_NOOVERWRITE, 0, FALSE, > > FALSE, TRUE}, > > + {"D3DLOCK_NOOVERWRITE", D3DLOCK_NOOVERWRITE, > > D3DUSAGE_DYNAMIC, TRUE, FALSE, FALSE}, > > + {"D3DLOCK_DISCARD", D3DLOCK_DISCARD, 0, FALSE, FALSE, > > FALSE}, > > + {"D3DLOCK_DISCARD", D3DLOCK_DISCARD, D3DUSAGE_DYNAMIC, > > FALSE, FALSE, FALSE}, > > + {"D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD", > > + D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD, 0, FALSE, > > FALSE, TRUE}, > > + {"D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD", > > + D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD, > > D3DUSAGE_DYNAMIC, TRUE, TRUE, TRUE}, > > + {"D3DLOCK_NOOVERWRITE | D3DLOCK_READONLY", > > + D3DLOCK_NOOVERWRITE | D3DLOCK_READONLY, 0, FALSE, > > FALSE, TRUE}, > > + {"D3DLOCK_NOOVERWRITE | D3DLOCK_READONLY", > > + D3DLOCK_NOOVERWRITE | D3DLOCK_READONLY, > > D3DUSAGE_DYNAMIC, TRUE, TRUE, TRUE}, > > + }; > [...] > > + todo_wine_if (tests[i].todo) > > + { > > + ok(tests[i].unsyncronized == unsyncronized || > > broken(tests[i].broken), > > + "Expected buffer mapped %s. Flags = %s. Usage > > = %#x.\n", > > + tests[i].unsyncronized ? "unsyncronized" : > > "syncronized", tests[i].flags_s, tests[i].usage); > > + } > I assume the broken() is for the testbot, but the way this is written, > half the cases aren't really tested on Windows. Also, what makes these > fail on the testbot? > I can replace some of the broken test results with another flag for AMD behaviour. AMD allows D3DLOCK_NOOVERWRITE for non dynamic buffers on d3d9, too. I guess the WARP driver filters D3DLOCK_NOOVERWRITE | D3DLOCK_DISCARD calls. > > + timestamp1 = GetTickCount(); > > + > > + hr = IDirect3DDevice9_BeginScene(device); > > + ok(SUCCEEDED(hr), "Failed to begin scene, hr %#x.\n", hr); > > + hr = IDirect3DDevice9_DrawPrimitive(device, > > D3DPT_TRIANGLESTRIP, 0, tri); > > + ok(SUCCEEDED(hr), "Failed to draw, hr %#x.\n", hr); > > + hr = IDirect3DDevice9_EndScene(device); > > + ok(SUCCEEDED(hr), "Failed to end scene, hr %#x.\n", hr); > > + > > + /* dummy read to wait for GPU being done */ > > + color = getPixelColor(device, 160, 360); > > + > > + timestamp2 = GetTickCount(); > > + > > + /* Adjust buffer size for 1 second draw call duration */ > > + size = (size * 1000UL) / (timestamp2 - timestamp1 + 1); > Does this work reliably? GetTickCount() has very limited resolution, I > think there's a real chance that the draw will take only a fraction of > GetTickCount()'s resolution. Yes it does. I first thought about using timestamp queries, but those aren't supported on ddraw and d3d8 and are broken on my AMD test system, so I'm using GetTickCount instead. It allows to test to pass on fast systems (Intel Core i5 + HD6870) and slow systems (Intel Atom + Nvidia ION). The buffersize can't be hardcoded as the Windows driver nukes the display driver whenever a draw call takes more than a few seconds, which is the case when using huge buffers to make them pass on fast systems. Regards, Patrick

July 28, 2016

Re: wine-devel Digest, Vol 132, Issue 55

by Anton Baskanov

On Wed, Jul 27, 2016 at 11:07 PM, <wine-devel-request(a)winehq.org> wrote: > ---------- Forwarded message ---------- > From: Nikolay Sivov <bunglehead(a)gmail.com> > To: wine-devel(a)winehq.org > Cc: > Date: Wed, 27 Jul 2016 17:39:26 +0300 > Subject: Re: [PATCH 1/6] amstream: Fix AMAudioData::QueryInterface. > On 27.07.2016 15:51, Anton Baskanov wrote: > > Signed-off-by: Anton Baskanov <baskanov(a)gmail.com> > > --- > > dlls/amstream/audiodata.c | 1 - > > dlls/amstream/tests/Makefile.in | 3 +- > > dlls/amstream/tests/audiodata.c | 67 > +++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 69 insertions(+), 2 deletions(-) > > create mode 100755 dlls/amstream/tests/audiodata.c > > Hi, Anton. > > Please keep tests in existing amstream.c file. > > + > > +static void test_query_interface(void) > > +{ > > + IUnknown *unknown = create_audio_data(); > > + IMemoryData *memory_data = NULL; > > + IAudioData *audio_data = NULL; > > + > > + HRESULT result = S_OK; > > result initialization is not needed; > > > + > > + result = IUnknown_QueryInterface(unknown, &IID_IMemoryData, (void > **)&memory_data); > > + ok(E_NOINTERFACE == result, "got 0x%08x\n", result); > > + if (S_OK == result) > > + { > > + IMemoryData_Release(memory_data); > > + } > > If it always fails, there's no reason add conditional Release. > > > + > > + result = IUnknown_QueryInterface(unknown, &IID_IAudioData, (void > **)&audio_data); > > + ok(S_OK == result, "got 0x%08x\n", result); > > + if (S_OK == result) > > + { > > + IAudioData_Release(audio_data); > > + } > > + > > + IUnknown_Release(unknown); > > +} > > Could you check if you can query IMemoryData from IAudioData? It seems > really weird if parent interface is not supported at all. > Hi, Nikolay. Thanks for the review! I've checked QueryInterface from IAudioData, and it also fails with IID_IMemoryData. I've fixed the issues and sent the updated patches.

July 28, 2016

Re: [PATCH 2/2] DInput : Use vendor and product ids to generate product GUID like it's done on Windows

by Bruno Jesus

On Thu, Jul 28, 2016 at 6:08 AM, DavidL <david.dljunk(a)gmail.com> wrote: > I'm not really familiar with the intricacies of the guid generation ... but > looking at the Mac dinput code ... should a similar patch be applied there? Hi, IMO yes in order to sync the output of all drivers to be the same independent of host OS. You can test using the ditool.exe from: https://sourceforge.net/projects/x360ce/files/ditool.zip Best wishes, Bruno

July 28, 2016

Re: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests

by Dmitry Timoshkov

Piotr Caban <piotr(a)codeweavers.com> wrote: > >>>>>>>>>> + sprintf(cmd, "\"\"%c\"\"\"%s\" \t \"misc\" cmd", name[0], name+1); > >>>>>>>>>> + memset(&startup, 0, sizeof(startup)); > >>>>>>>>>> + startup.cb = sizeof(startup); > >>>>>>>>>> + CreateProcessA(path, cmd, NULL, NULL, TRUE, > >>>>>>>>>> + CREATE_DEFAULT_ERROR_MODE|NORMAL_PRIORITY_CLASS, > >>>>>>>>>> + NULL, NULL, &startup, &proc); > >>>>>>>>>> + winetest_wait_child_process(proc.hProcess); > >>>>>>>>> > >>>>>>>>> Thanks for adding the tests. It shouldn't be too hard to also test other > >>>>>>>>> white space characters, at least testing '\r' and '\n' would be nice to > >>>>>>>>> have. > >>>>>>>>> > >>>>>>>> CreateProcess will fail in case of '\r' or '\n' characters. > >>>>>>> > >>>>>>> In a simple test CreateProcess() works just fine here under Windows 7 when a > >>>>>>> passed in command line contains '\r' and/or '\n', where does it fail for you? > >>>>>>> > >>>>>> I'll try to be a little more precise - '\r' is treated as another > >>>>>> argument so it can't be used before first argument (misc in this case). > >>>>> > >>>>> Then it clearly deserves an additional test case. And a test for '\n' since > >>>>> it doesn't break anything. > >>>> '\n' behaves the same as '\r' according to my test. > >>>> > >>>> I think I have already explained you while '\r' case can't be tested in > >>>> reasonable way. > >>> > >>> I don't see why '\r' and '\n' can't be tested. If CreateProcess() call doesn't > >>> fail, what you need to test is the resulting command line that the child process > >>> receives. It's clear and simple. > >>> > >> I've already wrote you that '\r' is treated as argument - because of > >> that it can't be added before "misc" argument (the test executable needs > >> first argument to be "misc"). The function > >> _get_narrow_winmain_command_line only interprets part of the command > >> line before first argument. > > > > How about adding '\r' after "misc" or at whatever place you think appropriate > > and simply test both the resulting command line and the result of narrow one? > > Just execute the process and test whatever it gets as the command line and > > the narrow one? What is so hard with that? > > > Such test will be useless. It's already shown that > _get_narrow_winmain_command_line returns substring of GetCommandLineA. > It also doesn't touch anything past the beginning of first argument. There is no such a thing as a useless test IMHO, any test exercises the API and shows its result preventing possible regressions in future. -- Dmitry.

July 28, 2016

Re: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests

by Piotr Caban

On 07/28/16 17:25, Dmitry Timoshkov wrote: > Piotr Caban <piotr(a)codeweavers.com> wrote: > >>>>>>>>>> + sprintf(cmd, "\"\"%c\"\"\"%s\" \t \"misc\" cmd", name[0], name+1); >>>>>>>>>> + memset(&startup, 0, sizeof(startup)); >>>>>>>>>> + startup.cb = sizeof(startup); >>>>>>>>>> + CreateProcessA(path, cmd, NULL, NULL, TRUE, >>>>>>>>>> + CREATE_DEFAULT_ERROR_MODE|NORMAL_PRIORITY_CLASS, >>>>>>>>>> + NULL, NULL, &startup, &proc); >>>>>>>>>> + winetest_wait_child_process(proc.hProcess); >>>>>>>>> >>>>>>>>> Thanks for adding the tests. It shouldn't be too hard to also test other >>>>>>>>> white space characters, at least testing '\r' and '\n' would be nice to >>>>>>>>> have. >>>>>>>>> >>>>>>>> CreateProcess will fail in case of '\r' or '\n' characters. >>>>>>> >>>>>>> In a simple test CreateProcess() works just fine here under Windows 7 when a >>>>>>> passed in command line contains '\r' and/or '\n', where does it fail for you? >>>>>>> >>>>>> I'll try to be a little more precise - '\r' is treated as another >>>>>> argument so it can't be used before first argument (misc in this case). >>>>> >>>>> Then it clearly deserves an additional test case. And a test for '\n' since >>>>> it doesn't break anything. >>>> '\n' behaves the same as '\r' according to my test. >>>> >>>> I think I have already explained you while '\r' case can't be tested in >>>> reasonable way. >>> >>> I don't see why '\r' and '\n' can't be tested. If CreateProcess() call doesn't >>> fail, what you need to test is the resulting command line that the child process >>> receives. It's clear and simple. >>> >> I've already wrote you that '\r' is treated as argument - because of >> that it can't be added before "misc" argument (the test executable needs >> first argument to be "misc"). The function >> _get_narrow_winmain_command_line only interprets part of the command >> line before first argument. > > How about adding '\r' after "misc" or at whatever place you think appropriate > and simply test both the resulting command line and the result of narrow one? > Just execute the process and test whatever it gets as the command line and > the narrow one? What is so hard with that? > Such test will be useless. It's already shown that _get_narrow_winmain_command_line returns substring of GetCommandLineA. It also doesn't touch anything past the beginning of first argument.

July 28, 2016

Re: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests

by Dmitry Timoshkov

Piotr Caban <piotr(a)codeweavers.com> wrote: > >>>>>>>> + sprintf(cmd, "\"\"%c\"\"\"%s\" \t \"misc\" cmd", name[0], name+1); > >>>>>>>> + memset(&startup, 0, sizeof(startup)); > >>>>>>>> + startup.cb = sizeof(startup); > >>>>>>>> + CreateProcessA(path, cmd, NULL, NULL, TRUE, > >>>>>>>> + CREATE_DEFAULT_ERROR_MODE|NORMAL_PRIORITY_CLASS, > >>>>>>>> + NULL, NULL, &startup, &proc); > >>>>>>>> + winetest_wait_child_process(proc.hProcess); > >>>>>>> > >>>>>>> Thanks for adding the tests. It shouldn't be too hard to also test other > >>>>>>> white space characters, at least testing '\r' and '\n' would be nice to > >>>>>>> have. > >>>>>>> > >>>>>> CreateProcess will fail in case of '\r' or '\n' characters. > >>>>> > >>>>> In a simple test CreateProcess() works just fine here under Windows 7 when a > >>>>> passed in command line contains '\r' and/or '\n', where does it fail for you? > >>>>> > >>>> I'll try to be a little more precise - '\r' is treated as another > >>>> argument so it can't be used before first argument (misc in this case). > >>> > >>> Then it clearly deserves an additional test case. And a test for '\n' since > >>> it doesn't break anything. > >> '\n' behaves the same as '\r' according to my test. > >> > >> I think I have already explained you while '\r' case can't be tested in > >> reasonable way. > > > > I don't see why '\r' and '\n' can't be tested. If CreateProcess() call doesn't > > fail, what you need to test is the resulting command line that the child process > > receives. It's clear and simple. > > > I've already wrote you that '\r' is treated as argument - because of > that it can't be added before "misc" argument (the test executable needs > first argument to be "misc"). The function > _get_narrow_winmain_command_line only interprets part of the command > line before first argument. How about adding '\r' after "misc" or at whatever place you think appropriate and simply test both the resulting command line and the result of narrow one? Just execute the process and test whatever it gets as the command line and the narrow one? What is so hard with that? -- Dmitry.

July 28, 2016

Re: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests

by Piotr Caban

On 07/28/16 15:43, Dmitry Timoshkov wrote: > Piotr Caban <piotr(a)codeweavers.com> wrote: > >>>>>>>> + sprintf(cmd, "\"\"%c\"\"\"%s\" \t \"misc\" cmd", name[0], name+1); >>>>>>>> + memset(&startup, 0, sizeof(startup)); >>>>>>>> + startup.cb = sizeof(startup); >>>>>>>> + CreateProcessA(path, cmd, NULL, NULL, TRUE, >>>>>>>> + CREATE_DEFAULT_ERROR_MODE|NORMAL_PRIORITY_CLASS, >>>>>>>> + NULL, NULL, &startup, &proc); >>>>>>>> + winetest_wait_child_process(proc.hProcess); >>>>>>> >>>>>>> Thanks for adding the tests. It shouldn't be too hard to also test other >>>>>>> white space characters, at least testing '\r' and '\n' would be nice to >>>>>>> have. >>>>>>> >>>>>> CreateProcess will fail in case of '\r' or '\n' characters. >>>>> >>>>> In a simple test CreateProcess() works just fine here under Windows 7 when a >>>>> passed in command line contains '\r' and/or '\n', where does it fail for you? >>>>> >>>> I'll try to be a little more precise - '\r' is treated as another >>>> argument so it can't be used before first argument (misc in this case). >>> >>> Then it clearly deserves an additional test case. And a test for '\n' since >>> it doesn't break anything. >> '\n' behaves the same as '\r' according to my test. >> >> I think I have already explained you while '\r' case can't be tested in >> reasonable way. > > I don't see why '\r' and '\n' can't be tested. If CreateProcess() call doesn't > fail, what you need to test is the resulting command line that the child process > receives. It's clear and simple. > I've already wrote you that '\r' is treated as argument - because of that it can't be added before "misc" argument (the test executable needs first argument to be "misc"). The function _get_narrow_winmain_command_line only interprets part of the command line before first argument.

July 28, 2016

Re: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests

by Dmitry Timoshkov

Piotr Caban <piotr(a)codeweavers.com> wrote: > >>>>>> + sprintf(cmd, "\"\"%c\"\"\"%s\" \t \"misc\" cmd", name[0], name+1); > >>>>>> + memset(&startup, 0, sizeof(startup)); > >>>>>> + startup.cb = sizeof(startup); > >>>>>> + CreateProcessA(path, cmd, NULL, NULL, TRUE, > >>>>>> + CREATE_DEFAULT_ERROR_MODE|NORMAL_PRIORITY_CLASS, > >>>>>> + NULL, NULL, &startup, &proc); > >>>>>> + winetest_wait_child_process(proc.hProcess); > >>>>> > >>>>> Thanks for adding the tests. It shouldn't be too hard to also test other > >>>>> white space characters, at least testing '\r' and '\n' would be nice to > >>>>> have. > >>>>> > >>>> CreateProcess will fail in case of '\r' or '\n' characters. > >>> > >>> In a simple test CreateProcess() works just fine here under Windows 7 when a > >>> passed in command line contains '\r' and/or '\n', where does it fail for you? > >>> > >> I'll try to be a little more precise - '\r' is treated as another > >> argument so it can't be used before first argument (misc in this case). > > > > Then it clearly deserves an additional test case. And a test for '\n' since > > it doesn't break anything. > '\n' behaves the same as '\r' according to my test. > > I think I have already explained you while '\r' case can't be tested in > reasonable way. I don't see why '\r' and '\n' can't be tested. If CreateProcess() call doesn't fail, what you need to test is the resulting command line that the child process receives. It's clear and simple. -- Dmitry.

July 28, 2016

Re: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests

by Piotr Caban

On 07/28/16 15:27, Dmitry Timoshkov wrote: > Piotr Caban <piotr(a)codeweavers.com> wrote: > >>>>>> + sprintf(cmd, "\"\"%c\"\"\"%s\" \t \"misc\" cmd", name[0], name+1); >>>>>> + memset(&startup, 0, sizeof(startup)); >>>>>> + startup.cb = sizeof(startup); >>>>>> + CreateProcessA(path, cmd, NULL, NULL, TRUE, >>>>>> + CREATE_DEFAULT_ERROR_MODE|NORMAL_PRIORITY_CLASS, >>>>>> + NULL, NULL, &startup, &proc); >>>>>> + winetest_wait_child_process(proc.hProcess); >>>>> >>>>> Thanks for adding the tests. It shouldn't be too hard to also test other >>>>> white space characters, at least testing '\r' and '\n' would be nice to >>>>> have. >>>>> >>>> CreateProcess will fail in case of '\r' or '\n' characters. >>> >>> In a simple test CreateProcess() works just fine here under Windows 7 when a >>> passed in command line contains '\r' and/or '\n', where does it fail for you? >>> >> I'll try to be a little more precise - '\r' is treated as another >> argument so it can't be used before first argument (misc in this case). > > Then it clearly deserves an additional test case. And a test for '\n' since > it doesn't break anything. '\n' behaves the same as '\r' according to my test. I think I have already explained you while '\r' case can't be tested in reasonable way.

July 28, 2016

Re: [PATCH 2/2 v4] ucrtbase: Add _get_narrow_winmain_command_line tests

by Dmitry Timoshkov

Piotr Caban <piotr(a)codeweavers.com> wrote: > >>>> + sprintf(cmd, "\"\"%c\"\"\"%s\" \t \"misc\" cmd", name[0], name+1); > >>>> + memset(&startup, 0, sizeof(startup)); > >>>> + startup.cb = sizeof(startup); > >>>> + CreateProcessA(path, cmd, NULL, NULL, TRUE, > >>>> + CREATE_DEFAULT_ERROR_MODE|NORMAL_PRIORITY_CLASS, > >>>> + NULL, NULL, &startup, &proc); > >>>> + winetest_wait_child_process(proc.hProcess); > >>> > >>> Thanks for adding the tests. It shouldn't be too hard to also test other > >>> white space characters, at least testing '\r' and '\n' would be nice to > >>> have. > >>> > >> CreateProcess will fail in case of '\r' or '\n' characters. > > > > In a simple test CreateProcess() works just fine here under Windows 7 when a > > passed in command line contains '\r' and/or '\n', where does it fail for you? > > > I'll try to be a little more precise - '\r' is treated as another > argument so it can't be used before first argument (misc in this case). Then it clearly deserves an additional test case. And a test for '\n' since it doesn't break anything. -- Dmitry.

July 28, 2016