Changeset 220742 in webkit

Timestamp:

Aug 15, 2017 7:16:29 AM (7 years ago)

Author:

Michael Catanzaro

Message:

Unreviewed, rolling out r220700.

Broke debug bot

Reverted changeset:

"[CoordGraphics] Simplify CoordinatedGraphicsScene state
updates"
​https://bugs.webkit.org/show_bug.cgi?id=175528
​http://trac.webkit.org/changeset/220700

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp (modified) (4 diffs)
• Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h (modified) (3 diffs)
• Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp (modified) (5 diffs)
• Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h (modified) (3 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2017-08-15  Michael Catanzaro  <mcatanzaro@igalia.com>
+
+        Unreviewed, rolling out r220700.
+
+        Broke debug bot
+
+        Reverted changeset:
+
+        "[CoordGraphics] Simplify CoordinatedGraphicsScene state
+        updates"
+        https://bugs.webkit.org/show_bug.cgi?id=175528
+        http://trac.webkit.org/changeset/220700
+
 2017-08-15  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp

@@ -78 +78 @@
 }
 
-void CoordinatedGraphicsScene::applyStateChanges(const Vector<CoordinatedGraphicsState>& states)
+void CoordinatedGraphicsScene::paintToCurrentGLContext(const TransformationMatrix& matrix, float opacity, const FloatRect& clipRect, const Color& backgroundColor, bool drawsBackground, const FloatPoint& contentPosition, TextureMapper::PaintFlags PaintFlags)
 {
     if (!m_textureMapper) {
@@ -85 +85 @@
     }
 
-    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)
-{
+    syncRemoteContent();
+
     adjustPositionForFixedLayers(contentPosition);
     TextureMapperLayer* currentRootLayer = rootLayer();
@@ -587 +581 @@
 }
 
+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()
 {
@@ -632 +643 @@
     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 || m_isActive == active)
-        return;
-
+    if (!m_client)
+        return;
+
+    if (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)

trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h

@@ -69 +69 @@
     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&);
@@ -126 +125 @@
     WebCore::TextureMapperLayer* rootLayer() { return m_rootLayer.get(); }
 
+    void syncRemoteContent();
     void adjustPositionForFixedLayers(const WebCore::FloatPoint& contentPosition);
 
@@ -149 +149 @@
     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;

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

@@ -209 +209 @@
     bool drawsBackground;
     bool needsResize;
-    Vector<WebCore::CoordinatedGraphicsState> states;
-    Vector<uint32_t> atlasesToRemove;
-
     {
         LockHolder locker(m_attributes.lock);
@@ -220 +217 @@
         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;
@@ -255 +233 @@
     }
 
-    m_scene->applyStateChanges(states);
-    m_scene->releaseUpdateAtlases(atlasesToRemove);
     m_scene->paintToCurrentGLContext(viewportTransform, 1, FloatRect { FloatPoint { }, viewportSize },
         Color::transparent, !drawsBackground, scrollPosition, m_paintFlags);
@@ -303 +279 @@
 void ThreadedCompositor::updateSceneState(const CoordinatedGraphicsState& state)
 {
-    LockHolder locker(m_attributes.lock);
-    m_attributes.states.append(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;
+    });
+
     m_compositingRunLoop->scheduleUpdate();
 }
@@ -310 +303 @@
 void ThreadedCompositor::releaseUpdateAtlases(Vector<uint32_t>&& atlasesToRemove)
 {
-    LockHolder locker(m_attributes.lock);
-    m_attributes.atlasesToRemove.appendVector(atlasesToRemove);
+    ASSERT(RunLoop::isMain());
+    m_scene->appendUpdate([scene = makeRef(*m_scene), atlasesToRemove = WTFMove(atlasesToRemove)] {
+        scene->releaseUpdateAtlases(atlasesToRemove);
+    });
     m_compositingRunLoop->scheduleUpdate();
 }

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

@@ -30 +30 @@
 #include "CompositingRunLoop.h"
 #include "CoordinatedGraphicsScene.h"
-#include <WebCore/CoordinatedGraphicsState.h>
 #include <WebCore/GLContext.h>
 #include <WebCore/IntSize.h>
@@ -42 +41 @@
 #include <WebCore/DisplayRefreshMonitor.h>
 #endif
+
+namespace WebCore {
+struct CoordinatedGraphicsState;
+}
 
 namespace WebKit {
@@ -124 +127 @@
         bool needsResize { false };
 
-        Vector<WebCore::CoordinatedGraphicsState> states;
-        Vector<uint32_t> atlasesToRemove;
-
         bool clientRendersNextFrame { false };
         bool coordinateUpdateCompletionWithClient { false };