Fix repositioning of fixed elements on zooming

https://bugs.webkit.org/show_bug.cgi?id=105223

Reviewed by Beth Dakin.

Source/WebCore:

When zoomed, scrolling would move the layers of fixed-position
elements oddly. This happened because on the scrolling thread we
passed a scale of 1, rather than the actual page scale to
scrollOffsetForFixedPosition().

Fix by plumbing the page scale through the scrolling state node
to the scrolling node.

Test: platform/mac/tiled-drawing/fixed/four-bars-zoomed.html

* page/scrolling/ScrollingStateScrollingNode.cpp:
(WebCore::ScrollingStateScrollingNode::ScrollingStateScrollingNode):
(WebCore::ScrollingStateScrollingNode::setFrameScaleFactor):
(WebCore::ScrollingStateScrollingNode::dumpProperties):
* page/scrolling/ScrollingStateScrollingNode.h:
(WebCore::ScrollingStateScrollingNode::frameScaleFactor):
(ScrollingStateScrollingNode):
* page/scrolling/ScrollingTreeScrollingNode.cpp:
(WebCore::ScrollingTreeScrollingNode::ScrollingTreeScrollingNode):
(WebCore::ScrollingTreeScrollingNode::update):
* page/scrolling/ScrollingTreeScrollingNode.h:
(WebCore::ScrollingTreeScrollingNode::frameScaleFactor):
(ScrollingTreeScrollingNode):
* page/scrolling/mac/ScrollingCoordinatorMac.h:
(ScrollParameters):
* page/scrolling/mac/ScrollingCoordinatorMac.mm:
(WebCore::ScrollingCoordinatorMac::frameViewLayoutUpdated):
(WebCore::ScrollingCoordinatorMac::setScrollParametersForNode):
* page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
(WebCore::ScrollingTreeScrollingNodeMac::setScrollLayerPosition):

LayoutTests:

Testcase that zoomed with fixed-position elements.

* platform/mac/tiled-drawing/fixed/four-bars-zoomed-expected.txt: Added.
* platform/mac/tiled-drawing/fixed/four-bars-zoomed.html: Added.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index cc97466..612e06d 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2012-12-17  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix repositioning of fixed elements on zooming
+        https://bugs.webkit.org/show_bug.cgi?id=105223
+
+        Reviewed by Beth Dakin.
+
+        Testcase that zoomed with fixed-position elements.
+
+        * platform/mac/tiled-drawing/fixed/four-bars-zoomed-expected.txt: Added.
+        * platform/mac/tiled-drawing/fixed/four-bars-zoomed.html: Added.
+
 2012-12-17  Dima Gorbik  <dgorbik@apple.com>
 
         Implement matching cue by the class name with ::cue pseudo element

diff --git a/LayoutTests/platform/mac/tiled-drawing/fixed/four-bars-zoomed-expected.txt b/LayoutTests/platform/mac/tiled-drawing/fixed/four-bars-zoomed-expected.txt
new file mode 100644 (file)
index 0000000..ed419b2
--- /dev/null
+++ b/LayoutTests/platform/mac/tiled-drawing/fixed/four-bars-zoomed-expected.txt
@@ -0,0 +1,65 @@
+(Scrolling node
+  (viewport rect 0 0 785 585)
+  (contents size 1805 5108)
+  (frame scale factor 2.30)
+  (requested scroll position 2 0)
+  (children 3
+    (Fixed node
+      (anchor edges: AnchorEdgeLeft AnchorEdgeTop)
+      (viewport rect at last layout: 0.00 0.00 785.00 585.00)
+    )
+    (Fixed node
+      (anchor edges: AnchorEdgeLeft AnchorEdgeTop)
+      (viewport rect at last layout: 0.00 0.00 785.00 585.00)
+      (layer position at last layout 10.00 200.00)
+    )
+    (Fixed node
+      (anchor edges: AnchorEdgeLeft AnchorEdgeBottom)
+      (viewport rect at last layout: 0.00 0.00 785.00 585.00)
+      (layer position at last layout 0.00 501.00)
+    )
+  )
+)
+
+(GraphicsLayer
+  (bounds 1805.00 5108.00)
+  (visible rect 2.00, 0.00 785.00 x 585.00)
+  (children 1
+    (GraphicsLayer
+      (anchor 0.00 0.00)
+      (bounds 785.00 2221.00)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+      (transform [2.30 0.00 0.00 0.00] [0.00 2.30 0.00 0.00] [0.00 0.00 1.00 0.00] [0.00 0.00 0.00 1.00])
+      (visible rect 0.87, 0.00 341.30 x 254.35)
+      (tile cache coverage 0, 0 784 x 890)
+      (tile size 512 x 512)
+      (top left tile 0, 0 tiles grid 4 x 4)
+      (children 1
+        (GraphicsLayer
+          (visible rect 0.00, 0.00 0.00 x 0.00)
+          (children 3
+            (GraphicsLayer
+              (bounds 778.00 74.00)
+              (drawsContent 1)
+              (visible rect 0.87, 0.00 341.30 x 74.00)
+            )
+            (GraphicsLayer
+              (position 10.00 200.00)
+              (bounds 174.00 324.00)
+              (drawsContent 1)
+              (visible rect 0.00, 0.00 174.00 x 54.35)
+            )
+            (GraphicsLayer
+              (position 0.00 501.00)
+              (bounds 778.00 74.00)
+              (drawsContent 1)
+              (visible rect 0.00, 0.00 0.00 x 0.00)
+            )
+          )
+        )
+      )
+    )
+  )
+)
+This is the top bar. This is the left bar. This is the right bar. This is the bottom bar.

diff --git a/LayoutTests/platform/mac/tiled-drawing/fixed/four-bars-zoomed.html b/LayoutTests/platform/mac/tiled-drawing/fixed/four-bars-zoomed.html
new file mode 100644 (file)
index 0000000..9dd7e22
--- /dev/null
+++ b/LayoutTests/platform/mac/tiled-drawing/fixed/four-bars-zoomed.html
@@ -0,0 +1,88 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+      body {
+        height: 2200px;
+      }
+      
+      .fixed {
+        position: fixed;
+        top: 0;
+        left: 0;
+        margin: 10px;
+        height: 50px;
+        background-color: rgba(0, 128, 0, 0.8);
+        border: 2px solid black;
+        -webkit-box-shadow: 0 0 10px black;
+      }
+      
+      .top, .bottom {
+        width: 96%;
+      }
+
+      .left {
+        top: 200px;
+        left: 10px;
+        width: 150px;
+        height: 300px;
+      }
+
+      .right {
+        top: 200px;
+        left: auto;
+        right: 10px;
+        width: 150px;
+        height: 300px;
+      }
+      
+      .bottom {
+        top: auto;
+        bottom: 10px;
+      }
+      
+    </style>
+    <script>
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    }
+
+    function doScroll()
+    {
+        window.setTimeout(function() {
+            if (window.eventSender)
+                eventSender.scalePageBy(2.3, 2.3);
+
+            if (window.testRunner) {
+                document.getElementById('results').textContent = window.internals.scrollingStateTreeAsText(document) + '\n' +
+                internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_VISIBLE_RECTS | internals.LAYER_TREE_INCLUDES_TILE_CACHES);
+
+                testRunner.notifyDone()
+            }
+        }, 10);
+    }
+
+    window.addEventListener('load', doScroll, false);
+    </script>
+</head>
+<body>
+
+<pre id="results"></pre>
+
+<div class="fixed top">
+  This is the top bar.
+</div>
+<div class="fixed left">
+  This is the left bar.
+</div>
+<div class="fixed right">
+  This is the right bar.
+</div>
+<div class="fixed bottom">
+  This is the bottom bar.
+</div>
+
+</body>
+</html>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 5b11d87..e00d98a 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,41 @@
+2012-12-17  Simon Fraser  <simon.fraser@apple.com>
+
+        Fix repositioning of fixed elements on zooming
+        https://bugs.webkit.org/show_bug.cgi?id=105223
+
+        Reviewed by Beth Dakin.
+
+        When zoomed, scrolling would move the layers of fixed-position
+        elements oddly. This happened because on the scrolling thread we
+        passed a scale of 1, rather than the actual page scale to 
+        scrollOffsetForFixedPosition().
+        
+        Fix by plumbing the page scale through the scrolling state node
+        to the scrolling node.
+
+        Test: platform/mac/tiled-drawing/fixed/four-bars-zoomed.html
+
+        * page/scrolling/ScrollingStateScrollingNode.cpp:
+        (WebCore::ScrollingStateScrollingNode::ScrollingStateScrollingNode):
+        (WebCore::ScrollingStateScrollingNode::setFrameScaleFactor):
+        (WebCore::ScrollingStateScrollingNode::dumpProperties):
+        * page/scrolling/ScrollingStateScrollingNode.h:
+        (WebCore::ScrollingStateScrollingNode::frameScaleFactor):
+        (ScrollingStateScrollingNode):
+        * page/scrolling/ScrollingTreeScrollingNode.cpp:
+        (WebCore::ScrollingTreeScrollingNode::ScrollingTreeScrollingNode):
+        (WebCore::ScrollingTreeScrollingNode::update):
+        * page/scrolling/ScrollingTreeScrollingNode.h:
+        (WebCore::ScrollingTreeScrollingNode::frameScaleFactor):
+        (ScrollingTreeScrollingNode):
+        * page/scrolling/mac/ScrollingCoordinatorMac.h:
+        (ScrollParameters):
+        * page/scrolling/mac/ScrollingCoordinatorMac.mm:
+        (WebCore::ScrollingCoordinatorMac::frameViewLayoutUpdated):
+        (WebCore::ScrollingCoordinatorMac::setScrollParametersForNode):
+        * page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:
+        (WebCore::ScrollingTreeScrollingNodeMac::setScrollLayerPosition):
+
 2012-12-17  Dima Gorbik  <dgorbik@apple.com>
 
         Implement matching cue by the class name with ::cue pseudo element

diff --git a/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp b/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp
index 1ad5fa2..9368c3e 100644 (file)
--- a/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp
+++ b/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp
@@ -42,6 +42,7 @@ PassOwnPtr<ScrollingStateScrollingNode> ScrollingStateScrollingNode::create(Scro
 ScrollingStateScrollingNode::ScrollingStateScrollingNode(ScrollingStateTree* stateTree, ScrollingNodeID nodeID)
     : ScrollingStateNode(stateTree, nodeID)
     , m_changedProperties(0)
+    , m_frameScaleFactor(1)
     , m_wheelEventHandlerCount(0)
     , m_shouldUpdateScrollLayerPositionOnMainThread(0)
     , m_horizontalScrollElasticity(ScrollElasticityNone)
@@ -59,6 +60,7 @@ ScrollingStateScrollingNode::ScrollingStateScrollingNode(const ScrollingStateScr
     , m_changedProperties(stateNode.changedProperties())
     , m_viewportRect(stateNode.viewportRect())
     , m_contentsSize(stateNode.contentsSize())
+    , m_frameScaleFactor(stateNode.frameScaleFactor())
     , m_nonFastScrollableRegion(stateNode.nonFastScrollableRegion())
     , m_wheelEventHandlerCount(stateNode.wheelEventHandlerCount())
     , m_shouldUpdateScrollLayerPositionOnMainThread(stateNode.shouldUpdateScrollLayerPositionOnMainThread())
@@ -103,6 +105,17 @@ void ScrollingStateScrollingNode::setContentsSize(const IntSize& contentsSize)
     m_scrollingStateTree->setHasChangedProperties(true);
 }
 
+void ScrollingStateScrollingNode::setFrameScaleFactor(float scaleFactor)
+{
+    if (m_frameScaleFactor == scaleFactor)
+        return;
+
+    m_frameScaleFactor = scaleFactor;
+
+    m_changedProperties |= FrameScaleFactor;
+    m_scrollingStateTree->setHasChangedProperties(true);
+}
+
 void ScrollingStateScrollingNode::setNonFastScrollableRegion(const Region& nonFastScrollableRegion)
 {
     if (m_nonFastScrollableRegion == nonFastScrollableRegion)
@@ -225,6 +238,11 @@ void ScrollingStateScrollingNode::dumpProperties(TextStream& ts, int indent) con
         ts << "(contents size " << m_contentsSize.width() << " " << m_contentsSize.height() << ")\n";
     }
 
+    if (m_frameScaleFactor != 1) {
+        writeIndent(ts, indent + 1);
+        ts << "(frame scale factor " << m_frameScaleFactor << ")\n";
+    }
+
     if (m_shouldUpdateScrollLayerPositionOnMainThread) {
         writeIndent(ts, indent + 1);
         ts << "(Scrolling on main thread because: " << ScrollingCoordinator::mainThreadScrollingReasonsAsText(m_shouldUpdateScrollLayerPositionOnMainThread) << ")\n";
@@ -239,7 +257,6 @@ void ScrollingStateScrollingNode::dumpProperties(TextStream& ts, int indent) con
         writeIndent(ts, indent + 1);
         ts << "(scroll origin " << m_scrollOrigin.x() << " " << m_scrollOrigin.y() << ")\n";
     }
-
 }
 
 } // namespace WebCore

diff --git a/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h b/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h
index 5902e27..f45527e 100644 (file)
--- a/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h
+++ b/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.h
@@ -48,17 +48,18 @@ public:
     enum ChangedProperty {
         ViewportRect = 1 << 0,
         ContentsSize = 1 << 1,
-        NonFastScrollableRegion = 1 << 2,
-        WheelEventHandlerCount = 1 << 3,
-        ShouldUpdateScrollLayerPositionOnMainThread = 1 << 4,
-        HorizontalScrollElasticity = 1 << 5,
-        VerticalScrollElasticity = 1 << 6,
-        HasEnabledHorizontalScrollbar = 1 << 7,
-        HasEnabledVerticalScrollbar = 1 << 8,
-        HorizontalScrollbarMode = 1 << 9,
-        VerticalScrollbarMode = 1 << 10,
-        ScrollOrigin = 1 << 11,
-        RequestedScrollPosition = 1 << 12,
+        FrameScaleFactor = 1 << 2,
+        NonFastScrollableRegion = 1 << 3,
+        WheelEventHandlerCount = 1 << 4,
+        ShouldUpdateScrollLayerPositionOnMainThread = 1 << 5,
+        HorizontalScrollElasticity = 1 << 6,
+        VerticalScrollElasticity = 1 << 7,
+        HasEnabledHorizontalScrollbar = 1 << 8,
+        HasEnabledVerticalScrollbar = 1 << 9,
+        HorizontalScrollbarMode = 1 << 10,
+        VerticalScrollbarMode = 1 << 11,
+        ScrollOrigin = 1 << 12,
+        RequestedScrollPosition = 1 << 13,
     };
 
     virtual bool isScrollingNode() OVERRIDE { return true; }
@@ -73,6 +74,9 @@ public:
     const IntSize& contentsSize() const { return m_contentsSize; }
     void setContentsSize(const IntSize&);
 
+    float frameScaleFactor() const { return m_frameScaleFactor; }
+    void setFrameScaleFactor(float);
+
     const Region& nonFastScrollableRegion() const { return m_nonFastScrollableRegion; }
     void setNonFastScrollableRegion(const Region&);
 
@@ -118,6 +122,8 @@ private:
 
     IntRect m_viewportRect;
     IntSize m_contentsSize;
+    
+    float m_frameScaleFactor;
 
     Region m_nonFastScrollableRegion;
 

diff --git a/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp b/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp
index 9d73f25..7e60803 100644 (file)
--- a/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp
+++ b/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp
@@ -34,6 +34,7 @@ namespace WebCore {
 
 ScrollingTreeScrollingNode::ScrollingTreeScrollingNode(ScrollingTree* scrollingTree)
     : ScrollingTreeNode(scrollingTree)
+    , m_frameScaleFactor(1)
     , m_shouldUpdateScrollLayerPositionOnMainThread(0)
     , m_horizontalScrollElasticity(ScrollElasticityNone)
     , m_verticalScrollElasticity(ScrollElasticityNone)
@@ -58,6 +59,9 @@ void ScrollingTreeScrollingNode::update(ScrollingStateNode* stateNode)
     if (state->changedProperties() & ScrollingStateScrollingNode::ContentsSize)
         m_contentsSize = state->contentsSize();
 
+    if (state->changedProperties() & ScrollingStateScrollingNode::FrameScaleFactor)
+        m_frameScaleFactor = state->frameScaleFactor();
+
     if (state->changedProperties() & ScrollingStateScrollingNode::ShouldUpdateScrollLayerPositionOnMainThread)
         m_shouldUpdateScrollLayerPositionOnMainThread = state->shouldUpdateScrollLayerPositionOnMainThread();
 

diff --git a/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h b/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h
index 7b62fd8..4a69f50 100644 (file)
--- a/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h
+++ b/Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h
@@ -61,6 +61,8 @@ protected:
     const IntRect& viewportRect() const { return m_viewportRect; }
     const IntSize& contentsSize() const { return m_contentsSize; }
 
+    float frameScaleFactor() const { return m_frameScaleFactor; }
+
     ScrollElasticity horizontalScrollElasticity() const { return m_horizontalScrollElasticity; }
     ScrollElasticity verticalScrollElasticity() const { return m_verticalScrollElasticity; }
 
@@ -75,6 +77,8 @@ private:
     IntRect m_viewportRect;
     IntSize m_contentsSize;
     IntPoint m_scrollOrigin;
+    
+    float m_frameScaleFactor;
 
     MainThreadScrollingReasons m_shouldUpdateScrollLayerPositionOnMainThread;
 

diff --git a/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h b/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h
index 10f235f..3c3849e 100644 (file)
--- a/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h
+++ b/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.h
@@ -108,6 +108,8 @@ private:
 
         IntRect viewportRect;
         IntSize contentsSize;
+        
+        float frameScaleFactor;
     };
 
     void setScrollParametersForNode(const ScrollParameters&, ScrollingStateScrollingNode*);

diff --git a/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm b/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm
index ceb9679..833b240 100644 (file)
--- a/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm
+++ b/Source/WebCore/page/scrolling/mac/ScrollingCoordinatorMac.mm
@@ -128,6 +128,7 @@ void ScrollingCoordinatorMac::frameViewLayoutUpdated(FrameView* frameView)
     scrollParameters.scrollOrigin = frameView->scrollOrigin();
     scrollParameters.viewportRect = IntRect(IntPoint(), frameView->visibleContentRect().size());
     scrollParameters.contentsSize = frameView->contentsSize();
+    scrollParameters.frameScaleFactor = frameView->frame()->frameScaleFactor();
 
     setScrollParametersForNode(scrollParameters, node);
 }
@@ -329,6 +330,8 @@ void ScrollingCoordinatorMac::setScrollParametersForNode(const ScrollParameters&
     node->setScrollOrigin(scrollParameters.scrollOrigin);
     node->setViewportRect(scrollParameters.viewportRect);
     node->setContentsSize(scrollParameters.contentsSize);
+    node->setFrameScaleFactor(scrollParameters.frameScaleFactor);
+
     scheduleTreeStateCommit();
 }
 

diff --git a/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm b/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm
index 303dc54..967219c 100644 (file)
--- a/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm
+++ b/Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm
@@ -279,8 +279,7 @@ void ScrollingTreeScrollingNodeMac::setScrollLayerPosition(const IntPoint& posit
     if (!m_children)
         return;
 
-    IntSize scrollOffsetForFixedChildren = WebCore::scrollOffsetForFixedPosition(viewportRect(), contentsSize(), position,
-        scrollOrigin(), 1, false);
+    IntSize scrollOffsetForFixedChildren = WebCore::scrollOffsetForFixedPosition(viewportRect(), contentsSize(), position, scrollOrigin(), frameScaleFactor(), false);
     IntRect viewportRect = this->viewportRect();
     viewportRect.setLocation(toPoint(scrollOffsetForFixedChildren));