April 2017 - Wine-Devel - list.winehq.org

Wine-Staging Performance Regression since 2.0rc6

by Bjoern Bidar/Thaodan

Hello guys I hope this is the right place to ask. Since after 2.0rc6 many Wow user getting worse performance. For comparsion: 2.0rc6: 65fps 2.1/2.2: about 40 2.4: no change either even the main menu lags when you look close. To test just download the game and open it even the main menu lags as said before. Me and many users don't know how to help in debugging the issue, so im asking here. Greetings Björn

April 8, 2017

Re: [PATCH 2/4] winex11: move XDestroyWindow() after free_gl_drawable() in set_win_format()

by Miklós Máté

On 05/04/17 19:52, Alexandre Julliard wrote: > Miklós Máté<mtmkls(a)gmail.com> writes: > >> On 04/04/17 16:06, Alexandre Julliard wrote: >>> Miklós Máté<mtmkls(a)gmail.com> writes: >>> >>>> On 04/04/17 12:11, Alexandre Julliard wrote: >>>>> Miklós Máté<mtmkls(a)gmail.com> writes: >>>>> >>>>>> @@ -1334,21 +1334,21 @@ static void free_gl_drawable( struct gl_drawable *gl ) >>>>>> /*********************************************************************** >>>>>> * create_gl_drawable >>>>>> */ >>>>>> -static BOOL create_gl_drawable( HWND hwnd, struct gl_drawable *gl ) >>>>>> +static BOOL create_gl_drawable( HWND hwnd, struct gl_drawable *gl, struct x11drv_win_data *data ) >>>>>> { >>>>>> gl->drawable = 0; >>>>>> if (GetAncestor( hwnd, GA_PARENT ) == GetDesktopWindow()) >>>>>> /* top-level window */ >>>>>> { >>>>>> - struct x11drv_win_data *data = get_win_data( hwnd ); >>>>>> - >>>>>> if (data) >>>>>> { >>>>>> gl->type = DC_GL_WINDOW; >>>>>> gl->window = create_client_window( data, gl->visual ); >>>>>> if (gl->window) >>>>>> + { >>>>>> gl->drawable = pglXCreateWindow( gdi_display, gl->format->fbconfig, gl->window, NULL ); >>>>>> - release_win_data( data ); >>>>>> + XSync( gdi_display, False ); >>>>> Do you really need all the extra XSync calls? We try to avoid server >>>>> round-trips as much as possible. >>>>> >>>> Yes. We must ensure that glXCreateWindow() and glXDestroyWindow() >>>> refer to a valid X window, so the command queue of data->display and >>>> gdi_display have to be in sync around those calls. >>> If the goal is that the X window is valid, I'd have expected an XSync on >>> the thread display. Why is the XSync on gdi_display needed here? What is >>> the scenario that fails? >>> >>> I'm not questioning that some XSyncs are necessary (and we already have >>> a few), but I want to make sure we don't add more than needed. >>> >> This one prevents crash in the following scenario: >> >> 1. create_gl_drawable() >> >> 2. X11DRV_ThreadDetach() calls XCloseDisplay( data->display ); >> >> 3. the Xserver processes the glXCreateWindow in the command queue of >> gdi_display -> X window is not valid >> >> This happens sometimes during the ddraw and d3d9 tests. > Shouldn't this be happening even without your other changes then? > Well, according to my notes from half a year ago the ddraw test of wine-csmt definitely crashes without this, but now I can't reproduce it. I don't know how likely the above scenario is in the real world. MM

April 8, 2017

Re: [PATCH 1/2 v4] ntdll/tests: Check debug registers of a newly created thread

by Christian Inci

Thank you very much. Fixed in v5. On 04/08/2017 04:33 AM, Dmitry Timoshkov wrote: > Hi Christian, > > Christian Inci <chris.wine(a)broke-the-inter.net> wrote: > >> + thread = CreateThread(NULL, 0, dummy_thread, NULL, CREATE_SUSPENDED, NULL); >> + ok(thread != INVALID_HANDLE_VALUE, "CreateThread failed with %d\n", GetLastError()); >> + memset(&ctx, 0, sizeof(ctx)); >> + ctx.ContextFlags = CONTEXT_DEBUG_REGISTERS; >> + status = pNtGetContextThread(thread, &ctx); >> + ok(status == STATUS_SUCCESS, "NtGetContextThread failed with %x\n", status); >> + ok(!ctx.Dr0, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr0); >> + ok(!ctx.Dr1, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr1); >> + ok(!ctx.Dr2, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr2); >> + ok(!ctx.Dr3, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr3); >> + todo_wine ok(!ctx.Dr6, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr6); >> + todo_wine ok(!ctx.Dr7, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr7); >> + CloseHandle(thread); >> +} > > The thread needs to be explicitly terminated. >

April 8, 2017

Re: [PATCH 1/2 v4] ntdll/tests: Check debug registers of a newly created thread

by Dmitry Timoshkov

Hi Christian, Christian Inci <chris.wine(a)broke-the-inter.net> wrote: > + thread = CreateThread(NULL, 0, dummy_thread, NULL, CREATE_SUSPENDED, NULL); > + ok(thread != INVALID_HANDLE_VALUE, "CreateThread failed with %d\n", GetLastError()); > + memset(&ctx, 0, sizeof(ctx)); > + ctx.ContextFlags = CONTEXT_DEBUG_REGISTERS; > + status = pNtGetContextThread(thread, &ctx); > + ok(status == STATUS_SUCCESS, "NtGetContextThread failed with %x\n", status); > + ok(!ctx.Dr0, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr0); > + ok(!ctx.Dr1, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr1); > + ok(!ctx.Dr2, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr2); > + ok(!ctx.Dr3, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr3); > + todo_wine ok(!ctx.Dr6, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr6); > + todo_wine ok(!ctx.Dr7, "expected 0, got %lx\n", (DWORD_PTR)ctx.Dr7); > + CloseHandle(thread); > +} The thread needs to be explicitly terminated. -- Dmitry.

April 8, 2017

Re: [PATCH] tools/winedump: Change duplicate flag.

by Zebediah Figura

On 04/07/2017 03:02 PM, Alexandre Julliard wrote: > Zebediah Figura <z.figura12(a)gmail.com> writes: > >> On 04/07/2017 02:29 PM, Alexandre Julliard wrote: >>> Zebediah Figura <z.figura12(a)gmail.com> writes: >>> >>>> On 04/07/2017 02:09 PM, Alexandre Julliard wrote: >>>>> Zebediah Figura <z.figura12(a)gmail.com> writes: >>>>> >>>>>> diff --git a/tools/winedump/main.c b/tools/winedump/main.c >>>>>> index a4ebb43..5c683f6 100644 >>>>>> --- a/tools/winedump/main.c >>>>>> +++ b/tools/winedump/main.c >>>>>> @@ -226,7 +226,7 @@ static const struct my_option option_table[] = { >>>>>> {"-v", SPEC, 0, do_verbose, "-v Show lots of detail while working (verbose)."}, >>>>>> {"dump", DUMP, 0, do_dump, "dump <file> Dump the contents of 'file' (dll, exe, lib...)"}, >>>>>> {"-C", DUMP, 0, do_symdmngl, "-C Turn on symbol demangling"}, >>>>>> - {"-f", DUMP, 0, do_dumphead, "-f Dump file header information"}, >>>>>> + {"-F", DUMP, 0, do_dumphead, "-F Dump file header information"}, >>>>> >>>>> Not a big deal, but is there a reason for changing this? >>>>> >>>> >>>> The -f flag is assigned to two different options, yielding this one >>>> unreachable. >>> >>> No, it works fine. The options are specific to each mode. >> >> If "dump" isn't explicitly specified, as "winedump file.exe -f", it >> fails with "Only one file can be treated at once". If this is intended, >> then perhaps "winedump file.exe" without any extra arguments should also >> fail? > > That wouldn't be an improvement. What is intended is that the "dump" > keyword is optional, since that's what we want 99% of the time. So the > dump options should work the same way with or without an explicit "dump" > keyword. If that's not the case, the option parser has to be fixed. > Okay, I'll submit a patch to fix that instead.

April 7, 2017

Re: [PATCH 1/2 v2] ntdll/tests: Check debug registers of a newly created thread

by Christian Inci

Forgot to send the response to wine-devel too. On 04/07/2017 10:01 PM, Christian Inci wrote: > Thank you very much for letting me know. > > Setting Dr7 with the "wrong" bits set will result into this error. > I'm going to change the testcase right now. > > On 04/07/2017 09:07 PM, Alexandre Julliard wrote: >> Christian Inci <chris.wine(a)broke-the-inter.net> writes: >> >>> +static void test_thread_debug_registers(void) >>> +{ >>> + HANDLE thread; >>> + CONTEXT ctx; >>> + NTSTATUS status; >>> + >>> + ctx.ContextFlags = CONTEXT_DEBUG_REGISTERS; >>> + ctx.Dr0 = 0xffffffff; >>> + ctx.Dr1 = 0xffffffff; >>> + ctx.Dr2 = 0xffffffff; >>> + ctx.Dr3 = 0xffffffff; >>> + ctx.Dr6 = 0xffffffff; >>> + ctx.Dr7 = 0xffffffff; >>> + status = pNtSetContextThread(GetCurrentThread(), &ctx); >>> + todo_wine ok(status == STATUS_SUCCESS, "NtSetContextThread failed with %x\n", status); >> >> It would be a more convincing test if setting the context succeeded. The >> failure is of course interesting too, but that could be a separate test. >>

April 7, 2017

Re: [PATCH] tools/winedump: Change duplicate flag.

by Alexandre Julliard

Zebediah Figura <z.figura12(a)gmail.com> writes: > On 04/07/2017 02:29 PM, Alexandre Julliard wrote: >> Zebediah Figura <z.figura12(a)gmail.com> writes: >> >>> On 04/07/2017 02:09 PM, Alexandre Julliard wrote: >>>> Zebediah Figura <z.figura12(a)gmail.com> writes: >>>> >>>>> diff --git a/tools/winedump/main.c b/tools/winedump/main.c >>>>> index a4ebb43..5c683f6 100644 >>>>> --- a/tools/winedump/main.c >>>>> +++ b/tools/winedump/main.c >>>>> @@ -226,7 +226,7 @@ static const struct my_option option_table[] = { >>>>> {"-v", SPEC, 0, do_verbose, "-v Show lots of detail while working (verbose)."}, >>>>> {"dump", DUMP, 0, do_dump, "dump <file> Dump the contents of 'file' (dll, exe, lib...)"}, >>>>> {"-C", DUMP, 0, do_symdmngl, "-C Turn on symbol demangling"}, >>>>> - {"-f", DUMP, 0, do_dumphead, "-f Dump file header information"}, >>>>> + {"-F", DUMP, 0, do_dumphead, "-F Dump file header information"}, >>>> >>>> Not a big deal, but is there a reason for changing this? >>>> >>> >>> The -f flag is assigned to two different options, yielding this one >>> unreachable. >> >> No, it works fine. The options are specific to each mode. > > If "dump" isn't explicitly specified, as "winedump file.exe -f", it > fails with "Only one file can be treated at once". If this is intended, > then perhaps "winedump file.exe" without any extra arguments should also > fail? That wouldn't be an improvement. What is intended is that the "dump" keyword is optional, since that's what we want 99% of the time. So the dump options should work the same way with or without an explicit "dump" keyword. If that's not the case, the option parser has to be fixed. -- Alexandre Julliard julliard(a)winehq.org

April 7, 2017

Re: [PATCH v4 4/4] user32/tests: Fix an intermittent failure. (resend)

by Marvin

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=30041 Your paranoid android. === w7u (32 bit msg) === msg.c:12062: Test failed: WmMouseHoverSeq: 0: the msg sequence is not complete: expected 0118 - actual 0000 msg.c:12064: Test failed: wrong tme.dwFlags 00000001, expected 00000000 msg.c:12064: Test failed: wrong tme.hwndTrack 0018017E, expected 00000000 msg.c:12064: Test failed: wrong tme.dwHoverTime 400, expected 0 === w7pro64 (32 bit msg) === msg.c:12609: Test failed: 54: ShowWindow(SW_MINIMIZE): 4: in msg 0x0047 expecting wParam 0x8170 got 0x1803 msg.c:12609: Test failed: 54: ShowWindow(SW_MINIMIZE): 5: the msg 0x0003 was expected, but got msg 0x0047 instead msg.c:12609: Test failed: 54: ShowWindow(SW_MINIMIZE): 6: the msg 0x0005 was expected, but got msg 0x0003 instead msg.c:12609: Test failed: 54: ShowWindow(SW_MINIMIZE): 7: the msg sequence is not complete: expected 0000 - actual 0005

April 7, 2017

Re: [PATCH] tools/winedump: Change duplicate flag.

by Zebediah Figura

On 04/07/2017 02:29 PM, Alexandre Julliard wrote: > Zebediah Figura <z.figura12(a)gmail.com> writes: > >> On 04/07/2017 02:09 PM, Alexandre Julliard wrote: >>> Zebediah Figura <z.figura12(a)gmail.com> writes: >>> >>>> diff --git a/tools/winedump/main.c b/tools/winedump/main.c >>>> index a4ebb43..5c683f6 100644 >>>> --- a/tools/winedump/main.c >>>> +++ b/tools/winedump/main.c >>>> @@ -226,7 +226,7 @@ static const struct my_option option_table[] = { >>>> {"-v", SPEC, 0, do_verbose, "-v Show lots of detail while working (verbose)."}, >>>> {"dump", DUMP, 0, do_dump, "dump <file> Dump the contents of 'file' (dll, exe, lib...)"}, >>>> {"-C", DUMP, 0, do_symdmngl, "-C Turn on symbol demangling"}, >>>> - {"-f", DUMP, 0, do_dumphead, "-f Dump file header information"}, >>>> + {"-F", DUMP, 0, do_dumphead, "-F Dump file header information"}, >>> >>> Not a big deal, but is there a reason for changing this? >>> >> >> The -f flag is assigned to two different options, yielding this one >> unreachable. > > No, it works fine. The options are specific to each mode. > If "dump" isn't explicitly specified, as "winedump file.exe -f", it fails with "Only one file can be treated at once". If this is intended, then perhaps "winedump file.exe" without any extra arguments should also fail?

April 7, 2017

Re: [PATCH] tools/winedump: Change duplicate flag.

by Alexandre Julliard

Zebediah Figura <z.figura12(a)gmail.com> writes: > On 04/07/2017 02:09 PM, Alexandre Julliard wrote: >> Zebediah Figura <z.figura12(a)gmail.com> writes: >> >>> diff --git a/tools/winedump/main.c b/tools/winedump/main.c >>> index a4ebb43..5c683f6 100644 >>> --- a/tools/winedump/main.c >>> +++ b/tools/winedump/main.c >>> @@ -226,7 +226,7 @@ static const struct my_option option_table[] = { >>> {"-v", SPEC, 0, do_verbose, "-v Show lots of detail while working (verbose)."}, >>> {"dump", DUMP, 0, do_dump, "dump <file> Dump the contents of 'file' (dll, exe, lib...)"}, >>> {"-C", DUMP, 0, do_symdmngl, "-C Turn on symbol demangling"}, >>> - {"-f", DUMP, 0, do_dumphead, "-f Dump file header information"}, >>> + {"-F", DUMP, 0, do_dumphead, "-F Dump file header information"}, >> >> Not a big deal, but is there a reason for changing this? >> > > The -f flag is assigned to two different options, yielding this one > unreachable. No, it works fine. The options are specific to each mode. -- Alexandre Julliard julliard(a)winehq.org

April 7, 2017