macOS: Handle update requests via setNeedsDisplay more carefully

Instead of trying to detect situations where we need to send a real
expose event instead of an update request in response to a drawRect
call, we keep track of when we've asked the view to display due to
a requestUpdate call, and only deliver the corresponding drawRect
as an update request if no other code has asked the view to
display.

This should cover all cases of asking the view to display, issued
from our code or from AppKit, such as the view changing its backing
scale factor, or otherwise needing a real expose event.

Task-number: QTBUG-65663
Change-Id: I1783787823aee889dad8e48f34a1cb0f1b7b06bd
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

3 files changed, 39 insertions, 27 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm
index 92df6eac49..af2931c747 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.mm
+++ b/src/plugins/platforms/cocoa/qcocoawindow.mm
@@ -1157,23 +1157,6 @@ void QCocoaWindow::handleExposeEvent(const QRegion &region)
         m_exposedRect = QRect();
     }
 
-    QWindowPrivate *windowPrivate = qt_window_private(window());
-    if (windowPrivate->updateRequestPending) {
-        // We can only deliver update request events when the window is exposed,
-        // and we also have to make sure we deliver any change to the exposed
-        // rect as a real expose event (including going from non-exposed to
-        // exposed). FIXME: Should this logic live in QGuiApplication?
-        if (isExposed() && m_exposedRect == previouslyExposedRect) {
-            qCDebug(lcQpaCocoaWindow) << "QCocoaWindow::handleExposeEvent" << window() << region << "as update request";
-            windowPrivate->deliverUpdateRequest();
-            return;
-        } else {
-            // Since updateRequestPending is still set, we will issue a deferred setNeedsDisplay
-            // from drawRect and get back into this code on the next display cycle, delivering
-            // the pending update request.
-        }
-    }
-
     qCDebug(lcQpaCocoaWindow) << "QCocoaWindow::handleExposeEvent" << window() << region << "isExposed" << isExposed();
     QWindowSystemInterface::handleExposeEvent<QWindowSystemInterface::SynchronousDelivery>(window(), region);
 }
@@ -1348,7 +1331,7 @@ void QCocoaWindow::recreateWindowIfNeeded()
 void QCocoaWindow::requestUpdate()
 {
     qCDebug(lcQpaCocoaWindow) << "QCocoaWindow::requestUpdate" << window();
-    [m_view setNeedsDisplay:YES];
+    [m_view requestUpdate];
 }
 
 void QCocoaWindow::requestActivateWindow()
diff --git a/src/plugins/platforms/cocoa/qnsview.h b/src/plugins/platforms/cocoa/qnsview.h
index f8903725a6..e2ea862cd5 100644
--- a/src/plugins/platforms/cocoa/qnsview.h
+++ b/src/plugins/platforms/cocoa/qnsview.h
@@ -81,6 +81,7 @@ Q_FORWARD_DECLARE_OBJC_CLASS(QT_MANGLE_NAMESPACE(QNSViewMouseMoveHelper));
     NSEvent *m_currentlyInterpretedKeyEvent;
     bool m_isMenuView;
     QSet<quint32> m_acceptedKeyDowns;
+    bool m_updateRequested;
 }
 
 @property (nonatomic, retain) NSCursor *cursor;
@@ -105,6 +106,8 @@ Q_FORWARD_DECLARE_OBJC_CLASS(QT_MANGLE_NAMESPACE(QNSViewMouseMoveHelper));
 
 - (void)resetMouseButtons;
 
+- (void)requestUpdate;
+
 - (void)handleMouseEvent:(NSEvent *)theEvent;
 - (bool)handleMouseDownEvent:(NSEvent *)theEvent withButton:(int)buttonNumber;
 - (bool)handleMouseDraggedEvent:(NSEvent *)theEvent withButton:(int)buttonNumber;
diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm
index 61f0012a2d..e3bee95ad9 100644
--- a/src/plugins/platforms/cocoa/qnsview.mm
+++ b/src/plugins/platforms/cocoa/qnsview.mm
@@ -147,6 +147,7 @@ Q_LOGGING_CATEGORY(lcQpaTablet, "qt.qpa.input.tablet")
         m_isMenuView = false;
         self.focusRingType = NSFocusRingTypeNone;
         self.cursor = nil;
+        m_updateRequested = false;
     }
     return self;
 }
@@ -299,6 +300,25 @@ Q_LOGGING_CATEGORY(lcQpaTablet, "qt.qpa.input.tablet")
     return m_platformWindow->isOpaque();
 }
 
+- (void)requestUpdate
+{
+    if (self.needsDisplay) {
+        // If the view already has needsDisplay set it means that there may be code waiting for
+        // a real expose event, so we can't issue setNeedsDisplay now as a way to trigger an
+        // update request. We will re-trigger requestUpdate from drawRect.
+        return;
+    }
+
+    [self setNeedsDisplay:YES];
+    m_updateRequested = true;
+}
+
+- (void)setNeedsDisplayInRect:(NSRect)rect
+{
+    [super setNeedsDisplayInRect:rect];
+    m_updateRequested = false;
+}
+
 - (void)drawRect:(NSRect)dirtyRect
 {
     Q_UNUSED(dirtyRect);
@@ -322,18 +342,24 @@ Q_LOGGING_CATEGORY(lcQpaTablet, "qt.qpa.input.tablet")
     }
 #endif
 
-    m_platformWindow->handleExposeEvent(exposedRegion);
+    QWindowPrivate *windowPrivate = qt_window_private(m_platformWindow->window());
+
+    if (m_updateRequested) {
+        Q_ASSERT(windowPrivate->updateRequestPending);
+        qCDebug(lcQpaCocoaWindow) << "Delivering update request to" << m_platformWindow->window();
+        windowPrivate->deliverUpdateRequest();
+        m_updateRequested = false;
+    } else {
+        m_platformWindow->handleExposeEvent(exposedRegion);
+    }
 
-    if (qt_window_private(m_platformWindow->window())->updateRequestPending) {
-        // A call to QWindow::requestUpdate was issued during the expose event, or we
-        // had to deliver a real expose event and still need to deliver the update.
-        // But AppKit will reset the needsDisplay state of the view after completing
+    if (windowPrivate->updateRequestPending) {
+        // A call to QWindow::requestUpdate was issued during event delivery above,
+        // but AppKit will reset the needsDisplay state of the view after completing
         // the current display cycle, so we need to defer the request to redisplay.
         // FIXME: Perhaps this should be a trigger to enable CADisplayLink?
-        qCDebug(lcQpaCocoaWindow) << "[QNSView drawRect:] issuing deferred setNeedsDisplay due to pending update request";
-        dispatch_async(dispatch_get_main_queue (), ^{
-            [self setNeedsDisplay:YES];
-        });
+        qCDebug(lcQpaCocoaWindow) << "Pending update request, triggering re-display";
+        dispatch_async(dispatch_get_main_queue (), ^{ [self requestUpdate]; });
     }
 }