From 2832ad2f9dd15c7c2e8effdbbd1d24517e6ce53c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Fri, 26 Jan 2024 17:22:34 +0100 Subject: [PATCH] macOS: Expand and tighten plumbing of QWindow's requested color space MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A QWindow can have a requested target color space via its QSurfaceFormat. This allows users who know what color space their source material is in, or who do their own color matching, to ensure a consistent target color space. In the past we plumbed this to NSWindow's colorSpace property. This meant that you could only have one color space per top level, even if child windows were in play, and we could only set the color space if we were the ones creating the NSWindow, failing the color space request in cases where the window was embedded in a non-Qt window hierarchy. We now store the requested color space in our QNSView, and propagate it to both the IOSurfaces we use in QCALayerBackingStore, and to the view's layer, in case it's a CAMetalLayer. We also pick up any changes to the backing properties of the view, and ensure we update the color space accordingly. We still propagate the color space to NSWindow, as for OpenGL we don't use CAOpenGLLayer (which has a colorSpace property), but instead use NSOpenGLContext. This is not something we're going to change, so as a workaround we set the NSWindow color space, which does affect GL drawing via NSOpenGLContext. The granular color spaces we set on the IOSurfaces and CAMetalLayer will override the NSWindow state. Change-Id: I5d9765d95140b8523ee09f70ff09a8c9400ffdc7 Reviewed-by: Pavel Dubsky Reviewed-by: Allan Sandfeld Jensen Reviewed-by: Artem Dyomin Reviewed-by: Jøger Hansegård (cherry picked from commit ef378898454964ce4d66d2656d2d5e4c7ac69b22) Reviewed-by: Qt Cherry-pick Bot --- .../platforms/cocoa/qcocoabackingstore.mm | 15 ++++---- src/plugins/platforms/cocoa/qcocoawindow.mm | 14 +++++--- src/plugins/platforms/cocoa/qnsview.h | 6 ++++ src/plugins/platforms/cocoa/qnsview.mm | 3 ++ .../platforms/cocoa/qnsview_drawing.mm | 35 ++++++++++++++----- 5 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/plugins/platforms/cocoa/qcocoabackingstore.mm b/src/plugins/platforms/cocoa/qcocoabackingstore.mm index 6970c44a12a..b211b5d02dd 100644 --- a/src/plugins/platforms/cocoa/qcocoabackingstore.mm +++ b/src/plugins/platforms/cocoa/qcocoabackingstore.mm @@ -23,8 +23,9 @@ QCocoaBackingStore::QCocoaBackingStore(QWindow *window) QCFType QCocoaBackingStore::colorSpace() const { - NSView *view = static_cast(window()->handle())->view(); - return QCFType::constructFromGet(view.window.colorSpace.CGColorSpace); + const auto *platformWindow = static_cast(window()->handle()); + const QNSView *view = qnsview_cast(platformWindow->view()); + return QCFType::constructFromGet(view.colorSpace.CGColorSpace); } // ---------------------------------------------------------------------------- @@ -188,8 +189,9 @@ bool QCALayerBackingStore::recreateBackBufferIfNeeded() } #endif - qCInfo(lcQpaBackingStore) << "Creating surface of" << requestedBufferSize - << "based on requested" << m_requestedSize << "and dpr =" << devicePixelRatio; + qCInfo(lcQpaBackingStore)<< "Creating surface of" << requestedBufferSize + << "for" << window() << "based on requested" << m_requestedSize + << "dpr =" << devicePixelRatio << "and color space" << colorSpace(); static auto pixelFormat = QImage::toPixelFormat(QImage::Format_ARGB32_Premultiplied); auto *newBackBuffer = new GraphicsBuffer(requestedBufferSize, devicePixelRatio, pixelFormat, colorSpace()); @@ -477,10 +479,11 @@ void QCALayerBackingStore::backingPropertiesChanged() qCDebug(lcQpaBackingStore) << "Backing properties for" << window() << "did change"; - qCDebug(lcQpaBackingStore) << "Updating color space of existing buffers"; + const auto newColorSpace = colorSpace(); + qCDebug(lcQpaBackingStore) << "Updating color space of existing buffers to" << newColorSpace; for (auto &buffer : m_buffers) { if (buffer) - buffer->setColorSpace(colorSpace()); + buffer->setColorSpace(newColorSpace); } } diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm index 14c72fd91a6..4a245a0f8af 100644 --- a/src/plugins/platforms/cocoa/qcocoawindow.mm +++ b/src/plugins/platforms/cocoa/qcocoawindow.mm @@ -1812,11 +1812,15 @@ QCocoaNSWindow *QCocoaWindow::createNSWindow(bool shouldBePanel) applyContentBorderThickness(nsWindow); - if (QColorSpace colorSpace = format().colorSpace(); colorSpace.isValid()) { - NSData *iccData = colorSpace.iccProfile().toNSData(); - nsWindow.colorSpace = [[[NSColorSpace alloc] initWithICCProfileData:iccData] autorelease]; - qCDebug(lcQpaDrawing) << "Set" << this << "color space to" << nsWindow.colorSpace; - } + // We propagate the view's color space granulary to both the IOSurfaces + // used for QSurface::RasterSurface, as well as the CAMetalLayer used for + // QSurface::MetalSurface, but for QSurface::OpenGLSurface we don't have + // that option as we use NSOpenGLContext instead of CAOpenGLLayer. As a + // workaround we set the NSWindow's color space, which affects GL drawing + // with NSOpenGLContext as well. This does not conflict with the granular + // modifications we do to each surface for raster or Metal. + if (auto *qtView = qnsview_cast(m_view)) + nsWindow.colorSpace = qtView.colorSpace; return nsWindow; } diff --git a/src/plugins/platforms/cocoa/qnsview.h b/src/plugins/platforms/cocoa/qnsview.h index e41f5a72966..7f845a5c3bc 100644 --- a/src/plugins/platforms/cocoa/qnsview.h +++ b/src/plugins/platforms/cocoa/qnsview.h @@ -32,6 +32,12 @@ QT_DECLARE_NAMESPACED_OBJC_INTERFACE(QNSView, NSView - (void)cancelComposingText; @end +Q_FORWARD_DECLARE_OBJC_CLASS(NSColorSpace); + +@interface QNSView (DrawingAPI) +@property (nonatomic, readonly) NSColorSpace *colorSpace; +@end + @interface QNSView (QtExtras) @property (nonatomic, readonly) QCocoaWindow *platformWindow; @end diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm index b3b3ec40cd1..8450b3d4207 100644 --- a/src/plugins/platforms/cocoa/qnsview.mm +++ b/src/plugins/platforms/cocoa/qnsview.mm @@ -90,6 +90,7 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QNSViewMenuHelper); @property (assign) NSView* previousSuperview; @property (assign) NSWindow* previousWindow; @property (retain) QNSViewMenuHelper* menuHelper; +@property (nonatomic, retain) NSColorSpace *colorSpace; @end @implementation QNSView { @@ -119,6 +120,8 @@ QT_NAMESPACE_ALIAS_OBJC_CLASS(QNSViewMenuHelper); NSDraggingContext m_lastSeenContext; } +@synthesize colorSpace = m_colorSpace; + - (instancetype)initWithCocoaWindow:(QCocoaWindow *)platformWindow { if ((self = [super initWithFrame:NSZeroRect])) { diff --git a/src/plugins/platforms/cocoa/qnsview_drawing.mm b/src/plugins/platforms/cocoa/qnsview_drawing.mm index 77fa8259acc..bf102e43f85 100644 --- a/src/plugins/platforms/cocoa/qnsview_drawing.mm +++ b/src/plugins/platforms/cocoa/qnsview_drawing.mm @@ -13,6 +13,14 @@ << " QT_MAC_WANTS_LAYER/_q_mac_wantsLayer has no effect."; } + // Pick up and persist requested color space from surface format + const QSurfaceFormat surfaceFormat = m_platformWindow->format(); + if (QColorSpace colorSpace = surfaceFormat.colorSpace(); colorSpace.isValid()) { + NSData *iccData = colorSpace.iccProfile().toNSData(); + self.colorSpace = [[[NSColorSpace alloc] initWithICCProfileData:iccData] autorelease]; + } + + // Trigger creation of the layer self.wantsLayer = YES; } @@ -28,6 +36,12 @@ return YES; } +- (NSColorSpace*)colorSpace +{ + // If no explicit color space was set, use the NSWindow's color space + return m_colorSpace ? m_colorSpace : self.window.colorSpace; +} + // ----------------------- Layer setup ----------------------- - (BOOL)shouldUseMetalLayer @@ -93,12 +107,7 @@ [super setLayer:layer]; - // When adding a view to a view hierarchy the backing properties will change - // which results in updating the contents scale, but in case of switching the - // layer on a view that's already in a view hierarchy we need to manually ensure - // the scale is up to date. - if (self.superview) - [self updateLayerContentsScale]; + [self propagateBackingProperties]; if (self.opaque && lcQpaDrawing().isDebugEnabled()) { // If the view claims to be opaque we expect it to fill the entire @@ -131,8 +140,7 @@ { qCDebug(lcQpaDrawing) << "Backing properties changed for" << self; - if (self.layer) - [self updateLayerContentsScale]; + [self propagateBackingProperties]; // Ideally we would plumb this situation through QPA in a way that lets // clients invalidate their own caches, recreate QBackingStore, etc. @@ -141,8 +149,11 @@ [self setNeedsDisplay:YES]; } -- (void)updateLayerContentsScale +- (void)propagateBackingProperties { + if (!self.layer) + return; + // We expect clients to fill the layer with retina aware content, // based on the devicePixelRatio of the QWindow, so we set the // layer's content scale to match that. By going via devicePixelRatio @@ -153,6 +164,12 @@ auto devicePixelRatio = m_platformWindow->devicePixelRatio(); qCDebug(lcQpaDrawing) << "Updating" << self.layer << "content scale to" << devicePixelRatio; self.layer.contentsScale = devicePixelRatio; + + if ([self.layer isKindOfClass:CAMetalLayer.class]) { + CAMetalLayer *metalLayer = static_cast(self.layer); + metalLayer.colorspace = self.colorSpace.CGColorSpace; + qCDebug(lcQpaDrawing) << "Set" << metalLayer << "color space to" << metalLayer.colorspace; + } } /*