<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[181504] trunk/Source</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/181504">181504</a></dd>
<dt>Author</dt> <dd>bfulgham@apple.com</dd>
<dt>Date</dt> <dd>2015-03-14 22:11:19 -0700 (Sat, 14 Mar 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Source/WebCore:
[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):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorepagescrollingAxisScrollSnapOffsetscpp">trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp</a></li>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2UIProcessAPICocoaWKWebViewmm">trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm</a></li>
<li><a href="#trunkSourceWebKit2UIProcessiosRemoteScrollingCoordinatorProxyIOSmm">trunk/Source/WebKit2/UIProcess/ios/RemoteScrollingCoordinatorProxyIOS.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (181503 => 181504)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-03-15 04:58:55 UTC (rev 181503)
+++ trunk/Source/WebCore/ChangeLog 2015-03-15 05:11:19 UTC (rev 181504)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+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.
+
</ins><span class="cx"> 2015-03-14 Dean Jackson <dino@apple.com>
</span><span class="cx">
</span><span class="cx"> Feature flag for Animations Level 2
</span></span></pre></div>
<a id="trunkSourceWebCorepagescrollingAxisScrollSnapOffsetscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp (181503 => 181504)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp 2015-03-15 04:58:55 UTC (rev 181503)
+++ trunk/Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp 2015-03-15 05:11:19 UTC (rev 181504)
</span><span class="lines">@@ -83,7 +83,6 @@
</span><span class="cx"> LayoutUnit curSnapPositionShift = 0;
</span><span class="cx"> LayoutUnit maxScrollOffset = scrollSize - viewSize;
</span><span class="cx"> LayoutUnit lastSnapPosition = curSnapPositionShift;
</span><del>- snapOffsets.append(0);
</del><span class="cx"> do {
</span><span class="cx"> for (auto& snapPosition : snapOffsetSubsequence) {
</span><span class="cx"> LayoutUnit potentialSnapPosition = curSnapPositionShift + snapPosition - destinationOffset;
</span><span class="lines">@@ -98,6 +97,10 @@
</span><span class="cx"> }
</span><span class="cx"> curSnapPositionShift = lastSnapPosition + repeatOffset;
</span><span class="cx"> } while (hasRepeat && curSnapPositionShift < maxScrollOffset);
</span><ins>+
+ if (snapOffsets.isEmpty())
+ snapOffsets.append(0);
+
</ins><span class="cx"> // Always put a snap point on the maximum scroll offset.
</span><span class="cx"> // Not a part of the spec, but necessary to prevent unreachable content when snapping.
</span><span class="cx"> if (snapOffsets.last() != maxScrollOffset)
</span></span></pre></div>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (181503 => 181504)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog 2015-03-15 04:58:55 UTC (rev 181503)
+++ trunk/Source/WebKit2/ChangeLog 2015-03-15 05:11:19 UTC (rev 181504)
</span><span class="lines">@@ -1,3 +1,28 @@
</span><ins>+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):
+
</ins><span class="cx"> 2015-03-14 Dean Jackson <dino@apple.com>
</span><span class="cx">
</span><span class="cx"> Feature flag for Animations Level 2
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessAPICocoaWKWebViewmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm (181503 => 181504)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm 2015-03-15 04:58:55 UTC (rev 181503)
+++ trunk/Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm 2015-03-15 05:11:19 UTC (rev 181504)
</span><span class="lines">@@ -1339,7 +1339,7 @@
</span><span class="cx"> // FIXME: We will want to detect whether snapping will occur before beginning to drag. See WebPageProxy::didCommitLayerTree.
</span><span class="cx"> WebKit::RemoteScrollingCoordinatorProxy* coordinator = _page->scrollingCoordinatorProxy();
</span><span class="cx"> ASSERT(scrollView == _scrollView.get());
</span><del>- scrollView.decelerationRate = (coordinator && coordinator->shouldSetScrollViewDecelerationRateFast()) ? UIScrollViewDecelerationRateFast : [_scrollView preferredScrollDecelerationFactor];;
</del><ins>+ scrollView.decelerationRate = (coordinator && coordinator->shouldSetScrollViewDecelerationRateFast()) ? UIScrollViewDecelerationRateFast : [_scrollView preferredScrollDecelerationFactor];
</ins><span class="cx"> #endif
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessiosRemoteScrollingCoordinatorProxyIOSmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/ios/RemoteScrollingCoordinatorProxyIOS.mm (181503 => 181504)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/ios/RemoteScrollingCoordinatorProxyIOS.mm 2015-03-15 04:58:55 UTC (rev 181503)
+++ trunk/Source/WebKit2/UIProcess/ios/RemoteScrollingCoordinatorProxyIOS.mm 2015-03-15 05:11:19 UTC (rev 181504)
</span><span class="lines">@@ -149,7 +149,10 @@
</span><span class="cx"> ASSERT(root && root->isFrameScrollingNode());
</span><span class="cx"> ScrollingTreeFrameScrollingNode* rootFrame = static_cast<ScrollingTreeFrameScrollingNode*>(root);
</span><span class="cx"> const Vector<float>& snapOffsets = axis == ScrollEventAxis::Horizontal ? rootFrame->horizontalSnapOffsets() : rootFrame->verticalSnapOffsets();
</span><del>- return closestSnapOffset<float, float>(snapOffsets, scrollDestination, velocity);
</del><ins>+
+ float scaledScrollDestination = scrollDestination / m_webPageProxy.displayedContentScale();
+ float rawClosestSnapOffset = closestSnapOffset<float, float>(snapOffsets, scaledScrollDestination, velocity);
+ return rawClosestSnapOffset * m_webPageProxy.displayedContentScale();
</ins><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>