- Revision
- 220672
- Author
- zandober...@gmail.com
- Date
- 2017-08-14 03:23:35 -0700 (Mon, 14 Aug 2017)
Log Message
[ThreadedCompositor] Improve composition and update state handling
https://bugs.webkit.org/show_bug.cgi?id=172448
Reviewed by Carlos Garcia Campos.
In the ThreadedCompositor and related classes, we now track composition
and scene update states separately. This enables properly piping in the
DisplayRefreshMonitor object, leveraging it for both display refresh
notifications and signalling to the client that the scene update is
completed.
In CompositingRunLoop, two separate states are now used, one for
compositing and the other for scene update. Access to both should be done
while the related Lock object is held. The pending update state is also
tracked in order to schedule any required updates as soon as the current
state update is completed.
In ThreadedCompositor, the information about client notification and
update completion coordination is now also kept under a Lock object,
moving away from atomics. DisplayRefreshMonitor can now utilize just
two methods in order to either schedule an update or report that update
as finished.
* Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:
(WebKit::CompositingRunLoop::CompositingRunLoop):
(WebKit::CompositingRunLoop::scheduleUpdate):
(WebKit::CompositingRunLoop::stopUpdates):
(WebKit::CompositingRunLoop::compositionCompleted):
(WebKit::CompositingRunLoop::updateCompleted):
(WebKit::CompositingRunLoop::updateTimerFired):
(WebKit::CompositingRunLoop::isActive): Deleted.
* Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:
(WebKit::CompositingRunLoop::stateLock):
* Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
(WebKit::m_displayRefreshMonitor):
(WebKit::ThreadedCompositor::sceneUpdateFinished):
(WebKit::ThreadedCompositor::updateSceneState):
(WebKit::ThreadedCompositor::requestDisplayRefreshMonitorUpdate):
(WebKit::ThreadedCompositor::handleDisplayRefreshMonitorUpdate):
(WebKit::ThreadedCompositor::renderNextFrameIfNeeded): Deleted.
(WebKit::ThreadedCompositor::completeCoordinatedUpdateIfNeeded): Deleted.
(WebKit::ThreadedCompositor::coordinateUpdateCompletionWithClient): Deleted.
* Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h:
* Shared/CoordinatedGraphics/threadedcompositor/ThreadedDisplayRefreshMonitor.cpp:
(WebKit::ThreadedDisplayRefreshMonitor::requestRefreshCallback):
(WebKit::ThreadedDisplayRefreshMonitor::displayRefreshCallback):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (220671 => 220672)
--- trunk/Source/WebKit/ChangeLog 2017-08-14 09:46:33 UTC (rev 220671)
+++ trunk/Source/WebKit/ChangeLog 2017-08-14 10:23:35 UTC (rev 220672)
@@ -1,3 +1,52 @@
+2017-08-14 Zan Dobersek <zdober...@igalia.com>
+
+ [ThreadedCompositor] Improve composition and update state handling
+ https://bugs.webkit.org/show_bug.cgi?id=172448
+
+ Reviewed by Carlos Garcia Campos.
+
+ In the ThreadedCompositor and related classes, we now track composition
+ and scene update states separately. This enables properly piping in the
+ DisplayRefreshMonitor object, leveraging it for both display refresh
+ notifications and signalling to the client that the scene update is
+ completed.
+
+ In CompositingRunLoop, two separate states are now used, one for
+ compositing and the other for scene update. Access to both should be done
+ while the related Lock object is held. The pending update state is also
+ tracked in order to schedule any required updates as soon as the current
+ state update is completed.
+
+ In ThreadedCompositor, the information about client notification and
+ update completion coordination is now also kept under a Lock object,
+ moving away from atomics. DisplayRefreshMonitor can now utilize just
+ two methods in order to either schedule an update or report that update
+ as finished.
+
+ * Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:
+ (WebKit::CompositingRunLoop::CompositingRunLoop):
+ (WebKit::CompositingRunLoop::scheduleUpdate):
+ (WebKit::CompositingRunLoop::stopUpdates):
+ (WebKit::CompositingRunLoop::compositionCompleted):
+ (WebKit::CompositingRunLoop::updateCompleted):
+ (WebKit::CompositingRunLoop::updateTimerFired):
+ (WebKit::CompositingRunLoop::isActive): Deleted.
+ * Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:
+ (WebKit::CompositingRunLoop::stateLock):
+ * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
+ (WebKit::m_displayRefreshMonitor):
+ (WebKit::ThreadedCompositor::sceneUpdateFinished):
+ (WebKit::ThreadedCompositor::updateSceneState):
+ (WebKit::ThreadedCompositor::requestDisplayRefreshMonitorUpdate):
+ (WebKit::ThreadedCompositor::handleDisplayRefreshMonitorUpdate):
+ (WebKit::ThreadedCompositor::renderNextFrameIfNeeded): Deleted.
+ (WebKit::ThreadedCompositor::completeCoordinatedUpdateIfNeeded): Deleted.
+ (WebKit::ThreadedCompositor::coordinateUpdateCompletionWithClient): Deleted.
+ * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h:
+ * Shared/CoordinatedGraphics/threadedcompositor/ThreadedDisplayRefreshMonitor.cpp:
+ (WebKit::ThreadedDisplayRefreshMonitor::requestRefreshCallback):
+ (WebKit::ThreadedDisplayRefreshMonitor::displayRefreshCallback):
+
2017-08-13 Manuel Rego Casasnovas <r...@igalia.com>
Composition underline color is always black
Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp (220671 => 220672)
--- trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp 2017-08-14 09:46:33 UTC (rev 220671)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp 2017-08-14 10:23:35 UTC (rev 220672)
@@ -109,8 +109,6 @@
: m_updateTimer(WorkQueuePool::singleton().runLoop(this), this, &CompositingRunLoop::updateTimerFired)
, m_updateFunction(WTFMove(updateFunction))
{
- m_updateState.store(UpdateState::Completed);
-
#if USE(GLIB_EVENT_LOOP)
m_updateTimer.setPriority(RunLoopSourcePriority::CompositingThreadUpdateTimer);
m_updateTimer.setName("[WebKit] CompositingRunLoop");
@@ -144,43 +142,119 @@
m_dispatchSyncCondition.wait(m_dispatchSyncConditionMutex);
}
-bool CompositingRunLoop::isActive()
+void CompositingRunLoop::scheduleUpdate()
{
- return m_updateState.load() != UpdateState::Completed;
+ LockHolder stateLocker(m_state.lock);
+ scheduleUpdate(stateLocker);
}
-void CompositingRunLoop::scheduleUpdate()
+void CompositingRunLoop::scheduleUpdate(LockHolder& stateLocker)
{
- if (m_updateState.compareExchangeStrong(UpdateState::Completed, UpdateState::InProgress) == UpdateState::Completed) {
+ // An update was requested. Depending on the state:
+ // - if Idle, enter the Scheduled state and start the update timer,
+ // - if Scheduled, do nothing,
+ // - if InProgress or PendingCompletion, mark an update as pending, meaning another
+ // update will be scheduled as soon as the current one is completed.
+
+ UNUSED_PARAM(stateLocker);
+
+ switch (m_state.update) {
+ case UpdateState::Idle:
+ m_state.update = UpdateState::Scheduled;
m_updateTimer.startOneShot(0);
return;
+ case UpdateState::Scheduled:
+ return;
+ case UpdateState::InProgress:
+ case UpdateState::PendingCompletion:
+ m_state.pendingUpdate = true;
+ return;
}
-
- if (m_updateState.compareExchangeStrong(UpdateState::InProgress, UpdateState::PendingAfterCompletion) == UpdateState::InProgress)
- return;
}
void CompositingRunLoop::stopUpdates()
{
+ // Stop everything.
+
+ LockHolder locker(m_state.lock);
m_updateTimer.stop();
- m_updateState.store(UpdateState::Completed);
+ m_state.composition = CompositionState::Idle;
+ m_state.update = UpdateState::Idle;
+ m_state.pendingUpdate = false;
}
-void CompositingRunLoop::updateCompleted()
+void CompositingRunLoop::compositionCompleted(LockHolder& stateLocker)
{
- if (m_updateState.compareExchangeStrong(UpdateState::InProgress, UpdateState::Completed) == UpdateState::InProgress)
+ // Composition has been signaled as completed, pushing the state into Idle.
+ // Depending on the state of the scene update:
+ // - if Idle, Scheduled or InProgress, do nothing,
+ // - if PendingCompletion, schedule a new update in case a pending update was marked,
+ // or push the scene update state into Idle otherwise.
+
+ UNUSED_PARAM(stateLocker);
+
+ m_state.composition = CompositionState::Idle;
+
+ switch (m_state.update) {
+ case UpdateState::Idle:
+ case UpdateState::Scheduled:
+ case UpdateState::InProgress:
return;
+ case UpdateState::PendingCompletion:
+ if (m_state.pendingUpdate) {
+ m_state.pendingUpdate = false;
+ m_state.update = UpdateState::Scheduled;
+ m_updateTimer.startOneShot(0);
+ return;
+ }
- if (m_updateState.compareExchangeStrong(UpdateState::PendingAfterCompletion, UpdateState::InProgress) == UpdateState::PendingAfterCompletion) {
- m_updateTimer.startOneShot(0);
+ m_state.update = UpdateState::Idle;
return;
}
+}
- ASSERT_NOT_REACHED();
+void CompositingRunLoop::updateCompleted(LockHolder& stateLocker)
+{
+ // Scene update has been signaled as completed. Depending on the state:
+ // - if Idle, Scheduled or InProgress, do nothing,
+ // - if InProgress, push the state into PendingCompletion if the composition state is
+ // InProgress, otherwise schedule a new update in case a pending update was marked,
+ // otherwise push the scene update state into Idle.
+
+ UNUSED_PARAM(stateLocker);
+
+ switch (m_state.update) {
+ case UpdateState::Idle:
+ case UpdateState::Scheduled:
+ return;
+ case UpdateState::InProgress:
+ if (m_state.composition == CompositionState::InProgress) {
+ m_state.update = UpdateState::PendingCompletion;
+ return;
+ }
+
+ if (m_state.pendingUpdate) {
+ m_state.pendingUpdate = false;
+ m_state.update = UpdateState::Scheduled;
+ m_updateTimer.startOneShot(0);
+ return;
+ }
+
+ m_state.update = UpdateState::Idle;
+ return;
+ case UpdateState::PendingCompletion:
+ return;
+ }
}
void CompositingRunLoop::updateTimerFired()
{
+ {
+ // Both composition and scene update are now in progress.
+ LockHolder locker(m_state.lock);
+ m_state.composition = CompositionState::InProgress;
+ m_state.update = UpdateState::InProgress;
+ }
m_updateFunction();
}
Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h (220671 => 220672)
--- trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h 2017-08-14 09:46:33 UTC (rev 220671)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h 2017-08-14 10:23:35 UTC (rev 220672)
@@ -48,17 +48,25 @@
void performTask(Function<void ()>&&);
void performTaskSync(Function<void ()>&&);
- bool isActive();
+ Lock& stateLock() { return m_state.lock; }
+
void scheduleUpdate();
+ void scheduleUpdate(LockHolder&);
void stopUpdates();
- void updateCompleted();
+ void compositionCompleted(LockHolder&);
+ void updateCompleted(LockHolder&);
private:
+ enum class CompositionState {
+ Idle,
+ InProgress,
+ };
enum class UpdateState {
- Completed,
+ Idle,
+ Scheduled,
InProgress,
- PendingAfterCompletion,
+ PendingCompletion,
};
void updateTimerFired();
@@ -65,9 +73,16 @@
RunLoop::Timer<CompositingRunLoop> m_updateTimer;
Function<void ()> m_updateFunction;
- Atomic<UpdateState> m_updateState;
Lock m_dispatchSyncConditionMutex;
Condition m_dispatchSyncCondition;
+
+
+ struct {
+ Lock lock;
+ CompositionState composition { CompositionState::Idle };
+ UpdateState update { UpdateState::Idle };
+ bool pendingUpdate { false };
+ } m_state;
};
} // namespace WebKit
Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp (220671 => 220672)
--- trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp 2017-08-14 09:46:33 UTC (rev 220671)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp 2017-08-14 10:23:35 UTC (rev 220672)
@@ -68,9 +68,6 @@
m_attributes.needsResize = !viewportSize.isEmpty();
}
- m_clientRendersNextFrame.store(false);
- m_coordinateUpdateCompletionWithClient.store(false);
-
m_compositingRunLoop->performTaskSync([this, protectedThis = makeRef(*this)] {
m_scene = adoptRef(new CoordinatedGraphicsScene(this));
m_nativeSurfaceHandle = m_client.nativeSurfaceHandleForCompositing();
@@ -246,14 +243,37 @@
void ThreadedCompositor::sceneUpdateFinished()
{
- bool shouldDispatchDisplayRefreshCallback = m_clientRendersNextFrame.load()
- || m_displayRefreshMonitor->requiresDisplayRefreshCallback();
- bool shouldCoordinateUpdateCompletionWithClient = m_coordinateUpdateCompletionWithClient.load();
+ // The composition has finished. Now we have to determine how to manage
+ // the scene update completion.
+ // The DisplayRefreshMonitor will be used to dispatch a callback on the client thread if:
+ // - clientRendersNextFrame is true (i.e. client has to be notified about the finished update), or
+ // - a DisplayRefreshMonitor callback was requested from the Web engine
+ bool shouldDispatchDisplayRefreshCallback { false };
+
+ // If coordinateUpdateCompletionWithClient is true, the scene update completion has to be
+ // delayed until the DisplayRefreshMonitor callback.
+ bool shouldCoordinateUpdateCompletionWithClient { false };
+
+ {
+ LockHolder locker(m_attributes.lock);
+ shouldDispatchDisplayRefreshCallback = m_attributes.clientRendersNextFrame
+ || m_displayRefreshMonitor->requiresDisplayRefreshCallback();
+ shouldCoordinateUpdateCompletionWithClient = m_attributes.coordinateUpdateCompletionWithClient;
+ }
+
+ LockHolder stateLocker(m_compositingRunLoop->stateLock());
+
+ // Schedule the DisplayRefreshMonitor callback, if necessary.
if (shouldDispatchDisplayRefreshCallback)
m_displayRefreshMonitor->dispatchDisplayRefreshCallback();
+
+ // Mark the scene update as completed if no coordination is required and if not in a forced repaint.
if (!shouldCoordinateUpdateCompletionWithClient && !m_inForceRepaint)
- m_compositingRunLoop->updateCompleted();
+ m_compositingRunLoop->updateCompleted(stateLocker);
+
+ // Independent of the scene update, the composition itself is now completed.
+ m_compositingRunLoop->compositionCompleted(stateLocker);
}
void ThreadedCompositor::updateSceneState(const CoordinatedGraphicsState& state)
@@ -262,15 +282,19 @@
m_scene->appendUpdate([this, scene = makeRef(*m_scene), state] {
scene->commitSceneState(state);
- m_clientRendersNextFrame.store(true);
+ LockHolder locker(m_attributes.lock);
+
+ // Client has to be notified upon finishing this scene update.
+ m_attributes.clientRendersNextFrame = true;
+
+ // Coordinate scene update completion with the client in case of changed or updated platform layers.
// Do not change m_coordinateUpdateCompletionWithClient while in force repaint.
- if (m_inForceRepaint)
- return;
- bool coordinateUpdate = std::any_of(state.layersToUpdate.begin(), state.layersToUpdate.end(),
+ bool coordinateUpdate = !m_inForceRepaint && std::any_of(state.layersToUpdate.begin(), state.layersToUpdate.end(),
[](const std::pair<CoordinatedLayerID, CoordinatedGraphicsLayerState>& it) {
return it.second.platformLayerChanged || it.second.platformLayerUpdated;
});
- m_coordinateUpdateCompletionWithClient.store(coordinateUpdate);
+
+ m_attributes.coordinateUpdateCompletionWithClient |= coordinateUpdate;
});
m_compositingRunLoop->scheduleUpdate();
@@ -291,23 +315,56 @@
return m_displayRefreshMonitor.copyRef();
}
-void ThreadedCompositor::renderNextFrameIfNeeded()
+void ThreadedCompositor::requestDisplayRefreshMonitorUpdate()
{
- if (m_clientRendersNextFrame.compareExchangeStrong(true, false))
- m_scene->renderNextFrame();
+ // This is invoked by ThreadedDisplayRefreshMonitor when a fresh update is required.
+
+ LockHolder stateLocker(m_compositingRunLoop->stateLock());
+ {
+ // coordinateUpdateCompletionWithClient is set to true in order to delay the scene update
+ // completion until the DisplayRefreshMonitor is fired on the main thread after the composition
+ // is completed.
+ LockHolder locker(m_attributes.lock);
+ m_attributes.coordinateUpdateCompletionWithClient = true;
+ }
+ m_compositingRunLoop->scheduleUpdate(stateLocker);
}
-void ThreadedCompositor::completeCoordinatedUpdateIfNeeded()
+void ThreadedCompositor::handleDisplayRefreshMonitorUpdate(bool hasBeenRescheduled)
{
- if (m_coordinateUpdateCompletionWithClient.compareExchangeStrong(true, false))
- m_compositingRunLoop->updateCompleted();
-}
+ // Retrieve the clientRendersNextFrame and coordinateUpdateCompletionWithClient.
+ bool clientRendersNextFrame { false };
+ bool coordinateUpdateCompletionWithClient { false };
+ {
+ LockHolder locker(m_attributes.lock);
+ clientRendersNextFrame = std::exchange(m_attributes.clientRendersNextFrame, false);
+ coordinateUpdateCompletionWithClient = std::exchange(m_attributes.coordinateUpdateCompletionWithClient, false);
+ }
-void ThreadedCompositor::coordinateUpdateCompletionWithClient()
-{
- m_coordinateUpdateCompletionWithClient.store(true);
- if (!m_compositingRunLoop->isActive())
- m_compositingRunLoop->scheduleUpdate();
+ // If clientRendersNextFrame is true, the client is finally notified about the scene update nearing
+ // completion. The client will use this opportunity to clean up resources as appropriate. It can also
+ // perform any layer flush that was requested during the composition, or by any DisplayRefreshMonitor
+ // notifications that have been handled at this point.
+ if (clientRendersNextFrame)
+ m_client.renderNextFrame();
+
+ LockHolder stateLocker(m_compositingRunLoop->stateLock());
+
+ // If required, mark the current scene update as completed. CompositingRunLoop will take care of
+ // scheduling a new update in case an update was marked as pending due to previous layer flushes
+ // or DisplayRefreshMonitor notifications.
+ if (coordinateUpdateCompletionWithClient)
+ m_compositingRunLoop->updateCompleted(stateLocker);
+
+ // If the DisplayRefreshMonitor was scheduled again, we immediately demand the update completion
+ // coordination (like we do in requestDisplayRefreshMonitorUpdate()) and request an update.
+ if (hasBeenRescheduled) {
+ {
+ LockHolder locker(m_attributes.lock);
+ m_attributes.coordinateUpdateCompletionWithClient = true;
+ }
+ m_compositingRunLoop->scheduleUpdate(stateLocker);
+ }
}
#endif
Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h (220671 => 220672)
--- trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h 2017-08-14 09:46:33 UTC (rev 220671)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h 2017-08-14 10:23:35 UTC (rev 220672)
@@ -88,9 +88,8 @@
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
RefPtr<WebCore::DisplayRefreshMonitor> displayRefreshMonitor(WebCore::PlatformDisplayID);
- void renderNextFrameIfNeeded();
- void completeCoordinatedUpdateIfNeeded();
- void coordinateUpdateCompletionWithClient();
+ void requestDisplayRefreshMonitorUpdate();
+ void handleDisplayRefreshMonitorUpdate(bool hasBeenRescheduled);
#endif
void frameComplete();
@@ -126,14 +125,14 @@
float scaleFactor { 1 };
bool drawsBackground { true };
bool needsResize { false };
+
+ bool clientRendersNextFrame { false };
+ bool coordinateUpdateCompletionWithClient { false };
} m_attributes;
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
Ref<ThreadedDisplayRefreshMonitor> m_displayRefreshMonitor;
#endif
-
- Atomic<bool> m_clientRendersNextFrame;
- Atomic<bool> m_coordinateUpdateCompletionWithClient;
};
} // namespace WebKit
Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedDisplayRefreshMonitor.cpp (220671 => 220672)
--- trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedDisplayRefreshMonitor.cpp 2017-08-14 09:46:33 UTC (rev 220671)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedDisplayRefreshMonitor.cpp 2017-08-14 10:23:35 UTC (rev 220672)
@@ -53,8 +53,11 @@
LockHolder locker(mutex());
setIsScheduled(true);
+ // Only request an update in case we're not currently handling the display
+ // refresh notifications under ThreadedDisplayRefreshMonitor::displayRefreshCallback().
+ // Any such schedule request is handled in that method after the notifications.
if (isPreviousFrameDone())
- m_compositor->coordinateUpdateCompletionWithClient();
+ m_compositor->requestDisplayRefreshMonitorUpdate();
return true;
}
@@ -80,7 +83,7 @@
void ThreadedDisplayRefreshMonitor::displayRefreshCallback()
{
- bool shouldHandleDisplayRefreshNotification = false;
+ bool shouldHandleDisplayRefreshNotification { false };
{
LockHolder locker(mutex());
shouldHandleDisplayRefreshNotification = isScheduled() && isPreviousFrameDone();
@@ -91,12 +94,18 @@
if (shouldHandleDisplayRefreshNotification)
DisplayRefreshMonitor::handleDisplayRefreshedNotificationOnMainThread(this);
- if (m_compositor) {
- m_compositor->renderNextFrameIfNeeded();
- m_compositor->completeCoordinatedUpdateIfNeeded();
- if (isScheduled())
- m_compositor->coordinateUpdateCompletionWithClient();
+ // Retrieve the scheduled status for this DisplayRefreshMonitor.
+ bool hasBeenRescheduled { false };
+ {
+ LockHolder locker(mutex());
+ hasBeenRescheduled = isScheduled();
}
+
+ // Notify the compositor about the completed DisplayRefreshMonitor update, passing
+ // along information about any schedule request that might have occurred during
+ // the notification handling.
+ if (m_compositor)
+ m_compositor->handleDisplayRefreshMonitorUpdate(hasBeenRescheduled);
}
} // namespace WebKit
