macOS: Don't throw away backingstore buffers when backing properties change

Clients such as QtWidgets that do their own dirty tracking will assume
they can just flush in response to the expose event, without repainting
anything. Since we have no way at the moment to inform these clients that
the backingstore content might be invalid we can't just throw it away.

It turns out that to pick up changes in color spaces we can just tag
the existing buffers with the new color space, so we don't need to
throw it away. And for the older surface-backed mode we tag the color
space on flush, so we didn't need to invalidate anything in the first
place.

Fixes: QTBUG-80844
Task-number: QTBUG-77749
Change-Id: Icb1ceb178894bb43887cdf03fb855d2d614b5ab0
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
(cherry picked from commit 7ac4e55cb979dff890f1575e771e5f2def9e3131)
(squashed in commit c027d9b61e15)

4 files changed, 30 insertions, 30 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoabackingstore.h b/src/plugins/platforms/cocoa/qcocoabackingstore.h
index a874936ce6..b57deacb57 100644
--- a/src/plugins/platforms/cocoa/qcocoabackingstore.h
+++ b/src/plugins/platforms/cocoa/qcocoabackingstore.h
@@ -54,8 +54,6 @@ class QCocoaBackingStore : public QRasterBackingStore
 protected:
     QCocoaBackingStore(QWindow *window);
     QCFType<CGColorSpaceRef> colorSpace() const;
-    QMacNotificationObserver m_backingPropertiesObserver;
-    virtual void backingPropertiesChanged() = 0;
 };
 
 class QNSWindowBackingStore : public QCocoaBackingStore
@@ -71,7 +69,6 @@ private:
     bool windowHasUnifiedToolbar() const;
     QImage::Format format() const override;
     void redrawRoundedBottomCorners(CGRect) const;
-    void backingPropertiesChanged() override;
 };
 
 class QCALayerBackingStore : public QCocoaBackingStore
@@ -118,7 +115,8 @@ private:
     bool recreateBackBufferIfNeeded();
     bool prepareForFlush();
 
-    void backingPropertiesChanged() override;
+    void backingPropertiesChanged();
+    QMacNotificationObserver m_backingPropertiesObserver;
 
     std::list<std::unique_ptr<GraphicsBuffer>> m_buffers;
 };
diff --git a/src/plugins/platforms/cocoa/qcocoabackingstore.mm b/src/plugins/platforms/cocoa/qcocoabackingstore.mm
index 2343f16b9f..1d2c78d750 100644
--- a/src/plugins/platforms/cocoa/qcocoabackingstore.mm
+++ b/src/plugins/platforms/cocoa/qcocoabackingstore.mm
@@ -51,17 +51,6 @@ QT_BEGIN_NAMESPACE
 QCocoaBackingStore::QCocoaBackingStore(QWindow *window)
     : QRasterBackingStore(window)
 {
-    // Ideally this would be plumbed from the platform layer to QtGui, and
-    // the QBackingStore would be recreated, but we don't have that code yet,
-    // so at least make sure we invalidate our backingstore when the backing
-    // properties (color space e.g.) are changed.
-    NSView *view = static_cast<QCocoaWindow *>(window->handle())->view();
-    m_backingPropertiesObserver = QMacNotificationObserver(view.window,
-        NSWindowDidChangeBackingPropertiesNotification, [this]() {
-            qCDebug(lcQpaBackingStore) << "Backing properties for"
-                << this->window() << "did change";
-            backingPropertiesChanged();
-        });
 }
 
 QCFType<CGColorSpaceRef> QCocoaBackingStore::colorSpace() const
@@ -341,11 +330,6 @@ void QNSWindowBackingStore::redrawRoundedBottomCorners(CGRect windowRect) const
 #endif
 }
 
-void QNSWindowBackingStore::backingPropertiesChanged()
-{
-    m_image = QImage();
-}
-
 // ----------------------------------------------------------------------------
 
 // https://stackoverflow.com/a/52722575/2761869
@@ -365,6 +349,18 @@ QCALayerBackingStore::QCALayerBackingStore(QWindow *window)
 {
     qCDebug(lcQpaBackingStore) << "Creating QCALayerBackingStore for" << window;
     m_buffers.resize(1);
+
+    // Ideally this would be plumbed from the platform layer to QtGui, and
+    // the QBackingStore would be recreated, but we don't have that code yet,
+    // so at least make sure we update our backingstore when the backing
+    // properties (color space e.g.) are changed.
+    NSView *view = static_cast<QCocoaWindow *>(window->handle())->view();
+    m_backingPropertiesObserver = QMacNotificationObserver(view.window,
+        NSWindowDidChangeBackingPropertiesNotification, [this]() {
+            qCDebug(lcQpaBackingStore) << "Backing properties for"
+                << this->window() << "did change";
+            backingPropertiesChanged();
+        });
 }
 
 QCALayerBackingStore::~QCALayerBackingStore()
@@ -632,8 +628,9 @@ QImage QCALayerBackingStore::toImage() const
 
 void QCALayerBackingStore::backingPropertiesChanged()
 {
-    m_buffers.clear();
-    m_buffers.resize(1);
+    qCDebug(lcQpaBackingStore) << "Updating color space of existing buffers";
+    for (auto &buffer : m_buffers)
+        buffer->setColorSpace(colorSpace());
 }
 
 QPlatformGraphicsBuffer *QCALayerBackingStore::graphicsBuffer() const
@@ -710,10 +707,11 @@ bool QCALayerBackingStore::prepareForFlush()
 
 QCALayerBackingStore::GraphicsBuffer::GraphicsBuffer(const QSize &size, qreal devicePixelRatio,
                                 const QPixelFormat &format, QCFType<CGColorSpaceRef> colorSpace)
-    : QIOSurfaceGraphicsBuffer(size, format, colorSpace)
+    : QIOSurfaceGraphicsBuffer(size, format)
     , dirtyRegion(0, 0, size.width() / devicePixelRatio, size.height() / devicePixelRatio)
     , m_devicePixelRatio(devicePixelRatio)
 {
+    setColorSpace(colorSpace);
 }
 
 QImage *QCALayerBackingStore::GraphicsBuffer::asImage()
diff --git a/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.h b/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.h
index 872773cb7a..e070ba977d 100644
--- a/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.h
+++ b/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.h
@@ -48,9 +48,11 @@ QT_BEGIN_NAMESPACE
 class QIOSurfaceGraphicsBuffer : public QPlatformGraphicsBuffer
 {
 public:
-    QIOSurfaceGraphicsBuffer(const QSize &size, const QPixelFormat &format, QCFType<CGColorSpaceRef> colorSpace);
+    QIOSurfaceGraphicsBuffer(const QSize &size, const QPixelFormat &format);
     ~QIOSurfaceGraphicsBuffer();
 
+    void setColorSpace(QCFType<CGColorSpaceRef> colorSpace);
+
     const uchar *data() const override;
     uchar *data() override;
     int bytesPerLine() const override;
diff --git a/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.mm b/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.mm
index a367487e85..285e316d4a 100644
--- a/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.mm
+++ b/src/plugins/platforms/cocoa/qiosurfacegraphicsbuffer.mm
@@ -53,7 +53,7 @@ QT_BEGIN_NAMESPACE
 
 Q_LOGGING_CATEGORY(lcQpaIOSurface, "qt.qpa.backingstore.iosurface");
 
-QIOSurfaceGraphicsBuffer::QIOSurfaceGraphicsBuffer(const QSize &size, const QPixelFormat &format, QCFType<CGColorSpaceRef> colorSpace)
+QIOSurfaceGraphicsBuffer::QIOSurfaceGraphicsBuffer(const QSize &size, const QPixelFormat &format)
     : QPlatformGraphicsBuffer(size, format)
 {
     const size_t width = size.width();
@@ -81,17 +81,19 @@ QIOSurfaceGraphicsBuffer::QIOSurfaceGraphicsBuffer(const QSize &size, const QPix
 
     Q_ASSERT(size_t(bytesPerLine()) == bytesPerRow);
     Q_ASSERT(size_t(byteCount()) == totalBytes);
-
-    if (colorSpace) {
-        IOSurfaceSetValue(m_surface, CFSTR("IOSurfaceColorSpace"),
-            QCFType<CFPropertyListRef>(CGColorSpaceCopyPropertyList(colorSpace)));
-    }
 }
 
 QIOSurfaceGraphicsBuffer::~QIOSurfaceGraphicsBuffer()
 {
 }
 
+void QIOSurfaceGraphicsBuffer::setColorSpace(QCFType<CGColorSpaceRef> colorSpace)
+{
+    Q_ASSERT(colorSpace);
+    IOSurfaceSetValue(m_surface, CFSTR("IOSurfaceColorSpace"),
+        QCFType<CFPropertyListRef>(CGColorSpaceCopyPropertyList(colorSpace)));
+}
+
 const uchar *QIOSurfaceGraphicsBuffer::data() const
 {
     return (const uchar *)IOSurfaceGetBaseAddress(m_surface);