[iOS WK2] Some accerated overflow-scroll doesn't scroll correctly
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 May 2014 02:15:17 +0000 (02:15 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 May 2014 02:15:17 +0000 (02:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=132375

Reviewed by Tim Horton.

Source/WebCore:
We set the size of the scrolling layer (which becomes the bounds of
the UIScrollView) to a non-pixel-snapped padding box size, but the
size of the contents layer is an integral-snapped scroll size.
This would result in a fractional difference between the two, which
makes us thing that the element is scrollable when it really is not.

Fix by setting the size of the scroll layer to pixel snapped client size,
which is what we also use for scrollability computation.

Added some FIXMEs in code that requires pixel snapping.

Also use #if PLATFORM(IOS)/#else to bracket some code that never runs on iOS
but tries to do something similar to iOS-only code.

* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateGraphicsLayerGeometry):

LayoutTests:
New test that dumps compositing layers on iOS so we can see the sizes of the
scroll layers that get created.

* compositing/overflow/subpixel-overflow-expected.txt: Added.
* compositing/overflow/subpixel-overflow.html: Added.
* platform/ios-sim/compositing/overflow/subpixel-overflow-expected.txt: Added.
* platform/mac/compositing/overflow/composited-scrolling-creates-a-stacking-container-expected.txt:
This is a progression; the old code failed to take the scrollbar width into
account, and the new code does.

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

LayoutTests/ChangeLog
LayoutTests/compositing/overflow/subpixel-overflow-expected.txt [new file with mode: 0644]
LayoutTests/compositing/overflow/subpixel-overflow.html [new file with mode: 0644]
LayoutTests/platform/ios-sim/compositing/overflow/subpixel-overflow-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/compositing/overflow/composited-scrolling-creates-a-stacking-container-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerBacking.cpp

index f3c49e2..82ba3fc 100644 (file)
@@ -1,5 +1,23 @@
 2014-04-30  Simon Fraser  <simon.fraser@apple.com>
 
+        [iOS WK2] Some accerated overflow-scroll doesn't scroll correctly
+        https://bugs.webkit.org/show_bug.cgi?id=132375
+
+        Reviewed by Tim Horton.
+        
+        New test that dumps compositing layers on iOS so we can see the sizes of the
+        scroll layers that get created.
+        
+        * compositing/overflow/subpixel-overflow-expected.txt: Added.
+        * compositing/overflow/subpixel-overflow.html: Added.
+        * platform/ios-sim/compositing/overflow/subpixel-overflow-expected.txt: Added.
+        * platform/mac/compositing/overflow/composited-scrolling-creates-a-stacking-container-expected.txt:
+        This is a progression; the old code failed to take the scrollbar width into
+        account, and the new code does.
+
+
+2014-04-30  Simon Fraser  <simon.fraser@apple.com>
+
         Rebaseline compositing/overflow tests for iOS.
 
         * platform/ios-sim/compositing/overflow/composited-scrolling-creates-a-stacking-container-expected.txt:
diff --git a/LayoutTests/compositing/overflow/subpixel-overflow-expected.txt b/LayoutTests/compositing/overflow/subpixel-overflow-expected.txt
new file mode 100644 (file)
index 0000000..40ba98a
--- /dev/null
@@ -0,0 +1,6 @@
+Content
+Content
+Content
+
diff --git a/LayoutTests/compositing/overflow/subpixel-overflow.html b/LayoutTests/compositing/overflow/subpixel-overflow.html
new file mode 100644 (file)
index 0000000..2a0b654
--- /dev/null
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+        html {
+            -webkit-overflow-scrolling: touch;
+        }
+        
+        .container {
+            margin: 10px;
+            width: 200px;
+            border: 1px solid black;
+            overflow: auto;
+            display: inline-block;
+        }
+        
+        .contents {
+            width: 100%;
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText();
+
+        window.addEventListener('load', function() {
+            if (window.testRunner)
+                document.getElementById('layers').innerHTML = window.internals.layerTreeAsText(document);
+        }, true);
+    </script>
+</head>
+<body>
+
+    <!-- None of these should be scrollable -->
+    <div class="container">
+        <div class="contents" style="height: 200.25px">
+            Content
+        </div>
+    </div>
+
+    <div class="container">
+        <div class="contents" style="height: 200.5px">
+            Content
+        </div>
+    </div>
+
+    <div class="container">
+        <div class="contents" style="height: 200.75px">
+            Content
+        </div>
+    </div>
+<pre id="layers"></pre>
+</body>
+</html>
diff --git a/LayoutTests/platform/ios-sim/compositing/overflow/subpixel-overflow-expected.txt b/LayoutTests/platform/ios-sim/compositing/overflow/subpixel-overflow-expected.txt
new file mode 100644 (file)
index 0000000..9a0dc7f
--- /dev/null
@@ -0,0 +1,67 @@
+Content
+Content
+Content
+ (GraphicsLayer
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (children 3
+        (GraphicsLayer
+          (position 18.00 18.00)
+          (bounds 202.00 202.50)
+          (drawsContent 1)
+          (children 1
+            (GraphicsLayer
+              (position 1.00 1.00)
+              (bounds 200.00 200.00)
+              (children 1
+                (GraphicsLayer
+                  (bounds 200.00 200.00)
+                  (drawsContent 1)
+                )
+              )
+            )
+          )
+        )
+        (GraphicsLayer
+          (position 244.00 18.00)
+          (bounds 202.00 202.50)
+          (drawsContent 1)
+          (children 1
+            (GraphicsLayer
+              (position 1.00 1.00)
+              (bounds 200.00 201.00)
+              (children 1
+                (GraphicsLayer
+                  (bounds 200.00 201.00)
+                  (drawsContent 1)
+                )
+              )
+            )
+          )
+        )
+        (GraphicsLayer
+          (position 470.00 18.00)
+          (bounds 202.00 203.00)
+          (drawsContent 1)
+          (children 1
+            (GraphicsLayer
+              (position 1.00 1.00)
+              (bounds 200.00 201.00)
+              (children 1
+                (GraphicsLayer
+                  (bounds 200.00 201.00)
+                  (drawsContent 1)
+                )
+              )
+            )
+          )
+        )
+      )
+    )
+  )
+)
+
index df32657..39737d7 100644 (file)
@@ -12,7 +12,7 @@
           (children 4
             (GraphicsLayer
               (position 1.00 1.00)
-              (bounds 200.00 200.00)
+              (bounds 185.00 185.00)
               (children 1
                 (GraphicsLayer
                   (bounds 200.00 300.00)
index 5c99b2c..f3cb4dc 100644 (file)
@@ -1,3 +1,27 @@
+2014-04-30  Simon Fraser  <simon.fraser@apple.com>
+
+        [iOS WK2] Some accerated overflow-scroll doesn't scroll correctly
+        https://bugs.webkit.org/show_bug.cgi?id=132375
+
+        Reviewed by Tim Horton.
+        
+        We set the size of the scrolling layer (which becomes the bounds of
+        the UIScrollView) to a non-pixel-snapped padding box size, but the
+        size of the contents layer is an integral-snapped scroll size.
+        This would result in a fractional difference between the two, which
+        makes us thing that the element is scrollable when it really is not.
+        
+        Fix by setting the size of the scroll layer to pixel snapped client size,
+        which is what we also use for scrollability computation.
+        
+        Added some FIXMEs in code that requires pixel snapping.
+        
+        Also use #if PLATFORM(IOS)/#else to bracket some code that never runs on iOS
+        but tries to do something similar to iOS-only code.
+
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::updateGraphicsLayerGeometry):
+
 2014-04-30  David Hyatt  <hyatt@apple.com>
 
         REGRESSION (r168046): [New Multicolumn] Painting order is wrong for columns and fixed positioned elements
index af58308..75b570e 100644 (file)
@@ -743,6 +743,7 @@ void RenderLayerBacking::updateGraphicsLayerGeometry()
     if (compAncestor && compAncestor->backing()->hasClippingLayer()) {
         // If the compositing ancestor has a layer to clip children, we parent in that, and therefore
         // position relative to it.
+        // FIXME: need to do some pixel snapping here.
         LayoutRect clippingBox = clipBox(toRenderBox(compAncestor->renderer()));
         graphicsLayerParentLocation = clippingBox.location();
     } else if (compAncestor)
@@ -757,18 +758,19 @@ void RenderLayerBacking::updateGraphicsLayerGeometry()
             renderBox->width() - renderBox->borderLeft() - renderBox->borderRight(),
             renderBox->height() - renderBox->borderTop() - renderBox->borderBottom());
 
-        LayoutSize scrollOffset = compAncestor->scrolledContentOffset();
+        IntSize scrollOffset = compAncestor->scrolledContentOffset();
+        // FIXME: pixel snap the padding box.
         graphicsLayerParentLocation = paddingBox.location() - scrollOffset;
     }
-#endif
-
+#else
     if (compAncestor && compAncestor->needsCompositedScrolling()) {
         RenderBox& renderBox = toRenderBox(compAncestor->renderer());
         LayoutSize scrollOffset = compAncestor->scrolledContentOffset();
         LayoutPoint scrollOrigin(renderBox.borderLeft(), renderBox.borderTop());
         graphicsLayerParentLocation = scrollOrigin - scrollOffset;
     }
-    
+#endif
+
     if (compAncestor && m_ancestorClippingLayer) {
         // Call calculateRects to get the backgroundRect which is what is used to clip the contents of this
         // layer. Note that we call it with temporaryClipRects = true because normally when computing clip rects
@@ -813,6 +815,7 @@ void RenderLayerBacking::updateGraphicsLayerGeometry()
     // If we have a layer that clips children, position it.
     LayoutRect clippingBox;
     if (GraphicsLayer* clipLayer = clippingLayer()) {
+        // FIXME: need to do some pixel snapping here.
         clippingBox = clipBox(toRenderBox(renderer()));
         clipLayer->setPosition(FloatPoint(clippingBox.location() - localCompositingBounds.location()));
         clipLayer->setSize(clippingBox.size());
@@ -909,9 +912,11 @@ void RenderLayerBacking::updateGraphicsLayerGeometry()
         LayoutRect paddingBox(renderBox.borderLeft(), renderBox.borderTop(), renderBox.width() - renderBox.borderLeft() - renderBox.borderRight(), renderBox.height() - renderBox.borderTop() - renderBox.borderBottom());
         LayoutSize scrollOffset = m_owningLayer.scrollOffset();
 
+        // FIXME: need to do some pixel snapping here.
         m_scrollingLayer->setPosition(FloatPoint(paddingBox.location() - localCompositingBounds.location()));
 
-        m_scrollingLayer->setSize(paddingBox.size());
+        IntSize pixelSnappedClientSize(renderBox.pixelSnappedClientWidth(), renderBox.pixelSnappedClientHeight());
+        m_scrollingLayer->setSize(pixelSnappedClientSize);
 #if PLATFORM(IOS)
         FloatSize oldScrollingLayerOffset = m_scrollingLayer->offsetFromRenderer();
         m_scrollingLayer->setOffsetFromRenderer(FloatPoint() - paddingBox.location());