Changeset 185681 in webkit

Timestamp:

Jun 17, 2015, 5:32:33 PM (11 years ago)

Author:

Brent Fulgham

Message:

Overflow regions with scroll snap points are not reliably rubber banding
​https://bugs.webkit.org/show_bug.cgi?id=142522
<rdar://problem/20100726>

Reviewed by Darin Adler.

Source/WebCore:

When computing the target scroll destination, update the nearest snap point index
and other bookkeeping, but keep the original gesture target if it would have taken
us beyond either limit of the scroll container.

• page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
• page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:

(WebCore::ScrollingTreeFrameScrollingNodeMac::scrollExtents): Add new method
to support client API.

• platform/ScrollAnimator.cpp:

(WebCore::ScrollAnimator::scrollExtents): Add new method to support client API.

• platform/ScrollAnimator.h:
• platform/cocoa/ScrollController.h:

(WebCore::ScrollControllerClient::scrollExtents): Added new pure virtual method to API.

• platform/cocoa/ScrollController.mm:

(WebCore::ScrollController::beginScrollSnapAnimation): Hold onto original user gesture
target, and use that instead of our nearest snap point if the gesture takes us past
either extreme of the scroll container.

Source/WebKit2:

Make sure we don't block rubberbanding behavior when a scroll gesture should take us past
the end of the scroll container.

• UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm:

(-[WKOverflowScrollViewDelegate scrollViewWillEndDragging:withVelocity:targetContentOffset:]): Don't adjust
target point if we were going to scroll past the edges of the scroll container.

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h (modified) (1 diff)
• WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm (modified) (2 diffs)
• WebCore/platform/ScrollAnimator.cpp (modified) (2 diffs)
• WebCore/platform/ScrollAnimator.h (modified) (1 diff)
• WebCore/platform/cocoa/ScrollController.h (modified) (2 diffs)
• WebCore/platform/cocoa/ScrollController.mm (modified) (2 diffs)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-06-17  Brent Fulgham  <bfulgham@apple.com>
+
+        Overflow regions with scroll snap points are not reliably rubber banding
+        https://bugs.webkit.org/show_bug.cgi?id=142522
+        <rdar://problem/20100726>
+
+        Reviewed by Darin Adler.
+
+        When computing the target scroll destination, update the nearest snap point index
+        and other bookkeeping, but keep the original gesture target if it would have taken
+        us beyond either limit of the scroll container.
+
+        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h:
+        * page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm:
+        (WebCore::ScrollingTreeFrameScrollingNodeMac::scrollExtents): Add new method
+        to support client API.
+        * platform/ScrollAnimator.cpp:
+        (WebCore::ScrollAnimator::scrollExtents): Add new method to support client API.
+        * platform/ScrollAnimator.h:
+        * platform/cocoa/ScrollController.h:
+        (WebCore::ScrollControllerClient::scrollExtents): Added new pure virtual method to API.
+        * platform/cocoa/ScrollController.mm:
+        (WebCore::ScrollController::beginScrollSnapAnimation): Hold onto original user gesture
+        target, and use that instead of our nearest snap point if the gesture takes us past
+        either extreme of the scroll container.
+
 2015-06-17  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.h

@@ -89 +89 @@
     void startScrollSnapTimer(ScrollEventAxis) override;
     void stopScrollSnapTimer(ScrollEventAxis) override;
+    LayoutSize scrollExtent() const override;
 #endif
 

trunk/Source/WebCore/page/scrolling/mac/ScrollingTreeFrameScrollingNodeMac.mm

@@ -30 +30 @@
 
 #import "FrameView.h"
+#import "LayoutSize.h"
 #import "Logging.h"
 #import "NSScrollerImpDetails.h"
@@ -588 +589 @@
         scrollingTree().setMainFrameIsScrollSnapping(false);
 }
+    
+LayoutSize ScrollingTreeFrameScrollingNodeMac::scrollExtent() const
+{
+    return LayoutSize(totalContentsSize());
+}
 #endif
 

trunk/Source/WebCore/platform/ScrollAnimator.cpp

@@ -34 +34 @@
 
 #include "FloatPoint.h"
+#include "LayoutSize.h"
 #include "PlatformWheelEvent.h"
 #include "ScrollableArea.h"
@@ -214 +215 @@
         scrollToOffsetWithoutAnimation(FloatPoint(currentPosition.x(), currentPosition.y() + delta));
 }
+
+LayoutSize ScrollAnimator::scrollExtent() const
+{
+    return m_scrollableArea.contentsSize();
+}
 #endif
 

trunk/Source/WebCore/platform/ScrollAnimator.h

@@ -136 +136 @@
     bool activeScrollSnapIndexDidChange() const;
     unsigned activeScrollSnapIndexForAxis(ScrollEventAxis) const;
+    LayoutSize scrollExtent() const override;
 #endif
 

trunk/Source/WebCore/platform/cocoa/ScrollController.h

@@ -42 +42 @@
 namespace WebCore {
 
+class LayoutSize;
 class PlatformWheelEvent;
 class ScrollableArea;
@@ -103 +104 @@
         return 0;
     }
+
+    virtual LayoutSize scrollExtent() const = 0;
 #endif
 };

trunk/Source/WebCore/platform/cocoa/ScrollController.mm

@@ -27 +27 @@
 #include "ScrollController.h"
 
+#include "LayoutSize.h"
 #include "PlatformWheelEvent.h"
 #include "WebCoreSystemInterface.h"
@@ -753 +754 @@
     LayoutUnit offset = m_client.scrollOffsetOnAxis(axis);
     float initialWheelDelta = newState == ScrollSnapState::Gliding ? snapState.averageInitialWheelDelta() : 0;
-    LayoutUnit projectedScrollDestination = newState == ScrollSnapState::Gliding ? snapState.m_beginTrackingWheelDeltaOffset + LayoutUnit(projectedInertialScrollDistance(initialWheelDelta)) : offset;
+    LayoutUnit scaledProjectedScrollDestination = newState == ScrollSnapState::Gliding ? snapState.m_beginTrackingWheelDeltaOffset + LayoutUnit(projectedInertialScrollDistance(initialWheelDelta)) : offset;
     if (snapState.m_snapOffsets.isEmpty())
         return;
 
     float scaleFactor = m_client.pageScaleFactor();
-    
-    projectedScrollDestination = std::min(std::max(LayoutUnit(projectedScrollDestination / scaleFactor), snapState.m_snapOffsets.first()), snapState.m_snapOffsets.last());
+    LayoutUnit originalProjectedScrollDestination = scaledProjectedScrollDestination / scaleFactor;
+    
+    LayoutUnit clampedScrollDestination = std::min(std::max(originalProjectedScrollDestination, snapState.m_snapOffsets.first()), snapState.m_snapOffsets.last());
     snapState.m_initialOffset = offset;
     m_activeScrollSnapIndexDidChange = false;
-    snapState.m_targetOffset = scaleFactor * closestSnapOffset<LayoutUnit, float>(snapState.m_snapOffsets, projectedScrollDestination, initialWheelDelta, snapState.m_activeSnapIndex);
+    snapState.m_targetOffset = scaleFactor * closestSnapOffset<LayoutUnit, float>(snapState.m_snapOffsets, clampedScrollDestination, initialWheelDelta, snapState.m_activeSnapIndex);
     if (snapState.m_initialOffset == snapState.m_targetOffset)
         return;
+
+    LayoutUnit scrollExtent = (axis == ScrollEventAxis::Horizontal) ? m_client.scrollExtent().width() : m_client.scrollExtent().height();
+    LayoutUnit projectedScrollDestination = clampedScrollDestination;
+    if (originalProjectedScrollDestination < 0 || originalProjectedScrollDestination > scrollExtent)
+        projectedScrollDestination = originalProjectedScrollDestination;
     
     m_activeScrollSnapIndexDidChange = true;

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-06-17  Brent Fulgham  <bfulgham@apple.com>
+
+        Overflow regions with scroll snap points are not reliably rubber banding
+        https://bugs.webkit.org/show_bug.cgi?id=142522
+        <rdar://problem/20100726>
+
+        Reviewed by Darin Adler.
+
+        Make sure we don't block rubberbanding behavior when a scroll gesture should take us past
+        the end of the scroll container.
+
+        * UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm:
+        (-[WKOverflowScrollViewDelegate scrollViewWillEndDragging:withVelocity:targetContentOffset:]): Don't adjust
+        target point if we were going to scroll past the edges of the scroll container.
+
 2015-06-17  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebKit2/UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm

@@ -81 +81 @@
 - (void)scrollViewWillEndDragging:(UIScrollView *)scrollView withVelocity:(CGPoint)velocity targetContentOffset:(inout CGPoint *)targetContentOffset
 {
+    CGFloat horizontalTarget = targetContentOffset->x;
+    CGFloat verticalTarget = targetContentOffset->y;
+
     unsigned ignore;
-    if (!_scrollingTreeNode->horizontalSnapOffsets().isEmpty())
-        targetContentOffset->x = closestSnapOffset<float, CGFloat>(_scrollingTreeNode->horizontalSnapOffsets(), targetContentOffset->x, velocity.x, ignore);
-    if (!_scrollingTreeNode->verticalSnapOffsets().isEmpty())
-        targetContentOffset->y = closestSnapOffset<float, CGFloat>(_scrollingTreeNode->verticalSnapOffsets(), targetContentOffset->y, velocity.y, ignore);
+    if (!_scrollingTreeNode->horizontalSnapOffsets().isEmpty() && horizontalTarget >= 0 && horizontalTarget <= scrollView.contentSize.width)
+        targetContentOffset->x = closestSnapOffset<float, CGFloat>(_scrollingTreeNode->horizontalSnapOffsets(), horizontalTarget, velocity.x, ignore);
+    if (!_scrollingTreeNode->verticalSnapOffsets().isEmpty() && verticalTarget >= 0 && verticalTarget <= scrollView.contentSize.height)
+        targetContentOffset->y = closestSnapOffset<float, CGFloat>(_scrollingTreeNode->verticalSnapOffsets(), verticalTarget, velocity.y, ignore);
 }
 #endif