From f7f1259b7b7f07f9e37ccaa4a53f07286f0c5541 Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Wed, 23 Mar 2016 16:14:07 -0700 Subject: [PATCH] Scrollable physics should be reasonable when sizes change Previously, when the content extent changed during a scroll interaction, we'd stop the current scroll interaction and reset the scroll offset. Now we try to continue the scroll interaction (e.g., drag, fling, or overscroll) even through the underlying scroll behavior has changed. For physics-based scroll interactions, we keep the current position and velocity and recompute the operative forces. For drag interactions, we keep the current position and continue to let the user drag the scroll offset. After this patch, we still disrupt non-physical scroll animations that are operating outside the new scroll bounds because it's not clear how we can sensibly modify them to work with the new scroll bounds. --- .../src/animation/animation_controller.dart | 9 ++++++ packages/flutter/lib/src/material/tabs.dart | 8 ++--- .../flutter/lib/src/widgets/editable.dart | 2 +- .../lib/src/widgets/pageable_list.dart | 2 +- .../flutter/lib/src/widgets/scrollable.dart | 32 +++++++++++++++---- .../lib/src/widgets/scrollable_grid.dart | 2 +- .../lib/src/widgets/scrollable_list.dart | 4 +-- 7 files changed, 44 insertions(+), 15 deletions(-) diff --git a/packages/flutter/lib/src/animation/animation_controller.dart b/packages/flutter/lib/src/animation/animation_controller.dart index 38063591ac..ef42916373 100644 --- a/packages/flutter/lib/src/animation/animation_controller.dart +++ b/packages/flutter/lib/src/animation/animation_controller.dart @@ -124,6 +124,12 @@ class AnimationController extends Animation _checkStatusChanged(); } + /// The amount of time that has passed between the time the animation started and the most recent tick of the animation. + /// + /// If the controller is not animating, the last elapsed duration is null; + Duration get lastElapsedDuration => _lastElapsedDuration; + Duration _lastElapsedDuration; + /// Whether this animation is currently animating in either the forward or reverse direction. bool get isAnimating => _ticker.isTicking; @@ -205,6 +211,7 @@ class AnimationController extends Animation assert(simulation != null); assert(!isAnimating); _simulation = simulation; + _lastElapsedDuration = const Duration(); _value = simulation.x(0.0).clamp(lowerBound, upperBound); Future result = _ticker.start(); _checkStatusChanged(); @@ -214,6 +221,7 @@ class AnimationController extends Animation /// Stops running this animation. void stop() { _simulation = null; + _lastElapsedDuration = null; _ticker.stop(); } @@ -233,6 +241,7 @@ class AnimationController extends Animation } void _tick(Duration elapsed) { + _lastElapsedDuration = elapsed; double elapsedInSeconds = elapsed.inMicroseconds.toDouble() / Duration.MICROSECONDS_PER_SECOND; _value = _simulation.x(elapsedInSeconds).clamp(lowerBound, upperBound); if (_simulation.isDone(elapsedInSeconds)) diff --git a/packages/flutter/lib/src/material/tabs.dart b/packages/flutter/lib/src/material/tabs.dart index 34e0c9a733..28dd1fcfa4 100644 --- a/packages/flutter/lib/src/material/tabs.dart +++ b/packages/flutter/lib/src/material/tabs.dart @@ -768,7 +768,7 @@ class _TabBarState extends ScrollableState> implements TabBarSelect } void _updateScrollBehavior() { - scrollTo(scrollBehavior.updateExtents( + didUpdateScrollBehavior(scrollBehavior.updateExtents( containerExtent: config.scrollDirection == Axis.vertical ? _viewportSize.height : _viewportSize.width, contentExtent: _tabWidths.reduce((double sum, double width) => sum + width), scrollOffset: scrollOffset @@ -943,11 +943,11 @@ class _TabBarViewState extends PageableListState> implements Ta void _updateScrollBehaviorForSelectedIndex(int selectedIndex) { if (selectedIndex == 0) { - scrollTo(scrollBehavior.updateExtents(contentExtent: 2.0, containerExtent: 1.0, scrollOffset: 0.0)); + didUpdateScrollBehavior(scrollBehavior.updateExtents(contentExtent: 2.0, containerExtent: 1.0, scrollOffset: 0.0)); } else if (selectedIndex == _tabCount - 1) { - scrollTo(scrollBehavior.updateExtents(contentExtent: 2.0, containerExtent: 1.0, scrollOffset: 1.0)); + didUpdateScrollBehavior(scrollBehavior.updateExtents(contentExtent: 2.0, containerExtent: 1.0, scrollOffset: 1.0)); } else { - scrollTo(scrollBehavior.updateExtents(contentExtent: 3.0, containerExtent: 1.0, scrollOffset: 1.0)); + didUpdateScrollBehavior(scrollBehavior.updateExtents(contentExtent: 3.0, containerExtent: 1.0, scrollOffset: 1.0)); } } diff --git a/packages/flutter/lib/src/widgets/editable.dart b/packages/flutter/lib/src/widgets/editable.dart index 0abfd02c4a..801d9a636c 100644 --- a/packages/flutter/lib/src/widgets/editable.dart +++ b/packages/flutter/lib/src/widgets/editable.dart @@ -235,7 +235,7 @@ class RawInputLineState extends ScrollableState { // render object via our return value. _containerWidth = dimensions.containerSize.width; _contentWidth = dimensions.contentSize.width; - scrollTo(scrollBehavior.updateExtents( + didUpdateScrollBehavior(scrollBehavior.updateExtents( contentExtent: _contentWidth, containerExtent: _containerWidth, // Set the scroll offset to match the content width so that the diff --git a/packages/flutter/lib/src/widgets/pageable_list.dart b/packages/flutter/lib/src/widgets/pageable_list.dart index e06f8d07b0..26ddb9c8d5 100644 --- a/packages/flutter/lib/src/widgets/pageable_list.dart +++ b/packages/flutter/lib/src/widgets/pageable_list.dart @@ -147,7 +147,7 @@ class PageableListState extends ScrollableState { void _updateScrollBehavior() { config.scrollableListPainter?.contentExtent = _itemCount.toDouble(); - scrollTo(scrollBehavior.updateExtents( + didUpdateScrollBehavior(scrollBehavior.updateExtents( contentExtent: _itemCount.toDouble(), containerExtent: 1.0, scrollOffset: scrollOffset diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart index 1f0d8bbd75..9ce51a8ce1 100644 --- a/packages/flutter/lib/src/widgets/scrollable.dart +++ b/packages/flutter/lib/src/widgets/scrollable.dart @@ -226,10 +226,12 @@ abstract class ScrollableState extends State { _scrollOffset = PageStorage.of(context)?.readState(context) ?? config.initialScrollOffset ?? 0.0; } + Simulation _simulation; AnimationController _controller; @override void dispose() { + _simulation = null; _controller.stop(); super.dispose(); } @@ -358,6 +360,7 @@ abstract class ScrollableState extends State { return new Future.value(); if (duration == null) { + _simulation = null; _controller.stop(); _setScrollOffset(newScrollOffset); return new Future.value(); @@ -368,12 +371,27 @@ abstract class ScrollableState extends State { } Future _animateTo(double newScrollOffset, Duration duration, Curve curve) { + _simulation = null; _controller.stop(); _controller.value = scrollOffset; _startScroll(); return _controller.animateTo(newScrollOffset, duration: duration, curve: curve).then(_endScroll); } + void didUpdateScrollBehavior(double newScrollOffset) { + if (newScrollOffset == _scrollOffset) + return; + if (_numberOfInProgressScrolls > 0) { + if (_simulation != null) { + double dx = _simulation.dx(_controller.lastElapsedDuration.inMicroseconds / Duration.MICROSECONDS_PER_SECOND); + // TODO(abarth): We should be consistent about the units we use for velocity (i.e., per second). + _startToEndAnimation(dx / Duration.MILLISECONDS_PER_SECOND); + } + return; + } + scrollTo(newScrollOffset); + } + /// Fling the scroll offset with the given velocity. /// /// Calling this function starts a physics-based animation of the scroll @@ -390,17 +408,18 @@ abstract class ScrollableState extends State { /// Calling this function starts a physics-based animation of the scroll /// offset either to a snap point or to within the scrolling bounds. The /// physics simulation used is determined by the scroll behavior. - Future settleScrollOffset() { + Future settleScrollOffset() { return _startToEndAnimation(0.0); } Future _startToEndAnimation(double scrollVelocity) { + _simulation = null; _controller.stop(); - Simulation simulation = _createSnapSimulation(scrollVelocity) ?? _createFlingSimulation(scrollVelocity); - if (simulation == null) + _simulation = _createSnapSimulation(scrollVelocity) ?? _createFlingSimulation(scrollVelocity); + if (_simulation == null) return new Future.value(); _startScroll(); - return _controller.animateWith(simulation).then(_endScroll); + return _controller.animateWith(_simulation).then(_endScroll); } /// Whether this scrollable should attempt to snap scroll offsets. @@ -471,6 +490,7 @@ abstract class ScrollableState extends State { } void _handleDragDown(_) { + _simulation = null; _controller.stop(); } @@ -653,7 +673,7 @@ class _ScrollableViewportState extends ScrollableState { // render object via our return value. _viewportSize = config.scrollDirection == Axis.vertical ? dimensions.containerSize.height : dimensions.containerSize.width; _childSize = config.scrollDirection == Axis.vertical ? dimensions.contentSize.height : dimensions.contentSize.width; - scrollTo(scrollBehavior.updateExtents( + didUpdateScrollBehavior(scrollBehavior.updateExtents( contentExtent: _childSize, containerExtent: _viewportSize, scrollOffset: scrollOffset @@ -819,7 +839,7 @@ class ScrollableMixedWidgetListState extends ScrollableState { void _handleExtentsChanged(double contentExtent, double containerExtent) { setState(() { - scrollTo(scrollBehavior.updateExtents( + didUpdateScrollBehavior(scrollBehavior.updateExtents( contentExtent: contentExtent, containerExtent: containerExtent, scrollOffset: scrollOffset diff --git a/packages/flutter/lib/src/widgets/scrollable_list.dart b/packages/flutter/lib/src/widgets/scrollable_list.dart index 2c8a91505b..3f7aefe919 100644 --- a/packages/flutter/lib/src/widgets/scrollable_list.dart +++ b/packages/flutter/lib/src/widgets/scrollable_list.dart @@ -55,7 +55,7 @@ class _ScrollableListState extends ScrollableState { void _handleExtentsChanged(double contentExtent, double containerExtent) { config.scrollableListPainter?.contentExtent = contentExtent; setState(() { - scrollTo(scrollBehavior.updateExtents( + didUpdateScrollBehavior(scrollBehavior.updateExtents( contentExtent: config.itemsWrap ? double.INFINITY : contentExtent, containerExtent: containerExtent, scrollOffset: scrollOffset @@ -338,7 +338,7 @@ class _ScrollableLazyListState extends ScrollableState { void _handleExtentsChanged(double contentExtent, double containerExtent) { config.scrollableListPainter?.contentExtent = contentExtent; setState(() { - scrollTo(scrollBehavior.updateExtents( + didUpdateScrollBehavior(scrollBehavior.updateExtents( contentExtent: contentExtent, containerExtent: containerExtent, scrollOffset: scrollOffset