Skip to content

Commit

Permalink
Simplify frame scheduling logic in RemoteLayerTreeDrawingArea.
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=259999
<rdar://113657855>

Reviewed by Simon Fraser.

Previously, both scheduleRenderingUpdate and external calls to triggerRenderingUpdate would wait for the next displayDidRefresh
if one was pending. scheduleRenderingUpdate would check for this explicitly at call time. TriggerRenderingUpdate would
always run updateRendering on a 0ms timer, and then check at that point and reschedule (by setting m_deferredRenderingUpdateWhileWaitingForBackingStoreSwap=true).

This reverses the logic of triggerRenderingUpdate to be the same as scheduleRenderingUpdate by making them use the same code (a new scheduleRenderingUpdate function
with an enum to specify the exact behaviour). In very rare cases this could be a behaviour change (since we're checking state at a different point).

The difference between these two is now more clear, in the case where there isn't a displayDidRefresh pending, schedule has an extra async task dispatch that trigger
does not. This difference affects performance benchmark results, so trying to remove it is outside the scope of this patch.

It should now never be possible for displayDidRefresh to be pending when updateRendering is called, so m_deferredRenderingUpdateWhileWaitingForBackingStoreSwap is removed.
Setting of m_displayDidRefreshIsPending=true happens earlier, since we can sometimes recurse back in to trigger rendering during a rendering update.

Existing internal callers of triggerRenderingUpdate (and startRenderingUpdateTimer) have been changed to use the enum version of scheduleRenderingUpdate to make it clearer. Most
of these are using the as soon as possible variant, but almost certainly don't need to and could be changed.

The two call sites that synchronously called updateRendering have also been changed to scheduleRenderingUpdate(AsSoonAsPossible). These likely weren't synchronous previously (since
updateRendering aborted and rescheduled if displayDidRefresh was pending), but it was non-deterministic. Making them always just schedule seems a lot safer and easier to understand.

Renames m_waitingForBackingStoreSwap to m_displayDidRefreshIsPending since that's the name of the message
we're actually waiting for.

The logic in ::displayDidRefresh cleaned up in response to the above changes, since most of the complexity it checked for no longer exists.

* Source/WebKit/WebProcess/WebPage/DrawingArea.h:
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h:
(WebKit::RemoteLayerTreeDrawingArea::displayDidRefreshIsPending const):
* Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::setRootCompositingLayer):
(WebKit::RemoteLayerTreeDrawingArea::updateGeometry):
(WebKit::RemoteLayerTreeDrawingArea::setLayerTreeStateIsFrozen):
(WebKit::RemoteLayerTreeDrawingArea::forceRepaint):
(WebKit::RemoteLayerTreeDrawingArea::setExposedContentRect):
(WebKit::RemoteLayerTreeDrawingArea::startRenderingUpdateTimer):
(WebKit::RemoteLayerTreeDrawingArea::triggerRenderingUpdate):
(WebKit::RemoteLayerTreeDrawingArea::updateRendering):
(WebKit::RemoteLayerTreeDrawingArea::displayDidRefresh):
(WebKit::RemoteLayerTreeDrawingArea::activityStateDidChange):
(WebKit::RemoteLayerTreeDrawingArea::dispatchAfterEnsuringDrawing):
(WebKit::RemoteLayerTreeDrawingArea::scheduleRenderingUpdateTimerFired):
(WebKit::RemoteLayerTreeDrawingArea::scheduleRenderingUpdate):
* Source/WebKit/WebProcess/WebPage/mac/RemoteLayerTreeDrawingAreaMac.mm:
(WebKit::RemoteLayerTreeDrawingAreaMac::applyTransientZoomToPage):

Canonical link: https://commits.webkit.org/266884@main
  • Loading branch information
@mattwoodrow
mattwoodrow committed Aug 14, 2023
1 parent 2f426da commit 7dd240a
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 64 deletions.
12 changes: 5 additions & 7 deletions Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@
{
for (auto& transaction : transactions)
commitLayerTreeTransaction(connection, transaction.first, transaction.second);

if (std::exchange(m_commitLayerTreeMessageState, NeedsDisplayDidRefresh) == MissedCommit)
didRefreshDisplay();

scheduleDisplayRefreshCallbacks();
}

void RemoteLayerTreeDrawingAreaProxy::commitLayerTreeTransaction(IPC::Connection& connection, const RemoteLayerTreeTransaction& layerTreeTransaction, const RemoteScrollingCoordinatorTransaction& scrollingTreeTransaction)
Expand Down Expand Up @@ -246,13 +251,6 @@

m_webPageProxy.layerTreeCommitComplete();

if (!layerTreeTransaction.isMainFrameProcessTransaction())
connection.send(Messages::DrawingArea::DisplayDidRefresh(), m_identifier);
else if (std::exchange(m_commitLayerTreeMessageState, NeedsDisplayDidRefresh) == MissedCommit)
didRefreshDisplay();

scheduleDisplayRefreshCallbacks();

if (didUpdateEditorState)
m_webPageProxy.dispatchDidUpdateEditorState();

Expand Down
5 changes: 5 additions & 0 deletions Source/WebKit/WebProcess/WebPage/DrawingArea.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,13 @@ class DrawingArea : public IPC::MessageReceiver, public WebCore::DisplayRefreshM
virtual void setRootCompositingLayer(WebCore::Frame&, WebCore::GraphicsLayer*) = 0;
virtual void addRootFrame(WebCore::FrameIdentifier) { }
// FIXME: Add a corresponding removeRootFrame.

// Cause a rendering update to happen as soon as possible.
virtual void triggerRenderingUpdate() = 0;
// Cause a rendering update to happen at the next suitable time,
// as determined by the drawing area.
virtual bool scheduleRenderingUpdate() { return false; }

virtual void renderingUpdateFramesPerSecondChanged() { }

virtual void willStartRenderingUpdateDisplay();
Expand Down
30 changes: 22 additions & 8 deletions Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,23 @@ class RemoteLayerTreeDrawingArea : public DrawingArea, public WebCore::GraphicsL
TransactionID nextTransactionID() const { return m_currentTransactionID.next(); }
TransactionID lastCommittedTransactionID() const { return m_currentTransactionID; }

bool displayDidRefreshIsPending() const { return m_waitingForBackingStoreSwap; }

protected:
void updateRendering();
enum class ScheduleRenderingUrgency {
// FIXME: There are many consumers of this (and triggerRenderingUpdate), do they all actually need the ASAP behaviour?
AsSoonAsPossible,
NextSuitableTime,
};
void scheduleRenderingUpdate(ScheduleRenderingUrgency);

private:
enum class State {
Idle,
WaitingForDisplayDidRefresh,
WaitingForUpdateRenderingTimer,
WaitingForCallOnMainRunLoop,
WaitingForScheduleRenderingTimer,
};

// DrawingArea
void setNeedsDisplay() override;
void setNeedsDisplayInRect(const WebCore::IntRect&) override;
Expand All @@ -71,8 +82,11 @@ class RemoteLayerTreeDrawingArea : public DrawingArea, public WebCore::GraphicsL
WebCore::GraphicsLayerFactory* graphicsLayerFactory() override;
void setRootCompositingLayer(WebCore::Frame&, WebCore::GraphicsLayer*) override;
void addRootFrame(WebCore::FrameIdentifier) final;
void triggerRenderingUpdate() override;
void triggerRenderingUpdate() final;
bool scheduleRenderingUpdate() final;

void updateRendering();

void renderingUpdateFramesPerSecondChanged() final;
void attachViewOverlayGraphicsLayer(WebCore::FrameIdentifier, WebCore::GraphicsLayer*) override;

Expand Down Expand Up @@ -162,14 +176,13 @@ class RemoteLayerTreeDrawingArea : public DrawingArea, public WebCore::GraphicsL

std::optional<WebCore::FloatRect> m_viewExposedRect;

State m_state { State::Idle };

WebCore::Timer m_updateRenderingTimer;
bool m_isRenderingSuspended { false };
bool m_hasDeferredRenderingUpdate { false };
bool m_inUpdateRendering { false };

bool m_waitingForBackingStoreSwap { false };
bool m_deferredRenderingUpdateWhileWaitingForBackingStoreSwap { false };

OSObjectPtr<dispatch_queue_t> m_commitQueue;
RefPtr<BackingStoreFlusher> m_pendingBackingStoreFlusher;

Expand All @@ -182,7 +195,8 @@ class RemoteLayerTreeDrawingArea : public DrawingArea, public WebCore::GraphicsL
WebCore::Timer m_scheduleRenderingTimer;
std::optional<WebCore::FramesPerSecond> m_preferredFramesPerSecond;
Seconds m_preferredRenderingUpdateInterval;
bool m_isScheduled { false };

bool m_updateRenderingIsPending { false };
};

inline bool RemoteLayerTreeDrawingArea::addMilestonesToDispatch(OptionSet<WebCore::LayoutMilestone> paintMilestones)
Expand Down
105 changes: 57 additions & 48 deletions Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@
rootLayer.contentLayer = rootGraphicsLayer;
}
updateRootLayers();
triggerRenderingUpdate();
scheduleRenderingUpdate(ScheduleRenderingUrgency::AsSoonAsPossible);
}

void RemoteLayerTreeDrawingArea::updateGeometry(const IntSize& viewSize, bool flushSynchronously, const WTF::MachSendRight&, CompletionHandler<void()>&& completionHandler)
Expand All @@ -169,7 +169,7 @@
size = contentSize;
}

triggerRenderingUpdate();
scheduleRenderingUpdate(ScheduleRenderingUrgency::AsSoonAsPossible);
completionHandler();
}

Expand Down Expand Up @@ -223,7 +223,7 @@

if (!m_isRenderingSuspended && m_hasDeferredRenderingUpdate) {
m_hasDeferredRenderingUpdate = false;
startRenderingUpdateTimer();
scheduleRenderingUpdate(ScheduleRenderingUrgency::AsSoonAsPossible);
}
}

Expand All @@ -233,7 +233,7 @@
return;

m_webPage.corePage()->forceRepaintAllFrames();
updateRendering();
scheduleRenderingUpdate(ScheduleRenderingUrgency::AsSoonAsPossible);
}

void RemoteLayerTreeDrawingArea::acceleratedAnimationDidStart(WebCore::PlatformLayerIdentifier layerID, const String& key, MonotonicTime startTime)
Expand Down Expand Up @@ -272,24 +272,23 @@
return;

frameView->setExposedContentRect(exposedContentRect);
triggerRenderingUpdate();
scheduleRenderingUpdate(ScheduleRenderingUrgency::AsSoonAsPossible);
}

void RemoteLayerTreeDrawingArea::startRenderingUpdateTimer()
{
if (m_updateRenderingTimer.isActive())
RELEASE_ASSERT(m_state != State::WaitingForDisplayDidRefresh);
if (m_updateRenderingTimer.isActive()) {
RELEASE_ASSERT(m_state == State::WaitingForUpdateRenderingTimer);
return;
}
m_state = State::WaitingForUpdateRenderingTimer;
m_updateRenderingTimer.startOneShot(0_s);
}

void RemoteLayerTreeDrawingArea::triggerRenderingUpdate()
{
if (m_isRenderingSuspended) {
m_hasDeferredRenderingUpdate = true;
return;
}

startRenderingUpdateTimer();
scheduleRenderingUpdate(ScheduleRenderingUrgency::AsSoonAsPossible);
}

void RemoteLayerTreeDrawingArea::setNextRenderingUpdateRequiresSynchronousImageDecoding()
Expand All @@ -299,23 +298,24 @@

void RemoteLayerTreeDrawingArea::updateRendering()
{
RELEASE_ASSERT(m_updateRenderingIsPending);
RELEASE_ASSERT(m_state == State::WaitingForUpdateRenderingTimer);
m_updateRenderingIsPending = false;

if (m_isRenderingSuspended) {
m_hasDeferredRenderingUpdate = true;
return;
}

if (m_waitingForBackingStoreSwap) {
m_deferredRenderingUpdateWhileWaitingForBackingStoreSwap = true;
return;
}

// This function is not reentrant, e.g. a rAF callback may force repaint.
if (m_inUpdateRendering)
return;
RELEASE_ASSERT(!m_inUpdateRendering);

if (auto* page = m_webPage.corePage(); page && !page->rootFrames().computeSize())
return;

RELEASE_ASSERT(!m_updateRenderingTimer.isActive());
m_state = State::WaitingForDisplayDidRefresh;

scaleViewToFitDocumentIfNeeded();

SetForScope change(m_inUpdateRendering, true);
Expand Down Expand Up @@ -369,9 +369,7 @@
layerTransaction.setActivityStateChangeID(std::exchange(m_activityStateChangeID, ActivityStateChangeAsynchronous));

willCommitLayerTree(layerTransaction);

m_waitingForBackingStoreSwap = true;


send(Messages::RemoteLayerTreeDrawingAreaProxy::WillCommitLayerTree(layerTransaction.transactionID()));

RemoteScrollingCoordinatorTransaction scrollingTransaction;
Expand Down Expand Up @@ -422,20 +420,18 @@
// FIXME: This should use a counted replacement for setLayerTreeStateIsFrozen, but
// the callers of that function are not strictly paired.

auto wasWaitingForBackingStoreSwap = std::exchange(m_waitingForBackingStoreSwap, false);
RELEASE_ASSERT(m_state == State::WaitingForDisplayDidRefresh);

if (!WebProcess::singleton().shouldUseRemoteRenderingFor(WebCore::RenderingPurpose::DOM)) {
// This empty transaction serves to trigger CA's garbage collection of IOSurfaces. See <rdar://problem/16110687>
[CATransaction begin];
[CATransaction commit];
}

if (m_deferredRenderingUpdateWhileWaitingForBackingStoreSwap || (m_isScheduled && !m_scheduleRenderingTimer.isActive())) {
triggerRenderingUpdate();
m_deferredRenderingUpdateWhileWaitingForBackingStoreSwap = false;
m_isScheduled = false;
} else if (wasWaitingForBackingStoreSwap && m_updateRenderingTimer.isActive())
m_deferredRenderingUpdateWhileWaitingForBackingStoreSwap = true;
m_state = State::Idle;
RELEASE_ASSERT(!m_updateRenderingTimer.isActive());
if (m_updateRenderingIsPending && !m_scheduleRenderingTimer.isActive())
startRenderingUpdateTimer();
else
send(Messages::RemoteLayerTreeDrawingAreaProxy::CommitLayerTreeNotTriggered(nextTransactionID()));
}
Expand Down Expand Up @@ -497,7 +493,7 @@
if (activityStateChangeID != ActivityStateChangeAsynchronous) {
m_remoteLayerTreeContext->setNextRenderingUpdateRequiresSynchronousImageDecoding();
m_activityStateChangeID = activityStateChangeID;
startRenderingUpdateTimer();
scheduleRenderingUpdate(ScheduleRenderingUrgency::AsSoonAsPossible);
}

// FIXME: We may want to match behavior in TiledCoreAnimationDrawingArea by firing these callbacks after the next compositing flush, rather than immediately after
Expand All @@ -512,7 +508,7 @@
m_remoteLayerTreeContext->setNextRenderingUpdateRequiresSynchronousImageDecoding();

m_pendingCallbackIDs.append(callbackID);
triggerRenderingUpdate();
scheduleRenderingUpdate(ScheduleRenderingUrgency::AsSoonAsPossible);
}

void RemoteLayerTreeDrawingArea::adoptLayersFromDrawingArea(DrawingArea& oldDrawingArea)
Expand All @@ -526,32 +522,45 @@

void RemoteLayerTreeDrawingArea::scheduleRenderingUpdateTimerFired()
{
triggerRenderingUpdate();
m_isScheduled = false;
if (m_state == State::WaitingForScheduleRenderingTimer)
startRenderingUpdateTimer();
}

bool RemoteLayerTreeDrawingArea::scheduleRenderingUpdate()
void RemoteLayerTreeDrawingArea::scheduleRenderingUpdate(ScheduleRenderingUrgency urgency)
{
if (m_isScheduled)
return true;
if (m_updateRenderingIsPending && urgency != ScheduleRenderingUrgency::AsSoonAsPossible)
return;

tracePoint(RemoteLayerTreeScheduleRenderingUpdate, m_waitingForBackingStoreSwap);
tracePoint(RemoteLayerTreeScheduleRenderingUpdate, m_state == State::WaitingForDisplayDidRefresh);

m_isScheduled = true;
m_updateRenderingIsPending = true;

if (m_preferredFramesPerSecond) {
if (displayDidRefreshIsPending())
return true;

callOnMainRunLoop([self = WeakPtr { this }] () {
if (self) {
self->m_isScheduled = false;
self->triggerRenderingUpdate();
}
});
} else
if (m_state == State::WaitingForDisplayDidRefresh)
return;

if (urgency == ScheduleRenderingUrgency::AsSoonAsPossible)
startRenderingUpdateTimer();
else {
m_state = State::WaitingForCallOnMainRunLoop;
callOnMainRunLoop([self = WeakPtr { this }] () {
// It's possible that an ASAP request got ahead of this
// and started a rendering update already and we're no
// longer waiting for this callback.
if (self && self->m_state == State::WaitingForCallOnMainRunLoop)
self->startRenderingUpdateTimer();
});
}
} else if (m_state == State::Idle || m_state == State::WaitingForDisplayDidRefresh) {
ASSERT(!m_scheduleRenderingTimer.isActive());
m_state = State::WaitingForScheduleRenderingTimer;
m_scheduleRenderingTimer.startOneShot(m_preferredRenderingUpdateInterval);
}
}

bool RemoteLayerTreeDrawingArea::scheduleRenderingUpdate()
{
scheduleRenderingUpdate(ScheduleRenderingUrgency::NextSuitableTime);
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/WebPage/mac/RemoteLayerTreeDrawingAreaMac.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
FloatRect unobscuredContentRect = frameView->unobscuredContentRectIncludingScrollbars();
unscrolledOrigin.moveBy(-unobscuredContentRect.location());
m_webPage.scalePage(scale / m_webPage.viewScaleFactor(), roundedIntPoint(-unscrolledOrigin));
updateRendering();
scheduleRenderingUpdate(ScheduleRenderingUrgency::AsSoonAsPossible);
}

void RemoteLayerTreeDrawingAreaMac::adjustTransientZoom(double scale, WebCore::FloatPoint origin)
Expand Down

0 comments on commit 7dd240a

Please sign in to comment.