Merge r220792 - [CoordGraphics] Simplify CoordinatedGraphicsScene sta…

…te updates

https://bugs.webkit.org/show_bug.cgi?id=175528
<rdar://problem/33876795>

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:

Source/WebKit/ChangeLog

@@ -1,3 +1,55 @@
+2017-08-16  Zan Dobersek  <zdobersek@igalia.com>
+
+        [CoordGraphics] Simplify CoordinatedGraphicsScene state updates
+        https://bugs.webkit.org/show_bug.cgi?id=175528
+        <rdar://problem/33876795>
+
+        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  <zdobersek@igalia.com>
 
         [ThreadedCompositor] Improve composition and update state handling

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<CoordinatedGraphicsState>& states)
 {
     if (!m_textureMapper) {
         m_textureMapper = TextureMapper::create();
         static_cast<TextureMapperGL*>(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<Function<void()>> 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<void()>&& 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();

Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h

@@ -68,9 +68,10 @@ class CoordinatedGraphicsScene : public ThreadSafeRefCounted<CoordinatedGraphics
 public:
     explicit CoordinatedGraphicsScene(CoordinatedGraphicsSceneClient*);
     virtual ~CoordinatedGraphicsScene();
+
+    void applyStateChanges(const Vector<WebCore::CoordinatedGraphicsState>&);
     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<void()>&&);
 
     WebCore::TextureMapperLayer* findScrollableContentsLayerAt(const WebCore::FloatPoint&);
 
@@ -124,7 +125,6 @@ class CoordinatedGraphicsScene : public ThreadSafeRefCounted<CoordinatedGraphics
     WebCore::TextureMapperLayer* getLayerByIDIfExists(WebCore::CoordinatedLayerID);
     WebCore::TextureMapperLayer* rootLayer() { return m_rootLayer.get(); }
 
-    void syncRemoteContent();
     void adjustPositionForFixedLayers(const WebCore::FloatPoint& contentPosition);
 
     void dispatchOnMainThread(Function<void()>&&);
@@ -149,10 +149,6 @@ class CoordinatedGraphicsScene : public ThreadSafeRefCounted<CoordinatedGraphics
     WebCore::TextureMapperGL* texmapGL() override;
 #endif
 
-    // Render queue can be accessed ony from main thread or updatePaintNode call stack!
-    Vector<Function<void()>> m_renderQueue;
-    Lock m_renderQueueMutex;
-
     std::unique_ptr<WebCore::TextureMapper> m_textureMapper;
 
     HashMap<WebCore::CoordinatedImageBackingID, RefPtr<CoordinatedBackingStore>> m_imageBackings;

Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp

@@ -208,6 +208,9 @@ void ThreadedCompositor::renderLayerTree()
     float scaleFactor;
     bool drawsBackground;
     bool needsResize;
+    Vector<WebCore::CoordinatedGraphicsState> states;
+    Vector<uint32_t> 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<CoordinatedLayerID, CoordinatedGraphicsLayerState>& 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<uint32_t>&& atlasesToRemove)
+void ThreadedCompositor::releaseUpdateAtlases(const Vector<uint32_t>& 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();
 }
 

Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h

@@ -29,6 +29,7 @@
 
 #include "CompositingRunLoop.h"
 #include "CoordinatedGraphicsScene.h"
+#include <WebCore/CoordinatedGraphicsState.h>
 #include <WebCore/GLContext.h>
 #include <WebCore/IntSize.h>
 #include <WebCore/TextureMapper.h>
@@ -41,10 +42,6 @@
 #include <WebCore/DisplayRefreshMonitor.h>
 #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<uint32_t>&&);
+    void releaseUpdateAtlases(const Vector<uint32_t>&);
 
     void invalidate();
 
@@ -126,6 +123,9 @@ class ThreadedCompositor : public CoordinatedGraphicsSceneClient, public ThreadS
         bool drawsBackground { true };
         bool needsResize { false };
 
+        Vector<WebCore::CoordinatedGraphicsState> states;
+        Vector<uint32_t> atlasesToRemove;
+
         bool clientRendersNextFrame { false };
         bool coordinateUpdateCompletionWithClient { false };
     } m_attributes;

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

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<uint32_t>&&) = 0;
+        virtual void releaseUpdateAtlases(const Vector<uint32_t>&) = 0;
     };
 
     CompositingCoordinator(WebCore::Page*, CompositingCoordinator::Client&);

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<uint32_t>&&) override { };
+    void releaseUpdateAtlases(const Vector<uint32_t>&) override { };
 
 private:
     void layerFlushTimerFired();

Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/ThreadedCoordinatedLayerTreeHost.cpp

@@ -249,9 +249,9 @@ void ThreadedCoordinatedLayerTreeHost::commitSceneState(const CoordinatedGraphic
     m_compositor->updateSceneState(state);
 }
 
-void ThreadedCoordinatedLayerTreeHost::releaseUpdateAtlases(Vector<uint32_t>&& atlasesToRemove)
+void ThreadedCoordinatedLayerTreeHost::releaseUpdateAtlases(const Vector<uint32_t>& atlasesToRemove)
 {
-    m_compositor->releaseUpdateAtlases(WTFMove(atlasesToRemove));
+    m_compositor->releaseUpdateAtlases(atlasesToRemove);
 }
 
 void ThreadedCoordinatedLayerTreeHost::setIsDiscardable(bool discardable)

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<uint32_t>&&) override;
+    void releaseUpdateAtlases(const Vector<uint32_t>&) override;
 
 #if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
     RefPtr<WebCore::DisplayRefreshMonitor> createDisplayRefreshMonitor(WebCore::PlatformDisplayID) override;