Title: [91542] trunk/Source/WebCore
Revision
91542
Author
[email protected]
Date
2011-07-21 18:02:35 -0700 (Thu, 21 Jul 2011)

Log Message

GraphicsLayers in subframes can get sync'd multiple times
https://bugs.webkit.org/show_bug.cgi?id=52489

Reviewed by James Robinson.

Avoid doing a 'syncCompositingState' pass on the GraphicsLayers
for subframes, when those GraphicsLayers are rooted in the
parent document.

* page/FrameView.cpp:
(WebCore::FrameView::syncCompositingStateForThisFrame): Add a parameter
that contains the rootmost frame on which sync was called. This is used
to indicate to the compositor whether it's the root of the sync.
(WebCore::FrameView::syncCompositingStateIncludingSubframes): Pass the current
Frame in.
(WebCore::FrameView::paintContents): The sync is for this frame, so pass m_frame.
* page/FrameView.h:
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::flushPendingLayerChanges): New parameter
to specify whether this compositor is in the rootmost Frame being flushed.
We can avoid doing work if we know that flushing some ancestor frame would
have also traversed our GraphicsLayers.
* rendering/RenderLayerCompositor.h:
(WebCore::RenderLayerCompositor::isFlushingLayers): Make this private, since
callers are probably most intersted in enclosingCompositorFlushingLayers().

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (91541 => 91542)


--- trunk/Source/WebCore/ChangeLog	2011-07-22 00:48:07 UTC (rev 91541)
+++ trunk/Source/WebCore/ChangeLog	2011-07-22 01:02:35 UTC (rev 91542)
@@ -1,3 +1,31 @@
+2011-07-21  Simon Fraser  <[email protected]>
+
+        GraphicsLayers in subframes can get sync'd multiple times
+        https://bugs.webkit.org/show_bug.cgi?id=52489
+
+        Reviewed by James Robinson.
+        
+        Avoid doing a 'syncCompositingState' pass on the GraphicsLayers
+        for subframes, when those GraphicsLayers are rooted in the
+        parent document.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::syncCompositingStateForThisFrame): Add a parameter
+        that contains the rootmost frame on which sync was called. This is used
+        to indicate to the compositor whether it's the root of the sync.
+        (WebCore::FrameView::syncCompositingStateIncludingSubframes): Pass the current
+        Frame in.
+        (WebCore::FrameView::paintContents): The sync is for this frame, so pass m_frame.
+        * page/FrameView.h:
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::flushPendingLayerChanges): New parameter
+        to specify whether this compositor is in the rootmost Frame being flushed.
+        We can avoid doing work if we know that flushing some ancestor frame would
+        have also traversed our GraphicsLayers.
+        * rendering/RenderLayerCompositor.h:
+        (WebCore::RenderLayerCompositor::isFlushingLayers): Make this private, since
+        callers are probably most intersted in enclosingCompositorFlushingLayers().
+
 2011-07-21  Rafael Brandao  <[email protected]>
 
         Local files cannot load icons.

Modified: trunk/Source/WebCore/page/FrameView.cpp (91541 => 91542)


--- trunk/Source/WebCore/page/FrameView.cpp	2011-07-22 00:48:07 UTC (rev 91541)
+++ trunk/Source/WebCore/page/FrameView.cpp	2011-07-22 01:02:35 UTC (rev 91542)
@@ -684,7 +684,7 @@
     return view->compositor()->layerForScrollCorner();
 }
 
-bool FrameView::syncCompositingStateForThisFrame()
+bool FrameView::syncCompositingStateForThisFrame(Frame* rootFrameForSync)
 {
     ASSERT(m_frame->view() == this);
     RenderView* view = m_frame->contentRenderer();
@@ -696,7 +696,7 @@
     if (needsLayout())
         return false;
 
-    view->compositor()->flushPendingLayerChanges();
+    view->compositor()->flushPendingLayerChanges(rootFrameForSync == m_frame);
 
 #if ENABLE(FULLSCREEN_API)
     // The fullScreenRenderer's graphicsLayer  has been re-parented, and the above recursive syncCompositingState
@@ -789,10 +789,10 @@
 bool FrameView::syncCompositingStateIncludingSubframes()
 {
 #if USE(ACCELERATED_COMPOSITING)
-    bool allFramesSynced = syncCompositingStateForThisFrame();
+    bool allFramesSynced = syncCompositingStateForThisFrame(m_frame.get());
     
     for (Frame* child = m_frame->tree()->firstChild(); child; child = child->tree()->traverseNext(m_frame.get())) {
-        bool synced = child->view()->syncCompositingStateForThisFrame();
+        bool synced = child->view()->syncCompositingStateForThisFrame(m_frame.get());
         allFramesSynced &= synced;
     }
     return allFramesSynced;
@@ -2523,7 +2523,7 @@
 
 #if USE(ACCELERATED_COMPOSITING)
     if (!p->paintingDisabled())
-        syncCompositingStateForThisFrame();
+        syncCompositingStateForThisFrame(m_frame.get());
 #endif
 
     PaintBehavior oldPaintBehavior = m_paintBehavior;

Modified: trunk/Source/WebCore/page/FrameView.h (91541 => 91542)


--- trunk/Source/WebCore/page/FrameView.h	2011-07-22 00:48:07 UTC (rev 91541)
+++ trunk/Source/WebCore/page/FrameView.h	2011-07-22 01:02:35 UTC (rev 91542)
@@ -110,7 +110,7 @@
 
 #if USE(ACCELERATED_COMPOSITING)
     void updateCompositingLayers();
-    bool syncCompositingStateForThisFrame();
+    bool syncCompositingStateForThisFrame(Frame* rootFrameForSync);
 
     void clearBackingStores();
     void restoreBackingStores();

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (91541 => 91542)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2011-07-22 00:48:07 UTC (rev 91541)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp	2011-07-22 01:02:35 UTC (rev 91542)
@@ -191,15 +191,18 @@
     page->chrome()->client()->scheduleCompositingLayerSync();
 }
 
-void RenderLayerCompositor::flushPendingLayerChanges()
+void RenderLayerCompositor::flushPendingLayerChanges(bool isFlushRoot)
 {
+    // FrameView::syncCompositingStateIncludingSubframes() flushes each subframe,
+    // but GraphicsLayer::syncCompositingState() will cross frame boundaries
+    // if the GraphicsLayers are connected (the RootLayerAttachedViaEnclosingFrame case).
+    // As long as we're not the root of the flush, we can bail.
+    if (!isFlushRoot && rootLayerAttachment() == RootLayerAttachedViaEnclosingFrame)
+        return;
+    
     ASSERT(!m_flushingLayers);
     m_flushingLayers = true;
 
-    // FIXME: FrameView::syncCompositingStateRecursive() calls this for each
-    // frame, so when compositing layers are connected between frames, we'll
-    // end up syncing subframe's layers multiple times.
-    // https://bugs.webkit.org/show_bug.cgi?id=52489
     if (GraphicsLayer* rootLayer = rootGraphicsLayer())
         rootLayer->syncCompositingState();
 

Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.h (91541 => 91542)


--- trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2011-07-22 00:48:07 UTC (rev 91541)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.h	2011-07-22 01:02:35 UTC (rev 91542)
@@ -87,8 +87,7 @@
     // GraphicsLayers buffer state, which gets pushed to the underlying platform layers
     // at specific times.
     void scheduleLayerFlush();
-    void flushPendingLayerChanges();
-    bool isFlushingLayers() const { return m_flushingLayers; }
+    void flushPendingLayerChanges(bool isFlushRoot = true);
     
     // flushPendingLayerChanges() flushes the entire GraphicsLayer tree, which can cross frame boundaries.
     // This call returns the rootmost compositor that is being flushed (including self).
@@ -260,6 +259,8 @@
 
     void notifyIFramesOfCompositingChange();
 
+    bool isFlushingLayers() const { return m_flushingLayers; }
+
     // Whether a running transition or animation enforces the need for a compositing layer.
     bool requiresCompositingForAnimation(RenderObject*) const;
     bool requiresCompositingForTransform(RenderObject*) const;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to