macOS: Defer update request on drawRect: when a real expose event is needed

The semantics of QWindow::requestUpdate() is that it's used when the
window needs updates outside of the normal window invalidation callbacks
such as expose and resize events, e.g. when doing animations.

As a result, user code might not be prepared to handle window
invalidations in the update-request callback, assuming those
will still be delivered as normal, so that's what we do.

This was exposed by resizing one of the simpler Qt Quick examples,
where the resize's expose event was delivered as an update request,
but didn't trigger an unconditional draw of the window as it should,
as the scenegraph didn't change in response to the resize, which is
typical for an update request.

Change-Id: Ida8f85f1cf61c332aa9b199520e6854c48d3ab40
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>

3 files changed, 24 insertions, 22 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoawindow.h b/src/plugins/platforms/cocoa/qcocoawindow.h
index 3230c32133..6fbe29f683 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.h
+++ b/src/plugins/platforms/cocoa/qcocoawindow.h
@@ -262,7 +262,7 @@ public: // for QNSView
 
     bool m_hasModalSession;
     bool m_frameStrutEventsEnabled;
-    bool m_isExposed;
+    QRect m_exposedRect;
     int m_registerTouchCount;
     bool m_resizableTransientParent;
 
diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm
index 8e5a9268ec..c825f03464 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.mm
+++ b/src/plugins/platforms/cocoa/qcocoawindow.mm
@@ -150,7 +150,6 @@ QCocoaWindow::QCocoaWindow(QWindow *win, WId nativeHandle)
     , m_needsInvalidateShadow(false)
     , m_hasModalSession(false)
     , m_frameStrutEventsEnabled(false)
-    , m_isExposed(false)
     , m_registerTouchCount(0)
     , m_resizableTransientParent(false)
     , m_alertRequest(NoAlertRequest)
@@ -692,7 +691,7 @@ void QCocoaWindow::lower()
 
 bool QCocoaWindow::isExposed() const
 {
-    return m_isExposed;
+    return !m_exposedRect.isEmpty();
 }
 
 bool QCocoaWindow::isOpaque() const
@@ -1120,7 +1119,7 @@ void QCocoaWindow::handleGeometryChange()
 
 void QCocoaWindow::handleExposeEvent(const QRegion &region)
 {
-    const bool wasExposed = isExposed();
+    const QRect previouslyExposedRect = m_exposedRect;
 
     // Ideally we'd implement isExposed() in terms of these properties,
     // plus the occlusionState of the NSWindow, and let the expose event
@@ -1133,27 +1132,29 @@ void QCocoaWindow::handleExposeEvent(const QRegion &region)
     // a window being obscured is an empty region, and in the case of
     // a drawRect call is a non-null region, even if occlusionState
     // is still hidden. This ensures the window is prepared for display.
-    m_isExposed = m_view.window.visible
-        && m_view.window.screen
-        && !geometry().size().isEmpty()
-        && !region.isEmpty()
-        && !m_view.hiddenOrHasHiddenAncestor;
+    if (m_view.window.visible && m_view.window.screen
+            && !geometry().size().isEmpty() && !region.isEmpty()
+            && !m_view.hiddenOrHasHiddenAncestor) {
+        m_exposedRect = region.boundingRect();
+    } else {
+        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 the first expose event after
-        // becoming exposed as a real expose event, otherwise the exposed state
-        // of the QWindow is never updated.
-        // FIXME: Should this logic live in QGuiApplication?
-        if (wasExposed && m_isExposed) {
+        // 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.
         }
-
-        // FIXME: Should we re-trigger setNeedsDisplay in case of !wasExposed && m_isExposed?
-        // Or possibly send the expose event first, and then the update request?
     }
 
     qCDebug(lcQpaCocoaWindow) << "QCocoaWindow::handleExposeEvent" << window() << region << "isExposed" << isExposed();
diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm
index 054dca122f..9708aa4ce6 100644
--- a/src/plugins/platforms/cocoa/qnsview.mm
+++ b/src/plugins/platforms/cocoa/qnsview.mm
@@ -324,12 +324,13 @@ Q_LOGGING_CATEGORY(lcQpaTablet, "qt.qpa.input.tablet")
 
     m_platformWindow->handleExposeEvent(exposedRegion);
 
-    // A call to QWindow::requestUpdate was issued during the expose event, 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?
     if (qt_window_private(m_platformWindow->window())->updateRequestPending) {
-        qCDebug(lcQpaCocoaWindow) << "[QNSView drawRect:] deferring setNeedsDisplay";
+        // 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
+        // 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];
         });