[Threaded Compositor] Modernize and simplify threaded compositor code
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jun 2016 07:52:25 +0000 (07:52 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jun 2016 07:52:25 +0000 (07:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158615

Reviewed by Žan Doberšek.

* Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:
(WebKit::CompositingRunLoop::performTask): Use NoncopyableFunction.
(WebKit::CompositingRunLoop::performTaskSync): Ditto.
(WebKit::CompositingRunLoop::startUpdateTimer): Just renamed to start instead of set.
(WebKit::CompositingRunLoop::run): Expose run/stop methods instead of the internal RunLoop object.
(WebKit::CompositingRunLoop::stop): Also stop the update timer instead of relying on the caller to do it.
(WebKit::CompositingRunLoop::setUpdateTimer): Deleted.
(WebKit::CompositingRunLoop::stopUpdateTimer): Deleted.
* Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:
* Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
(WebKit::ThreadedCompositor::setNativeSurfaceHandleForCompositing): Protects this directly in lambda capture.
(WebKit::ThreadedCompositor::setDeviceScaleFactor): Ditto.
(WebKit::ThreadedCompositor::didChangeViewportSize): Ditto.
(WebKit::ThreadedCompositor::didChangeViewportAttribute): Ditto.
(WebKit::ThreadedCompositor::didChangeContentsSize): Ditto.
(WebKit::ThreadedCompositor::scrollTo): Ditto.
(WebKit::ThreadedCompositor::scrollBy): Ditto.
(WebKit::ThreadedCompositor::updateViewport): Use startUpdateTimer().
(WebKit::ThreadedCompositor::scheduleDisplayImmediately): Ditto.
(WebKit::ThreadedCompositor::didChangeVisibleRect): Improve lambda captures.
(WebKit::ThreadedCompositor::renderLayerTree): Use m_viewportController directly.
(WebKit::ThreadedCompositor::createCompositingThread): Use createThread() version that receives a function.
(WebKit::ThreadedCompositor::runCompositingThread): Use run method and don't stop the update timer when the run
loop finishes.
(WebKit::ThreadedCompositor::terminateCompositingThread): Use stop method.
(WebKit::ThreadedCompositor::ThreadedCompositor): Deleted.
(WebKit::ThreadedCompositor::compositingThreadEntry): Deleted.
* Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h:
(WebKit::ThreadedCompositor::viewportController): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@202038 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp
Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h
Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp
Source/WebKit2/Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h

index 6f88d95..e54f4c1 100644 (file)
@@ -1,5 +1,42 @@
 2016-06-14  Carlos Garcia Campos  <cgarcia@igalia.com>
 
+        [Threaded Compositor] Modernize and simplify threaded compositor code
+        https://bugs.webkit.org/show_bug.cgi?id=158615
+
+        Reviewed by Žan Doberšek.
+
+        * Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.cpp:
+        (WebKit::CompositingRunLoop::performTask): Use NoncopyableFunction.
+        (WebKit::CompositingRunLoop::performTaskSync): Ditto.
+        (WebKit::CompositingRunLoop::startUpdateTimer): Just renamed to start instead of set.
+        (WebKit::CompositingRunLoop::run): Expose run/stop methods instead of the internal RunLoop object.
+        (WebKit::CompositingRunLoop::stop): Also stop the update timer instead of relying on the caller to do it.
+        (WebKit::CompositingRunLoop::setUpdateTimer): Deleted.
+        (WebKit::CompositingRunLoop::stopUpdateTimer): Deleted.
+        * Shared/CoordinatedGraphics/threadedcompositor/CompositingRunLoop.h:
+        * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.cpp:
+        (WebKit::ThreadedCompositor::setNativeSurfaceHandleForCompositing): Protects this directly in lambda capture.
+        (WebKit::ThreadedCompositor::setDeviceScaleFactor): Ditto.
+        (WebKit::ThreadedCompositor::didChangeViewportSize): Ditto.
+        (WebKit::ThreadedCompositor::didChangeViewportAttribute): Ditto.
+        (WebKit::ThreadedCompositor::didChangeContentsSize): Ditto.
+        (WebKit::ThreadedCompositor::scrollTo): Ditto.
+        (WebKit::ThreadedCompositor::scrollBy): Ditto.
+        (WebKit::ThreadedCompositor::updateViewport): Use startUpdateTimer().
+        (WebKit::ThreadedCompositor::scheduleDisplayImmediately): Ditto.
+        (WebKit::ThreadedCompositor::didChangeVisibleRect): Improve lambda captures.
+        (WebKit::ThreadedCompositor::renderLayerTree): Use m_viewportController directly.
+        (WebKit::ThreadedCompositor::createCompositingThread): Use createThread() version that receives a function.
+        (WebKit::ThreadedCompositor::runCompositingThread): Use run method and don't stop the update timer when the run
+        loop finishes.
+        (WebKit::ThreadedCompositor::terminateCompositingThread): Use stop method.
+        (WebKit::ThreadedCompositor::ThreadedCompositor): Deleted.
+        (WebKit::ThreadedCompositor::compositingThreadEntry): Deleted.
+        * Shared/CoordinatedGraphics/threadedcompositor/ThreadedCompositor.h:
+        (WebKit::ThreadedCompositor::viewportController): Deleted.
+
+2016-06-14  Carlos Garcia Campos  <cgarcia@igalia.com>
+
         [Threaded Compositor] Flickering and rendering artifacts when resizing the web view
         https://bugs.webkit.org/show_bug.cgi?id=154070
 
index f4ace10..5df5f33 100644 (file)
@@ -40,13 +40,13 @@ CompositingRunLoop::CompositingRunLoop(std::function<void ()>&& updateFunction)
 {
 }
 
-void CompositingRunLoop::performTask(std::function<void ()>&& function)
+void CompositingRunLoop::performTask(NoncopyableFunction<void ()>&& function)
 {
     ASSERT(isMainThread());
     m_runLoop.dispatch(WTFMove(function));
 }
 
-void CompositingRunLoop::performTaskSync(std::function<void ()>&& function)
+void CompositingRunLoop::performTaskSync(NoncopyableFunction<void ()>&& function)
 {
     ASSERT(isMainThread());
     LockHolder locker(m_dispatchSyncConditionMutex);
@@ -58,7 +58,7 @@ void CompositingRunLoop::performTaskSync(std::function<void ()>&& function)
     m_dispatchSyncCondition.wait(m_dispatchSyncConditionMutex);
 }
 
-void CompositingRunLoop::setUpdateTimer(UpdateTiming timing)
+void CompositingRunLoop::startUpdateTimer(UpdateTiming timing)
 {
     if (m_updateTimer.isActive())
         return;
@@ -71,18 +71,23 @@ void CompositingRunLoop::setUpdateTimer(UpdateTiming timing)
     m_updateTimer.startOneShot(nextUpdateTime);
 }
 
-void CompositingRunLoop::stopUpdateTimer()
-{
-    if (m_updateTimer.isActive())
-        m_updateTimer.stop();
-}
-
 void CompositingRunLoop::updateTimerFired()
 {
     m_updateFunction();
     m_lastUpdateTime = monotonicallyIncreasingTime();
 }
 
+void CompositingRunLoop::run()
+{
+    m_runLoop.run();
+}
+
+void CompositingRunLoop::stop()
+{
+    m_updateTimer.stop();
+    m_runLoop.stop();
+}
+
 } // namespace WebKit
 
 #endif // USE(COORDINATED_GRAPHICS_THREADED)
index fc0f95c..61fe2e8 100644 (file)
@@ -46,13 +46,13 @@ public:
 
     CompositingRunLoop(std::function<void ()>&&);
 
-    void performTask(std::function<void ()>&&);
-    void performTaskSync(std::function<void ()>&&);
+    void performTask(NoncopyableFunction<void ()>&&);
+    void performTaskSync(NoncopyableFunction<void ()>&&);
 
-    void setUpdateTimer(UpdateTiming timing = Immediate);
-    void stopUpdateTimer();
+    void startUpdateTimer(UpdateTiming = Immediate);
 
-    RunLoop& runLoop() { return m_runLoop; }
+    void run();
+    void stop();
 
 private:
     void updateTimerFired();
index 1c68e67..4ae3451 100644 (file)
@@ -50,8 +50,6 @@ Ref<ThreadedCompositor> ThreadedCompositor::create(Client* client)
 
 ThreadedCompositor::ThreadedCompositor(Client* client)
     : m_client(client)
-    , m_deviceScaleFactor(1)
-    , m_threadIdentifier(0)
 {
     createCompositingThread();
 }
@@ -63,59 +61,52 @@ ThreadedCompositor::~ThreadedCompositor()
 
 void ThreadedCompositor::setNativeSurfaceHandleForCompositing(uint64_t handle)
 {
-    RefPtr<ThreadedCompositor> protector(this);
-    m_compositingRunLoop->performTask([protector, handle] {
-        protector->m_nativeSurfaceHandle = handle;
-        protector->m_scene->setActive(true);
+    m_compositingRunLoop->performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), handle] {
+        m_nativeSurfaceHandle = handle;
+        m_scene->setActive(true);
     });
 }
 
 void ThreadedCompositor::setDeviceScaleFactor(float scale)
 {
-    RefPtr<ThreadedCompositor> protector(this);
-    m_compositingRunLoop->performTask([protector, scale] {
-        protector->m_deviceScaleFactor = scale;
-        protector->scheduleDisplayImmediately();
+    m_compositingRunLoop->performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), scale] {
+        m_deviceScaleFactor = scale;
+        scheduleDisplayImmediately();
     });
 }
 
 void ThreadedCompositor::didChangeViewportSize(const IntSize& size)
 {
-    RefPtr<ThreadedCompositor> protector(this);
-    m_compositingRunLoop->performTaskSync([protector, size] {
-        protector->viewportController()->didChangeViewportSize(size);
+    m_compositingRunLoop->performTaskSync([this, protectedThis = Ref<ThreadedCompositor>(*this), size] {
+        m_viewportController->didChangeViewportSize(size);
     });
 }
 
 void ThreadedCompositor::didChangeViewportAttribute(const ViewportAttributes& attr)
 {
-    RefPtr<ThreadedCompositor> protector(this);
-    m_compositingRunLoop->performTask([protector, attr] {
-        protector->viewportController()->didChangeViewportAttribute(attr);
+    m_compositingRunLoop->performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), attr] {
+        m_viewportController->didChangeViewportAttribute(attr);
     });
 }
 
 void ThreadedCompositor::didChangeContentsSize(const IntSize& size)
 {
-    RefPtr<ThreadedCompositor> protector(this);
-    m_compositingRunLoop->performTask([protector, size] {
-        protector->viewportController()->didChangeContentsSize(size);
+    m_compositingRunLoop->performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), size] {
+        m_viewportController->didChangeContentsSize(size);
     });
 }
 
 void ThreadedCompositor::scrollTo(const IntPoint& position)
 {
-    RefPtr<ThreadedCompositor> protector(this);
-    m_compositingRunLoop->performTask([protector, position] {
-        protector->viewportController()->scrollTo(position);
+    m_compositingRunLoop->performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), position] {
+        m_viewportController->scrollTo(position);
     });
 }
 
 void ThreadedCompositor::scrollBy(const IntSize& delta)
 {
-    RefPtr<ThreadedCompositor> protector(this);
-    m_compositingRunLoop->performTask([protector, delta] {
-        protector->viewportController()->scrollBy(delta);
+    m_compositingRunLoop->performTask([this, protectedThis = Ref<ThreadedCompositor>(*this), delta] {
+        m_viewportController->scrollBy(delta);
     });
 }
 
@@ -131,7 +122,7 @@ void ThreadedCompositor::renderNextFrame()
 
 void ThreadedCompositor::updateViewport()
 {
-    m_compositingRunLoop->setUpdateTimer(CompositingRunLoop::WaitUntilNextFrame);
+    m_compositingRunLoop->startUpdateTimer(CompositingRunLoop::WaitUntilNextFrame);
 }
 
 void ThreadedCompositor::commitScrollOffset(uint32_t layerID, const IntSize& offset)
@@ -173,18 +164,15 @@ GLContext* ThreadedCompositor::glContext()
 
 void ThreadedCompositor::scheduleDisplayImmediately()
 {
-    m_compositingRunLoop->setUpdateTimer(CompositingRunLoop::Immediate);
+    m_compositingRunLoop->startUpdateTimer(CompositingRunLoop::Immediate);
 }
 
 void ThreadedCompositor::didChangeVisibleRect()
 {
     ASSERT(&RunLoop::current() == &m_compositingRunLoop->runLoop());
 
-    RefPtr<ThreadedCompositor> protector(this);
-    FloatRect visibleRect = viewportController()->visibleContentsRect();
-    float scale = viewportController()->pageScaleFactor();
-    RunLoop::main().dispatch([protector, visibleRect, scale] {
-        protector->m_client->setVisibleContentsRect(visibleRect, FloatPoint::zero(), scale);
+    RunLoop::main().dispatch([this, protectedThis = Ref<ThreadedCompositor>(*this), visibleRect = m_viewportController->visibleContentsRect(), scale = m_viewportController->pageScaleFactor()] {
+        m_client->setVisibleContentsRect(visibleRect, FloatPoint::zero(), scale);
     });
 
     scheduleDisplayImmediately();
@@ -202,8 +190,8 @@ void ThreadedCompositor::renderLayerTree()
     FloatRect clipRect(0, 0, m_viewportSize.width(), m_viewportSize.height());
 
     TransformationMatrix viewportTransform;
-    FloatPoint scrollPostion = viewportController()->visibleContentsRect().location();
-    viewportTransform.scale(viewportController()->pageScaleFactor() * m_deviceScaleFactor);
+    FloatPoint scrollPostion = m_viewportController->visibleContentsRect().location();
+    viewportTransform.scale(m_viewportController->pageScaleFactor() * m_deviceScaleFactor);
     viewportTransform.translate(-scrollPostion.x(), -scrollPostion.y());
 
     m_scene->paintToCurrentGLContext(viewportTransform, 1, clipRect, Color::white, false, scrollPostion);
@@ -222,19 +210,13 @@ void ThreadedCompositor::updateSceneState(const CoordinatedGraphicsState& state)
     scheduleDisplayImmediately();
 }
 
-void ThreadedCompositor::compositingThreadEntry(void* coordinatedCompositor)
-{
-    static_cast<ThreadedCompositor*>(coordinatedCompositor)->runCompositingThread();
-}
-
 void ThreadedCompositor::createCompositingThread()
 {
     if (m_threadIdentifier)
         return;
 
     LockHolder locker(m_initializeRunLoopConditionMutex);
-    m_threadIdentifier = createThread(compositingThreadEntry, this, "WebCore: ThreadedCompositor");
-
+    m_threadIdentifier = createThread("WebKit: ThreadedCompositor", [this] { runCompositingThread(); });
     m_initializeRunLoopCondition.wait(m_initializeRunLoopConditionMutex);
 }
 
@@ -252,9 +234,8 @@ void ThreadedCompositor::runCompositingThread()
         m_initializeRunLoopCondition.notifyOne();
     }
 
-    m_compositingRunLoop->runLoop().run();
+    m_compositingRunLoop->run();
 
-    m_compositingRunLoop->stopUpdateTimer();
     m_scene->purgeGLResources();
 
     {
@@ -274,7 +255,7 @@ void ThreadedCompositor::terminateCompositingThread()
     LockHolder locker(m_terminateRunLoopConditionMutex);
 
     m_scene->detach();
-    m_compositingRunLoop->runLoop().stop();
+    m_compositingRunLoop->stop();
 
     m_terminateRunLoopCondition.wait(m_terminateRunLoopConditionMutex);
 }
index 64416b8..3700322 100644 (file)
@@ -90,7 +90,6 @@ private:
 
     bool ensureGLContext();
     WebCore::GLContext* glContext();
-    SimpleViewportController* viewportController() { return m_viewportController.get(); }
 
     void createCompositingThread();
     void runCompositingThread();
@@ -104,12 +103,12 @@ private:
     std::unique_ptr<WebCore::GLContext> m_context;
 
     WebCore::IntSize m_viewportSize;
-    float m_deviceScaleFactor;
-    uint64_t m_nativeSurfaceHandle;
+    float m_deviceScaleFactor { 1 };
+    uint64_t m_nativeSurfaceHandle { 0 };
 
     std::unique_ptr<CompositingRunLoop> m_compositingRunLoop;
 
-    ThreadIdentifier m_threadIdentifier;
+    ThreadIdentifier m_threadIdentifier { 0 };
     Condition m_initializeRunLoopCondition;
     Lock m_initializeRunLoopConditionMutex;
     Condition m_terminateRunLoopCondition;