Changeset 162050 in webkit

Timestamp:

Jan 14, 2014 10:43:39 PM (10 years ago)

Author:

mihnea@adobe.com

Message:

[CSSRegions] Incorrect repaint of fixed element with transformed parent
​https://bugs.webkit.org/show_bug.cgi?id=125756

Reviewed by Simon Fraser.

Source/WebCore:

When collecting the layers for fixed positioned elements with named flow
as a containing block, use layers collection at named flow layer level
instead of relying on the positioned elements collection.

With this approach, there are situations with nested named flows laid out in
auto-height regions and thus assuming 2 step layouts, in which is enough to lay
out the inner named flow once instead of twice with a performance gain.
In such situations, the layers lists for the inner named flow are not yet updated
when collectFixedPositionedLayers is called to paint the fixed positioned layers.
Therefore I called updateLayerListsIfNeeded for all named flows layers, via the flow thread controller, before inspecting the named flow layers to avoid hitting the assertions when
accessing the named flows layers lists.

Tests: fast/regions/repaint/fixed-in-named-flow-cb-changed.html

fast/regions/repaint/fixed-in-named-flow-cb-changed2.html

• rendering/FlowThreadController.cpp:

(WebCore::FlowThreadController::updateNamedFlowsLayerListsIfNeeded):
(WebCore::FlowThreadController::collectFixedPositionedLayers):

• rendering/FlowThreadController.h:
• rendering/RenderLayer.cpp:

(WebCore::RenderLayer::paintFixedLayersInNamedFlows):

LayoutTests:

Add tests for a fixed positioned element with a parent that:

1. dynamically gets a transform, in which case the fixed positioned element should be positioned

relative to its parent

2. dynamically loses its transform, in which case the fixed positioned element should be positioned

relative to the view.

• fast/regions/repaint/fixed-in-named-flow-cb-changed-expected.html: Added.
• fast/regions/repaint/fixed-in-named-flow-cb-changed.html: Added.
• fast/regions/repaint/fixed-in-named-flow-cb-changed2-expected.html: Added.
• fast/regions/repaint/fixed-in-named-flow-cb-changed2.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/regions/repaint/fixed-in-named-flow-cb-changed-expected.html (added)
• LayoutTests/fast/regions/repaint/fixed-in-named-flow-cb-changed.html (added)
• LayoutTests/fast/regions/repaint/fixed-in-named-flow-cb-changed2-expected.html (added)
• LayoutTests/fast/regions/repaint/fixed-in-named-flow-cb-changed2.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/FlowThreadController.cpp (modified) (2 diffs)
• Source/WebCore/rendering/FlowThreadController.h (modified) (1 diff)
• Source/WebCore/rendering/RenderLayer.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2014-01-14  Mihnea Ovidenie  <mihnea@adobe.com>
+
+        [CSSRegions] Incorrect repaint of fixed element with transformed parent
+        https://bugs.webkit.org/show_bug.cgi?id=125756
+
+        Reviewed by Simon Fraser.
+
+        Add tests for a fixed positioned element with a parent that:
+        1. dynamically gets a transform, in which case the fixed positioned element should be positioned
+        relative to its parent
+        2. dynamically loses its transform, in which case the fixed positioned element should be positioned
+        relative to the view.
+
+        * fast/regions/repaint/fixed-in-named-flow-cb-changed-expected.html: Added.
+        * fast/regions/repaint/fixed-in-named-flow-cb-changed.html: Added.
+        * fast/regions/repaint/fixed-in-named-flow-cb-changed2-expected.html: Added.
+        * fast/regions/repaint/fixed-in-named-flow-cb-changed2.html: Added.
+
 2014-01-14  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-01-14  Mihnea Ovidenie  <mihnea@adobe.com>
+
+        [CSSRegions] Incorrect repaint of fixed element with transformed parent
+        https://bugs.webkit.org/show_bug.cgi?id=125756
+
+        Reviewed by Simon Fraser.
+
+        When collecting the layers for fixed positioned elements with named flow
+        as a containing block, use layers collection at named flow layer level
+        instead of relying on the positioned elements collection.
+
+        With this approach, there are situations with nested named flows laid out in
+        auto-height regions and thus assuming 2 step layouts, in which is enough to lay
+        out the inner named flow once instead of twice with a performance gain.
+        In such situations, the layers lists for the inner named flow are not yet updated
+        when collectFixedPositionedLayers is called to paint the fixed positioned layers.
+        Therefore I called updateLayerListsIfNeeded for all named flows layers, via the flow thread controller, before inspecting the named flow layers to avoid hitting the assertions when
+        accessing the named flows layers lists.
+
+        Tests: fast/regions/repaint/fixed-in-named-flow-cb-changed.html
+               fast/regions/repaint/fixed-in-named-flow-cb-changed2.html
+
+        * rendering/FlowThreadController.cpp:
+        (WebCore::FlowThreadController::updateNamedFlowsLayerListsIfNeeded):
+        (WebCore::FlowThreadController::collectFixedPositionedLayers):
+        * rendering/FlowThreadController.h:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::paintFixedLayersInNamedFlows):
+
 2014-01-14  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebCore/rendering/FlowThreadController.cpp

@@ -280 +280 @@
 }
 
+void FlowThreadController::updateNamedFlowsLayerListsIfNeeded()
+{
+    for (auto iter = m_renderNamedFlowThreadList->begin(), end = m_renderNamedFlowThreadList->end(); iter != end; ++iter) {
+        RenderNamedFlowThread* flowRenderer = *iter;
+        flowRenderer->layer()->updateLayerListsIfNeeded();
+    }
+}
+
 static inline bool compareZIndex(RenderLayer* first, RenderLayer* second)
 {
@@ -297 +305 @@
             continue;
 
-        // Iterate over the fixed positioned elements in the flow thread
-        TrackedRendererListHashSet* positionedDescendants = flowRenderer->positionedObjects();
-        if (positionedDescendants) {
-            for (auto it = positionedDescendants->begin(), end = positionedDescendants->end(); it != end; ++it) {
-                RenderBox* box = *it;
-                if (!box->fixedPositionedWithNamedFlowContainingBlock())
+        RenderLayer* flowThreadLayer = flowRenderer->layer();
+        if (Vector<RenderLayer*>* negZOrderList = flowThreadLayer->negZOrderList()) {
+            for (size_t i = 0, size = negZOrderList->size(); i < size; ++i) {
+                RenderLayer* currLayer = negZOrderList->at(i);
+                if (currLayer->renderer().style().position() != FixedPosition)
                     continue;
-                fixedPosLayers.append(box->layer());
+                fixedPosLayers.append(currLayer);
+            }
+        }
+
+        if (Vector<RenderLayer*>* posZOrderList = flowThreadLayer->posZOrderList()) {
+            for (size_t i = 0, size = posZOrderList->size(); i < size; ++i) {
+                RenderLayer* currLayer = posZOrderList->at(i);
+                if (currLayer->renderer().style().position() != FixedPosition)
+                    continue;
+                fixedPosLayers.append(currLayer);
             }
         }

trunk/Source/WebCore/rendering/FlowThreadController.h

@@ -81 +81 @@
     void updateFlowThreadsIntoFinalPhase();
 
+    void updateNamedFlowsLayerListsIfNeeded();
     // Collect the fixed positioned layers that have the named flows as containing block
     // These layers are painted and hit-tested by RenderView

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -4058 +4058 @@
         return;
 
+    // Ensure the flow threads hierarchy is up-to-date before using it.
+    renderer().view().flowThreadController().updateNamedFlowsLayerListsIfNeeded();
+
     // Collect the fixed layers in a list to be painted
     Vector<RenderLayer*> fixedLayers;