Source/WebCore:
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 15 Mar 2015 05:11:19 +0000 (05:11 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 15 Mar 2015 05:11:19 +0000 (05:11 +0000)
[iOS] scroll snap points are animating to the wrong positions...
https://bugs.webkit.org/show_bug.cgi?id=142705
<rdar://problem/20136946>

Reviewed by Simon Fraser.

Avoid adding an extra '0' snap point to our set. We always start with one zero; this
extra append just forces us to do more steps in our search for nearest snap point.

* page/scrolling/AxisScrollSnapOffsets.cpp:
(WebCore::updateFromStyle): Remove extra '0' appended to offsets.

Source/WebKit2:
[iOS] scroll snap points are animating to the wrong positions.
https://bugs.webkit.org/show_bug.cgi?id=142705
<rdar://problem/20136946>

Reviewed by Simon Fraser.

Scroll snapping was landing in the wrong place on iOS because of two problems:
(1) It was searching for the closest snap offset point using unscaled 'screen' pixels,
which caused it to always choose one of the earliest snap point options.
(2) It was then selecting a scaled snap point coordinate and passing it back to UIKit
to animate the snap. This caused it to select a target point beyond the 'screen' pixel
we want to hit.

The solution to both problems are to scale the scroll destination UIKit suggests so that
we search among the scaled points with a valid value. Then, we need to scale the returned
value back to screen units before handing it back to UIKit to process.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView scrollViewWillBeginDragging:]): Drive-by fix. Get rid of extra ';' at
the end of the line.
* UIProcess/ios/RemoteScrollingCoordinatorProxyIOS.mm:
(WebKit::RemoteScrollingCoordinatorProxy::closestSnapOffsetForMainFrameScrolling):

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

Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit2/UIProcess/ios/RemoteScrollingCoordinatorProxyIOS.mm

index 4a30403..dc11f75 100644 (file)
@@ -1,3 +1,17 @@
+2015-03-14  Brent Fulgham  <bfulgham@apple.com>
+
+        [iOS] scroll snap points are animating to the wrong positions...
+        https://bugs.webkit.org/show_bug.cgi?id=142705
+        <rdar://problem/20136946>
+
+        Reviewed by Simon Fraser.
+
+        Avoid adding an extra '0' snap point to our set. We always start with one zero; this
+        extra append just forces us to do more steps in our search for nearest snap point.
+
+        * page/scrolling/AxisScrollSnapOffsets.cpp:
+        (WebCore::updateFromStyle): Remove extra '0' appended to offsets.
+
 2015-03-14  Dean Jackson  <dino@apple.com>
 
         Feature flag for Animations Level 2
index bde4495..02db439 100644 (file)
@@ -83,7 +83,6 @@ static void updateFromStyle(Vector<LayoutUnit>& snapOffsets, const RenderStyle&
     LayoutUnit curSnapPositionShift = 0;
     LayoutUnit maxScrollOffset = scrollSize - viewSize;
     LayoutUnit lastSnapPosition = curSnapPositionShift;
-    snapOffsets.append(0);
     do {
         for (auto& snapPosition : snapOffsetSubsequence) {
             LayoutUnit potentialSnapPosition = curSnapPositionShift + snapPosition - destinationOffset;
@@ -98,6 +97,10 @@ static void updateFromStyle(Vector<LayoutUnit>& snapOffsets, const RenderStyle&
         }
         curSnapPositionShift = lastSnapPosition + repeatOffset;
     } while (hasRepeat && curSnapPositionShift < maxScrollOffset);
+
+    if (snapOffsets.isEmpty())
+        snapOffsets.append(0);
+
     // Always put a snap point on the maximum scroll offset.
     // Not a part of the spec, but necessary to prevent unreachable content when snapping.
     if (snapOffsets.last() != maxScrollOffset)
index 85ecec1..2bd8df4 100644 (file)
@@ -1,3 +1,28 @@
+2015-03-14  Brent Fulgham  <bfulgham@apple.com>
+
+        [iOS] scroll snap points are animating to the wrong positions.
+        https://bugs.webkit.org/show_bug.cgi?id=142705
+        <rdar://problem/20136946>
+
+        Reviewed by Simon Fraser.
+
+        Scroll snapping was landing in the wrong place on iOS because of two problems:
+        (1) It was searching for the closest snap offset point using unscaled 'screen' pixels,
+        which caused it to always choose one of the earliest snap point options.
+        (2) It was then selecting a scaled snap point coordinate and passing it back to UIKit
+        to animate the snap. This caused it to select a target point beyond the 'screen' pixel
+        we want to hit.
+        
+        The solution to both problems are to scale the scroll destination UIKit suggests so that
+        we search among the scaled points with a valid value. Then, we need to scale the returned
+        value back to screen units before handing it back to UIKit to process.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView scrollViewWillBeginDragging:]): Drive-by fix. Get rid of extra ';' at
+        the end of the line.
+        * UIProcess/ios/RemoteScrollingCoordinatorProxyIOS.mm:
+        (WebKit::RemoteScrollingCoordinatorProxy::closestSnapOffsetForMainFrameScrolling):
+
 2015-03-14  Dean Jackson  <dino@apple.com>
 
         Feature flag for Animations Level 2
index 8147654..2826f3e 100644 (file)
@@ -1339,7 +1339,7 @@ static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOff
     // FIXME: We will want to detect whether snapping will occur before beginning to drag. See WebPageProxy::didCommitLayerTree.
     WebKit::RemoteScrollingCoordinatorProxy* coordinator = _page->scrollingCoordinatorProxy();
     ASSERT(scrollView == _scrollView.get());
-    scrollView.decelerationRate = (coordinator && coordinator->shouldSetScrollViewDecelerationRateFast()) ? UIScrollViewDecelerationRateFast : [_scrollView preferredScrollDecelerationFactor];;
+    scrollView.decelerationRate = (coordinator && coordinator->shouldSetScrollViewDecelerationRateFast()) ? UIScrollViewDecelerationRateFast : [_scrollView preferredScrollDecelerationFactor];
 #endif
 }
 
index f8ab235..3d240a7 100644 (file)
@@ -149,7 +149,10 @@ float RemoteScrollingCoordinatorProxy::closestSnapOffsetForMainFrameScrolling(Sc
     ASSERT(root && root->isFrameScrollingNode());
     ScrollingTreeFrameScrollingNode* rootFrame = static_cast<ScrollingTreeFrameScrollingNode*>(root);
     const Vector<float>& snapOffsets = axis == ScrollEventAxis::Horizontal ? rootFrame->horizontalSnapOffsets() : rootFrame->verticalSnapOffsets();
-    return closestSnapOffset<float, float>(snapOffsets, scrollDestination, velocity);
+
+    float scaledScrollDestination = scrollDestination / m_webPageProxy.displayedContentScale();
+    float rawClosestSnapOffset = closestSnapOffset<float, float>(snapOffsets, scaledScrollDestination, velocity);
+    return rawClosestSnapOffset * m_webPageProxy.displayedContentScale();
 }
 #endif