[PATCH v3 0/5] MR5798: winemac: Push window surface updates to the main thread.
Instead of accessing the surface with macdrv_get_surface_display_image. -- v3: winemac: Remove now unnecessary cocoa window surface pointer. winemac: Push window surface image updates to the main thread. winemac: Create window surface CGImageRef on surface flush. winemac: Create a provider for the surface and a HBITMAP wrapping it. winemac: Remove unused macdrv_get_surface_display_image copy_data parameter. https://gitlab.winehq.org/wine/wine/-/merge_requests/5798
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/cocoa_window.m | 2 +- dlls/winemac.drv/macdrv_cocoa.h | 2 +- dlls/winemac.drv/surface.c | 12 ++---------- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index 61a6ab2cd4e..34c39bb3d76 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -532,7 +532,7 @@ - (void) updateLayer imageRect.origin.y *= layer.contentsScale; imageRect.size.width *= layer.contentsScale; imageRect.size.height *= layer.contentsScale; - image = macdrv_get_surface_display_image(window.surface, &imageRect, FALSE, window.colorKeyed, + image = macdrv_get_surface_display_image(window.surface, &imageRect, window.colorKeyed, window.colorKeyRed, window.colorKeyGreen, window.colorKeyBlue); if (image) diff --git a/dlls/winemac.drv/macdrv_cocoa.h b/dlls/winemac.drv/macdrv_cocoa.h index ba5803f88dc..bdc7d9980e1 100644 --- a/dlls/winemac.drv/macdrv_cocoa.h +++ b/dlls/winemac.drv/macdrv_cocoa.h @@ -553,7 +553,7 @@ extern void macdrv_order_cocoa_window(macdrv_window w, macdrv_window prev, extern void macdrv_get_cocoa_window_frame(macdrv_window w, CGRect* out_frame); extern void macdrv_set_cocoa_parent_window(macdrv_window w, macdrv_window parent); extern void macdrv_set_window_surface(macdrv_window w, struct window_surface *window_surface); -extern CGImageRef macdrv_get_surface_display_image(struct window_surface *window_surface, CGRect *rect, int copy_data, int color_keyed, +extern CGImageRef macdrv_get_surface_display_image(struct window_surface *window_surface, CGRect *rect, int color_keyed, CGFloat key_red, CGFloat key_green, CGFloat key_blue); extern void macdrv_window_needs_display(macdrv_window w, CGRect rect); extern void macdrv_set_window_shape(macdrv_window w, const CGRect *rects, int count); diff --git a/dlls/winemac.drv/surface.c b/dlls/winemac.drv/surface.c index 37e2c466926..4e8b3c4824f 100644 --- a/dlls/winemac.drv/surface.c +++ b/dlls/winemac.drv/surface.c @@ -191,7 +191,7 @@ void set_surface_use_alpha(struct window_surface *window_surface, BOOL use_alpha * must not use Win32 or Wine functions, including debug * logging. */ -CGImageRef macdrv_get_surface_display_image(struct window_surface *window_surface, CGRect *rect, int copy_data, int color_keyed, +CGImageRef macdrv_get_surface_display_image(struct window_surface *window_surface, CGRect *rect, int color_keyed, CGFloat key_red, CGFloat key_green, CGFloat key_blue) { CGImageRef cgimage = NULL; @@ -220,15 +220,7 @@ CGImageRef macdrv_get_surface_display_image(struct window_surface *window_surfac offset = CGRectGetMinX(visrect) * 4 + CGRectGetMinY(visrect) * bytes_per_row; size = min(CGRectGetHeight(visrect) * bytes_per_row, surface->info.bmiHeader.biSizeImage - offset); - - if (copy_data) - { - CFDataRef data = CFDataCreate(NULL, (UInt8 *)window_surface->color_bits + offset, size); - provider = CGDataProviderCreateWithCFData(data); - CFRelease(data); - } - else - provider = CGDataProviderCreateWithData(NULL, (UInt8 *)window_surface->color_bits + offset, size, NULL); + provider = CGDataProviderCreateWithData(NULL, (UInt8 *)window_surface->color_bits + offset, size, NULL); alphaInfo = surface->use_alpha ? kCGImageAlphaPremultipliedFirst : kCGImageAlphaNoneSkipFirst; cgimage = CGImageCreate(CGRectGetWidth(visrect), CGRectGetHeight(visrect), -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5798
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/surface.c | 55 ++++++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/dlls/winemac.drv/surface.c b/dlls/winemac.drv/surface.c index 4e8b3c4824f..d215e126110 100644 --- a/dlls/winemac.drv/surface.c +++ b/dlls/winemac.drv/surface.c @@ -65,11 +65,31 @@ struct macdrv_window_surface struct window_surface header; macdrv_window window; BOOL use_alpha; + CGDataProviderRef provider; BITMAPINFO info; /* variable size, must be last */ }; static struct macdrv_window_surface *get_mac_surface(struct window_surface *surface); +static void data_provider_destroy(void *info, const void *data, size_t size) +{ + free(info); +} + +static CGDataProviderRef data_provider_create(size_t size, void **bits) +{ + CGDataProviderRef provider; + void *data; + + if (!(data = malloc(size))) return NULL; + + provider = CGDataProviderCreateWithData(data, data, size, data_provider_destroy); + if (!provider) free(data); + else *bits = data; + + return provider; +} + /*********************************************************************** * macdrv_surface_get_bitmap_info */ @@ -107,6 +127,7 @@ static void macdrv_surface_destroy(struct window_surface *window_surface) struct macdrv_window_surface *surface = get_mac_surface(window_surface); TRACE("freeing %p\n", surface); + CGDataProviderRelease(surface->provider); free(surface); } @@ -130,11 +151,16 @@ static struct macdrv_window_surface *get_mac_surface(struct window_surface *surf struct window_surface *create_surface(HWND hwnd, macdrv_window window, const RECT *rect, struct window_surface *old_surface, BOOL use_alpha) { - struct macdrv_window_surface *surface; + struct macdrv_window_surface *surface = NULL; int width = rect->right - rect->left, height = rect->bottom - rect->top; DWORD window_background; + D3DKMT_CREATEDCFROMMEMORY desc = {.Format = D3DDDIFMT_A8R8G8B8}; char buffer[FIELD_OFFSET(BITMAPINFO, bmiColors[256])]; BITMAPINFO *info = (BITMAPINFO *)buffer; + CGDataProviderRef provider; + HBITMAP bitmap = 0; + UINT status; + void *bits; memset(info, 0, sizeof(*info)); info->bmiHeader.biSize = sizeof(info->bmiHeader); @@ -145,14 +171,31 @@ struct window_surface *create_surface(HWND hwnd, macdrv_window window, const REC info->bmiHeader.biSizeImage = get_dib_image_size(info); info->bmiHeader.biCompression = BI_RGB; - surface = calloc(1, FIELD_OFFSET(struct macdrv_window_surface, info.bmiColors[3])); - if (!surface) return NULL; - if (!window_surface_init(&surface->header, &macdrv_surface_funcs, hwnd, rect, info, 0)) goto failed; + if (!(provider = data_provider_create(info->bmiHeader.biSizeImage, &bits))) return NULL; + + /* wrap the data in a HBITMAP so we can write to the surface pixels directly */ + desc.Width = info->bmiHeader.biWidth; + desc.Height = abs(info->bmiHeader.biHeight); + desc.Pitch = info->bmiHeader.biSizeImage / abs(info->bmiHeader.biHeight); + desc.pMemory = bits; + desc.hDeviceDc = NtUserGetDCEx(hwnd, 0, DCX_CACHE | DCX_WINDOW); + if ((status = NtGdiDdDDICreateDCFromMemory(&desc))) + ERR("Failed to create HBITMAP, status %#x\n", status); + else + { + bitmap = desc.hBitmap; + NtGdiDeleteObjectApp(desc.hDc); + } + if (desc.hDeviceDc) NtUserReleaseDC(hwnd, desc.hDeviceDc); + + if (!(surface = calloc(1, FIELD_OFFSET(struct macdrv_window_surface, info.bmiColors[3])))) goto failed; + if (!window_surface_init(&surface->header, &macdrv_surface_funcs, hwnd, rect, info, bitmap)) goto failed; memcpy(&surface->info, info, offsetof(BITMAPINFO, bmiColors[3])); surface->window = window; if (old_surface) surface->header.bounds = old_surface->bounds; surface->use_alpha = use_alpha; + surface->provider = provider; window_background = macdrv_window_background_color(); memset_pattern4(surface->header.color_bits, &window_background, info->bmiHeader.biSizeImage); @@ -163,7 +206,9 @@ struct window_surface *create_surface(HWND hwnd, macdrv_window window, const REC return &surface->header; failed: - window_surface_release(&surface->header); + if (surface) window_surface_release(&surface->header); + if (bitmap) NtGdiDeleteObjectApp(bitmap); + CGDataProviderRelease(provider); return NULL; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5798
From: Rémi Bernon <rbernon(a)codeweavers.com> Avoid accessing the surface color bits field from macdrv_get_surface_display_image. --- dlls/winemac.drv/surface.c | 47 +++++++++++++------------------------- 1 file changed, 16 insertions(+), 31 deletions(-) diff --git a/dlls/winemac.drv/surface.c b/dlls/winemac.drv/surface.c index d215e126110..5082bd2ebe2 100644 --- a/dlls/winemac.drv/surface.c +++ b/dlls/winemac.drv/surface.c @@ -66,6 +66,7 @@ struct macdrv_window_surface macdrv_window window; BOOL use_alpha; CGDataProviderRef provider; + CGImageRef color_image; BITMAPINFO info; /* variable size, must be last */ }; @@ -115,6 +116,18 @@ static void macdrv_surface_set_clip(struct window_surface *window_surface, const static BOOL macdrv_surface_flush(struct window_surface *window_surface, const RECT *rect, const RECT *dirty) { struct macdrv_window_surface *surface = get_mac_surface(window_surface); + CGImageAlphaInfo alpha_info = (surface->use_alpha ? kCGImageAlphaPremultipliedFirst : kCGImageAlphaNoneSkipFirst); + BITMAPINFO *color_info = &surface->info; + CGColorSpaceRef colorspace; + + if (surface->color_image) CGImageRelease(surface->color_image); + + colorspace = CGColorSpaceCreateWithName(kCGColorSpaceSRGB); + surface->color_image = CGImageCreate(color_info->bmiHeader.biWidth, abs(color_info->bmiHeader.biHeight), 8, 32, + color_info->bmiHeader.biSizeImage / abs(color_info->bmiHeader.biHeight), colorspace, + alpha_info | kCGBitmapByteOrder32Little, surface->provider, NULL, retina_on, kCGRenderingIntentDefault); + CGColorSpaceRelease(colorspace); + macdrv_window_needs_display(surface->window, cgrect_from_rect(*dirty)); return FALSE; /* bounds are reset asynchronously, from macdrv_get_surface_display_image */ } @@ -127,6 +140,7 @@ static void macdrv_surface_destroy(struct window_surface *window_surface) struct macdrv_window_surface *surface = get_mac_surface(window_surface); TRACE("freeing %p\n", surface); + if (surface->color_image) CGImageRelease(surface->color_image); CGDataProviderRelease(surface->provider); free(surface); } @@ -241,39 +255,11 @@ CGImageRef macdrv_get_surface_display_image(struct window_surface *window_surfac { CGImageRef cgimage = NULL; struct macdrv_window_surface *surface = get_mac_surface(window_surface); - RECT surface_rect = window_surface->rect; - int width, height; pthread_mutex_lock(&window_surface->mutex); - if (IsRectEmpty(&window_surface->bounds)) goto done; - - width = surface_rect.right - surface_rect.left; - height = surface_rect.bottom - surface_rect.top; - *rect = CGRectIntersection(cgrect_from_rect(surface_rect), *rect); - if (!CGRectIsEmpty(*rect)) + if (surface->color_image && !IsRectEmpty(&window_surface->bounds)) { - CGRect visrect; - CGColorSpaceRef colorspace; - CGDataProviderRef provider; - int bytes_per_row, offset, size; - CGImageAlphaInfo alphaInfo; - - visrect = CGRectOffset(*rect, -surface_rect.left, -surface_rect.top); - - colorspace = CGColorSpaceCreateWithName(kCGColorSpaceSRGB); - bytes_per_row = get_dib_stride(width, 32); - offset = CGRectGetMinX(visrect) * 4 + CGRectGetMinY(visrect) * bytes_per_row; - size = min(CGRectGetHeight(visrect) * bytes_per_row, - surface->info.bmiHeader.biSizeImage - offset); - provider = CGDataProviderCreateWithData(NULL, (UInt8 *)window_surface->color_bits + offset, size, NULL); - - alphaInfo = surface->use_alpha ? kCGImageAlphaPremultipliedFirst : kCGImageAlphaNoneSkipFirst; - cgimage = CGImageCreate(CGRectGetWidth(visrect), CGRectGetHeight(visrect), - 8, 32, bytes_per_row, colorspace, - alphaInfo | kCGBitmapByteOrder32Little, - provider, NULL, retina_on, kCGRenderingIntentDefault); - CGDataProviderRelease(provider); - CGColorSpaceRelease(colorspace); + cgimage = CGImageCreateWithImageInRect(surface->color_image, *rect); if (color_keyed) { @@ -290,7 +276,6 @@ CGImageRef macdrv_get_surface_display_image(struct window_surface *window_surfac } } -done: reset_bounds(&window_surface->bounds); pthread_mutex_unlock(&window_surface->mutex); return cgimage; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5798
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/cocoa_window.m | 44 ++++++++++++++++++--- dlls/winemac.drv/macdrv_cocoa.h | 4 +- dlls/winemac.drv/surface.c | 69 ++++----------------------------- 3 files changed, 47 insertions(+), 70 deletions(-) diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index 34c39bb3d76..82c3a4ee625 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -365,6 +365,8 @@ - (id) initWithFrame:(NSRect)frame device:(id<MTLDevice>)device; @interface WineContentView : WineBaseView <NSTextInputClient, NSViewLayerContentScaleDelegate> { + CGImageRef _colorImage; + NSMutableArray* glContexts; NSMutableArray* pendingGlContexts; BOOL _everHadGLContext; @@ -380,6 +382,7 @@ @interface WineContentView : WineBaseView <NSTextInputClient, NSViewLayerContent WineMetalView *_metalView; } +(a)property (retain, nonatomic) CGImageRef colorImage __attribute__((NSObject)); @property (readonly, nonatomic) BOOL everHadGLContext; - (void) addGLContext:(WineOpenGLContext*)context; @@ -391,6 +394,7 @@ - (void) wine_setBackingSize:(const int*)newBackingSize; - (WineMetalView*) newMetalViewWithDevice:(id<MTLDevice>)device; + - (void) updateColorImage:(CGImageRef)image rect:(CGRect)rect; @end @@ -483,6 +487,7 @@ - (NSFocusRingType) focusRingType @implementation WineContentView @synthesize everHadGLContext = _everHadGLContext; +(a)synthesize colorImage = _colorImage; - (instancetype) initWithFrame:(NSRect)frame { @@ -498,6 +503,7 @@ - (instancetype) initWithFrame:(NSRect)frame - (void) dealloc { + self.colorImage = nil; [markedText release]; [glContexts release]; [pendingGlContexts release]; @@ -532,8 +538,25 @@ - (void) updateLayer imageRect.origin.y *= layer.contentsScale; imageRect.size.width *= layer.contentsScale; imageRect.size.height *= layer.contentsScale; - image = macdrv_get_surface_display_image(window.surface, &imageRect, window.colorKeyed, - window.colorKeyRed, window.colorKeyGreen, window.colorKeyBlue); + + if (self.colorImage) + { + image = CGImageCreateWithImageInRect(self.colorImage, imageRect); + + if (window.colorKeyed) + { + CGImageRef maskedImage; + CGFloat components[] = { window.colorKeyRed - 0.5, window.colorKeyRed + 0.5, + window.colorKeyGreen - 0.5, window.colorKeyGreen + 0.5, + window.colorKeyBlue - 0.5, window.colorKeyBlue + 0.5 }; + maskedImage = CGImageCreateWithMaskingColors(image, components); + if (maskedImage) + { + CGImageRelease(image); + image = maskedImage; + } + } + } if (image) { @@ -552,6 +575,12 @@ - (void) updateLayer } } + - (void) updateColorImage:(CGImageRef)image rect:(CGRect)rect + { + self.colorImage = image; + [self setNeedsDisplayInRect:NSRectFromCGRect(rect)]; + } + - (void) viewWillDraw { [super viewWillDraw]; @@ -3493,19 +3522,22 @@ void macdrv_set_window_surface(macdrv_window w, struct window_surface *window_su } /*********************************************************************** - * macdrv_window_needs_display + * macdrv_window_set_color_image * - * Mark a window as needing display in a specified rect (in non-client + * Push a window surface color pixel update in a specified rect (in non-client * area coordinates). */ -void macdrv_window_needs_display(macdrv_window w, CGRect rect) +void macdrv_window_set_color_image(macdrv_window w, CGRect rect, CGImageRef image) { @autoreleasepool { WineWindow* window = (WineWindow*)w; + CGImageRetain(image); + OnMainThreadAsync(^{ - [[window contentView] setNeedsDisplayInRect:NSRectFromCGRect(cgrect_mac_from_win(rect))]; + [[window contentView] updateColorImage:image rect:cgrect_mac_from_win(rect)]; + CGImageRelease(image); }); } } diff --git a/dlls/winemac.drv/macdrv_cocoa.h b/dlls/winemac.drv/macdrv_cocoa.h index bdc7d9980e1..ee88f99002a 100644 --- a/dlls/winemac.drv/macdrv_cocoa.h +++ b/dlls/winemac.drv/macdrv_cocoa.h @@ -553,9 +553,7 @@ extern void macdrv_order_cocoa_window(macdrv_window w, macdrv_window prev, extern void macdrv_get_cocoa_window_frame(macdrv_window w, CGRect* out_frame); extern void macdrv_set_cocoa_parent_window(macdrv_window w, macdrv_window parent); extern void macdrv_set_window_surface(macdrv_window w, struct window_surface *window_surface); -extern CGImageRef macdrv_get_surface_display_image(struct window_surface *window_surface, CGRect *rect, int color_keyed, - CGFloat key_red, CGFloat key_green, CGFloat key_blue); -extern void macdrv_window_needs_display(macdrv_window w, CGRect rect); +extern void macdrv_window_set_color_image(macdrv_window w, CGRect rect, CGImageRef image); extern void macdrv_set_window_shape(macdrv_window w, const CGRect *rects, int count); extern void macdrv_set_window_alpha(macdrv_window w, CGFloat alpha); extern void macdrv_set_window_color_key(macdrv_window w, CGFloat keyRed, CGFloat keyGreen, diff --git a/dlls/winemac.drv/surface.c b/dlls/winemac.drv/surface.c index 5082bd2ebe2..56bdf5eeb30 100644 --- a/dlls/winemac.drv/surface.c +++ b/dlls/winemac.drv/surface.c @@ -53,12 +53,6 @@ static inline int get_dib_image_size(const BITMAPINFO *info) * abs(info->bmiHeader.biHeight); } -static inline void reset_bounds(RECT *bounds) -{ - bounds->left = bounds->top = INT_MAX; - bounds->right = bounds->bottom = INT_MIN; -} - struct macdrv_window_surface { @@ -66,7 +60,6 @@ struct macdrv_window_surface macdrv_window window; BOOL use_alpha; CGDataProviderRef provider; - CGImageRef color_image; BITMAPINFO info; /* variable size, must be last */ }; @@ -119,17 +112,18 @@ static BOOL macdrv_surface_flush(struct window_surface *window_surface, const RE CGImageAlphaInfo alpha_info = (surface->use_alpha ? kCGImageAlphaPremultipliedFirst : kCGImageAlphaNoneSkipFirst); BITMAPINFO *color_info = &surface->info; CGColorSpaceRef colorspace; - - if (surface->color_image) CGImageRelease(surface->color_image); + CGImageRef image; colorspace = CGColorSpaceCreateWithName(kCGColorSpaceSRGB); - surface->color_image = CGImageCreate(color_info->bmiHeader.biWidth, abs(color_info->bmiHeader.biHeight), 8, 32, - color_info->bmiHeader.biSizeImage / abs(color_info->bmiHeader.biHeight), colorspace, - alpha_info | kCGBitmapByteOrder32Little, surface->provider, NULL, retina_on, kCGRenderingIntentDefault); + image = CGImageCreate(color_info->bmiHeader.biWidth, abs(color_info->bmiHeader.biHeight), 8, 32, + color_info->bmiHeader.biSizeImage / abs(color_info->bmiHeader.biHeight), colorspace, + alpha_info | kCGBitmapByteOrder32Little, surface->provider, NULL, retina_on, kCGRenderingIntentDefault); CGColorSpaceRelease(colorspace); - macdrv_window_needs_display(surface->window, cgrect_from_rect(*dirty)); - return FALSE; /* bounds are reset asynchronously, from macdrv_get_surface_display_image */ + macdrv_window_set_color_image(surface->window, cgrect_from_rect(*dirty), image); + CGImageRelease(image); + + return TRUE; } /*********************************************************************** @@ -140,7 +134,6 @@ static void macdrv_surface_destroy(struct window_surface *window_surface) struct macdrv_window_surface *surface = get_mac_surface(window_surface); TRACE("freeing %p\n", surface); - if (surface->color_image) CGImageRelease(surface->color_image); CGDataProviderRelease(surface->provider); free(surface); } @@ -235,52 +228,6 @@ void set_surface_use_alpha(struct window_surface *window_surface, BOOL use_alpha if (surface) surface->use_alpha = use_alpha; } -/*********************************************************************** - * create_surface_image - * - * Caller must hold the surface lock. On input, *rect is the requested - * image rect, relative to the window whole_rect, a.k.a. visible_rect. - * On output, it's been intersected with that part backed by the surface - * and is the actual size of the returned image. copy_data indicates if - * the caller will keep the returned image beyond the point where the - * surface bits can be guaranteed to remain valid and unchanged. If so, - * the bits are copied instead of merely referenced by the image. - * - * IMPORTANT: This function is called from non-Wine threads, so it - * must not use Win32 or Wine functions, including debug - * logging. - */ -CGImageRef macdrv_get_surface_display_image(struct window_surface *window_surface, CGRect *rect, int color_keyed, - CGFloat key_red, CGFloat key_green, CGFloat key_blue) -{ - CGImageRef cgimage = NULL; - struct macdrv_window_surface *surface = get_mac_surface(window_surface); - - pthread_mutex_lock(&window_surface->mutex); - if (surface->color_image && !IsRectEmpty(&window_surface->bounds)) - { - cgimage = CGImageCreateWithImageInRect(surface->color_image, *rect); - - if (color_keyed) - { - CGImageRef maskedImage; - CGFloat components[] = { key_red - 0.5, key_red + 0.5, - key_green - 0.5, key_green + 0.5, - key_blue - 0.5, key_blue + 0.5 }; - maskedImage = CGImageCreateWithMaskingColors(cgimage, components); - if (maskedImage) - { - CGImageRelease(cgimage); - cgimage = maskedImage; - } - } - } - - reset_bounds(&window_surface->bounds); - pthread_mutex_unlock(&window_surface->mutex); - return cgimage; -} - /*********************************************************************** * surface_clip_to_visible_rect * -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5798
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/cocoa_window.h | 3 --- dlls/winemac.drv/cocoa_window.m | 19 +------------------ dlls/winemac.drv/macdrv_cocoa.h | 1 - dlls/winemac.drv/window.c | 2 -- 4 files changed, 1 insertion(+), 24 deletions(-) diff --git a/dlls/winemac.drv/cocoa_window.h b/dlls/winemac.drv/cocoa_window.h index 9539e4ebdd7..b5168d0c128 100644 --- a/dlls/winemac.drv/cocoa_window.h +++ b/dlls/winemac.drv/cocoa_window.h @@ -44,9 +44,6 @@ @interface WineWindow : NSPanel <NSWindowDelegate> void* hwnd; WineEventQueue* queue; - void* surface; - pthread_mutex_t* surface_mutex; - CGDirectDisplayID _lastDisplayID; NSTimeInterval _lastDisplayTime; diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index 82c3a4ee625..384d23b633a 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -412,8 +412,6 @@ @interface WineWindow () @property (nonatomic) void* hwnd; @property (retain, readwrite, nonatomic) WineEventQueue* queue; -(a)property (nonatomic) void* surface; - @property (nonatomic) BOOL shapeChangedSinceLastDraw; @property (readonly, nonatomic) BOOL needsTransparency; @@ -530,7 +528,7 @@ - (void) updateLayer if ([window contentView] != self) return; - if (window.closing || !window.surface) + if (window.closing) return; imageRect = layer.bounds; @@ -1027,7 +1025,6 @@ @implementation WineWindow @synthesize disabled, noForeground, preventsAppActivation, floating, fullscreen, fakingClose, closing, latentParentWindow, hwnd, queue; @synthesize drawnSinceShown; - @synthesize surface; @synthesize shapeChangedSinceLastDraw; @synthesize colorKeyed, colorKeyRed, colorKeyGreen, colorKeyBlue; @synthesize usePerPixelAlpha; @@ -3506,20 +3503,6 @@ void macdrv_set_cocoa_parent_window(macdrv_window w, macdrv_window parent) }); } -/*********************************************************************** - * macdrv_set_window_surface - */ -void macdrv_set_window_surface(macdrv_window w, struct window_surface *window_surface) -{ -(a)autoreleasepool -{ - WineWindow* window = (WineWindow*)w; - - OnMainThread(^{ - window.surface = window_surface; - }); -} -} /*********************************************************************** * macdrv_window_set_color_image diff --git a/dlls/winemac.drv/macdrv_cocoa.h b/dlls/winemac.drv/macdrv_cocoa.h index ee88f99002a..abf01888bdb 100644 --- a/dlls/winemac.drv/macdrv_cocoa.h +++ b/dlls/winemac.drv/macdrv_cocoa.h @@ -552,7 +552,6 @@ extern void macdrv_order_cocoa_window(macdrv_window w, macdrv_window prev, extern void macdrv_set_cocoa_window_frame(macdrv_window w, const CGRect* new_frame); extern void macdrv_get_cocoa_window_frame(macdrv_window w, CGRect* out_frame); extern void macdrv_set_cocoa_parent_window(macdrv_window w, macdrv_window parent); -extern void macdrv_set_window_surface(macdrv_window w, struct window_surface *window_surface); extern void macdrv_window_set_color_image(macdrv_window w, CGRect rect, CGImageRef image); extern void macdrv_set_window_shape(macdrv_window w, const CGRect *rects, int count); extern void macdrv_set_window_alpha(macdrv_window w, CGFloat alpha); diff --git a/dlls/winemac.drv/window.c b/dlls/winemac.drv/window.c index 2f3d937b83c..81c3bb593e5 100644 --- a/dlls/winemac.drv/window.c +++ b/dlls/winemac.drv/window.c @@ -1884,7 +1884,6 @@ BOOL macdrv_CreateLayeredWindow(HWND hwnd, const RECT *window_rect, COLORREF col if (!surface || !EqualRect(&surface->rect, &rect)) { data->surface = create_surface(data->hwnd, data->cocoa_window, &rect, NULL, TRUE); - macdrv_set_window_surface(data->cocoa_window, data->surface); if (surface) window_surface_release(surface); surface = data->surface; if (data->unminimized_surface) @@ -2057,7 +2056,6 @@ void macdrv_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, } else { - macdrv_set_window_surface(data->cocoa_window, surface); if (data->unminimized_surface) { window_surface_release(data->unminimized_surface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5798
Tim Clem (@tclem) commented about dlls/winemac.drv/cocoa_window.m:
WineMetalView *_metalView; }
+(a)property (retain, nonatomic) CGImageRef colorImage __attribute__((NSObject));
`__attribute__((NSObject))` is a pretty obscure feature. If you don't have a compelling reason for this to be a property, I'd probably just make it an ivar and handle the retain/release manually. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5798#note_72957
On Wed Jun 12 15:46:45 2024 +0000, Tim Clem wrote:
`__attribute__((NSObject))` is a pretty obscure feature. If you don't have a compelling reason for this to be a property, I'd probably just make it an ivar and handle the retain/release manually. Yeah I wasn't sure, I'll change it, thanks!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5798#note_72994
Could you say something about the impetus for this change? Is it paving the way for shared surfaces? I don't know that I'm the best person to review this, but it looks good to me aside from the other comment I made. I tested a few applications, and if CrystalMark is to be believed, it improves GDI performance. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5798#note_73091
Could you say something about the impetus for this change? Is it paving the way for shared surfaces?
Sort of, I think it could make things a bit more like how it ought to be, although I still haven't a clear idea how to do that yet. The main reason right now is that it fixes some ownership issue with the window surfaces: 1) The UI doesn't have a ref on either the surface or its data. It still keeps a reference on the image, which uses but doesn't own the surface data, while it's being referenced by the UI layer. 2) The window surface might get re-created at any point, especially during resize, and the UI surface pointer is swapped then but the layer isn't necessarily redrawn immediately and still has a reference through its image on the now invalid previous surface data. That issue doesn't seem to cause any problem so far for some unclear reason, but I think it still needs to be fixed. Then, I'm also refactoring the surface code to make it simpler and move more logic into win32u. Specifically I would like to re-implement the window shapes using 1bpp shape bitmap, in common code. That would make color keying and shape logic simpler, and less driver dependent. This change will be easier to implement with this MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5798#note_73104
I don't know that I'm the best person to review this, but it looks good to me aside from the other comment I made. I tested a few applications, and if CrystalMark is to be believed, it improves GDI performance.
Thanks, this is really appreciated. I think that if someone shows enough interest to look at the code and make comments, their advice is worth considering as a review. But then if you don't feel like it, feel free to remove yourself. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5798#note_73105
participants (2)
-
Rémi Bernon -
Tim Clem (＠tclem)