[PATCH v2 0/7] MR5470: win32u: Get rid of the window surface recursive mutex.
-- v2: winex11: Stop using a recursive mutex for the window surfaces. winewayland: Stop using a recursive mutex for the window surfaces. winemac: Stop using a recursive mutex for the window surfaces. wineandroid: Stop using a recursive mutex for the window surfaces. win32u: Stop using a recursive mutex for the offscreen surface. win32u: Remove surface recursive locking requirement. win32u: Flush window surface when it is fully unlocked. https://gitlab.winehq.org/wine/wine/-/merge_requests/5470
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5470
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/dibdrv/dc.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/dlls/win32u/dibdrv/dc.c b/dlls/win32u/dibdrv/dc.c index 38039e7d6ae..e1b2f66cb25 100644 --- a/dlls/win32u/dibdrv/dc.c +++ b/dlls/win32u/dibdrv/dc.c @@ -734,6 +734,7 @@ struct windrv_physdev struct gdi_physdev dev; struct dibdrv_physdev *dibdrv; struct window_surface *surface; + UINT lock_count; }; static const struct gdi_dc_funcs window_driver; @@ -743,25 +744,34 @@ 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; + + surface->funcs->lock( surface ); + if (!dev->lock_count++) + { + 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; + + surface->funcs->unlock( surface ); + if (!--dev->lock_count) + { + DWORD ticks = NtGetTickCount() - surface->draw_start_ticks; + 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 ) @@ -964,8 +974,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 | 36 +++++++++++++++++++++++------------- dlls/win32u/dibdrv/dc.c | 4 ++-- 2 files changed, 25 insertions(+), 15 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 e1b2f66cb25..48cbe51d708 100644 --- a/dlls/win32u/dibdrv/dc.c +++ b/dlls/win32u/dibdrv/dc.c @@ -749,9 +749,9 @@ static inline void lock_surface( struct windrv_physdev *dev ) { struct window_surface *surface = dev->surface; - surface->funcs->lock( surface ); if (!dev->lock_count++) { + surface->funcs->lock( surface ); if (IsRectEmpty( dev->dibdrv->bounds ) || !surface->draw_start_ticks) surface->draw_start_ticks = NtGetTickCount(); } @@ -761,10 +761,10 @@ static inline void unlock_surface( struct windrv_physdev *dev ) { struct window_surface *surface = dev->surface; - surface->funcs->unlock( surface ); if (!--dev->lock_count) { DWORD ticks = NtGetTickCount() - surface->draw_start_ticks; + surface->funcs->unlock( surface ); if (ticks > FLUSH_PERIOD) surface->funcs->flush( dev->surface ); } } -- 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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=145865 Your paranoid android. === debian11b (build log) === error: patch failed: dlls/win32u/dibdrv/dc.c:734 error: patch failed: dlls/win32u/dce.c:1268 error: patch failed: dlls/win32u/dibdrv/dc.c:749 error: patch failed: dlls/win32u/dce.c:179 error: patch failed: dlls/wineandroid.drv/window.c:807 error: patch failed: dlls/winemac.drv/surface.c:246 error: patch failed: dlls/winewayland.drv/window_surface.c:507 error: patch failed: dlls/winex11.drv/bitblt.c:1992 Task: Patch failed to apply
Should hopefully be working now. The problem was that flush also takes a lock. My intention here (and in some later changes) is to move more implementation details into some win32u exported surface helpers, factoring them between drivers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5470#note_71279
participants (3)
-
Huw Davies (＠huw) -
Marvin -
Rémi Bernon