Title: [282730] trunk/Source/WebCore
Revision
282730
Author
[email protected]
Date
2021-09-18 15:44:53 -0700 (Sat, 18 Sep 2021)

Log Message

Remove use of NSScrollAnimationHelper for animated scrolls
https://bugs.webkit.org/show_bug.cgi?id=230445

Reviewed by Martin Robinson.

NSScrollAnimationHelper provided little utility. Under the hood it's ust an ease-in-out
animation with a duration based on distance, and that's exactly what ScrollAnimationSmooth
is now, so we can remove use of NSScrollAnimationHelper in ScrollAnimatorMac and just
use the ScrollAnimationSmooth from the base class.

This unifies the scroll animation used for keyboard scrolling and CSS smooth scrolling.

* platform/ScrollAnimator.cpp:
(WebCore::ScrollAnimator::scroll):
(WebCore::ScrollAnimator::scrollToPositionWithoutAnimation):
(WebCore::ScrollAnimator::setCurrentPosition):
(WebCore::ScrollAnimator::scrollAnimationDidUpdate):
* platform/ScrollAnimator.h:
(WebCore::ScrollAnimator::platformAllowsScrollAnimation const):
* platform/mac/ScrollAnimatorMac.h:
* platform/mac/ScrollAnimatorMac.mm:
(WebCore::ScrollAnimatorMac::ScrollAnimatorMac):
(WebCore::scrollAnimationEnabledForSystem):
(WebCore::ScrollAnimatorMac::platformAllowsScrollAnimation const):
(WebCore::ScrollAnimatorMac::scroll):
(abs): Deleted.
(-[WebScrollAnimationHelperDelegate initWithScrollAnimator:]): Deleted.
(-[WebScrollAnimationHelperDelegate invalidate]): Deleted.
(-[WebScrollAnimationHelperDelegate bounds]): Deleted.
(-[WebScrollAnimationHelperDelegate _immediateScrollToPoint:]): Deleted.
(-[WebScrollAnimationHelperDelegate _pixelAlignProposedScrollPosition:]): Deleted.
(-[WebScrollAnimationHelperDelegate convertSizeToBase:]): Deleted.
(-[WebScrollAnimationHelperDelegate convertSizeFromBase:]): Deleted.
(-[WebScrollAnimationHelperDelegate convertSizeToBacking:]): Deleted.
(-[WebScrollAnimationHelperDelegate convertSizeFromBacking:]): Deleted.
(-[WebScrollAnimationHelperDelegate superview]): Deleted.
(-[WebScrollAnimationHelperDelegate documentView]): Deleted.
(-[WebScrollAnimationHelperDelegate window]): Deleted.
(-[WebScrollAnimationHelperDelegate _recursiveRecomputeToolTips]): Deleted.
(WebCore::ScrollAnimatorMac::scrollToPositionWithAnimation): Deleted.
(WebCore::ScrollAnimatorMac::scrollToPositionWithoutAnimation): Deleted.
(WebCore::ScrollAnimatorMac::immediateScrollToPositionForScrollAnimation): Deleted.
* platform/mac/ScrollbarsControllerMac.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (282729 => 282730)


--- trunk/Source/WebCore/ChangeLog	2021-09-18 20:41:26 UTC (rev 282729)
+++ trunk/Source/WebCore/ChangeLog	2021-09-18 22:44:53 UTC (rev 282730)
@@ -1,5 +1,51 @@
 2021-09-18  Simon Fraser  <[email protected]>
 
+        Remove use of NSScrollAnimationHelper for animated scrolls
+        https://bugs.webkit.org/show_bug.cgi?id=230445
+
+        Reviewed by Martin Robinson.
+
+        NSScrollAnimationHelper provided little utility. Under the hood it's ust an ease-in-out
+        animation with a duration based on distance, and that's exactly what ScrollAnimationSmooth
+        is now, so we can remove use of NSScrollAnimationHelper in ScrollAnimatorMac and just
+        use the ScrollAnimationSmooth from the base class.
+
+        This unifies the scroll animation used for keyboard scrolling and CSS smooth scrolling.
+
+        * platform/ScrollAnimator.cpp:
+        (WebCore::ScrollAnimator::scroll):
+        (WebCore::ScrollAnimator::scrollToPositionWithoutAnimation):
+        (WebCore::ScrollAnimator::setCurrentPosition):
+        (WebCore::ScrollAnimator::scrollAnimationDidUpdate):
+        * platform/ScrollAnimator.h:
+        (WebCore::ScrollAnimator::platformAllowsScrollAnimation const):
+        * platform/mac/ScrollAnimatorMac.h:
+        * platform/mac/ScrollAnimatorMac.mm:
+        (WebCore::ScrollAnimatorMac::ScrollAnimatorMac):
+        (WebCore::scrollAnimationEnabledForSystem):
+        (WebCore::ScrollAnimatorMac::platformAllowsScrollAnimation const):
+        (WebCore::ScrollAnimatorMac::scroll):
+        (abs): Deleted.
+        (-[WebScrollAnimationHelperDelegate initWithScrollAnimator:]): Deleted.
+        (-[WebScrollAnimationHelperDelegate invalidate]): Deleted.
+        (-[WebScrollAnimationHelperDelegate bounds]): Deleted.
+        (-[WebScrollAnimationHelperDelegate _immediateScrollToPoint:]): Deleted.
+        (-[WebScrollAnimationHelperDelegate _pixelAlignProposedScrollPosition:]): Deleted.
+        (-[WebScrollAnimationHelperDelegate convertSizeToBase:]): Deleted.
+        (-[WebScrollAnimationHelperDelegate convertSizeFromBase:]): Deleted.
+        (-[WebScrollAnimationHelperDelegate convertSizeToBacking:]): Deleted.
+        (-[WebScrollAnimationHelperDelegate convertSizeFromBacking:]): Deleted.
+        (-[WebScrollAnimationHelperDelegate superview]): Deleted.
+        (-[WebScrollAnimationHelperDelegate documentView]): Deleted.
+        (-[WebScrollAnimationHelperDelegate window]): Deleted.
+        (-[WebScrollAnimationHelperDelegate _recursiveRecomputeToolTips]): Deleted.
+        (WebCore::ScrollAnimatorMac::scrollToPositionWithAnimation): Deleted.
+        (WebCore::ScrollAnimatorMac::scrollToPositionWithoutAnimation): Deleted.
+        (WebCore::ScrollAnimatorMac::immediateScrollToPositionForScrollAnimation): Deleted.
+        * platform/mac/ScrollbarsControllerMac.h:
+
+2021-09-18  Simon Fraser  <[email protected]>
+
         Rename haveScrolledSincePageLoad() and push the state to ScrollbarsController
         https://bugs.webkit.org/show_bug.cgi?id=230444
 

Modified: trunk/Source/WebCore/platform/ScrollAnimator.cpp (282729 => 282730)


--- trunk/Source/WebCore/platform/ScrollAnimator.cpp	2021-09-18 20:41:26 UTC (rev 282729)
+++ trunk/Source/WebCore/platform/ScrollAnimator.cpp	2021-09-18 22:44:53 UTC (rev 282730)
@@ -71,7 +71,7 @@
     if (behavior.contains(ScrollBehavior::DoDirectionalSnapping)) {
         behavior.remove(ScrollBehavior::DoDirectionalSnapping);
         if (!m_scrollController.usesScrollSnap())
-            return scroll(orientation, granularity, step, multiplier, behavior);
+            return ScrollAnimator::scroll(orientation, granularity, step, multiplier, behavior);
 
         auto currentOffset = offsetFromPosition(currentPosition());
         auto newOffset = currentOffset + delta;
@@ -87,7 +87,7 @@
     }
 
 #if ENABLE(SMOOTH_SCROLLING) && !PLATFORM(IOS_FAMILY)
-    if (m_scrollableArea.scrollAnimatorEnabled() && !behavior.contains(ScrollBehavior::NeverAnimate))
+    if (m_scrollableArea.scrollAnimatorEnabled() && platformAllowsScrollAnimation() && !behavior.contains(ScrollBehavior::NeverAnimate))
         return m_scrollAnimation->startAnimatedScroll(orientation, granularity, m_currentPosition, step, multiplier);
 #endif
 
@@ -105,6 +105,7 @@
         return false;
 
     m_scrollAnimation->stop();
+
     m_currentPosition = adjustedPosition;
     notifyPositionChanged(adjustedPosition - currentPosition);
     updateActiveScrollSnapIndexForOffset();
@@ -238,6 +239,8 @@
 
 void ScrollAnimator::setCurrentPosition(const FloatPoint& position)
 {
+    WTFLogAlways("setCurrentPosition to %.2f", position.y());
+
     m_currentPosition = position;
     updateActiveScrollSnapIndexForOffset();
 }
@@ -383,6 +386,7 @@
 void ScrollAnimator::scrollAnimationDidUpdate(ScrollAnimation&, const FloatPoint& position)
 {
     FloatSize delta = position - m_currentPosition;
+    WTFLogAlways("scrollAnimationDidUpdate to %.2f", position.y());
     m_currentPosition = position;
     notifyPositionChanged(delta);
     updateActiveScrollSnapIndexForOffset();

Modified: trunk/Source/WebCore/platform/ScrollAnimator.h (282729 => 282730)


--- trunk/Source/WebCore/platform/ScrollAnimator.h	2021-09-18 20:41:26 UTC (rev 282729)
+++ trunk/Source/WebCore/platform/ScrollAnimator.h	2021-09-18 22:44:53 UTC (rev 282730)
@@ -141,6 +141,8 @@
 
 protected:
     virtual void notifyPositionChanged(const FloatSize& delta);
+    virtual bool platformAllowsScrollAnimation() const { return true; }
+
     void updateActiveScrollSnapIndexForOffset();
 
     FloatPoint offsetFromPosition(const FloatPoint& position);

Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h (282729 => 282730)


--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h	2021-09-18 20:41:26 UTC (rev 282729)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h	2021-09-18 22:44:53 UTC (rev 282730)
@@ -33,8 +33,6 @@
 #include "ScrollAnimator.h"
 #include <wtf/RetainPtr.h>
 
-OBJC_CLASS WebScrollAnimationHelperDelegate;
-
 namespace WebCore {
 
 class Scrollbar;
@@ -44,12 +42,8 @@
     ScrollAnimatorMac(ScrollableArea&);
     virtual ~ScrollAnimatorMac();
 
-    void immediateScrollToPositionForScrollAnimation(const FloatPoint& newPosition);
-
 private:
     bool scroll(ScrollbarOrientation, ScrollGranularity, float step, float multiplier, OptionSet<ScrollBehavior>) final;
-    bool scrollToPositionWithAnimation(const FloatPoint&) final;
-    bool scrollToPositionWithoutAnimation(const FloatPoint& position, ScrollClamping = ScrollClamping::Clamped) final;
 
 #if ENABLE(RUBBER_BANDING)
     bool shouldForwardWheelEventsToParent(const PlatformWheelEvent&) const;
@@ -56,6 +50,8 @@
     bool handleWheelEvent(const PlatformWheelEvent&) final;
 #endif
 
+    bool platformAllowsScrollAnimation() const;
+
     void handleWheelEventPhase(PlatformWheelEventPhase) final;
     
     void notifyPositionChanged(const FloatSize& delta) final;
@@ -82,9 +78,6 @@
     void immediateScrollBy(const FloatSize&) final;
     void adjustScrollPositionToBoundsIfNecessary() final;
 #endif
-
-    RetainPtr<id> m_scrollAnimationHelper;
-    RetainPtr<WebScrollAnimationHelperDelegate> m_scrollAnimationHelperDelegate;
 };
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm (282729 => 282730)


--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm	2021-09-18 20:41:26 UTC (rev 282729)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm	2021-09-18 22:44:53 UTC (rev 282730)
@@ -35,121 +35,8 @@
 #import "ScrollView.h"
 #import "ScrollableArea.h"
 #import "ScrollbarsController.h"
-#import <wtf/BlockObjCExceptions.h>
-#import <wtf/NakedPtr.h>
 #import <wtf/text/TextStream.h>
 
-using WebCore::ScrollableArea;
-using WebCore::ScrollAnimatorMac;
-using WebCore::Scrollbar;
-using WebCore::GraphicsLayer;
-using WebCore::VerticalScrollbar;
-using WebCore::IntRect;
-
-@interface NSObject (ScrollAnimationHelperDetails)
-- (id)initWithDelegate:(id)delegate;
-- (void)_stopRun;
-- (BOOL)_isAnimating;
-- (NSPoint)targetOrigin;
-- (CGFloat)_progress;
-@end
-
-@interface WebScrollAnimationHelperDelegate : NSObject
-{
-    NakedPtr<WebCore::ScrollAnimatorMac> _animator;
-}
-- (id)initWithScrollAnimator:(NakedPtr<WebCore::ScrollAnimatorMac>)scrollAnimator;
-@end
-
-static NSSize abs(NSSize size)
-{
-    NSSize finalSize = size;
-    if (finalSize.width < 0)
-        finalSize.width = -finalSize.width;
-    if (finalSize.height < 0)
-        finalSize.height = -finalSize.height;
-    return finalSize;    
-}
-
-@implementation WebScrollAnimationHelperDelegate
-
-- (id)initWithScrollAnimator:(NakedPtr<WebCore::ScrollAnimatorMac>)scrollAnimator
-{
-    self = [super init];
-    if (!self)
-        return nil;
-
-    _animator = scrollAnimator;
-    return self;
-}
-
-- (void)invalidate
-{
-    _animator = nullptr;
-}
-
-- (NSRect)bounds
-{
-    if (!_animator)
-        return NSZeroRect;
-
-    WebCore::FloatPoint currentPosition = _animator->currentPosition();
-    return NSMakeRect(currentPosition.x(), currentPosition.y(), 0, 0);
-}
-
-- (void)_immediateScrollToPoint:(NSPoint)newPosition
-{
-    if (!_animator)
-        return;
-    _animator->immediateScrollToPositionForScrollAnimation(newPosition);
-}
-
-- (NSPoint)_pixelAlignProposedScrollPosition:(NSPoint)newOrigin
-{
-    return newOrigin;
-}
-
-- (NSSize)convertSizeToBase:(NSSize)size
-{
-    return abs(size);
-}
-
-- (NSSize)convertSizeFromBase:(NSSize)size
-{
-    return abs(size);
-}
-
-- (NSSize)convertSizeToBacking:(NSSize)size
-{
-    return abs(size);
-}
-
-- (NSSize)convertSizeFromBacking:(NSSize)size
-{
-    return abs(size);
-}
-
-- (id)superview
-{
-    return nil;
-}
-
-- (id)documentView
-{
-    return nil;
-}
-
-- (id)window
-{
-    return nil;
-}
-
-- (void)_recursiveRecomputeToolTips
-{
-}
-
-@end
-
 namespace WebCore {
 
 std::unique_ptr<ScrollAnimator> ScrollAnimator::create(ScrollableArea& scrollableArea)
@@ -160,17 +47,13 @@
 ScrollAnimatorMac::ScrollAnimatorMac(ScrollableArea& scrollableArea)
     : ScrollAnimator(scrollableArea)
 {
-    m_scrollAnimationHelperDelegate = adoptNS([[WebScrollAnimationHelperDelegate alloc] initWithScrollAnimator:this]);
-    m_scrollAnimationHelper = adoptNS([[NSClassFromString(@"NSScrollAnimationHelper") alloc] initWithDelegate:m_scrollAnimationHelperDelegate.get()]);
-
 }
 
 ScrollAnimatorMac::~ScrollAnimatorMac() = default;
 
-static bool scrollAnimationEnabledForSystem()
+bool ScrollAnimatorMac::platformAllowsScrollAnimation() const
 {
-    NSString* scrollAnimationDefaultsKey = @"NSScrollAnimationEnabled";
-    static bool enabled = [[NSUserDefaults standardUserDefaults] boolForKey:scrollAnimationDefaultsKey];
+    static bool enabled = [[NSUserDefaults standardUserDefaults] boolForKey:@"NSScrollAnimationEnabled"];
     return enabled;
 }
 
@@ -196,52 +79,9 @@
 {
     m_scrollableArea.scrollbarsController().setScrollbarAnimationsUnsuspendedByUserInteraction(true);
 
-    // This method doesn't do directional snapping, but our base class does. It will call into
-    // ScrollAnimatorMac::scroll again with the snapped positions and ScrollBehavior::Default.
-    if (behavior.contains(ScrollBehavior::DoDirectionalSnapping))
-        return ScrollAnimator::scroll(orientation, granularity, step, multiplier, behavior);
-
-    bool shouldAnimate = scrollAnimationEnabledForSystem() && m_scrollableArea.scrollAnimatorEnabled() && granularity != ScrollByPixel
-        && !behavior.contains(ScrollBehavior::NeverAnimate);
-    FloatPoint newPosition = this->currentPosition() + deltaFromStep(orientation, step, multiplier);
-    newPosition = newPosition.constrainedBetween(scrollableArea().minimumScrollPosition(), scrollableArea().maximumScrollPosition());
-
-    LOG_WITH_STREAM(Scrolling, stream << "ScrollAnimatorMac::scroll from " << currentPosition() << " to " << newPosition);
-
-    if (!shouldAnimate)
-        return scrollToPositionWithoutAnimation(newPosition);
-
-    if ([m_scrollAnimationHelper _isAnimating]) {
-        NSPoint targetOrigin = [m_scrollAnimationHelper targetOrigin];
-        if (orientation == HorizontalScrollbar)
-            newPosition.setY(targetOrigin.y);
-        else
-            newPosition.setX(targetOrigin.x);
-    }
-
-    LOG_WITH_STREAM(Scrolling, stream << "ScrollAnimatorMac::scroll " << " from " << currentPosition() << " to " << newPosition);
-    [m_scrollAnimationHelper scrollToPoint:newPosition];
-    return true;
+    return ScrollAnimator::scroll(orientation, granularity, step, multiplier, behavior);
 }
 
-bool ScrollAnimatorMac::scrollToPositionWithAnimation(const FloatPoint& newPosition)
-{
-    bool positionChanged = newPosition != currentPosition();
-    if (!positionChanged && !scrollableArea().scrollOriginChanged())
-        return false;
-
-    // FIXME: This is used primarily by smooth scrolling. This should ideally use native scroll animations.
-    // See: https://bugs.webkit.org/show_bug.cgi?id=218857
-    [m_scrollAnimationHelper _stopRun];
-    return ScrollAnimator::scrollToPositionWithAnimation(newPosition);
-}
-
-bool ScrollAnimatorMac::scrollToPositionWithoutAnimation(const FloatPoint& position, ScrollClamping clamping)
-{
-    [m_scrollAnimationHelper _stopRun];
-    return ScrollAnimator::scrollToPositionWithoutAnimation(position, clamping);
-}
-
 FloatPoint ScrollAnimatorMac::adjustScrollPositionIfNecessary(const FloatPoint& position) const
 {
     if (!m_scrollableArea.constrainsScrollingToContentEdge())
@@ -281,12 +121,6 @@
     return m_scrollController.isScrollSnapInProgress();
 }
 
-void ScrollAnimatorMac::immediateScrollToPositionForScrollAnimation(const FloatPoint& newPosition)
-{
-    ASSERT(m_scrollAnimationHelper);
-    ScrollAnimator::scrollToPositionWithoutAnimation(newPosition);
-}
-
 void ScrollAnimatorMac::notifyPositionChanged(const FloatSize& delta)
 {
     m_scrollableArea.scrollbarsController().notifyContentAreaScrolled(delta);

Modified: trunk/Source/WebCore/platform/mac/ScrollbarsControllerMac.h (282729 => 282730)


--- trunk/Source/WebCore/platform/mac/ScrollbarsControllerMac.h	2021-09-18 20:41:26 UTC (rev 282729)
+++ trunk/Source/WebCore/platform/mac/ScrollbarsControllerMac.h	2021-09-18 22:44:53 UTC (rev 282730)
@@ -33,7 +33,6 @@
 
 #include <wtf/RetainPtr.h>
 
-OBJC_CLASS WebScrollAnimationHelperDelegate;
 OBJC_CLASS WebScrollerImpPairDelegate;
 OBJC_CLASS WebScrollerImpDelegate;
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to