diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog index c01624a16d0b..4734963bea3c 100644 --- a/Source/WebKit/ChangeLog +++ b/Source/WebKit/ChangeLog @@ -1,3 +1,55 @@ +2017-08-16 Zan Dobersek + + [CoordGraphics] Simplify CoordinatedGraphicsScene state updates + https://bugs.webkit.org/show_bug.cgi?id=175528 + + + Reviewed by Carlos Garcia Campos. + + Hold the information about state updates in ThreadedCompositor, in the + m_attributes struct. This way we don't need to store the updates in + functors and accumulate them in the CoordinatedGraphicsScene class, but + instead just apply any pending state update or atlas removal before the + scene is rendered. + + This removes the need to update the CoordinatedGraphicsScene object from + the main thread with data that ultimately has to be handled on the + composition thread. Similarly, when updating CoordinatedGraphicsScene, we + only need to synchronize on the m_attributes lock object once in order to + retrieve the scene update information, instead of having each functor do + that repeatedly. + + Outside of CoordinatedGraphicsScene and ThreadedCompositor classes, the + CompositingCoordinator class now passes the atlases-to-remove Vector by + a const lvalue reference down to ThreadedCompositor, and then manually + clears the Vector. Before the Vector was passed as an rvalue reference, + depending on the ThreadedCompositor code to clear out the original Vector + object by moving its resources into the functor object. This doesn't occur + anymore because the Vector object is now appended to another Vector. + + * Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp: + (WebKit::CoordinatedGraphicsScene::applyStateChanges): + (WebKit::CoordinatedGraphicsScene::paintToCurrentGLContext): + (WebKit::CoordinatedGraphicsScene::detach): + (WebKit::CoordinatedGraphicsScene::setActive): + (WebKit::CoordinatedGraphicsScene::syncRemoteContent): Deleted. + (WebKit::CoordinatedGraphicsScene::appendUpdate): Deleted. + * Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h: + * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp: + (WebKit::ThreadedCompositor::renderLayerTree): + (WebKit::ThreadedCompositor::updateSceneState): + (WebKit::ThreadedCompositor::releaseUpdateAtlases): + * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h: + * WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp: + (WebKit::CompositingCoordinator::flushPendingLayerChanges): + (WebKit::CompositingCoordinator::releaseAtlases): + (WebKit::CompositingCoordinator::clearUpdateAtlases): + * WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h: + * WebProcess/WebPage/CoordinatedGraphics/CoordinatedLayerTreeHost.h: + * WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp: + (WebKit::ThreadedCoordinatedLayerTreeHost::releaseUpdateAtlases): + * WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.h: + 2017-08-14 Zan Dobersek [ThreadedCompositor] Improve composition and update state handling diff --git a/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp b/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp index 35e12d782f7c..12fc063302f7 100644 --- a/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp +++ b/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp @@ -77,15 +77,21 @@ CoordinatedGraphicsScene::~CoordinatedGraphicsScene() { } -void CoordinatedGraphicsScene::paintToCurrentGLContext(const TransformationMatrix& matrix, float opacity, const FloatRect& clipRect, const Color& backgroundColor, bool drawsBackground, const FloatPoint& contentPosition, TextureMapper::PaintFlags PaintFlags) +void CoordinatedGraphicsScene::applyStateChanges(const Vector& states) { if (!m_textureMapper) { m_textureMapper = TextureMapper::create(); static_cast(m_textureMapper.get())->setEnableEdgeDistanceAntialiasing(true); } - syncRemoteContent(); + ensureRootLayer(); + + for (auto& state : states) + commitSceneState(state); +} +void CoordinatedGraphicsScene::paintToCurrentGLContext(const TransformationMatrix& matrix, float opacity, const FloatRect& clipRect, const Color& backgroundColor, bool drawsBackground, const FloatPoint& contentPosition, TextureMapper::PaintFlags PaintFlags) +{ adjustPositionForFixedLayers(contentPosition); TextureMapperLayer* currentRootLayer = rootLayer(); if (!currentRootLayer) @@ -580,23 +586,6 @@ void CoordinatedGraphicsScene::ensureRootLayer() m_rootLayer->setTextureMapper(m_textureMapper.get()); } -void CoordinatedGraphicsScene::syncRemoteContent() -{ - // We enqueue messages and execute them during paint, as they require an active GL context. - ensureRootLayer(); - - Vector> renderQueue; - bool calledOnMainThread = RunLoop::isMain(); - if (!calledOnMainThread) - m_renderQueueMutex.lock(); - renderQueue = WTFMove(m_renderQueue); - if (!calledOnMainThread) - m_renderQueueMutex.unlock(); - - for (auto& function : renderQueue) - function(); -} - void CoordinatedGraphicsScene::purgeGLResources() { ASSERT(!m_client); @@ -642,32 +631,13 @@ void CoordinatedGraphicsScene::detach() ASSERT(RunLoop::isMain()); m_isActive = false; m_client = nullptr; - LockHolder locker(m_renderQueueMutex); - m_renderQueue.clear(); -} - -void CoordinatedGraphicsScene::appendUpdate(Function&& function) -{ - if (!m_isActive) - return; - - ASSERT(RunLoop::isMain()); - LockHolder locker(m_renderQueueMutex); - m_renderQueue.append(WTFMove(function)); } void CoordinatedGraphicsScene::setActive(bool active) { - if (!m_client) - return; - - if (m_isActive == active) + if (!m_client || m_isActive == active) return; - // Have to clear render queue in both cases. - // If there are some updates in queue during activation then those updates are from previous instance of paint node - // and cannot be applied to the newly created instance. - m_renderQueue.clear(); m_isActive = active; if (m_isActive) renderNextFrame(); diff --git a/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h b/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h index 6653b10b3605..4b1f04600a41 100644 --- a/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h +++ b/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h @@ -68,9 +68,10 @@ class CoordinatedGraphicsScene : public ThreadSafeRefCounted&); void paintToCurrentGLContext(const WebCore::TransformationMatrix&, float, const WebCore::FloatRect&, const WebCore::Color& backgroundColor, bool drawsBackground, const WebCore::FloatPoint&, WebCore::TextureMapper::PaintFlags = 0); void detach(); - void appendUpdate(Function&&); WebCore::TextureMapperLayer* findScrollableContentsLayerAt(const WebCore::FloatPoint&); @@ -124,7 +125,6 @@ class CoordinatedGraphicsScene : public ThreadSafeRefCounted&&); @@ -149,10 +149,6 @@ class CoordinatedGraphicsScene : public ThreadSafeRefCounted> m_renderQueue; - Lock m_renderQueueMutex; - std::unique_ptr m_textureMapper; HashMap> m_imageBackings; diff --git a/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp b/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp index a64355d97c52..3c090e1cd2fe 100644 --- a/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp +++ b/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp @@ -208,6 +208,9 @@ void ThreadedCompositor::renderLayerTree() float scaleFactor; bool drawsBackground; bool needsResize; + Vector states; + Vector atlasesToRemove; + { LockHolder locker(m_attributes.lock); viewportSize = m_attributes.viewportSize; @@ -216,6 +219,25 @@ void ThreadedCompositor::renderLayerTree() drawsBackground = m_attributes.drawsBackground; needsResize = m_attributes.needsResize; + states = WTFMove(m_attributes.states); + atlasesToRemove = WTFMove(m_attributes.atlasesToRemove); + + if (!states.isEmpty()) { + // 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. + // But do not change coordinateUpdateCompletionWithClient while in force repaint because that + // demands immediate scene update completion regardless of platform layers. + if (!m_inForceRepaint) { + bool coordinateUpdate = false; + for (auto& state : states) + coordinateUpdate |= std::any_of(state.layersToUpdate.begin(), state.layersToUpdate.end(), + [](auto& it) { return it.second.platformLayerChanged || it.second.platformLayerUpdated; }); + m_attributes.coordinateUpdateCompletionWithClient = coordinateUpdate; + } + } + // Reset the needsResize attribute to false. m_attributes.needsResize = false; } @@ -232,6 +254,8 @@ void ThreadedCompositor::renderLayerTree() glClear(GL_COLOR_BUFFER_BIT); } + m_scene->applyStateChanges(states); + m_scene->releaseUpdateAtlases(atlasesToRemove); m_scene->paintToCurrentGLContext(viewportTransform, 1, FloatRect { FloatPoint { }, viewportSize }, Color::transparent, !drawsBackground, scrollPosition, m_paintFlags); @@ -278,34 +302,15 @@ void ThreadedCompositor::sceneUpdateFinished() void ThreadedCompositor::updateSceneState(const CoordinatedGraphicsState& state) { - ASSERT(RunLoop::isMain()); - m_scene->appendUpdate([this, scene = makeRef(*m_scene), state] { - scene->commitSceneState(state); - - 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. - bool coordinateUpdate = !m_inForceRepaint && std::any_of(state.layersToUpdate.begin(), state.layersToUpdate.end(), - [](const std::pair& it) { - return it.second.platformLayerChanged || it.second.platformLayerUpdated; - }); - - m_attributes.coordinateUpdateCompletionWithClient |= coordinateUpdate; - }); - + LockHolder locker(m_attributes.lock); + m_attributes.states.append(state); m_compositingRunLoop->scheduleUpdate(); } -void ThreadedCompositor::releaseUpdateAtlases(Vector&& atlasesToRemove) +void ThreadedCompositor::releaseUpdateAtlases(const Vector& atlasesToRemove) { - ASSERT(RunLoop::isMain()); - m_scene->appendUpdate([scene = makeRef(*m_scene), atlasesToRemove = WTFMove(atlasesToRemove)] { - scene->releaseUpdateAtlases(atlasesToRemove); - }); + LockHolder locker(m_attributes.lock); + m_attributes.atlasesToRemove.appendVector(atlasesToRemove); m_compositingRunLoop->scheduleUpdate(); } diff --git a/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h b/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h index e6100f8768cb..ee09da430b52 100644 --- a/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h +++ b/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h @@ -29,6 +29,7 @@ #include "CompositingRunLoop.h" #include "CoordinatedGraphicsScene.h" +#include #include #include #include @@ -41,10 +42,6 @@ #include #endif -namespace WebCore { -struct CoordinatedGraphicsState; -} - namespace WebKit { class CoordinatedGraphicsScene; @@ -80,7 +77,7 @@ class ThreadedCompositor : public CoordinatedGraphicsSceneClient, public ThreadS void setDrawsBackground(bool); void updateSceneState(const WebCore::CoordinatedGraphicsState&); - void releaseUpdateAtlases(Vector&&); + void releaseUpdateAtlases(const Vector&); void invalidate(); @@ -126,6 +123,9 @@ class ThreadedCompositor : public CoordinatedGraphicsSceneClient, public ThreadS bool drawsBackground { true }; bool needsResize { false }; + Vector states; + Vector atlasesToRemove; + bool clientRendersNextFrame { false }; bool coordinateUpdateCompletionWithClient { false }; } m_attributes; diff --git a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp index 1ce73fcdb1e2..0228a09530ae 100644 --- a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp +++ b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.cpp @@ -138,7 +138,8 @@ bool CompositingCoordinator::flushPendingLayerChanges() m_client.commitSceneState(m_state); if (!m_atlasesToRemove.isEmpty()) - m_client.releaseUpdateAtlases(WTFMove(m_atlasesToRemove)); + m_client.releaseUpdateAtlases(m_atlasesToRemove); + m_atlasesToRemove.clear(); clearPendingStateChanges(); m_shouldSyncFrame = false; @@ -442,7 +443,8 @@ void CompositingCoordinator::releaseAtlases(ReleaseAtlasPolicy policy) m_releaseInactiveAtlasesTimer.stop(); if (!m_atlasesToRemove.isEmpty()) - m_client.releaseUpdateAtlases(WTFMove(m_atlasesToRemove)); + m_client.releaseUpdateAtlases(m_atlasesToRemove); + m_atlasesToRemove.clear(); } void CompositingCoordinator::clearUpdateAtlases() @@ -454,7 +456,8 @@ void CompositingCoordinator::clearUpdateAtlases() m_updateAtlases.clear(); if (!m_atlasesToRemove.isEmpty()) - m_client.releaseUpdateAtlases(WTFMove(m_atlasesToRemove)); + m_client.releaseUpdateAtlases(m_atlasesToRemove); + m_atlasesToRemove.clear(); } } // namespace WebKit diff --git a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h index fb275631cff2..2d96d0a9c0e3 100644 --- a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h +++ b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h @@ -60,7 +60,7 @@ class CompositingCoordinator final : public WebCore::GraphicsLayerClient virtual void notifyFlushRequired() = 0; virtual void commitSceneState(const WebCore::CoordinatedGraphicsState&) = 0; virtual void paintLayerContents(const WebCore::GraphicsLayer*, WebCore::GraphicsContext&, const WebCore::IntRect& clipRect) = 0; - virtual void releaseUpdateAtlases(Vector&&) = 0; + virtual void releaseUpdateAtlases(const Vector&) = 0; }; CompositingCoordinator(WebCore::Page*, CompositingCoordinator::Client&); diff --git a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CoordinatedLayerTreeHost.h b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CoordinatedLayerTreeHost.h index 74bb5d359bf2..aa51cac72eda 100644 --- a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CoordinatedLayerTreeHost.h +++ b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CoordinatedLayerTreeHost.h @@ -75,7 +75,7 @@ class CoordinatedLayerTreeHost : public LayerTreeHost, public CompositingCoordin void notifyFlushRequired() override { scheduleLayerFlush(); }; void commitSceneState(const WebCore::CoordinatedGraphicsState&) override; void paintLayerContents(const WebCore::GraphicsLayer*, WebCore::GraphicsContext&, const WebCore::IntRect& clipRect) override; - void releaseUpdateAtlases(Vector&&) override { }; + void releaseUpdateAtlases(const Vector&) override { }; private: void layerFlushTimerFired(); diff --git a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp index 7bed39a15d7a..6f199a5f9c95 100644 --- a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp +++ b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp @@ -249,9 +249,9 @@ void ThreadedCoordinatedLayerTreeHost::commitSceneState(const CoordinatedGraphic m_compositor->updateSceneState(state); } -void ThreadedCoordinatedLayerTreeHost::releaseUpdateAtlases(Vector&& atlasesToRemove) +void ThreadedCoordinatedLayerTreeHost::releaseUpdateAtlases(const Vector& atlasesToRemove) { - m_compositor->releaseUpdateAtlases(WTFMove(atlasesToRemove)); + m_compositor->releaseUpdateAtlases(atlasesToRemove); } void ThreadedCoordinatedLayerTreeHost::setIsDiscardable(bool discardable) diff --git a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.h b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.h index 93debdcf8b62..2f2bc46ddb8d 100644 --- a/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.h +++ b/Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.h @@ -118,7 +118,7 @@ class ThreadedCoordinatedLayerTreeHost final : public CoordinatedLayerTreeHost, // CompositingCoordinator::Client void didFlushRootLayer(const WebCore::FloatRect&) override { } void commitSceneState(const WebCore::CoordinatedGraphicsState&) override; - void releaseUpdateAtlases(Vector&&) override; + void releaseUpdateAtlases(const Vector&) override; #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) RefPtr createDisplayRefreshMonitor(WebCore::PlatformDisplayID) override;