From 6b6cea65e23676307f7e8418dabf33f903460982 Mon Sep 17 00:00:00 2001 From: Kate Lovett Date: Mon, 24 Jan 2022 17:51:46 -0600 Subject: [PATCH] Revert "PageView scroll physics to match Android (#95423)" (#97150) This reverts commit daef08253b94f5fd1e188e16fc0d9524d4f426d2. --- .../flutter/lib/src/widgets/page_view.dart | 79 +------ .../lib/src/widgets/scroll_physics.dart | 4 +- .../lib/src/widgets/scroll_simulation.dart | 69 ------ .../flutter/test/widgets/page_view_test.dart | 206 ------------------ .../test/widgets/scroll_physics_test.dart | 24 +- .../test/widgets/scroll_simulation_test.dart | 41 ---- 6 files changed, 12 insertions(+), 411 deletions(-) diff --git a/packages/flutter/lib/src/widgets/page_view.dart b/packages/flutter/lib/src/widgets/page_view.dart index 64f97252dbc..c8046079029 100644 --- a/packages/flutter/lib/src/widgets/page_view.dart +++ b/packages/flutter/lib/src/widgets/page_view.dart @@ -21,7 +21,6 @@ import 'scroll_notification.dart'; import 'scroll_physics.dart'; import 'scroll_position.dart'; import 'scroll_position_with_single_context.dart'; -import 'scroll_simulation.dart'; import 'scroll_view.dart'; import 'scrollable.dart'; import 'sliver.dart'; @@ -525,24 +524,10 @@ class _ForceImplicitScrollPhysics extends ScrollPhysics { /// * [ScrollPhysics], the base class which defines the API for scrolling /// physics. /// * [PageView.physics], which can override the physics used by a page view. -/// * [PageScrollSimulation], which implements Android page view scroll physics, and -/// used by this class. class PageScrollPhysics extends ScrollPhysics { /// Creates physics for a [PageView]. const PageScrollPhysics({ ScrollPhysics? parent }) : super(parent: parent); - // See Android ViewPager constants - // https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:viewpager/viewpager/src/main/java/androidx/viewpager/widget/ViewPager.java;l=116;drc=1dcb8847e7aca80ee78c5d9864329b93dd276379 - static const int _kMaxSettleDuration = 600; - static const double _kMinFlingDistance = 25.0; - static const double _kMinFlingVelocity = 400.0; - - @override - double get minFlingDistance => _kMinFlingDistance; - - @override - double get minFlingVelocity => _kMinFlingVelocity; - @override PageScrollPhysics applyTo(ScrollPhysics? ancestor) { return PageScrollPhysics(parent: buildParent(ancestor)); @@ -560,24 +545,13 @@ class PageScrollPhysics extends ScrollPhysics { return page * position.viewportDimension; } - double _getTargetPage(double page, Tolerance tolerance, double velocity) { + double _getTargetPixels(ScrollMetrics position, Tolerance tolerance, double velocity) { + double page = _getPage(position); if (velocity < -tolerance.velocity) page -= 0.5; else if (velocity > tolerance.velocity) page += 0.5; - return page.roundToDouble(); - } - - double _getPageDelta(ScrollMetrics position, Tolerance tolerance, double velocity) { - final double page = _getPage(position); - final double targetPage = _getTargetPage(page, tolerance, velocity); - return targetPage - page; - } - - double _getTargetPixels(ScrollMetrics position, Tolerance tolerance, double velocity) { - final double page = _getPage(position); - final double targetPage = _getTargetPage(page, tolerance, velocity); - return _getPixels(position, targetPage); + return _getPixels(position, page.roundToDouble()); } @override @@ -589,54 +563,11 @@ class PageScrollPhysics extends ScrollPhysics { return super.createBallisticSimulation(position, velocity); final Tolerance tolerance = this.tolerance; final double target = _getTargetPixels(position, tolerance, velocity); - if (target != position.pixels) { - // See Android ViewPager smoothScrollTo logic - // https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:viewpager/viewpager/src/main/java/androidx/viewpager/widget/ViewPager.java;l=952;drc=1dcb8847e7aca80ee78c5d9864329b93dd276379 - final double delta = target - position.pixels; - final double width = position.viewportDimension; - final double halfWidth = width / 2; - final double distanceRatio = math.min(1.0, 1.0 * delta.abs() / width); - final double distance = halfWidth + halfWidth * _distanceInfluenceForSnapDuration(distanceRatio); - int duration; - if (velocity.abs() > 0) { - duration = 4 * (1000 * (distance / velocity).abs()).round(); - } else { - final double pageDelta = _getPageDelta(position, tolerance, velocity).abs(); - // A slightly different algorithm than on Android, because - // Flutter has different velocity estimate, which is more likely to - // return zero pointer velocity when user holds his finger after a fling, - // compared to Android's VelocityTracker. - // - // It was not clear why exactly this happens, since the estimate logic is - // the same as on Android, so it was decided to adjust this formula - // to produce similar results. - // - // On Android it looks like this: - // duration = ((pageDelta + 1) * 100).toInt(); - duration = ((pageDelta * 100 + 1) * 100).toInt(); - } - duration = math.min(duration, _kMaxSettleDuration); - return PageScrollSimulation( - position: position.pixels, - target: target, - duration: duration / 1000, - ); - } + if (target != position.pixels) + return ScrollSpringSimulation(spring, position.pixels, target, velocity, tolerance: tolerance); return null; } - // See Android ViewPager distanceInfluenceForSnapDuration. - // - // We want the duration of the page snap animation to be influenced by the distance that - // the screen has to travel, however, we don't want this duration to be effected in a - // purely linear fashion. Instead, we use this method to moderate the effect that the distance - // of travel has on the overall snap duration. - double _distanceInfluenceForSnapDuration(double value) { - value -= 0.5; // center the values about 0. - value *= 0.3 * math.pi / 2.0; - return math.sin(value); - } - @override bool get allowImplicitScrolling => false; } diff --git a/packages/flutter/lib/src/widgets/scroll_physics.dart b/packages/flutter/lib/src/widgets/scroll_physics.dart index 9027afe7d44..dca3b7a28ca 100644 --- a/packages/flutter/lib/src/widgets/scroll_physics.dart +++ b/packages/flutter/lib/src/widgets/scroll_physics.dart @@ -349,9 +349,7 @@ class ScrollPhysics { ratio: 1.1, ); - /// The spring which [createBallisticSimulation] should use to create - /// a [SpringSimulation] to correct the scroll position when it goes - /// out of range. + /// The spring to use for ballistic simulations. SpringDescription get spring => parent?.spring ?? _kDefaultSpring; /// The default accuracy to which scrolling is computed. diff --git a/packages/flutter/lib/src/widgets/scroll_simulation.dart b/packages/flutter/lib/src/widgets/scroll_simulation.dart index 4c10166f953..3e9a1c2befd 100644 --- a/packages/flutter/lib/src/widgets/scroll_simulation.dart +++ b/packages/flutter/lib/src/widgets/scroll_simulation.dart @@ -12,7 +12,6 @@ import 'package:flutter/physics.dart'; /// See also: /// /// * [ClampingScrollSimulation], which implements Android scroll physics. -/// * [PageScrollSimulation], which implements Android page view scroll physics. class BouncingScrollSimulation extends Simulation { /// Creates a simulation group for scrolling on iOS, with the given /// parameters. @@ -135,7 +134,6 @@ class BouncingScrollSimulation extends Simulation { /// See also: /// /// * [BouncingScrollSimulation], which implements iOS scroll physics. -/// * [PageScrollSimulation], which implements Android page view scroll physics. // // This class is based on Scroller.java from Android: // https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/widget @@ -231,70 +229,3 @@ class ClampingScrollSimulation extends Simulation { return time >= _duration; } } - -/// An implementation of scroll physics that matches Android page view. -/// -/// See also: -/// -/// * [BouncingScrollSimulation], which implements iOS scroll physics. -/// * [ClampingScrollSimulation], which implements Android scroll physics. -// -// This class ports Android logic from Scroller.java `startScroll`, applying -// the interpolator set in ViewPager.java. -// -// See Android Scroller.java `startScroll` source code -// https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/widget/Scroller.java;l=393;drc=ae5bcf23b5f0875e455790d6af387184dbd009c1 -class PageScrollSimulation extends Simulation { - /// Creates a scroll physics simulation that matches Android page view. - PageScrollSimulation({ - required this.position, - required this.target, - required this.duration, - }) : assert(position != target), - _delta = target - position, - _durationReciprocal = 1.0 / duration; - - /// The position at the beginning of the simulation. - final double position; - - /// The target, which will be reached at the end of the simulation. - final double target; - - /// The duration of the simulation in seconds. - final double duration; - - final double _delta; - final double _durationReciprocal; - - /// See Android ViewPager interpolator - /// https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:viewpager/viewpager/src/main/java/androidx/viewpager/widget/ViewPager.java;l=152;drc=1dcb8847e7aca80ee78c5d9864329b93dd276379 - double _interpolate(double t) { - // y = (t - 1)^5 + 1.0 - t -= 1.0; - return t * t * t * t * t + 1.0; - } - - /// A derivative from [_interpolate] function. - double _interpolateDx(double t) { - // y = 5 * (t - 1)^4 - t -= 1.0; - return 5.0 * t * t * t * t; - } - - @override - double x(double time) { - time = time.clamp(0.0, duration); - return position + _delta * _interpolate(time * _durationReciprocal); - } - - @override - double dx(double time) { - time = time.clamp(0.0, duration); - return _delta * _interpolateDx(time * _durationReciprocal); - } - - @override - bool isDone(double time) { - return time >= duration; - } -} diff --git a/packages/flutter/test/widgets/page_view_test.dart b/packages/flutter/test/widgets/page_view_test.dart index 66d35d336ae..0f353f592b2 100644 --- a/packages/flutter/test/widgets/page_view_test.dart +++ b/packages/flutter/test/widgets/page_view_test.dart @@ -1123,210 +1123,4 @@ void main() { await tester.pump(); expect(tester.takeException(), isNull); }); - - group('PageView uses PageScrollPhysics -', () { - const double expectedMinFlingVelocity = 400.0; - const int maxExpectedFrames = 41; // expected frames with _kMaxSettleDuration - - /// Creates a page view and drags/flings it with specified parameters. - /// - /// When `flingVelocity` is not null, uses [WidgetTester.fling] - /// otherwise uses [WidgetTester.drag]. - /// - /// Returns the length of animation in frames. - /// - /// At the end, runs expects: - /// - /// * `expectedEndValue` will be used to verify the end scroll position - /// * optional `expectedFrames` will be used to verify the length of animation - /// - Future testPageViewPhysics( - WidgetTester tester, { - double? flingVelocity, - required double offset, - required double expectedEndValue, - Object? expectedFrames, - }) async { - await tester.pumpWidget( - Directionality( - key: UniqueKey(), - textDirection: TextDirection.ltr, - child: PageView.builder( - itemCount: kStates.length, - itemBuilder: (BuildContext context, int index) { - return Container( - height: 200.0, - color: index.isEven - ? const Color(0xFF0000FF) - : const Color(0xFF00FF00), - child: Text(kStates[index]), - ); - }, - ), - ) - ); - - if (flingVelocity != null) - await tester.fling(find.byType(PageView), Offset(-offset, 0.0), flingVelocity); - else - await tester.drag(find.byType(PageView), Offset(-offset, 0.0)); - - final ScrollPosition position = tester.state(find.byType(Scrollable)).position; - - final List values = []; - for (int i = 0; i < 100; i++) { - values.add(position.pixels); - if (values[i] == expectedEndValue) { - await tester.pump(const Duration(milliseconds: 16)); - values.add(position.pixels); - break; - } - await tester.pump(const Duration(milliseconds: 16)); - } - // verify the simulation ended - expect(values[values.length - 2], values.last); - - if (expectedFrames != null) - expect(values, hasLength(expectedFrames)); - expect(position.pixels, expectedEndValue); - return values.length; - } - - testWidgets('drag and release', (WidgetTester tester) async { - await testPageViewPhysics( - tester, - offset: 1.0, - expectedFrames: 10, - expectedEndValue: 0.0, - ); - await testPageViewPhysics( - tester, - offset: 20.0, - expectedFrames: 25, - expectedEndValue: 0.0, - ); - await testPageViewPhysics( - tester, - offset: 40.0, - expectedFrames: maxExpectedFrames, - expectedEndValue: 0.0, - ); - await testPageViewPhysics( - tester, - offset: 399.0, - expectedFrames: maxExpectedFrames, - expectedEndValue: 0.0, - ); - await testPageViewPhysics( - tester, - offset: 400.0, - expectedFrames: maxExpectedFrames, - expectedEndValue: 800.0, - ); - await testPageViewPhysics( - tester, - offset: 799.0, - expectedFrames: 10, - expectedEndValue: 800.0, - ); - }); - - testWidgets('min fling velocity', (WidgetTester tester) async { - // fling with velocity less than min fling velocity - goes back to 0 - await testPageViewPhysics( - tester, - flingVelocity: 1.0, - offset: 200.0, - expectedEndValue: 0.0, - ); - await testPageViewPhysics( - tester, - flingVelocity: expectedMinFlingVelocity, - offset: 200.0, - expectedEndValue: 0.0, - ); - - // fling with velocity greater than min fling velocity - goes to the next page - await testPageViewPhysics( - tester, - flingVelocity: expectedMinFlingVelocity + 1, - offset: 200.0, - expectedEndValue: 800.0, - ); - }); - - testWidgets('fling has max settle duration, which decreases when user drags faster', (WidgetTester tester) async { - // The duration depends on the fling `offset` and `velocity` - - // For offset 200.0 - - // Fling very slowly - int frames = await testPageViewPhysics( - tester, - flingVelocity: 500.0, - offset: 200.0, - expectedFrames: maxExpectedFrames, - expectedEndValue: 800.0, - ); - // Fling about as fast to start going faster - frames = await testPageViewPhysics( - tester, - flingVelocity: 3000.0, - offset: 200.0, - expectedFrames: frames, - expectedEndValue: 800.0, - ); - // Go faster - frames = await testPageViewPhysics( - tester, - flingVelocity: 3050.0, - offset: 200.0, - expectedFrames: lessThan(frames), - expectedEndValue: 800.0, - ); - // Go even faster - frames = await testPageViewPhysics( - tester, - flingVelocity: 10000.0, - offset: 200.0, - expectedFrames: lessThan(frames), - expectedEndValue: 800.0, - ); - - // For offset 400.0 - - // Fling very slowly - frames = await testPageViewPhysics( - tester, - flingVelocity: 500.0, - offset: 400.0, - expectedFrames: maxExpectedFrames, - expectedEndValue: 800.0, - ); - // Fling about as fast to start going faster - frames = await testPageViewPhysics( - tester, - flingVelocity: 2650.0, - offset: 400.0, - expectedFrames: frames, - expectedEndValue: 800.0, - ); - // Go faster - frames = await testPageViewPhysics( - tester, - flingVelocity: 2700.0, - offset: 400.0, - expectedFrames: lessThan(frames), - expectedEndValue: 800.0, - ); - // Go even faster - frames = await testPageViewPhysics( - tester, - flingVelocity: 10000.0, - offset: 400.0, - expectedFrames: lessThan(frames), - expectedEndValue: 800.0, - ); - }); - }); } diff --git a/packages/flutter/test/widgets/scroll_physics_test.dart b/packages/flutter/test/widgets/scroll_physics_test.dart index 7d93cde8c0f..2178f51bb9e 100644 --- a/packages/flutter/test/widgets/scroll_physics_test.dart +++ b/packages/flutter/test/widgets/scroll_physics_test.dart @@ -93,7 +93,7 @@ void main() { ); }); - test('ScrollPhysics scrolling subclasses - initial velocity when creating the simulation', () { + test("ScrollPhysics scrolling subclasses - Creating the simulation doesn't alter the velocity for time 0", () { final ScrollMetrics position = FixedScrollMetrics( minScrollExtent: 0.0, maxScrollExtent: 100.0, @@ -104,25 +104,13 @@ void main() { const BouncingScrollPhysics bounce = BouncingScrollPhysics(); const ClampingScrollPhysics clamp = ClampingScrollPhysics(); + const PageScrollPhysics page = PageScrollPhysics(); - // Verify creating these simulations doesn't alter the velocity for time 0. - // // Calls to createBallisticSimulation may happen on every frame (i.e. when the maxScrollExtent changes) - // Changing velocity for time 0 may cause a sudden, unwanted damping/speedup effect. - expect(bounce.createBallisticSimulation(position, 1000)!.dx(0), 1000); - expect(clamp.createBallisticSimulation(position, 1000)!.dx(0), 1000); - - // Contrary to the other two scroll physics, PageScrollPhysics do intentionally produce - // a bigger velocity than supplied to them, but they won't suffer from this, because the simulation - // velocity depends on a ratio of velocity / pageDelta, and because they are not affected by - // https://github.com/flutter/flutter/issues/11599 - - // TODO(nt4f04uNd): remove this test, because it is artificial, in favor of a real one. - // Scroll simulations surely should be allowed to modify the speed they receive in whatever way they want. - // The proper fix for the original bug https://github.com/flutter/flutter/issues/24715 - // would be to ensure this is not called too often https://github.com/flutter/flutter/issues/11599 - // Proper test(s) should ensure the animations of the physics have the same duration - // for same fling conditions within different layouts. + // Changing velocity for time 0 may cause a sudden, unwanted damping/speedup effect + expect(bounce.createBallisticSimulation(position, 1000)!.dx(0), moreOrLessEquals(1000)); + expect(clamp.createBallisticSimulation(position, 1000)!.dx(0), moreOrLessEquals(1000)); + expect(page.createBallisticSimulation(position, 1000)!.dx(0), moreOrLessEquals(1000)); }); group('BouncingScrollPhysics test', () { diff --git a/packages/flutter/test/widgets/scroll_simulation_test.dart b/packages/flutter/test/widgets/scroll_simulation_test.dart index 7672306642f..69958f91e31 100644 --- a/packages/flutter/test/widgets/scroll_simulation_test.dart +++ b/packages/flutter/test/widgets/scroll_simulation_test.dart @@ -23,45 +23,4 @@ void main() { checkInitialConditions(75.0, 614.2093); checkInitialConditions(5469.0, 182.114534); }); - - test('PageScrollSimulation', () { - void checkSimulation(double position, double target, double duration) { - final double delta = target - position; - final PageScrollSimulation simulation = PageScrollSimulation(position: position, target: target, duration: duration); - late double lastX; - late double lastDx; - void expectX(double t, dynamic expectedValue) { - final double x = simulation.x(t); - expect(x, expectedValue); - lastX = x; - } - void expectDx(double t, dynamic expectedValue) { - final double dx = simulation.dx(t); - expect(dx, expectedValue); - lastDx = dx; - } - - // verify start values - expectX(0.0, position); - expectDx(0.0, moreOrLessEquals(5 * delta)); - expect(simulation.isDone(0.0), false); - - // verify intermediate values change monotonically - for (double t = 0.01; t < duration; t += 1) { - expectX(t, target > position ? greaterThan(lastX) : lessThan(lastX)); - expectDx(t, target > position ? lessThan(lastDx) : greaterThan(lastDx)); - expect(simulation.isDone(t), false); - } - - // verify end values - expectX(duration, target); - expectDx(duration, 0.0); - expect(simulation.isDone(duration), true); - } - - checkSimulation(0, 500, 1000); - checkSimulation(0, -500, 1000); - checkSimulation(1000, 5000, 100); - checkSimulation(100, -5000, 100); - }); }