[PATCH 0/6] MR5470: win32u: Get rid of the window surface recursive mutex.
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/dce.c | 36 +++++++++++++++++++++++------------- dlls/win32u/dibdrv/dc.c | 32 ++++++++++++++++++++------------ 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/dlls/win32u/dce.c b/dlls/win32u/dce.c index 5df30550cae..24a53b9191e 100644 --- a/dlls/win32u/dce.c +++ b/dlls/win32u/dce.c @@ -1268,7 +1268,7 @@ static BOOL send_erase( HWND hwnd, UINT flags, HRGN client_rgn, * Copy bits from a window surface; helper for move_window_bits and move_window_bits_parent. */ static void copy_bits_from_surface( HWND hwnd, struct window_surface *surface, - const RECT *dst, const RECT *src ) + const RECT *dst, const RECT *src, BOOL same ) { char buffer[FIELD_OFFSET( BITMAPINFO, bmiColors[256] )]; BITMAPINFO *info = (BITMAPINFO *)buffer; @@ -1277,13 +1277,23 @@ static void copy_bits_from_surface( HWND hwnd, struct window_surface *surface, HRGN rgn = get_update_region( hwnd, &flags, NULL ); HDC hdc = NtUserGetDCEx( hwnd, rgn, DCX_CACHE | DCX_WINDOW | DCX_EXCLUDERGN ); - bits = surface->funcs->get_info( surface, info ); - surface->funcs->lock( surface ); - NtGdiSetDIBitsToDeviceInternal( hdc, dst->left, dst->top, dst->right - dst->left, dst->bottom - dst->top, - src->left - surface->rect.left, surface->rect.bottom - src->bottom, - 0, surface->rect.bottom - surface->rect.top, - bits, info, DIB_RGB_COLORS, 0, 0, FALSE, NULL ); - surface->funcs->unlock( surface ); + if (same) + { + RECT rect = *src; + NtGdiStretchBlt( hdc, dst->left, dst->top, dst->right - dst->left, dst->bottom - dst->top, + hdc, rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top, SRCCOPY, 0 ); + } + else + { + bits = surface->funcs->get_info( surface, info ); + surface->funcs->lock( surface ); + NtGdiSetDIBitsToDeviceInternal( hdc, dst->left, dst->top, dst->right - dst->left, dst->bottom - dst->top, + src->left - surface->rect.left, surface->rect.bottom - src->bottom, + 0, surface->rect.bottom - surface->rect.top, + bits, info, DIB_RGB_COLORS, 0, 0, FALSE, NULL ); + surface->funcs->unlock( surface ); + } + NtUserReleaseDC( hwnd, hdc ); } @@ -1305,9 +1315,10 @@ void move_window_bits( HWND hwnd, struct window_surface *old_surface, src.top - old_visible_rect->top != dst.top - visible_rect->top) { TRACE( "copying %s -> %s\n", wine_dbgstr_rect( &src ), wine_dbgstr_rect( &dst )); - OffsetRect( &src, -old_visible_rect->left, -old_visible_rect->top ); + if (new_surface != old_surface) OffsetRect( &src, -old_visible_rect->left, -old_visible_rect->top ); + else OffsetRect( &src, -window_rect->left, -window_rect->top ); OffsetRect( &dst, -window_rect->left, -window_rect->top ); - copy_bits_from_surface( hwnd, old_surface, &dst, &src ); + copy_bits_from_surface( hwnd, old_surface, &dst, &src, new_surface == old_surface ); } } @@ -1336,13 +1347,12 @@ void move_window_bits_parent( HWND hwnd, HWND parent, const RECT *window_rect, c TRACE( "copying %s -> %s\n", wine_dbgstr_rect( &src ), wine_dbgstr_rect( &dst )); map_window_points( NtUserGetAncestor( hwnd, GA_PARENT ), parent, (POINT *)&src, 2, get_thread_dpi() ); - OffsetRect( &src, win->client_rect.left - win->visible_rect.left, - win->client_rect.top - win->visible_rect.top ); + OffsetRect( &src, -window_rect->left, -window_rect->top ); OffsetRect( &dst, -window_rect->left, -window_rect->top ); window_surface_add_ref( surface ); release_win_ptr( win ); - copy_bits_from_surface( hwnd, surface, &dst, &src ); + copy_bits_from_surface( hwnd, surface, &dst, &src, TRUE ); window_surface_release( surface ); } diff --git a/dlls/win32u/dibdrv/dc.c b/dlls/win32u/dibdrv/dc.c index 637b92f5256..f1eb0c96300 100644 --- a/dlls/win32u/dibdrv/dc.c +++ b/dlls/win32u/dibdrv/dc.c @@ -725,6 +725,7 @@ struct windrv_physdev struct gdi_physdev dev; struct dibdrv_physdev *dibdrv; struct window_surface *surface; + UINT surface_locks; }; static const struct gdi_dc_funcs window_driver; @@ -734,25 +735,32 @@ static inline struct windrv_physdev *get_windrv_physdev( PHYSDEV dev ) return (struct windrv_physdev *)dev; } +/* gdi_lock should not be locked */ static inline void lock_surface( struct windrv_physdev *dev ) { - /* gdi_lock should not be locked */ - dev->surface->funcs->lock( dev->surface ); - if (IsRectEmpty( dev->dibdrv->bounds ) || dev->surface->draw_start_ticks == 0) - dev->surface->draw_start_ticks = NtGetTickCount(); + struct window_surface *surface = dev->surface; + + if (!dev->surface_locks++) + surface->funcs->lock( surface ); + + if (IsRectEmpty( dev->dibdrv->bounds ) || !surface->draw_start_ticks) + surface->draw_start_ticks = NtGetTickCount(); } static inline void unlock_surface( struct windrv_physdev *dev ) { - BOOL should_flush = NtGetTickCount() - dev->surface->draw_start_ticks > FLUSH_PERIOD; - dev->surface->funcs->unlock( dev->surface ); - if (should_flush) dev->surface->funcs->flush( dev->surface ); + struct window_surface *surface = dev->surface; + DWORD ticks = NtGetTickCount() - surface->draw_start_ticks; + + if (!--dev->surface_locks) + surface->funcs->unlock( surface ); + + if (ticks > FLUSH_PERIOD) surface->funcs->flush( dev->surface ); } -static void unlock_bits_surface( struct gdi_image_bits *bits ) +static void unlock_windrv_bits( struct gdi_image_bits *bits ) { - struct window_surface *surface = bits->param; - surface->funcs->unlock( surface ); + unlock_surface( bits->param ); } void dibdrv_set_window_surface( DC *dc, struct window_surface *surface ) @@ -955,8 +963,8 @@ static DWORD windrv_GetImage( PHYSDEV dev, BITMAPINFO *info, { /* use the freeing callback to unlock the surface */ assert( !bits->free ); - bits->free = unlock_bits_surface; - bits->param = physdev->surface; + bits->free = unlock_windrv_bits; + bits->param = physdev; } else unlock_surface( physdev ); return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5470
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/dce.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/dlls/win32u/dce.c b/dlls/win32u/dce.c index 24a53b9191e..256fc5cdd3c 100644 --- a/dlls/win32u/dce.c +++ b/dlls/win32u/dce.c @@ -179,6 +179,7 @@ static void offscreen_window_surface_flush( struct window_surface *base ) static void offscreen_window_surface_destroy( struct window_surface *base ) { struct offscreen_window_surface *impl = impl_from_window_surface( base ); + pthread_mutex_destroy( &impl->mutex ); free( impl ); } @@ -198,7 +199,6 @@ void create_offscreen_window_surface( const RECT *visible_rect, struct window_su struct offscreen_window_surface *impl; SIZE_T size; RECT surface_rect = *visible_rect; - pthread_mutexattr_t attr; TRACE( "visible_rect %s, surface %p.\n", wine_dbgstr_rect( visible_rect ), surface ); @@ -224,10 +224,7 @@ void create_offscreen_window_surface( const RECT *visible_rect, struct window_su impl->header.ref = 1; impl->header.rect = surface_rect; - pthread_mutexattr_init( &attr ); - pthread_mutexattr_settype( &attr, PTHREAD_MUTEX_RECURSIVE ); - pthread_mutex_init( &impl->mutex, &attr ); - pthread_mutexattr_destroy( &attr ); + pthread_mutex_init( &impl->mutex, NULL ); reset_bounds( &impl->bounds ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5470
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/wineandroid.drv/window.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/dlls/wineandroid.drv/window.c b/dlls/wineandroid.drv/window.c index ed7ee808b29..0369842eefb 100644 --- a/dlls/wineandroid.drv/window.c +++ b/dlls/wineandroid.drv/window.c @@ -807,6 +807,7 @@ static void android_surface_destroy( struct window_surface *window_surface ) if (surface->region) NtGdiDeleteObjectApp( surface->region ); release_ioctl_window( surface->window ); free( surface->bits ); + pthread_mutex_destroy( &surface->mutex ); free( surface ); } @@ -901,7 +902,6 @@ static struct window_surface *create_surface( HWND hwnd, const RECT *rect, { struct android_window_surface *surface; int width = rect->right - rect->left, height = rect->bottom - rect->top; - pthread_mutexattr_t attr; surface = calloc( 1, FIELD_OFFSET( struct android_window_surface, info.bmiColors[3] )); if (!surface) return NULL; @@ -911,10 +911,7 @@ static struct window_surface *create_surface( HWND hwnd, const RECT *rect, surface->info.bmiHeader.biPlanes = 1; surface->info.bmiHeader.biSizeImage = get_dib_image_size( &surface->info ); - pthread_mutexattr_init( &attr ); - pthread_mutexattr_settype( &attr, PTHREAD_MUTEX_RECURSIVE ); - pthread_mutex_init( &surface->mutex, &attr ); - pthread_mutexattr_destroy( &attr ); + pthread_mutex_init( &surface->mutex, NULL ); surface->header.funcs = &android_surface_funcs; surface->header.rect = *rect; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5470
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/surface.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/dlls/winemac.drv/surface.c b/dlls/winemac.drv/surface.c index 8b238ff4853..65da12912c2 100644 --- a/dlls/winemac.drv/surface.c +++ b/dlls/winemac.drv/surface.c @@ -246,22 +246,13 @@ struct window_surface *create_surface(macdrv_window window, const RECT *rect, struct macdrv_window_surface *old_mac_surface = get_mac_surface(old_surface); int width = rect->right - rect->left, height = rect->bottom - rect->top; DWORD *colors; - pthread_mutexattr_t attr; int err; DWORD window_background; surface = calloc(1, FIELD_OFFSET(struct macdrv_window_surface, info.bmiColors[3])); if (!surface) return NULL; - err = pthread_mutexattr_init(&attr); - if (!err) - { - err = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); - if (!err) - err = pthread_mutex_init(&surface->mutex, &attr); - pthread_mutexattr_destroy(&attr); - } - if (err) + if ((err = pthread_mutex_init(&surface->mutex, NULL))) { free(surface); return NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5470
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winewayland.drv/window_surface.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/dlls/winewayland.drv/window_surface.c b/dlls/winewayland.drv/window_surface.c index 67cb2fe2d0b..cb5e1539072 100644 --- a/dlls/winewayland.drv/window_surface.c +++ b/dlls/winewayland.drv/window_surface.c @@ -507,7 +507,6 @@ struct window_surface *wayland_window_surface_create(HWND hwnd, const RECT *rect struct wayland_window_surface *wws; int width = rect->right - rect->left; int height = rect->bottom - rect->top; - pthread_mutexattr_t mutexattr; TRACE("hwnd %p rect %s\n", hwnd, wine_dbgstr_rect(rect)); @@ -522,10 +521,7 @@ struct window_surface *wayland_window_surface_create(HWND hwnd, const RECT *rect wws->info.bmiHeader.biPlanes = 1; wws->info.bmiHeader.biSizeImage = width * height * 4; - pthread_mutexattr_init(&mutexattr); - pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&wws->mutex, &mutexattr); - pthread_mutexattr_destroy(&mutexattr); + pthread_mutex_init(&wws->mutex, NULL); wws->header.funcs = &wayland_window_surface_funcs; wws->header.rect = *rect; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5470
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/bitblt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/winex11.drv/bitblt.c b/dlls/winex11.drv/bitblt.c index 94aa06773a7..5eccc4a2eb7 100644 --- a/dlls/winex11.drv/bitblt.c +++ b/dlls/winex11.drv/bitblt.c @@ -1992,6 +1992,8 @@ static void x11drv_surface_destroy( struct window_surface *window_surface ) XDestroyImage( surface->image ); } if (surface->region) NtGdiDeleteObjectApp( surface->region ); + + pthread_mutex_destroy( &surface->mutex ); free( surface ); } @@ -2027,7 +2029,7 @@ struct window_surface *create_surface( Window window, const XVisualInfo *vis, co surface->info.bmiHeader.biSizeImage = get_dib_image_size( &surface->info ); if (format->bits_per_pixel > 8) set_color_info( vis, &surface->info, use_alpha ); - init_recursive_mutex( &surface->mutex ); + pthread_mutex_init( &surface->mutex, NULL ); surface->header.funcs = &x11drv_surface_funcs; surface->header.rect = *rect; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5470
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=144793 Your paranoid android. === debian11b (64 bit WoW report) === comctl32: listbox: Timeout listview: Timeout tab: Timeout treeview: Timeout gdiplus: graphics: Timeout quartz: videorenderer.c:885: Test failed: Got hr 0x40237. videorenderer.c:895: Test failed: Wait failed. videorenderer.c:896: Test failed: Got hr 0x103. videorenderer: Timeout shell32: autocomplete: Timeout user32: listbox.c:1500: Test failed: Error creating the test file: 80 listbox: Timeout menu: Timeout msg: Timeout
I probably missed some cases where it was needed. I'll have a better look. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5470#note_67257
participants (2)
-
Marvin -
Rémi Bernon