[PATCH v3 0/2] MR7938: Draft: winemac: Use IOSurface to update native window layers instead of CGImage.
This consolidates the two separate `CGImage`s (`colorImage` and `shapeImage`) previously used in `WineContentView` into a single `IOSurface` that also holds the window mask information in its alpha channel now. The alpha channel is now being updated in `macdrv_surface_flush`, whenever the shape changes (and stores that change for the next update, to also keep the front buffer in sync). This reduces the delay from setting a `CGImage` to a layer from 50ms to about 1.5ms in the `IOSurface` case (also accounting for `vImageSelectChannels_ARGB8888` operation). Based on a patch by @bshanks. I am already putting it out for feedback, but I will so some more testing to see how the performance impact of this is and do more exhaustive testing with shaped windows. Ideally the double buffer solution can be avoided as well... Also the `HBITMAP` for the windows is currently backed by a `CGDataProviderRef` and could also become an `IOSurface` I believe. -- v3: winemac: Use IOSurface to update native window layers instead of CGImage. winemac: Also return alpha channel in macdrv_window_background_color. https://gitlab.winehq.org/wine/wine/-/merge_requests/7938
From: Marc-Aurel Zent <mzent(a)codeweavers.com> --- dlls/winemac.drv/cocoa_window.m | 12 ++++++------ dlls/winemac.drv/surface.c | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index 19c8fef4246..c55de3ecac0 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -3996,7 +3996,7 @@ void macdrv_set_view_backing_size(macdrv_view v, const int backing_size[2]) * macdrv_window_background_color * * Returns the standard Mac window background color as a 32-bit value of - * the form 0x00rrggbb. + * the form 0xaarrggbb. */ uint32_t macdrv_window_background_color(void) { @@ -4008,14 +4008,14 @@ uint32_t macdrv_window_background_color(void) // of it is to draw with it. dispatch_once(&once, ^{ OnMainThread(^{ - unsigned char rgbx[4]; - unsigned char *planes = rgbx; + unsigned char rgba[4]; + unsigned char *planes = rgba; NSBitmapImageRep *bitmap = [[NSBitmapImageRep alloc] initWithBitmapDataPlanes:&planes pixelsWide:1 pixelsHigh:1 bitsPerSample:8 - samplesPerPixel:3 - hasAlpha:NO + samplesPerPixel:4 + hasAlpha:YES isPlanar:NO colorSpaceName:NSCalibratedRGBColorSpace bitmapFormat:0 @@ -4027,7 +4027,7 @@ uint32_t macdrv_window_background_color(void) NSRectFill(NSMakeRect(0, 0, 1, 1)); [NSGraphicsContext restoreGraphicsState]; [bitmap release]; - result = rgbx[0] << 16 | rgbx[1] << 8 | rgbx[2]; + result = rgba[0] << 16 | rgba[1] << 8 | rgba[2] | rgba[3] << 24; }); }); diff --git a/dlls/winemac.drv/surface.c b/dlls/winemac.drv/surface.c index a849918dfa7..bf2acf31431 100644 --- a/dlls/winemac.drv/surface.c +++ b/dlls/winemac.drv/surface.c @@ -175,6 +175,7 @@ static struct window_surface *create_surface(HWND hwnd, macdrv_window window, co if (!(provider = data_provider_create(info->bmiHeader.biSizeImage, &bits))) return NULL; window_background = macdrv_window_background_color(); + window_background &= 0x00ffffff; memset_pattern4(bits, &window_background, info->bmiHeader.biSizeImage); /* wrap the data in a HBITMAP so we can write to the surface pixels directly */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7938
From: Marc-Aurel Zent <mzent(a)codeweavers.com> Based on a patch by Brendan Shanks. --- dlls/winemac.drv/Makefile.in | 2 + dlls/winemac.drv/cocoa_window.m | 71 +++++++----------- dlls/winemac.drv/macdrv_cocoa.h | 6 +- dlls/winemac.drv/surface.c | 128 ++++++++++++++++++++++++++------ 4 files changed, 140 insertions(+), 67 deletions(-) diff --git a/dlls/winemac.drv/Makefile.in b/dlls/winemac.drv/Makefile.in index 621388f2bd8..578d925c6de 100644 --- a/dlls/winemac.drv/Makefile.in +++ b/dlls/winemac.drv/Makefile.in @@ -4,11 +4,13 @@ IMPORTS = uuid rpcrt4 user32 gdi32 win32u DELAYIMPORTS = ole32 shell32 imm32 UNIX_LIBS = \ -lwin32u \ + -framework Accelerate \ -framework AppKit \ -framework Carbon \ -framework CoreVideo \ -framework Foundation \ -framework IOKit \ + -framework IOSurface \ -framework Metal \ -framework OpenGL \ -framework QuartzCore \ diff --git a/dlls/winemac.drv/cocoa_window.m b/dlls/winemac.drv/cocoa_window.m index c55de3ecac0..bb73c611910 100644 --- a/dlls/winemac.drv/cocoa_window.m +++ b/dlls/winemac.drv/cocoa_window.m @@ -359,8 +359,8 @@ - (id) initWithFrame:(NSRect)frame device:(id<MTLDevice>)device; @interface WineContentView : WineBaseView <NSTextInputClient, NSViewLayerContentScaleDelegate> { CGRect surfaceRect; - CGImageRef colorImage; - CGImageRef shapeImage; + IOSurfaceRef _IOSurface; + BOOL _hasShape; NSMutableArray* glContexts; NSMutableArray* pendingGlContexts; @@ -496,8 +496,6 @@ - (void) dealloc [markedText release]; [glContexts release]; [pendingGlContexts release]; - CGImageRelease(colorImage); - CGImageRelease(shapeImage); [super dealloc]; } @@ -530,26 +528,17 @@ - (void) updateLayer imageRect.size.width *= layer.contentsScale; imageRect.size.height *= layer.contentsScale; - maskedImage = shapeImage ? CGImageCreateWithMask(colorImage, shapeImage) - : CGImageRetain(colorImage); - image = CGImageCreateWithImageInRect(maskedImage, imageRect); - CGImageRelease(maskedImage); + layer.position = surfaceRect.origin; + layer.contents = (id)_IOSurface; + [window windowDidDrawContent]; - if (image) + // If the window may be transparent, then we have to invalidate the + // shadow every time we draw. Also, if this is the first time we've + // drawn since changing from transparent to opaque. + if (_hasShape || window.usePerPixelAlpha || window.shapeChangedSinceLastDraw) { - layer.position = surfaceRect.origin; - layer.contents = (id)image; - CFRelease(image); - [window windowDidDrawContent]; - - // If the window may be transparent, then we have to invalidate the - // shadow every time we draw. Also, if this is the first time we've - // drawn since changing from transparent to opaque. - if (shapeImage || window.usePerPixelAlpha || window.shapeChangedSinceLastDraw) - { - window.shapeChangedSinceLastDraw = FALSE; - [window invalidateShadow]; - } + window.shapeChangedSinceLastDraw = FALSE; + [window invalidateShadow]; } } @@ -558,21 +547,21 @@ - (void) setSurfaceRect:(CGRect)rect surfaceRect = rect; } - - (void) setColorImage:(CGImageRef)image + - (void) setIOSurface:(IOSurfaceRef)image { - CGImageRelease(colorImage); - colorImage = CGImageRetain(image); + CFRelease(_IOSurface); + _IOSurface = image; + CFRetain(_IOSurface); } - - (void) setShapeImage:(CGImageRef)image + - (void) setHasShape:(BOOL)has_shape { - CGImageRelease(shapeImage); - shapeImage = CGImageRetain(image); + _hasShape = has_shape; } - - (BOOL) hasShapeImage + - (BOOL) hasShape { - return !!shapeImage; + return _hasShape; } - (void) viewWillDraw @@ -2060,7 +2049,7 @@ - (void) setDisabled:(BOOL)newValue - (BOOL) needsTransparency { WineContentView *view = self.contentView; - return self.contentView.layer.mask || [view hasShapeImage] || self.usePerPixelAlpha || + return self.contentView.layer.mask || [view hasShape] || self.usePerPixelAlpha || (gl_surface_mode == GL_SURFACE_BEHIND && [view hasGLDescendant]); } @@ -3560,51 +3549,47 @@ void macdrv_set_cocoa_parent_window(macdrv_window w, macdrv_window parent) /*********************************************************************** - * macdrv_window_set_color_image + * macdrv_window_set_io_surface * * Push a window surface color pixel update in a specified rect (in non-client * area coordinates). */ -void macdrv_window_set_color_image(macdrv_window w, CGImageRef image, CGRect rect, CGRect dirty) +void macdrv_window_set_io_surface(macdrv_window w, IOSurfaceRef image, CGRect rect, CGRect dirty) { @autoreleasepool { WineWindow* window = (WineWindow*)w; - CGImageRetain(image); + CFRetain(image); OnMainThreadAsync(^{ WineContentView *view = [window contentView]; - [view setColorImage:image]; + [view setIOSurface:image]; [view setSurfaceRect:cgrect_mac_from_win(rect)]; [view setNeedsDisplayInRect:NSRectFromCGRect(cgrect_mac_from_win(dirty))]; - CGImageRelease(image); + CFRelease(image); }); } } /*********************************************************************** - * macdrv_window_set_shape_image + * macdrv_window_shape_changed */ -void macdrv_window_set_shape_image(macdrv_window w, CGImageRef image) +void macdrv_window_shape_changed(macdrv_window w, bool has_shape) { @autoreleasepool { WineWindow* window = (WineWindow*)w; - CGImageRetain(image); - OnMainThreadAsync(^{ WineContentView *view = [window contentView]; - [view setShapeImage:image]; + [view setHasShape:has_shape]; [view setNeedsDisplay:true]; [window checkTransparency]; - - CGImageRelease(image); }); } } diff --git a/dlls/winemac.drv/macdrv_cocoa.h b/dlls/winemac.drv/macdrv_cocoa.h index 6a6b6975f55..ef93b81be5c 100644 --- a/dlls/winemac.drv/macdrv_cocoa.h +++ b/dlls/winemac.drv/macdrv_cocoa.h @@ -44,8 +44,10 @@ # define ShowWindow MacShowWindow #endif +#include <Accelerate/Accelerate.h> #include <ApplicationServices/ApplicationServices.h> #include <Carbon/Carbon.h> +#include <IOSurface/IOSurface.h> #undef GetCurrentProcess #undef GetCurrentThread @@ -507,8 +509,8 @@ 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_window_set_color_image(macdrv_window w, CGImageRef image, CGRect rect, CGRect dirty); -extern void macdrv_window_set_shape_image(macdrv_window w, CGImageRef image); +extern void macdrv_window_set_io_surface(macdrv_window w, IOSurfaceRef image, CGRect rect, CGRect dirty); +extern void macdrv_window_shape_changed(macdrv_window w, bool has_shape); 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_window_use_per_pixel_alpha(macdrv_window w, bool use_per_pixel_alpha); diff --git a/dlls/winemac.drv/surface.c b/dlls/winemac.drv/surface.c index bf2acf31431..ae4746bbd9d 100644 --- a/dlls/winemac.drv/surface.c +++ b/dlls/winemac.drv/surface.c @@ -48,6 +48,9 @@ struct macdrv_window_surface struct window_surface header; macdrv_window window; CGDataProviderRef provider; + IOSurfaceRef front_buffer; + IOSurfaceRef back_buffer; + BOOL shape_changed; }; static struct macdrv_window_surface *get_mac_surface(struct window_surface *surface); @@ -85,39 +88,80 @@ static BOOL macdrv_surface_flush(struct window_surface *window_surface, const RE CGImageAlphaInfo alpha_info = (window_surface->alpha_mask ? kCGImageAlphaPremultipliedFirst : kCGImageAlphaNoneSkipFirst); CGColorSpaceRef colorspace; CGImageRef image; + IOSurfaceRef io_surface = surface->back_buffer; - colorspace = CGColorSpaceCreateWithName(kCGColorSpaceSRGB); - 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); + /* Swap the front and back buffers, so we are not writing into the presented surface */ + surface->back_buffer = surface->front_buffer; + surface->front_buffer = io_surface; + IOSurfaceLock(io_surface, 0, NULL); - macdrv_window_set_color_image(surface->window, image, cgrect_from_rect(*rect), cgrect_from_rect(*dirty)); - CGImageRelease(image); + /* First only copy the RBG componenets from color_bits as it uses BGRX */ + { + vImage_Buffer src = { + .data = color_bits, + .height = IOSurfaceGetHeight(io_surface), + .width = IOSurfaceGetWidth(io_surface), + .rowBytes = IOSurfaceGetBytesPerRow(io_surface), + }; + vImage_Buffer dst = { + .data = IOSurfaceGetBaseAddress(io_surface), + .height = IOSurfaceGetHeight(io_surface), + .width = IOSurfaceGetWidth(io_surface), + .rowBytes = IOSurfaceGetBytesPerRow(io_surface), + }; + vImageSelectChannels_ARGB8888(&src, &dst, &dst, 0x8 | 0x4 | 0x2, kvImageNoFlags); + } - if (shape_changed) + /* Also update the shape, when it changed during the last flush */ + if (shape_changed || surface->shape_changed) { + surface->shape_changed = FALSE; + + /* No shape, fully opaque */ if (!shape_bits) - macdrv_window_set_shape_image(surface->window, NULL); + { + Pixel_8888 alpha1 = { 0, 0, 0, 255 }; + vImage_Buffer dst = { + .data = IOSurfaceGetBaseAddress(io_surface), + .height = IOSurfaceGetHeight(io_surface), + .width = IOSurfaceGetWidth(io_surface), + .rowBytes = IOSurfaceGetBytesPerRow(io_surface), + }; + vImageOverwriteChannelsWithPixel_ARGB8888(alpha1, &dst, &dst, 0x1, kvImageNoFlags); + } + /* Extract the shape bits into the alpha component of the IOSurface. + * This is fairly slow, but only happens upon shape change into transparency. */ else { const BYTE *src = shape_bits; - CGDataProviderRef provider; - CGImageRef image; - BYTE *dst; - UINT i; + BYTE *dst = IOSurfaceGetBaseAddress(io_surface); + UINT x, y; + UINT src_row_bytes = shape_info->bmiHeader.biSizeImage / abs(shape_info->bmiHeader.biHeight); + + for (y = 0; y < IOSurfaceGetHeight(io_surface); y++) + { + const BYTE *src_row = src + y * src_row_bytes; + BYTE *dst_row = dst + y * IOSurfaceGetBytesPerRow(io_surface); + for (x = 0; x < IOSurfaceGetWidth(io_surface); x++) + { + BYTE bit = (src_row[x / 8] >> (7 - (x % 8))) & 1; + dst_row[x * 4] = bit ? 0 : 255; + } + } + } + } - if (!(provider = data_provider_create(shape_info->bmiHeader.biSizeImage, (void **)&dst))) return TRUE; - for (i = 0; i < shape_info->bmiHeader.biSizeImage; i++) dst[i] = ~src[i]; /* CGImage mask bits are inverted */ + IOSurfaceUnlock(io_surface, 0, NULL); + macdrv_window_set_io_surface(surface->window, io_surface, cgrect_from_rect(*rect), cgrect_from_rect(*dirty)); - image = CGImageMaskCreate(shape_info->bmiHeader.biWidth, abs(shape_info->bmiHeader.biHeight), 1, 1, - shape_info->bmiHeader.biSizeImage / abs(shape_info->bmiHeader.biHeight), - provider, NULL, retina_on); - CGDataProviderRelease(provider); + if (shape_changed) + { + surface->shape_changed = TRUE; - macdrv_window_set_shape_image(surface->window, image); - CGImageRelease(image); - } + if (!shape_bits) + macdrv_window_shape_changed(surface->window, FALSE); + else + macdrv_window_shape_changed(surface->window, TRUE); } return TRUE; @@ -132,6 +176,8 @@ static void macdrv_surface_destroy(struct window_surface *window_surface) TRACE("freeing %p\n", surface); CGDataProviderRelease(surface->provider); + CFRelease(surface->back_buffer); + CFRelease(surface->front_buffer); } static const struct window_surface_funcs macdrv_surface_funcs = @@ -163,6 +209,7 @@ static struct window_surface *create_surface(HWND hwnd, macdrv_window window, co HBITMAP bitmap = 0; UINT status; void *bits; + IOSurfaceRef io_surface1, io_surface2; memset(info, 0, sizeof(*info)); info->bmiHeader.biSize = sizeof(info->bmiHeader); @@ -175,6 +222,38 @@ static struct window_surface *create_surface(HWND hwnd, macdrv_window window, co if (!(provider = data_provider_create(info->bmiHeader.biSizeImage, &bits))) return NULL; window_background = macdrv_window_background_color(); + + { + CFDictionaryRef properties; + CFStringRef keys[] = { kIOSurfaceWidth, kIOSurfaceHeight, kIOSurfaceBytesPerElement, kIOSurfacePixelFormat }; + CFNumberRef values[4]; + uint32_t surfaceWidth = info->bmiHeader.biWidth; + uint32_t surfaceHeight = abs(info->bmiHeader.biHeight); + uint32_t surfaceBytesPerElement = 4; + uint32_t surfacePixelFormat = 0x42475241; /* kCVPixelFormatType_32BGRA */ + + values[0] = CFNumberCreate(NULL, kCFNumberSInt32Type, &surfaceWidth); + values[1] = CFNumberCreate(NULL, kCFNumberSInt32Type, &surfaceHeight); + values[2] = CFNumberCreate(NULL, kCFNumberSInt32Type, &surfaceBytesPerElement); + values[3] = CFNumberCreate(NULL, kCFNumberSInt32Type, &surfacePixelFormat); + + properties = CFDictionaryCreate(NULL, (void **)keys, (void **)values, ARRAY_SIZE(keys), &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); + io_surface1 = IOSurfaceCreate(properties); + io_surface2 = IOSurfaceCreate(properties); + CFRelease(properties); + CFRelease(values[0]); + CFRelease(values[1]); + CFRelease(values[2]); + CFRelease(values[3]); + + IOSurfaceLock(io_surface1, 0, NULL); + IOSurfaceLock(io_surface2, 0, NULL); + memset_pattern4(IOSurfaceGetBaseAddress(io_surface1), &window_background, info->bmiHeader.biSizeImage); + memset_pattern4(IOSurfaceGetBaseAddress(io_surface2), &window_background, info->bmiHeader.biSizeImage); + IOSurfaceUnlock(io_surface1, 0, NULL); + IOSurfaceUnlock(io_surface2, 0, NULL); + } + window_background &= 0x00ffffff; memset_pattern4(bits, &window_background, info->bmiHeader.biSizeImage); @@ -196,6 +275,8 @@ static struct window_surface *create_surface(HWND hwnd, macdrv_window window, co if (!(window_surface = window_surface_create(sizeof(*surface), &macdrv_surface_funcs, hwnd, rect, info, bitmap))) { if (bitmap) NtGdiDeleteObjectApp(bitmap); + CFRelease(io_surface1); + CFRelease(io_surface2); CGDataProviderRelease(provider); } else @@ -203,6 +284,9 @@ static struct window_surface *create_surface(HWND hwnd, macdrv_window window, co surface = get_mac_surface(window_surface); surface->window = window; surface->provider = provider; + surface->front_buffer = io_surface1; + surface->back_buffer = io_surface2; + surface->shape_changed = FALSE; } return window_surface; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7938
On Thu May 22 23:47:29 2025 +0000, Brendan Shanks wrote:
`WineWindow` should hold a ref to the IOSurface. There's an `IOSurface` ObjC class (toll-free bridged to IOSurfaceRef) which could save a few lines of code, or `CFRetain`/`CFRelease` should work too. I stuck with CF functions to keep things simple here for now
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7938#note_124664
On Thu May 22 23:47:29 2025 +0000, Brendan Shanks wrote:
Some comments describing what each of these operations does would be useful. Added some comments around here
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7938#note_124665
On Thu Dec 4 09:57:43 2025 +0000, Brendan Shanks wrote:
Need to unlock/lock `io_surface` around this call Fixed
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7938#note_124666
On Thu Dec 4 09:59:57 2025 +0000, Brendan Shanks wrote:
Just to stay alphabetical, this should go above Metal Should be fixed
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7938#note_124667
participants (2)
-
Marc-Aurel Zent -
Marc-Aurel Zent (＠mzent)