[PATCH 0/1] MR7094: winex11.drv: Flush before blitting offscreen drawable in wglFlush() and wglFinish().
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winex11.drv/opengl.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c index 594ffaea2dd..f6e8def0ade 100644 --- a/dlls/winex11.drv/opengl.c +++ b/dlls/winex11.drv/opengl.c @@ -1877,7 +1877,7 @@ static BOOL glxdrv_wglShareLists(struct wgl_context *org, struct wgl_context *de return TRUE; } -static void present_gl_drawable( HWND hwnd, HDC hdc, struct gl_drawable *gl, BOOL flush ) +static void present_gl_drawable( HWND hwnd, HDC hdc, struct gl_drawable *gl, BOOL flush, BOOL gl_finish ) { HWND toplevel = NtUserGetAncestor( hwnd, GA_ROOT ); struct x11drv_win_data *data; @@ -1896,6 +1896,7 @@ static void present_gl_drawable( HWND hwnd, HDC hdc, struct gl_drawable *gl, BOO window = get_dc_drawable( hdc, &rect ); region = get_dc_monitor_region( hwnd, hdc ); + if (gl_finish) pglFinish(); if (flush) XFlush( gdi_display ); NtUserGetClientRect( hwnd, &rect_dst, NtUserGetWinMonitorDpi( hwnd, MDT_RAW_DPI ) ); @@ -1929,7 +1930,7 @@ static void wglFinish(void) { sync_context(ctx); pglFinish(); - present_gl_drawable( hwnd, ctx->hdc, gl, FALSE ); + present_gl_drawable( hwnd, ctx->hdc, gl, TRUE, FALSE ); release_gl_drawable( gl ); } } @@ -1945,7 +1946,7 @@ static void wglFlush(void) { sync_context(ctx); pglFlush(); - present_gl_drawable( hwnd, ctx->hdc, gl, FALSE ); + present_gl_drawable( hwnd, ctx->hdc, gl, TRUE, TRUE ); release_gl_drawable( gl ); } } @@ -2867,7 +2868,7 @@ static BOOL glxdrv_wglSwapBuffers( HDC hdc ) if (ctx && drawable && pglXWaitForSbcOML) pglXWaitForSbcOML( gdi_display, gl->drawable, target_sbc, &ust, &msc, &sbc ); - present_gl_drawable( hwnd, ctx ? ctx->hdc : hdc, gl, !pglXWaitForSbcOML ); + present_gl_drawable( hwnd, ctx ? ctx->hdc : hdc, gl, !pglXWaitForSbcOML, FALSE ); update_gl_drawable_size( gl ); release_gl_drawable( gl ); return TRUE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7094
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=150732 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000008900D8, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
Commit 9d95bd5f4b54a392d97f0b58db8f78d675495544 (which introduced multiple d3d devices per ddraw support) regressed Master Magistrate. The game tries to create a separate d3d device for its popup dialog windows. Previously when it was failing it worked with some fallback path. Now when that succeeds one of the issues that it hits is that while drawing on the child window updating popup dialog it does the draw only once something changed and doesn't present repeatedly. That is done by blitting to primary surface in ddraw, ddraw / wined3d update GL frontbuffer in that case and call glFlush(). The image gets blitted from offscreen client window before the actual image gets there. GLX_OML_sync_control which is used in wglSwapBuffers is not that useful here because it only allows to wait for swap happen and not for onscreen present resulting from glFinish() or Flush (it allows to wait for the next monitor refresh but it doesn't guarantee that the image is actually presented). Besides, GLX_OML_sync_control is not available on Nvidia. XFlush( gdi_display ) is the fallback path already used in wglSwapBuffers when GLX_OML_sync_control is not available. Besides, we need glFinish() to wait for GL operation complete before blitting in wglFinish(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7094#note_91278
I'm still trying to understand what this code does, but the commit title doesn't seem to match the content. Also, why call glFinish() from inside present_gl_drawable(), instead of from the caller? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7094#note_91283
On Fri Jan 3 18:26:30 2025 +0000, Elizabeth Figura wrote:
I'm still trying to understand what this code does, but the commit title doesn't seem to match the content. Also, why call glFinish() from inside present_gl_drawable(), instead of from the caller? Maybe I should split that in two, one for ensuring XFlush( gdi_display) for both wglFinish and wglFlush, and another for forcing glFinish when offscreen blit is used in glFlush?
glFinish() should only be called from wglFlush() if we need to blit from offscreen drawable, and all the logic which detects / processes that is now inside present_gl_drawable(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7094#note_91284
Maybe I should split that in two, one for ensuring XFlush( gdi_display) for both wglFinish and wglFlush, and another for forcing glFinish when offscreen blit is used in glFlush?
That would probably help.
glFinish() should only be called from wglFlush() if we need to blit from offscreen drawable, and all the logic which detects / processes that is now inside present_gl_drawable().
Sorry, I don't understand? You're passing a flag to present_gl_drawable() to signal whether glFinish() should be called, so the logic is pretty clearly outside present_gl_drawable(). And as far as I can tell, the sequence isn't important; present_gl_drawable() doesn't do any GL work before glFinish() is called. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7094#note_91434
Looks okay to me, splitting would probably be nice too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7094#note_91452
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7094
participants (5)
-
Elizabeth Figura (＠zfigura) -
Marvin -
Paul Gofman
-
Paul Gofman (＠gofman)
-
Rémi Bernon