-- 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.
This merge request was approved by Huw Davies.
From: Rémi Bernon rbernon@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;
From: Rémi Bernon rbernon@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 ); } }
From: Rémi Bernon rbernon@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 );
From: Rémi Bernon rbernon@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;
From: Rémi Bernon rbernon@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;
From: Rémi Bernon rbernon@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;
From: Rémi Bernon rbernon@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;
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.
-
Huw Davies (＠huw) -
Marvin -
Rémi Bernon
-
Rémi Bernon (＠rbernon)