- Revision
- 220700
- Author
- zandober...@gmail.com
- Date
- 2017-08-14 09:41:18 -0700 (Mon, 14 Aug 2017)
Log Message
[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.
* 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:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (220699 => 220700)
--- trunk/Source/WebKit/ChangeLog 2017-08-14 16:37:01 UTC (rev 220699)
+++ trunk/Source/WebKit/ChangeLog 2017-08-14 16:41:18 UTC (rev 220700)
@@ -1,3 +1,37 @@
+2017-08-14 Zan Dobersek <zdober...@igalia.com>
+
+ [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.
+
+ * 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:
+
2017-08-14 Carlos Garcia Campos <cgar...@igalia.com>
[GTK][WPE] Avoid emitting WebKitFaviconDatabase::favicon-changed multiple times while setting an icon
Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp (220699 => 220700)
--- trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp 2017-08-14 16:37:01 UTC (rev 220699)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.cpp 2017-08-14 16:41:18 UTC (rev 220700)
@@ -77,7 +77,7 @@
{
}
-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();
@@ -84,8 +84,14 @@
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 @@
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 @@
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)
+ if (!m_client || m_isActive == active)
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)
renderNextFrame();
Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h (220699 => 220700)
--- trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h 2017-08-14 16:37:01 UTC (rev 220699)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/CoordinatedGraphicsScene.h 2017-08-14 16:41:18 UTC (rev 220700)
@@ -68,9 +68,10 @@
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 @@
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 @@
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;
Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp (220699 => 220700)
--- trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp 2017-08-14 16:37:01 UTC (rev 220699)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp 2017-08-14 16:41:18 UTC (rev 220700)
@@ -208,6 +208,9 @@
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 @@
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 @@
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::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)
{
- 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();
}
Modified: trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h (220699 => 220700)
--- trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h 2017-08-14 16:37:01 UTC (rev 220699)
+++ trunk/Source/WebKit/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h 2017-08-14 16:41:18 UTC (rev 220700)
@@ -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;
@@ -126,6 +123,9 @@
bool drawsBackground { true };
bool needsResize { false };
+ Vector<WebCore::CoordinatedGraphicsState> states;
+ Vector<uint32_t> atlasesToRemove;
+
bool clientRendersNextFrame { false };
bool coordinateUpdateCompletionWithClient { false };
} m_attributes;
