macOS: Send expose event at drawRect and trigger updates via setNeedsDisplay

This changes the drawing model on macOS from the following:

 1. Sending synchronous expose events directly from callbacks such as
    windowDidOrderOnScreen and windowDidChangeOcclusionState
 2. Waiting for a resulting flush of the backing store, and issuing
    setNeedsDisplay as a response
 3. Waiting for the asynchronous drawRect call in response to
    setNeedsDisplay, where the backing store is finally drawn
    to the window

To the following:

 1. Issue setNeedsDisplay as a response to callbacks such as
    windowDidOrderOnScreen and windowDidChangeOcclusionState,
    when needed (in many cases this is automatic by AppKit)
 2. Send synchronous expose events from the resulting drawRect
    callback
 3. Draw the backing store to the window when flushed

The new model matches how normal Cocoa application draw in response to
drawRect, and makes the backing store flush synchronous instead of having
to trigger a async setNeedsDisplay. This gives AppKit more information
about how much time we're spending in drawRect, as the actual drawing
and flushing all happens within the synchronous expose event.

Qt applications that draw outside of drawRect, e.g. in response to timers,
are still supported by manually locking focus of the view and flushing the
window at the end of the backingstore flush.

Task-number: QTBUG-50414
Change-Id: I2efb9ff8df51ab6e840ad20c497b71f53e21e1c2
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

4 files changed, 80 insertions, 116 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoawindow.h b/src/plugins/platforms/cocoa/qcocoawindow.h
index e2fb372dae..e20f033a43 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.h
+++ b/src/plugins/platforms/cocoa/qcocoawindow.h
@@ -188,10 +188,6 @@ public:
     void updateNSToolbar();
 
     qreal devicePixelRatio() const Q_DECL_OVERRIDE;
-    bool isWindowExposable();
-    void exposeWindow();
-    void obscureWindow();
-    void updateExposedGeometry();
     QWindow *childWindowAt(QPoint windowPoint);
     bool shouldRefuseKeyWindowAndFirstResponder();
 
@@ -238,6 +234,7 @@ public: // for QNSView
 
     void handleGeometryChange();
     void handleWindowStateChanged(HandleFlags flags = NoHandleFlags);
+    void handleExposeEvent(const QRegion &region);
 
     NSView *m_view;
     QCocoaNSWindow *m_nsWindow;
@@ -264,10 +261,7 @@ public: // for QNSView
 
     bool m_hasModalSession;
     bool m_frameStrutEventsEnabled;
-    bool m_geometryUpdateExposeAllowed;
     bool m_isExposed;
-    QRect m_exposedGeometry;
-    qreal m_exposedDevicePixelRatio;
     int m_registerTouchCount;
     bool m_resizableTransientParent;
 
diff --git a/src/plugins/platforms/cocoa/qcocoawindow.mm b/src/plugins/platforms/cocoa/qcocoawindow.mm
index 6de316560e..86599a2529 100644
--- a/src/plugins/platforms/cocoa/qcocoawindow.mm
+++ b/src/plugins/platforms/cocoa/qcocoawindow.mm
@@ -152,7 +152,6 @@ QCocoaWindow::QCocoaWindow(QWindow *win, WId nativeHandle)
     , m_windowCursor(0)
     , m_hasModalSession(false)
     , m_frameStrutEventsEnabled(false)
-    , m_geometryUpdateExposeAllowed(false)
     , m_isExposed(false)
     , m_registerTouchCount(0)
     , m_resizableTransientParent(false)
@@ -347,13 +346,6 @@ void QCocoaWindow::setVisible(bool visible)
 
         }
 
-        // This call is here to handle initial window show correctly:
-        // - top-level windows need to have backing store content ready when the
-        //   window is shown, sendin the expose event here makes that more likely.
-        // - QNSViews for child windows are initialy not hidden and won't get the
-        //   viewDidUnhide message.
-        exposeWindow();
-
         if (isContentView()) {
             QWindowSystemInterface::flushWindowSystemEvents(QEventLoop::ExcludeUserInputEvents);
 
@@ -877,7 +869,7 @@ void QCocoaWindow::viewDidChangeFrame()
 */
 void QCocoaWindow::viewDidChangeGlobalFrame()
 {
-    updateExposedGeometry();
+    [m_view setNeedsDisplay:YES];
 }
 
 void QCocoaWindow::windowDidEndLiveResize()
@@ -975,20 +967,20 @@ void QCocoaWindow::windowDidExitFullScreen()
 
 void QCocoaWindow::windowDidOrderOffScreen()
 {
-    obscureWindow();
+    handleExposeEvent(QRegion());
 }
 
 void QCocoaWindow::windowDidOrderOnScreen()
 {
-    exposeWindow();
+    [m_view setNeedsDisplay:YES];
 }
 
 void QCocoaWindow::windowDidChangeOcclusionState()
 {
     if (m_view.window.occlusionState & NSWindowOcclusionStateVisible)
-        exposeWindow();
+        [m_view setNeedsDisplay:YES];
     else
-        obscureWindow();
+        handleExposeEvent(QRegion());
 }
 
 void QCocoaWindow::windowDidChangeScreen()
@@ -998,8 +990,6 @@ void QCocoaWindow::windowDidChangeScreen()
 
     if (QCocoaScreen *cocoaScreen = QCocoaIntegration::instance()->screenForNSScreen(m_view.window.screen))
         QWindowSystemInterface::handleWindowScreenChanged(window(), cocoaScreen->screen());
-
-    updateExposedGeometry();
 }
 
 void QCocoaWindow::windowWillClose()
@@ -1064,7 +1054,6 @@ void QCocoaWindow::handleGeometryChange()
                                << "current" << geometry() << "new" << newGeometry;
 
     QWindowSystemInterface::handleGeometryChange(window(), newGeometry);
-    updateExposedGeometry();
 
     // Guard against processing window system events during QWindow::setGeometry
     // calls, which Qt and Qt applications do not expect.
@@ -1074,6 +1063,29 @@ void QCocoaWindow::handleGeometryChange()
         [qnsview_cast(m_view) clearBackingStore];
 }
 
+void QCocoaWindow::handleExposeEvent(const QRegion &region)
+{
+    // Ideally we'd implement isExposed() in terms of these properties,
+    // plus the occlusionState of the NSWindow, and let the expose event
+    // pull the exposed state out when needed. However, when the window
+    // is first shown we receive a drawRect call where the occlusionState
+    // of the window is still hidden, but we still want to prepare the
+    // window for display by issuing an expose event to Qt. To work around
+    // this we don't use the occlusionState directly, but instead base
+    // the exposed state on the region we get in, which in the case of
+    // 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;
+
+    qCDebug(lcQpaCocoaWindow) << "QCocoaWindow::handleExposeEvent" << window() << region << "isExposed" << isExposed();
+    QWindowSystemInterface::handleExposeEvent<QWindowSystemInterface::SynchronousDelivery>(window(), region);
+}
+
 void QCocoaWindow::handleWindowStateChanged(HandleFlags flags)
 {
     Qt::WindowState currentState = windowState();
@@ -1655,75 +1667,6 @@ qreal QCocoaWindow::devicePixelRatio() const
     return backingSize.height;
 }
 
-// Returns whether the window can be expose, which it can
-// if it is on screen and has a valid geometry.
-bool QCocoaWindow::isWindowExposable()
-{
-    QSize size = geometry().size();
-    bool validGeometry = (size.width() > 0 && size.height() > 0);
-    bool validScreen = ([[m_view window] screen] != 0);
-    bool nonHiddenSuperView = ![[m_view superview] isHidden];
-    return (validGeometry && validScreen && nonHiddenSuperView);
-}
-
-// Exposes the window by posting an expose event to QWindowSystemInterface
-void QCocoaWindow::exposeWindow()
-{
-    m_geometryUpdateExposeAllowed = true;
-
-    if (!isWindowExposable())
-        return;
-
-    if (!m_isExposed) {
-        m_isExposed = true;
-        m_exposedGeometry = geometry();
-        m_exposedDevicePixelRatio = devicePixelRatio();
-        QRect geometry(QPoint(0, 0), m_exposedGeometry.size());
-        qCDebug(lcQpaCocoaWindow) << "QCocoaWindow: exposeWindow" << window() << geometry;
-        QWindowSystemInterface::handleExposeEvent(window(), geometry);
-    }
-}
-
-// Obscures the window by posting an empty expose event to QWindowSystemInterface
-void QCocoaWindow::obscureWindow()
-{
-    if (m_isExposed) {
-        m_geometryUpdateExposeAllowed = false;
-        m_isExposed = false;
-
-        qCDebug(lcQpaCocoaWindow) << "QCocoaWindow::obscureWindow" << window();
-        QWindowSystemInterface::handleExposeEvent(window(), QRegion());
-    }
-}
-
-// Updates window geometry by posting an expose event to QWindowSystemInterface
-void QCocoaWindow::updateExposedGeometry()
-{
-    // updateExposedGeometry is not allowed to send the initial expose. If you want
-    // that call exposeWindow();
-    if (!m_geometryUpdateExposeAllowed)
-        return;
-
-    // Do not send incorrect exposes in case the window is not even visible yet.
-    // We might get here as a result of a resize() from QWidget's show(), for instance.
-    if (!window()->isVisible())
-        return;
-
-    if (!isWindowExposable())
-        return;
-
-    if (m_exposedGeometry.size() == geometry().size() && m_exposedDevicePixelRatio == devicePixelRatio())
-        return;
-
-    m_isExposed = true;
-    m_exposedGeometry = geometry();
-    m_exposedDevicePixelRatio = devicePixelRatio();
-
-    QRect geometry(QPoint(0, 0), m_exposedGeometry.size());
-    qCDebug(lcQpaCocoaWindow) << "QCocoaWindow::updateExposedGeometry" << window() << geometry;
-    QWindowSystemInterface::handleExposeEvent(window(), geometry);
-}
-
 QWindow *QCocoaWindow::childWindowAt(QPoint windowPoint)
 {
     QWindow *targetWindow = window();
diff --git a/src/plugins/platforms/cocoa/qnsview.h b/src/plugins/platforms/cocoa/qnsview.h
index 4ef2a7133d..770aae240e 100644
--- a/src/plugins/platforms/cocoa/qnsview.h
+++ b/src/plugins/platforms/cocoa/qnsview.h
@@ -102,7 +102,6 @@ Q_FORWARD_DECLARE_OBJC_CLASS(QT_MANGLE_NAMESPACE(QNSViewMouseMoveHelper));
 - (void)drawBackingStoreUsingCoreGraphics:(NSRect)dirtyRect;
 - (void)textInputContextKeyboardSelectionDidChangeNotification : (NSNotification *) textInputContextKeyboardSelectionDidChangeNotification;
 - (void)viewDidHide;
-- (void)viewDidUnhide;
 - (void)removeFromSuperview;
 
 - (BOOL)isFlipped;
diff --git a/src/plugins/platforms/cocoa/qnsview.mm b/src/plugins/platforms/cocoa/qnsview.mm
index 70d218aad3..2efbb85c88 100644
--- a/src/plugins/platforms/cocoa/qnsview.mm
+++ b/src/plugins/platforms/cocoa/qnsview.mm
@@ -260,7 +260,7 @@ static QTouchDevice *touchDevice = 0;
     if ([self superview]) {
         m_platformWindow->m_viewIsEmbedded = true;
         QWindowSystemInterface::handleGeometryChange(m_platformWindow->window(), m_platformWindow->geometry());
-        m_platformWindow->updateExposedGeometry();
+        [self setNeedsDisplay:YES];
         QWindowSystemInterface::flushWindowSystemEvents();
     } else {
         m_platformWindow->m_viewIsEmbedded = false;
@@ -301,12 +301,13 @@ static QTouchDevice *touchDevice = 0;
 
 - (void)viewDidHide
 {
-    m_platformWindow->obscureWindow();
-}
+    if (!m_platformWindow->isExposed())
+        return;
 
-- (void)viewDidUnhide
-{
-    m_platformWindow->exposeWindow();
+    m_platformWindow->handleExposeEvent(QRegion());
+
+    // Note: setNeedsDisplay is automatically called for
+    // viewDidUnhide so no reason to override it here.
 }
 
 - (void)removeFromSuperview
@@ -315,22 +316,54 @@ static QTouchDevice *touchDevice = 0;
     [super removeFromSuperview];
 }
 
-- (void) flushBackingStore:(QCocoaBackingStore *)backingStore region:(const QRegion &)region offset:(QPoint)offset
+- (void)flushBackingStore:(QCocoaBackingStore *)backingStore region:(const QRegion &)region offset:(QPoint)offset
 {
     qCDebug(lcQpaCocoaWindow) << "[QNSView flushBackingStore:]" << m_platformWindow->window() << region.rectCount() << region.boundingRect() << offset;
 
     m_backingStore = backingStore;
     m_backingStoreOffset = offset * m_backingStore->paintDevice()->devicePixelRatio();
 
-    // Prevent buildup of NSDisplayCycle objects during setNeedsDisplayInRect, which
-    // would normally be released as part of the root runloop's autorelease pool, but
-    // can be kept alive during repeated painting which starve the root runloop.
-    // FIXME: Move this to the event dispatcher, to cover more cases of starvation.
-    // FIXME: Figure out if there's a way to detect and/or prevent runloop starvation.
-    QMacAutoReleasePool pool;
+    // FIXME: Clean up this method now that the drawRect logic has been merged into it
+
+    const NSRect dirtyRect = region.boundingRect().toCGRect();
+
+    // Normally a NSView is drawn via drawRect, as part of the display cycle in the
+    // main runloop, via setNeedsDisplay and friends. AppKit will lock focus on each
+    // individual view, starting with the top level and then traversing any subviews,
+    // calling drawRect for each of them. This pull model results in expose events
+    // sent to Qt, which result in drawing to the backingstore and flushing it.
+    // Qt may also decide to paint and flush the backingstore via e.g. timers,
+    // or other events such as mouse events, in which case we're in a push model.
+    // If there is no focused view, it means we're in the latter case, and need
+    // to manually flush the NSWindow after drawing to its graphic context.
+    const bool drawingOutsideOfDisplayCycle = ![NSView focusView];
+
+    // We also need to ensure the flushed view has focus, so that the graphics
+    // context is set up correctly (coordinate system, clipping, etc). Outside
+    // of the normal display cycle there is no focused view, as explained above,
+    // so we have to handle it manually. There's also a corner case inside the
+    // normal display cycle due to way QWidgetBackingStore composits native child
+    // widgets, where we'll get a flush of a native child during the drawRect of
+    // its parent/ancestor, and the parent/ancestor being the one locked by AppKit.
+    // In this case we also need to lock and unlock focus manually.
+    const bool shouldHandleViewLockManually = [NSView focusView] != self;
+    if (shouldHandleViewLockManually && ![self lockFocusIfCanDraw]) {
+        qWarning() << "failed to lock focus of" << self;
+        return;
+    }
+
+    if (m_platformWindow->m_drawContentBorderGradient)
+        NSDrawWindowBackground(dirtyRect);
 
-    for (const QRect &rect : region)
-        [self setNeedsDisplayInRect:NSMakeRect(rect.x(), rect.y(), rect.width(), rect.height())];
+    [self drawBackingStoreUsingCoreGraphics:dirtyRect];
+
+    if (shouldHandleViewLockManually)
+        [self unlockFocus];
+
+    if (drawingOutsideOfDisplayCycle)
+        [self.window flushWindow];
+
+    [self invalidateWindowShadowIfNeeded];
 }
 
 - (void)clearBackingStore
@@ -398,7 +431,8 @@ static QTouchDevice *touchDevice = 0;
     if (!m_platformWindow)
         return;
 
-    qCDebug(lcQpaCocoaWindow) << "[QNSView drawRect:]" << m_platformWindow->window() << QRectF::fromCGRect(NSRectToCGRect(dirtyRect));
+    qCDebug(lcQpaCocoaWindow) << "[QNSView drawRect:]" << m_platformWindow->window()
+        << QRectF::fromCGRect(NSRectToCGRect(dirtyRect));
 
 #ifndef QT_NO_OPENGL
     if (m_glContext && m_shouldSetGLContextinDrawRect) {
@@ -407,13 +441,7 @@ static QTouchDevice *touchDevice = 0;
     }
 #endif
 
-    if (m_platformWindow->m_drawContentBorderGradient)
-        NSDrawWindowBackground(dirtyRect);
-
-    if (m_backingStore)
-        [self drawBackingStoreUsingCoreGraphics:dirtyRect];
-
-    [self invalidateWindowShadowIfNeeded];
+    m_platformWindow->handleExposeEvent(QRectF::fromCGRect(dirtyRect).toRect());
 }
 
 // Draws the backing store content to the QNSView using Core Graphics.