Changeset 141330 in webkit

Timestamp:

Jan 30, 2013, 3:22:25 PM (12 years ago)

Author:

Simon Fraser

Message:

Elements that dynamically become fixed sometimes jump to the top left on scrolling
​https://bugs.webkit.org/show_bug.cgi?id=108389

Source/WebCore:

Reviewed by Beth Dakin.

When an element became position:fixed and gained a compositing layer
as a result, we would compute its viewport constraints (including the
last GraphicsLayer position) before we had actually updated the GraphicsLayer
geometry for the first time. This would cause a jump to 0,0 on scrolling.

Fix by removing the call to updateViewportConstraintStatus() just after
creating the backing. Instead, hook in to registerScrollingLayers(),
which is called on every geometry update, to update the viewport
constraints for this layer.

Tested via existing tests, which now show correct positions at last
layout.

• rendering/RenderLayerBacking.cpp:

(WebCore::RenderLayerBacking::registerScrollingLayers):

• rendering/RenderLayerCompositor.cpp:

(WebCore::RenderLayerCompositor::updateBacking):

LayoutTests:

Reviewed by Beth Dakin.

Update test results; these results now correctly reflect the last layout
positions.

• platform/mac/tiled-drawing/fixed/absolute-inside-fixed-expected.txt:
• platform/mac/tiled-drawing/fixed/absolute-inside-out-of-view-fixed-expected.txt:
• platform/mac/tiled-drawing/fixed/fixed-in-overflow-expected.txt:
• platform/mac/tiled-drawing/fixed/four-bars-expected.txt:
• platform/mac/tiled-drawing/fixed/nested-fixed-expected.txt:
• platform/mac/tiled-drawing/fixed/percentage-inside-fixed-expected.txt:
• platform/mac/tiled-drawing/sticky/sticky-horizontal-expected.txt:
• platform/mac/tiled-drawing/sticky/sticky-vertical-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/tiled-drawing/fixed/absolute-inside-fixed-expected.txt (modified) (1 diff)
• LayoutTests/platform/mac/tiled-drawing/fixed/absolute-inside-out-of-view-fixed-expected.txt (modified) (1 diff)
• LayoutTests/platform/mac/tiled-drawing/fixed/fixed-in-overflow-expected.txt (modified) (1 diff)
• LayoutTests/platform/mac/tiled-drawing/fixed/four-bars-expected.txt (modified) (1 diff)
• LayoutTests/platform/mac/tiled-drawing/fixed/nested-fixed-expected.txt (modified) (1 diff)
• LayoutTests/platform/mac/tiled-drawing/fixed/percentage-inside-fixed-expected.txt (modified) (1 diff)
• LayoutTests/platform/mac/tiled-drawing/sticky/sticky-horizontal-expected.txt (modified) (6 diffs)
• LayoutTests/platform/mac/tiled-drawing/sticky/sticky-vertical-expected.txt (modified) (6 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerBacking.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerCompositor.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-01-30  Simon Fraser  <simon.fraser@apple.com>
+
+        Elements that dynamically become fixed sometimes jump to the top left on scrolling
+        https://bugs.webkit.org/show_bug.cgi?id=108389
+
+        Reviewed by Beth Dakin.
+        
+        Update test results; these results now correctly reflect the last layout
+        positions.
+
+        * platform/mac/tiled-drawing/fixed/absolute-inside-fixed-expected.txt:
+        * platform/mac/tiled-drawing/fixed/absolute-inside-out-of-view-fixed-expected.txt:
+        * platform/mac/tiled-drawing/fixed/fixed-in-overflow-expected.txt:
+        * platform/mac/tiled-drawing/fixed/four-bars-expected.txt:
+        * platform/mac/tiled-drawing/fixed/nested-fixed-expected.txt:
+        * platform/mac/tiled-drawing/fixed/percentage-inside-fixed-expected.txt:
+        * platform/mac/tiled-drawing/sticky/sticky-horizontal-expected.txt:
+        * platform/mac/tiled-drawing/sticky/sticky-vertical-expected.txt:
+
 2013-01-30  Jessie Berlin  <jberlin@apple.com>
 

trunk/LayoutTests/platform/mac/tiled-drawing/fixed/absolute-inside-fixed-expected.txt

@@ -7 +7 @@
       (anchor edges: AnchorEdgeLeft AnchorEdgeTop)
       (viewport rect at last layout: 0.00 200.00 785.00 600.00)
-      (layer position at last layout 8.00 8.00)
+      (layer position at last layout 8.00 208.00)
     )
   )

trunk/LayoutTests/platform/mac/tiled-drawing/fixed/absolute-inside-out-of-view-fixed-expected.txt

@@ -7 +7 @@
       (anchor edges: AnchorEdgeLeft AnchorEdgeTop)
       (viewport rect at last layout: 0.00 200.00 785.00 600.00)
+      (layer position at last layout 0.00 200.00)
     )
   )

trunk/LayoutTests/platform/mac/tiled-drawing/fixed/fixed-in-overflow-expected.txt

@@ -7 +7 @@
       (anchor edges: AnchorEdgeLeft AnchorEdgeTop)
       (viewport rect at last layout: 0.00 200.00 785.00 600.00)
-      (layer position at last layout 2.00 15.00)
+      (layer position at last layout 2.00 215.00)
     )
   )

trunk/LayoutTests/platform/mac/tiled-drawing/fixed/four-bars-expected.txt

@@ -7 +7 @@
       (anchor edges: AnchorEdgeLeft AnchorEdgeTop)
       (viewport rect at last layout: 0.00 200.00 785.00 600.00)
+      (layer position at last layout 0.00 200.00)
     )
     (Fixed node
       (anchor edges: AnchorEdgeLeft AnchorEdgeTop)
       (viewport rect at last layout: 0.00 200.00 785.00 600.00)
-      (layer position at last layout 10.00 200.00)
+      (layer position at last layout 10.00 400.00)
     )
     (Fixed node
       (anchor edges: AnchorEdgeRight AnchorEdgeTop)
       (viewport rect at last layout: 0.00 200.00 785.00 600.00)
-      (layer position at last layout 601.00 200.00)
+      (layer position at last layout 601.00 400.00)
     )
     (Fixed node
       (anchor edges: AnchorEdgeLeft AnchorEdgeBottom)
       (viewport rect at last layout: 0.00 200.00 785.00 600.00)
-      (layer position at last layout 0.00 516.00)
+      (layer position at last layout 0.00 716.00)
     )
   )

trunk/LayoutTests/platform/mac/tiled-drawing/fixed/nested-fixed-expected.txt

@@ -7 +7 @@
       (anchor edges: AnchorEdgeLeft AnchorEdgeTop)
       (viewport rect at last layout: 0.00 200.00 785.00 600.00)
-      (children 0
-      )
+      (layer position at last layout 0.00 200.00)
     )
   )

trunk/LayoutTests/platform/mac/tiled-drawing/fixed/percentage-inside-fixed-expected.txt

@@ -7 +7 @@
       (anchor edges: AnchorEdgeLeft AnchorEdgeTop)
       (viewport rect at last layout: 0.00 200.00 785.00 600.00)
-      (layer position at last layout 0.00 13.00)
+      (layer position at last layout 0.00 213.00)
     )
   )

trunk/LayoutTests/platform/mac/tiled-drawing/sticky/sticky-horizontal-expected.txt

@@ -12 +12 @@
       (sticky box rect 812.00 25.00 300.00 60.00)
       (sticky offset at last layout 8.00 0.00)
-      (layer position at last layout 812.00 25.00)
+      (layer position at last layout 820.00 25.00)
     )
     (Sticky node
@@ -21 +21 @@
       (sticky box rect 812.00 93.00 300.00 60.00)
       (sticky offset at last layout 0.00 0.00)
-      (layer position at last layout 612.00 93.00)
+      (layer position at last layout 812.00 93.00)
     )
     (Sticky node
@@ -30 +30 @@
       (sticky box rect 812.00 161.00 300.00 60.00)
       (sticky offset at last layout 148.00 0.00)
-      (layer position at last layout 812.00 161.00)
+      (layer position at last layout 960.00 161.00)
     )
     (Sticky node
@@ -39 +39 @@
       (sticky box rect 812.00 229.00 300.00 60.00)
       (sticky offset at last layout 0.00 0.00)
-      (layer position at last layout 612.00 229.00)
+      (layer position at last layout 812.00 229.00)
     )
     (Sticky node
@@ -49 +49 @@
       (sticky box rect 812.00 297.00 300.00 60.00)
       (sticky offset at last layout 8.00 0.00)
-      (layer position at last layout 612.00 297.00)
+      (layer position at last layout 820.00 297.00)
     )
     (Sticky node
@@ -59 +59 @@
       (sticky box rect 812.00 365.00 300.00 60.00)
       (sticky offset at last layout 148.00 0.00)
-      (layer position at last layout 612.00 365.00)
+      (layer position at last layout 960.00 365.00)
     )
   )

trunk/LayoutTests/platform/mac/tiled-drawing/sticky/sticky-vertical-expected.txt

@@ -16 +16 @@
       (sticky box rect 73.00 558.00 80.00 300.00)
       (sticky offset at last layout 0.00 42.00)
-      (layer position at last layout 73.00 558.00)
+      (layer position at last layout 73.00 600.00)
     )
     (Sticky node
@@ -25 +25 @@
       (sticky box rect 163.00 558.00 80.00 300.00)
       (sticky offset at last layout 0.00 0.00)
-      (layer position at last layout 163.00 406.00)
+      (layer position at last layout 163.00 558.00)
     )
     (Sticky node
@@ -34 +34 @@
       (sticky box rect 253.00 558.00 80.00 300.00)
       (sticky offset at last layout 0.00 142.00)
-      (layer position at last layout 253.00 558.00)
+      (layer position at last layout 253.00 700.00)
     )
     (Sticky node
@@ -43 +43 @@
       (sticky box rect 343.00 558.00 80.00 300.00)
       (sticky offset at last layout 0.00 0.00)
-      (layer position at last layout 343.00 406.00)
+      (layer position at last layout 343.00 558.00)
     )
     (Sticky node
@@ -53 +53 @@
       (sticky box rect 433.00 558.00 80.00 300.00)
       (sticky offset at last layout 0.00 42.00)
-      (layer position at last layout 433.00 406.00)
+      (layer position at last layout 433.00 600.00)
     )
     (Sticky node
@@ -63 +63 @@
       (sticky box rect 523.00 558.00 80.00 300.00)
       (sticky offset at last layout 0.00 142.00)
-      (layer position at last layout 523.00 406.00)
+      (layer position at last layout 523.00 700.00)
     )
   )

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-30  Simon Fraser  <simon.fraser@apple.com>
+
+        Elements that dynamically become fixed sometimes jump to the top left on scrolling
+        https://bugs.webkit.org/show_bug.cgi?id=108389
+
+        Reviewed by Beth Dakin.
+
+        When an element became position:fixed and gained a compositing layer
+        as a result, we would compute its viewport constraints (including the 
+        last GraphicsLayer position) before we had actually updated the GraphicsLayer
+        geometry for the first time. This would cause a jump to 0,0 on scrolling.
+        
+        Fix by removing the call to updateViewportConstraintStatus() just after
+        creating the backing. Instead, hook in to registerScrollingLayers(),
+        which is called on every geometry update, to update the viewport
+        constraints for this layer.
+        
+        Tested via existing tests, which now show correct positions at last
+        layout.
+
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::registerScrollingLayers):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::updateBacking):
+
 2013-01-30  Adam Barth  <abarth@webkit.org>
 

trunk/Source/WebCore/rendering/RenderLayerBacking.cpp

@@ -824 +824 @@
         return;
 
+    compositor()->updateViewportConstraintStatus(m_owningLayer);
+
     // FIXME: it would be nice to avoid all this work if the platform doesn't implement setLayerIsFixedToContainerLayer().
     if (renderer()->style()->position() == FixedPosition || compositor()->fixedPositionedByAncestor(m_owningLayer))

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -592 +592 @@
             layerChanged = true;
         }
-
-        // Need to add for every compositing layer, because a composited layer may change from being non-fixed to fixed.
-        updateViewportConstraintStatus(layer);
     } else {
         if (layer->backing()) {