March 2016 - Wine-devel - list.winehq.org

GSoC Idea: Tools - Winetest Scripting Interface
by Zhenbo Li 15 Mar '16

15 Mar '16

Hello, I'm Zhenbo Li. I participated in GSoC twice. This year, I want to work on "Tools - Winetest Scripting Interface". After "make test", checking the output in stdout manually is horrible. As ok() macro is used quite widely, my patches should cause no surprise. That is to say, other developers need to read neither my patches nor the changelog to continue their jobs, although ok() macro and other tools for test have been changed. I think there will be two steps for the goal: 1. ok() macro ===================== int winetest_vok( int condition, const char *msg, __winetest_va_list args ) { tls_data* data=get_tls_data(); if (data->todo_level){...} else { if (!condition) { printf( "%s:%d: Test failed: ", data->current_file, data->current_line ); vprintf(msg, args); InterlockedIncrement(&failures); return 0; } else{.....} } } Instead of printing all the data to stderr, I think we can write a wrapper static void test_print(const char *s) { if(want_print_to_file) fprintf(file_handle, s); printf(s); } It needs some work to make sure that the file_handle is opened/closed successfully. With that, we can record the failed tests to a file, and developers don't need to find them in stdout anymore. 2. compare the results ======================= On the idea page, it says that any scripting language is acceptable. I prefer Python3. To make the code simple, I think editing runtest script is the best solution. Currently, our logic in Makefile is htmllocation.ok: ../../../tools/runtest $(RUNTESTFLAGS) -T ../../.. -M mshtml.dll -p mshtml_test.exe.so htmllocation && touch $@ And in runtest, we have exec $WINETEST_WRAPPER "$topobjdir/wine" "$program" "$@" I don't want to touch Makefile. Instead, I'm going to add some options for runtest. For example, one can use ../../../tools/runtest $(RUNTESTFLAGS) -new_option expected_errors_generated before -T ../../.. -M mshtml.dll -p mshtml_test.exe.so htmllocation And in runtest script, we can expected_errors_generated and new_errors_got, then tell that whether the test result has been changed. It should be easy for developers to write a script for git bisect or other circumstance. Problems ========= This approach seems to be quite simple, and there are some conditions can't be handled. For example void test_foo(sturct Data *d){ ret = foo(d); ok(ret, "foo(%s) is %d\n", data_to_str(d), ret); } struct Data list[]={a,b,c,d,e}; void test(){ for(i in list) test_foo(i); } I think we should not do too much "smart deduce" here, and don't hide what's happening to other developers. Maybe we can add another option for runtest script: --scrict=false This is my humble idea for Wine's GSoC 2016. If this project goes well, working on wine test suit will be much more satisfying(although still boring). I appreciate for your comment. With you a happy weekend. -- Have a nice day! Zhenbo Li

6 15

Re: winemac: Call the EnumDisplayMonitors callback using a wrapper.
by Henri Verbeet 15 Mar '16

15 Mar '16

On 15 March 2016 at 05:50, Ken Thomases <ken(a)codeweavers.com> wrote: > Signed-off-by: Ken Thomases <ken(a)codeweavers.com> > --- > For <https://bugs.winehq.org/show_bug.cgi?id=40297>. > > dlls/winemac.drv/display.c | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) > Is this really specific to winemac, or would it be more appropriate to add the wrapper in user32?

1 0

Re: winemac: Call the EnumDisplayMonitors callback using a wrapper.
by Ken Thomases 15 Mar '16

15 Mar '16

On Mar 14, 2016, at 11:50 PM, Ken Thomases <ken(a)codeweavers.com> wrote: > > Signed-off-by: Ken Thomases <ken(a)codeweavers.com> > --- > For <https://bugs.winehq.org/show_bug.cgi?id=40297>. > > dlls/winemac.drv/display.c | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-) Dmitry pointed out that this version misaligns the stack, so ignore it. I'll submit another try tomorrow. -Ken

1 0

Re: [PATCH] crypt32/tests: Accept Windows 10 results.
by Alexandre Julliard 15 Mar '16

15 Mar '16

Detlef Riekenberg <wine.dev(a)web.de> writes: > @@ -336,11 +336,13 @@ static void testStringToBinaryA(void) > /* Bad strings */ > for (i = 0; i < sizeof(badStrings) / sizeof(badStrings[0]); i++) > { > - bufLen = 0; > + bufLen = 0xdeadbeef; > + SetLastError(0xdeadbeef); > + /* This succeeds on Windows 10 with our CRYPT_STRING_BASE64 test data */ > ret = pCryptStringToBinaryA(badStrings[i].str, 0, badStrings[i].format, > NULL, &bufLen, NULL, NULL); > - ok(!ret && GetLastError() == ERROR_INVALID_DATA, > - "Expected ERROR_INVALID_DATA, got ret=%d le=%u\n", ret, GetLastError()); > + ok((!ret && GetLastError() == ERROR_INVALID_DATA) || broken(ret), > + "%d: Expected FALSE with ERROR_INVALID_DATA, got ret=%d le=%u len=%d\n", i, ret, GetLastError(), bufLen); I'm not convinced this should be considered broken. It would be worth testing some other truly bad strings, and the CRYPT_STRING_STRICT flag also. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: msrle32: Use the correct number of colors when biClrUsed = 0 (try 2)
by Alexandre Julliard 15 Mar '16

15 Mar '16

Bruno Jesus <00cpxxx(a)gmail.com> writes: > @@ -1647,27 +1655,33 @@ static LRESULT DecompressBegin(CodecInfo *pi, LPCBITMAPINFOHEADER lpbiIn, > > if (lpbiIn->biCompression != BI_RGB) > { > + int colors; > + > + if (lpbiIn->biClrUsed == 0) > + colors = 1 << lpbiIn->biBitCount; > + else > + colors = lpbiIn->biClrUsed; I don't think you want to do that for biBitCount > 8. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH] winex11: Don't create a temporary context each time X11DRV_wglBindTexImageARB is called.
by Matteo Bruni 15 Mar '16

15 Mar '16

2016-03-12 14:57 GMT+01:00 Miklós Máté <mtmkls(a)gmail.com>: > Theoretically wglBindTexImageARB is supposed to be a fast > render-to-texture method, and creating a new context is anything but fast. > > This fixes horrible performance in Star Wars > Knights of the Old Republic when post-process effects are enabled. > > v2: fix context caching, fix destructor, fix comments > v3: better handling of context delete > > Signed-off-by: Miklós Máté <mtmkls(a)gmail.com> Thank you for following up after the review comments. Unfortunately I think there is one more (and hopefully last) bit which still needs addressing: > @@ -2301,6 +2314,7 @@ static struct wgl_pbuffer *X11DRV_wglCreatePbufferARB( HDC hdc, int iPixelFormat > SetLastError(ERROR_NO_SYSTEM_RESOURCES); > goto create_failed; /* unexpected error */ > } > + list_add_head( &pbuffer_list, &object->entry ); > TRACE("->(%p)\n", object); > return object; This needs to be done inside the critical section, so please put EnterCriticalSection() + LeaveCriticalSection() around it. FTR the wglCreateContext and wglCreateContextAttribsARB implementation functions indirectly acquire and release the critical section via get_gl_drawable() / release_gl_drawable(). It looks good to me otherwise.

1 0

Re: [v2 6/6] d3dx9: imlement setting named shader constants in effect.
by Matteo Bruni 15 Mar '16

15 Mar '16

2016-03-10 14:51 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > + unsigned i, j; We usually avoid the bare "unsigned". > + for (i = 0; i < parameters_count; i++) > + if (params[i] && params[i]->class == D3DXPC_OBJECT && params[i]->type == D3DXPT_SAMPLER) > + { > + struct d3dx_sampler *sampler; > + > + sampler = (struct d3dx_sampler *)params[i]->data; > + TRACE("sampler %s, register index %u, state count %u.\n", params[i]->name, > + cdesc[i].RegisterIndex, sampler->state_count); > + for (j = 0; j < sampler->state_count; j++) > + { > + hr = d3dx9_apply_state(effect, pass, &sampler->states[j], > + cdesc[i].RegisterIndex + (vs ? D3DVERTEXTEXTURESAMPLER0 : 0)); > + if (FAILED(hr)) > + ret = hr; > + } > + } It's better to add brackets to the for, for greater clarity.

1 0

Re: [2/2] user32: Skip menu loop for empty popup menus.
by Marvin 14 Mar '16

14 Mar '16

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=21431 Your paranoid android. === w864 (32 bit msg) === msg.c:6417: Test failed: Update region shouldn't be empty msg.c:6651: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6653: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different === w1064 (32 bit msg) === msg.c:6417: Test failed: Update region shouldn't be empty msg.c:6651: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6653: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different === w1064 (64 bit msg) === msg.c:6417: Test failed: Update region shouldn't be empty msg.c:6651: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6653: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different msg.c:6423: Test failed: Regions are different

1 0

Re: [PATCH 2/2] d3d11/tests: Port swapchain_flip to d3d11.
by Henri Verbeet 14 Mar '16

14 Mar '16

On 13 March 2016 at 19:08, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > --- > > I am not using create_test_context in this test because it would introduce > asymetry in how back buffer 0 is created that would make the test more > difficult to read in my opinion. It would probably be worth it, mostly because you also get to use draw_quad() that way. We could also consider making the "backbuffer" and "backbuffer_rtv" members arrays. For the moment it's probably more important that it matches the d3d10core test though. > > Signed-off-by: Stefan Dösinger <stefandoesinger(a)gmx.at> If you put the Signed-off-by here, it won't be put in the commit message.

2 2

Re: [PATCH 1/2] windowscodecs: Stub IWICMetadataQueryReader
by Vincent Povirk 14 Mar '16

14 Mar '16

You're referencing the block reader in MetadataQueryReader_CreateInstance but not releasing it in mqr_Release.

1 0