Modified: trunk/Source/WebKit2/ChangeLog (219066 => 219067)
--- trunk/Source/WebKit2/ChangeLog 2017-07-03 09:00:49 UTC (rev 219066)
+++ trunk/Source/WebKit2/ChangeLog 2017-07-03 09:56:45 UTC (rev 219067)
@@ -1,3 +1,32 @@
+2017-07-03 Zan Dobersek <[email protected]>
+
+ [ThreadedCompositor] Update and retrieve scene attributes under a Lock
+ https://bugs.webkit.org/show_bug.cgi?id=173762
+
+ Reviewed by Carlos Garcia Campos.
+
+ Instead of dispatching separate tasks on the composition run loop, update
+ various scene attributes by locking a common lock object and updating the
+ appropriate attribute.
+
+ In ThreadedCompositor::renderLayerTree(), where these attributes are used
+ in scene composition, the lock is again obtained and the attributes copied
+ into local variables, releasing the lock afterwards. The attribute values
+ in local copies are then used for that renderLayerTree() invocation.
+
+ This approach is more efficient than dispatching separate tasks that can
+ race against renderLayerTree() dispatches.
+
+ * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
+ (WebKit::ThreadedCompositor::ThreadedCompositor):
+ (WebKit::m_displayRefreshMonitor):
+ (WebKit::ThreadedCompositor::setScaleFactor):
+ (WebKit::ThreadedCompositor::setScrollPosition):
+ (WebKit::ThreadedCompositor::setViewportSize):
+ (WebKit::ThreadedCompositor::setDrawsBackground):
+ (WebKit::ThreadedCompositor::renderLayerTree):
+ * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h:
+
2017-07-01 Dan Bernstein <[email protected]>
[iOS] Remove code only needed when building for iOS 9.x
Modified: trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp (219066 => 219067)
--- trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp 2017-07-03 09:00:49 UTC (rev 219066)
+++ trunk/Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp 2017-07-03 09:56:45 UTC (rev 219067)
@@ -51,16 +51,21 @@
ThreadedCompositor::ThreadedCompositor(Client& client, WebPage& webPage, const IntSize& viewportSize, float scaleFactor, ShouldDoFrameSync doFrameSync, TextureMapper::PaintFlags paintFlags)
: m_client(client)
- , m_viewportSize(viewportSize)
- , m_scaleFactor(scaleFactor)
, m_doFrameSync(doFrameSync)
, m_paintFlags(paintFlags)
- , m_needsResize(!viewportSize.isEmpty())
, m_compositingRunLoop(std::make_unique<CompositingRunLoop>([this] { renderLayerTree(); }))
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
, m_displayRefreshMonitor(ThreadedDisplayRefreshMonitor::create(*this))
#endif
{
+ {
+ // Locking isn't really necessary here, but it's done for consistency.
+ LockHolder locker(m_attributes.lock);
+ m_attributes.viewportSize = viewportSize;
+ m_attributes.scaleFactor = scaleFactor;
+ m_attributes.needsResize = !viewportSize.isEmpty();
+ }
+
m_clientRendersNextFrame.store(false);
m_coordinateUpdateCompletionWithClient.store(false);
@@ -130,37 +135,33 @@
void ThreadedCompositor::setScaleFactor(float scale)
{
- m_compositingRunLoop->performTask([this, protectedThis = makeRef(*this), scale] {
- m_scaleFactor = scale;
- m_compositingRunLoop->scheduleUpdate();
- });
+ LockHolder locker(m_attributes.lock);
+ m_attributes.scaleFactor = scale;
+ m_compositingRunLoop->scheduleUpdate();
}
void ThreadedCompositor::setScrollPosition(const IntPoint& scrollPosition, float scale)
{
- m_compositingRunLoop->performTask([this, protectedThis = makeRef(*this), scrollPosition, scale] {
- m_scrollPosition = scrollPosition;
- m_scaleFactor = scale;
- m_compositingRunLoop->scheduleUpdate();
- });
+ LockHolder locker(m_attributes.lock);
+ m_attributes.scrollPosition = scrollPosition;
+ m_attributes.scaleFactor = scale;
+ m_compositingRunLoop->scheduleUpdate();
}
void ThreadedCompositor::setViewportSize(const IntSize& viewportSize, float scale)
{
- m_compositingRunLoop->performTaskSync([this, protectedThis = makeRef(*this), viewportSize, scale] {
- m_viewportSize = viewportSize;
- m_scaleFactor = scale;
- m_needsResize = true;
- m_compositingRunLoop->scheduleUpdate();
- });
+ LockHolder locker(m_attributes.lock);
+ m_attributes.viewportSize = viewportSize;
+ m_attributes.scaleFactor = scale;
+ m_attributes.needsResize = true;
+ m_compositingRunLoop->scheduleUpdate();
}
void ThreadedCompositor::setDrawsBackground(bool drawsBackground)
{
- m_compositingRunLoop->performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), drawsBackground] {
- m_drawsBackground = drawsBackground;
- m_compositingRunLoop->scheduleUpdate();
- });
+ LockHolder locker(m_attributes.lock);
+ m_attributes.drawsBackground = drawsBackground;
+ m_compositingRunLoop->scheduleUpdate();
}
void ThreadedCompositor::renderNextFrame()
@@ -202,22 +203,38 @@
m_client.willRenderFrame();
- if (m_needsResize) {
- glViewport(0, 0, m_viewportSize.width(), m_viewportSize.height());
- m_needsResize = false;
+ // Retrieve the scene attributes in a thread-safe manner.
+ WebCore::IntSize viewportSize;
+ WebCore::IntPoint scrollPosition;
+ float scaleFactor;
+ bool drawsBackground;
+ bool needsResize;
+ {
+ LockHolder locker(m_attributes.lock);
+ viewportSize = m_attributes.viewportSize;
+ scrollPosition = m_attributes.scrollPosition;
+ scaleFactor = m_attributes.scaleFactor;
+ drawsBackground = m_attributes.drawsBackground;
+ needsResize = m_attributes.needsResize;
+
+ // Reset the needsResize attribute to false.
+ m_attributes.needsResize = false;
}
- FloatRect clipRect(0, 0, m_viewportSize.width(), m_viewportSize.height());
+ if (needsResize)
+ glViewport(0, 0, viewportSize.width(), viewportSize.height());
+
TransformationMatrix viewportTransform;
- viewportTransform.scale(m_scaleFactor);
- viewportTransform.translate(-m_scrollPosition.x(), -m_scrollPosition.y());
+ viewportTransform.scale(scaleFactor);
+ viewportTransform.translate(-scrollPosition.x(), -scrollPosition.y());
- if (!m_drawsBackground) {
+ if (!drawsBackground) {
glClearColor(0, 0, 0, 0);
glClear(GL_COLOR_BUFFER_BIT);
}
- m_scene->paintToCurrentGLContext(viewportTransform, 1, clipRect, Color::transparent, !m_drawsBackground, m_scrollPosition, m_paintFlags);
+ m_scene->paintToCurrentGLContext(viewportTransform, 1, FloatRect { FloatPoint { }, viewportSize },
+ Color::transparent, !drawsBackground, scrollPosition, m_paintFlags);
m_context->swapBuffers();
