1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

macOS: Don't stop display-link once started and window is exposed

Browse Source 
We were stopping the display-link we use to drive update-requests as soon
as no windows on the screen needed an update, and then starting it again
once a new updateRequest came in.

However there is a cost to starting the display-link, as it starts a
dedicated display-link thread, which resulted in a delay of about one
vsync before getting the corresponding update request delivery.

For Qt Quick this is problematic, because it drives all visual changes
to the scene via updateRequest, so e.g. clicking a MouseArea to change
the color of a Rectangle would not be reflected on screen as fast as it
could.

More problematically, when advancing animations from a timer, which
happens if there is more than one window shown, we would conclude after
polishAndSync that no window needed updates, and stop the display link.
But then the animation timer kicked in and would advance animations,
resulting in an updateRequest, and we'd start the display link up again,
causing janky animations:

 - deliverUpdateRequest
 - polishAndSync, without advancing animations
 - stop display link
 - timer callback, advancing animations
 - requestUpdate
 - start display link
 - timer callback, advancing animations
 - requestUpdate
 - display link callback
 - deliverUpdateRequest

We now keep the display link running once it has been started, and
only stop it if the window is de-exposed (or destroyed) without any
pending update requests.

To avoid overhead of running logic related to update requests when
there's no need, we keep track of whether there is a window on the
screen that needs an updateRequest, and skip the signaling from the
display-link thread over to the main thread if there are none.

We could have moved the bail-out later, to the main thread before
we iterate the windows, or even not tracked pending update requests
at all, which would have given us even faster "response-time", but
doing so would only buy us a small benefit, at the cost of always
signaling the main thread and running a GCD callback. Moving to
the new CADisplayLink API available in macOS 14+ might help solve
this, as the callback always comes in on the same (main) thread.

[ChangeLog][macOS] Display-links are now kept running, even when
windows are not animating, as long as they are visible on screen.
This improves the responsiveness in fulfilling updateRequest with
a corresponding UpdateRequest event, which affects how fast a Qt
Quick window can render the result of a property change.

Task-number: QTBUG-129839
Change-Id: Ieebb60e0f619974849e72be1a41aec66508a1475
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
This commit is contained in:
Tor Arne Vestbø 2024-10-17 19:43:45 +02:00
parent 674475887c
commit a9412bca7f
3 changed files with 62 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/plugins/platforms/cocoa/qcocoascreen.h
View File

@ -50,7 +50,6 @@ public:
bool requestUpdate();
void deliverUpdateRequests();
bool isRunningDisplayLink() const;
static QCocoaScreen *primaryScreen();
static QCocoaScreen *get(NSScreen *nsScreen);
@ -95,7 +94,10 @@ private:
CVDisplayLinkRef m_displayLink = nullptr;
dispatch_source_t m_displayLinkSource = nullptr;
QAtomicInt m_pendingUpdates;
QAtomicInt m_pendingUpdateRequests;
QAtomicInt m_pendingDisplayLinkUpdates;
void maybeStopDisplayLink();
friend class QCocoaIntegration;
friend class QCocoaWindow;

64
src/plugins/platforms/cocoa/qcocoascreen.mm
View File

@ -268,6 +268,11 @@ bool QCocoaScreen::requestUpdate()
return false;
}
// Track how many update requests we have queued, so that we
// know whether the display-link thread should try to deliver
// update requests, or if it can bail out early.
++m_pendingUpdateRequests;
if (!m_displayLink) {
qCDebug(lcQpaScreenUpdates) << "Creating display link for" << this;
if (CVDisplayLinkCreateWithCGDisplay(m_displayId, &m_displayLink) != kCVReturnSuccess) {
@ -381,10 +386,13 @@ void QCocoaScreen::deliverUpdateRequests()
// We're explicitly not using the data of the GCD source to track the pending updates,
// as the data isn't reset to 0 until after the event handler, and also doesn't update
// during the event handler, both of which we need to track late frames.
const int pendingUpdates = ++m_pendingUpdates;
const int pendingUpdates = ++m_pendingDisplayLinkUpdates;
const int pendingUpdateRequests = m_pendingUpdateRequests;
DeferredDebugHelper screenUpdates(lcQpaScreenUpdates());
qDeferredDebug(screenUpdates) << "display link callback for screen " << m_displayId;
qDeferredDebug(screenUpdates) << "display link callback for screen " << m_displayId
<< " with " << pendingUpdateRequests << " pending update requests";
if (const int framesAheadOfDelivery = pendingUpdates - 1) {
// If we have more than one update pending it means that a previous display link callback
@ -394,6 +402,16 @@ void QCocoaScreen::deliverUpdateRequests()
qDeferredDebug(screenUpdates) << ", " << framesAheadOfDelivery << " frame(s) ahead";
}
if (!pendingUpdateRequests) {
// There's a cost to stopping and starting the display link thread,
// so once started we always keep it running, to avoid missing frames.
// In the case where we don't have any pending update requests we don't
// need to signal the main thread.
qDeferredDebug(screenUpdates) << "; skipping signaling dispatch source";
m_pendingDisplayLinkUpdates = 0;
return;
}
qDeferredDebug(screenUpdates) << "; signaling dispatch source";
if (!m_displayLinkSource) {
@ -410,13 +428,13 @@ void QCocoaScreen::deliverUpdateRequests()
DeferredDebugHelper screenUpdates(lcQpaScreenUpdates());
qDeferredDebug(screenUpdates) << "gcd event handler on main thread";
const int pendingUpdates = m_pendingUpdates;
const int pendingUpdates = m_pendingDisplayLinkUpdates;
if (pendingUpdates > 1)
qDeferredDebug(screenUpdates) << ", " << (pendingUpdates - 1) << " frame(s) behind display link";
screenUpdates.flushOutput();
bool pauseUpdates = true;
int pendingUpdateRequests = 0;
auto windows = QGuiApplication::allWindows();
for (int i = 0; i < windows.size(); ++i) {
@ -441,27 +459,45 @@ void QCocoaScreen::deliverUpdateRequests()
if (!platformWindow)
continue;
// Another update request was triggered, keep the display link running
// The update request delivery could result in another request
// from the window, or the platform window could decide to not
// deliver the request at this time.
if (platformWindow->hasPendingUpdateRequest())
pauseUpdates = false;
++pendingUpdateRequests;
}
if (pauseUpdates) {
// Pause the display link if there are no pending update requests
qCDebug(lcQpaScreenUpdates) << "Stopping display link for" << this;
CVDisplayLinkStop(m_displayLink);
}
m_pendingUpdateRequests = pendingUpdateRequests;
if (const int missedUpdates = m_pendingUpdates.fetchAndStoreRelaxed(0) - pendingUpdates) {
if (const int missedUpdates = m_pendingDisplayLinkUpdates.fetchAndStoreRelaxed(0) - pendingUpdates) {
qCWarning(lcQpaScreenUpdates) << "main thread missed" << missedUpdates
<< "update(s) from display link during update request delivery";
}
}
}
bool QCocoaScreen::isRunningDisplayLink() const
void QCocoaScreen::maybeStopDisplayLink()
{
return m_displayLink && CVDisplayLinkIsRunning(m_displayLink);
if (!CVDisplayLinkIsRunning(m_displayLink))
return;
const auto windows = QGuiApplication::allWindows();
for (auto *window : windows) {
if (window->screen() != screen())
continue;
QPointer<QCocoaWindow> platformWindow = static_cast<QCocoaWindow*>(window->handle());
if (!platformWindow)
continue;
if (window->isExposed())
return;
if (platformWindow->hasPendingUpdateRequest())
return;
}
qCDebug(lcQpaScreenUpdates) << "Stopping display link for" << this;
CVDisplayLinkStop(m_displayLink);
}
// -----------------------------------------------------------

8
src/plugins/platforms/cocoa/qcocoawindow.mm
View File

@ -179,6 +179,11 @@ QCocoaWindow::~QCocoaWindow()
[m_view release];
[m_nsWindow closeAndRelease];
// Disposing of the view and window should have resulted in an
// expose event with isExposed=false, but just in case we try
// to stop the display link here as well.
static_cast<QCocoaScreen *>(screen())->maybeStopDisplayLink();
}
QSurfaceFormat QCocoaWindow::format() const
@ -1546,6 +1551,9 @@ void QCocoaWindow::handleExposeEvent(const QRegion &region)
qCDebug(lcQpaDrawing) << "QCocoaWindow::handleExposeEvent" << window() << region << "isExposed" << isExposed();
QWindowSystemInterface::handleExposeEvent<QWindowSystemInterface::SynchronousDelivery>(window(), region);
if (!isExposed())
static_cast<QCocoaScreen *>(screen())->maybeStopDisplayLink();
}
// --------------------------------------------------------------------------