Unverified Commit 3e4e2809 authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Reland VelocityTracker update (again) (#138843)

This updates the implementation to use the stopwatch from the Clock object and piping it through to the TestWidgetsFlutterBinding so it will be kept in sync with FakeAsync.

Relands https://github.com/flutter/flutter/pull/137381 which attempted to reland #132291
Fixes https://github.com/flutter/flutter/issues/97761

The original change was reverted due to flakiness it introduced in tests that use fling gestures.
* https://github.com/flutter/flutter/issues/135728

It was reverted again due to a change in the leak tracking tests that broke it.
parent 50c9a377
...@@ -366,7 +366,7 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H ...@@ -366,7 +366,7 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H
if (resamplingEnabled) { if (resamplingEnabled) {
_resampler.addOrDispatch(event); _resampler.addOrDispatch(event);
_resampler.sample(samplingOffset, _samplingClock); _resampler.sample(samplingOffset, samplingClock);
return; return;
} }
...@@ -505,16 +505,10 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H ...@@ -505,16 +505,10 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H
_hitTests.clear(); _hitTests.clear();
} }
/// Overrides the sampling clock for debugging and testing.
///
/// This value is ignored in non-debug builds.
@protected
SamplingClock? get debugSamplingClock => null;
void _handleSampleTimeChanged() { void _handleSampleTimeChanged() {
if (!locked) { if (!locked) {
if (resamplingEnabled) { if (resamplingEnabled) {
_resampler.sample(samplingOffset, _samplingClock); _resampler.sample(samplingOffset, samplingClock);
} }
else { else {
_resampler.stop(); _resampler.stop();
...@@ -522,7 +516,18 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H ...@@ -522,7 +516,18 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H
} }
} }
SamplingClock get _samplingClock { /// Overrides the [samplingClock] for debugging and testing.
///
/// This value is ignored in non-debug builds.
@protected
SamplingClock? get debugSamplingClock => null;
/// Provides access to the current [DateTime] and `StopWatch` objects for
/// sampling.
///
/// Overridden by [debugSamplingClock] for debug builds and testing. Using
/// this object under test will maintain synchronization with [FakeAsync].
SamplingClock get samplingClock {
SamplingClock value = SamplingClock(); SamplingClock value = SamplingClock();
assert(() { assert(() {
final SamplingClock? debugValue = debugSamplingClock; final SamplingClock? debugValue = debugSamplingClock;
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'binding.dart';
import 'events.dart'; import 'events.dart';
import 'lsq_solver.dart'; import 'lsq_solver.dart';
...@@ -149,12 +150,21 @@ class VelocityTracker { ...@@ -149,12 +150,21 @@ class VelocityTracker {
/// The kind of pointer this tracker is for. /// The kind of pointer this tracker is for.
final PointerDeviceKind kind; final PointerDeviceKind kind;
// Time difference since the last sample was added
Stopwatch get _sinceLastSample {
_stopwatch ??= GestureBinding.instance.samplingClock.stopwatch();
return _stopwatch!;
}
Stopwatch? _stopwatch;
// Circular buffer; current sample at _index. // Circular buffer; current sample at _index.
final List<_PointAtTime?> _samples = List<_PointAtTime?>.filled(_historySize, null); final List<_PointAtTime?> _samples = List<_PointAtTime?>.filled(_historySize, null);
int _index = 0; int _index = 0;
/// Adds a position as the given time to the tracker. /// Adds a position as the given time to the tracker.
void addPosition(Duration time, Offset position) { void addPosition(Duration time, Offset position) {
_sinceLastSample.start();
_sinceLastSample.reset();
_index += 1; _index += 1;
if (_index == _historySize) { if (_index == _historySize) {
_index = 0; _index = 0;
...@@ -169,6 +179,16 @@ class VelocityTracker { ...@@ -169,6 +179,16 @@ class VelocityTracker {
/// ///
/// Returns null if there is no data on which to base an estimate. /// Returns null if there is no data on which to base an estimate.
VelocityEstimate? getVelocityEstimate() { VelocityEstimate? getVelocityEstimate() {
// Has user recently moved since last sample?
if (_sinceLastSample.elapsedMilliseconds > VelocityTracker._assumePointerMoveStoppedMilliseconds) {
return const VelocityEstimate(
pixelsPerSecond: Offset.zero,
confidence: 1.0,
duration: Duration.zero,
offset: Offset.zero,
);
}
final List<double> x = <double>[]; final List<double> x = <double>[];
final List<double> y = <double>[]; final List<double> y = <double>[];
final List<double> w = <double>[]; final List<double> w = <double>[];
...@@ -195,7 +215,7 @@ class VelocityTracker { ...@@ -195,7 +215,7 @@ class VelocityTracker {
final double age = (newestSample.time - sample.time).inMicroseconds.toDouble() / 1000; final double age = (newestSample.time - sample.time).inMicroseconds.toDouble() / 1000;
final double delta = (sample.time - previousSample.time).inMicroseconds.abs().toDouble() / 1000; final double delta = (sample.time - previousSample.time).inMicroseconds.abs().toDouble() / 1000;
previousSample = sample; previousSample = sample;
if (age > _horizonMilliseconds || delta > _assumePointerMoveStoppedMilliseconds) { if (age > _horizonMilliseconds || delta > VelocityTracker._assumePointerMoveStoppedMilliseconds) {
break; break;
} }
...@@ -288,6 +308,8 @@ class IOSScrollViewFlingVelocityTracker extends VelocityTracker { ...@@ -288,6 +308,8 @@ class IOSScrollViewFlingVelocityTracker extends VelocityTracker {
@override @override
void addPosition(Duration time, Offset position) { void addPosition(Duration time, Offset position) {
_sinceLastSample.start();
_sinceLastSample.reset();
assert(() { assert(() {
final _PointAtTime? previousPoint = _touchSamples[_index]; final _PointAtTime? previousPoint = _touchSamples[_index];
if (previousPoint == null || previousPoint.time <= time) { if (previousPoint == null || previousPoint.time <= time) {
...@@ -326,6 +348,16 @@ class IOSScrollViewFlingVelocityTracker extends VelocityTracker { ...@@ -326,6 +348,16 @@ class IOSScrollViewFlingVelocityTracker extends VelocityTracker {
@override @override
VelocityEstimate getVelocityEstimate() { VelocityEstimate getVelocityEstimate() {
// Has user recently moved since last sample?
if (_sinceLastSample.elapsedMilliseconds > VelocityTracker._assumePointerMoveStoppedMilliseconds) {
return const VelocityEstimate(
pixelsPerSecond: Offset.zero,
confidence: 1.0,
duration: Duration.zero,
offset: Offset.zero,
);
}
// The velocity estimated using this expression is an approximation of the // The velocity estimated using this expression is an approximation of the
// scroll velocity of an iOS scroll view at the moment the user touch was // scroll velocity of an iOS scroll view at the moment the user touch was
// released, not the final velocity of the iOS pan gesture recognizer // released, not the final velocity of the iOS pan gesture recognizer
...@@ -387,6 +419,16 @@ class MacOSScrollViewFlingVelocityTracker extends IOSScrollViewFlingVelocityTrac ...@@ -387,6 +419,16 @@ class MacOSScrollViewFlingVelocityTracker extends IOSScrollViewFlingVelocityTrac
@override @override
VelocityEstimate getVelocityEstimate() { VelocityEstimate getVelocityEstimate() {
// Has user recently moved since last sample?
if (_sinceLastSample.elapsedMilliseconds > VelocityTracker._assumePointerMoveStoppedMilliseconds) {
return const VelocityEstimate(
pixelsPerSecond: Offset.zero,
confidence: 1.0,
duration: Duration.zero,
offset: Offset.zero,
);
}
// The velocity estimated using this expression is an approximation of the // The velocity estimated using this expression is an approximation of the
// scroll velocity of a macOS scroll view at the moment the user touch was // scroll velocity of a macOS scroll view at the moment the user touch was
// released. // released.
......
...@@ -4,35 +4,17 @@ ...@@ -4,35 +4,17 @@
import 'dart:ui' as ui; import 'dart:ui' as ui;
import 'package:clock/clock.dart';
import 'package:flutter/gestures.dart'; import 'package:flutter/gestures.dart';
import 'package:flutter/scheduler.dart'; import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
class TestResampleEventFlutterBinding extends AutomatedTestWidgetsFlutterBinding {
@override
SamplingClock? get debugSamplingClock => TestSamplingClock(this.clock);
}
class TestSamplingClock implements SamplingClock {
TestSamplingClock(this._clock);
@override
DateTime now() => _clock.now();
@override
Stopwatch stopwatch() => _clock.stopwatch();
final Clock _clock;
}
void main() { void main() {
final TestWidgetsFlutterBinding binding = TestResampleEventFlutterBinding();
testWidgetsWithLeakTracking('PointerEvent resampling on a widget', (WidgetTester tester) async { testWidgetsWithLeakTracking('PointerEvent resampling on a widget', (WidgetTester tester) async {
assert(WidgetsBinding.instance == binding); Duration currentTestFrameTime() => Duration(
Duration currentTestFrameTime() => Duration(milliseconds: binding.clock.now().millisecondsSinceEpoch); milliseconds: TestWidgetsFlutterBinding.instance.clock.now().millisecondsSinceEpoch,
);
void requestFrame() => SchedulerBinding.instance.scheduleFrameCallback((_) {}); void requestFrame() => SchedulerBinding.instance.scheduleFrameCallback((_) {});
final Duration epoch = currentTestFrameTime(); final Duration epoch = currentTestFrameTime();
final ui.PointerDataPacket packet = ui.PointerDataPacket( final ui.PointerDataPacket packet = ui.PointerDataPacket(
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
import 'package:fake_async/fake_async.dart'; import 'package:fake_async/fake_async.dart';
import 'package:flutter/gestures.dart'; import 'package:flutter/gestures.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
class GestureTester { class GestureTester {
...@@ -26,7 +26,7 @@ typedef GestureTest = void Function(GestureTester tester); ...@@ -26,7 +26,7 @@ typedef GestureTest = void Function(GestureTester tester);
@isTest @isTest
void testGesture(String description, GestureTest callback) { void testGesture(String description, GestureTest callback) {
test(description, () { testWidgetsWithLeakTracking(description, (_) async {
FakeAsync().run((FakeAsync async) { FakeAsync().run((FakeAsync async) {
callback(GestureTester._(async)); callback(GestureTester._(async));
}); });
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
import 'package:flutter/gestures.dart'; import 'package:flutter/gestures.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
import 'velocity_tracker_data.dart'; import 'velocity_tracker_data.dart';
bool _withinTolerance(double actual, double expected) { bool _withinTolerance(double actual, double expected) {
...@@ -34,7 +36,7 @@ void main() { ...@@ -34,7 +36,7 @@ void main() {
Offset(-71.51939428321249, 3716.7385187526947), Offset(-71.51939428321249, 3716.7385187526947),
]; ];
test('Velocity tracker gives expected results', () { testWidgetsWithLeakTracking('Velocity tracker gives expected results', (WidgetTester tester) async {
final VelocityTracker tracker = VelocityTracker.withKind(PointerDeviceKind.touch); final VelocityTracker tracker = VelocityTracker.withKind(PointerDeviceKind.touch);
int i = 0; int i = 0;
for (final PointerEvent event in velocityEventData) { for (final PointerEvent event in velocityEventData) {
...@@ -48,7 +50,7 @@ void main() { ...@@ -48,7 +50,7 @@ void main() {
} }
}); });
test('Velocity control test', () { testWidgetsWithLeakTracking('Velocity control test', (WidgetTester tester) async {
const Velocity velocity1 = Velocity(pixelsPerSecond: Offset(7.0, 0.0)); const Velocity velocity1 = Velocity(pixelsPerSecond: Offset(7.0, 0.0));
const Velocity velocity2 = Velocity(pixelsPerSecond: Offset(12.0, 0.0)); const Velocity velocity2 = Velocity(pixelsPerSecond: Offset(12.0, 0.0));
expect(velocity1, equals(const Velocity(pixelsPerSecond: Offset(7.0, 0.0)))); expect(velocity1, equals(const Velocity(pixelsPerSecond: Offset(7.0, 0.0))));
...@@ -60,7 +62,7 @@ void main() { ...@@ -60,7 +62,7 @@ void main() {
expect(velocity1, hasOneLineDescription); expect(velocity1, hasOneLineDescription);
}); });
test('Interrupted velocity estimation', () { testWidgetsWithLeakTracking('Interrupted velocity estimation', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/pull/7510 // Regression test for https://github.com/flutter/flutter/pull/7510
final VelocityTracker tracker = VelocityTracker.withKind(PointerDeviceKind.touch); final VelocityTracker tracker = VelocityTracker.withKind(PointerDeviceKind.touch);
for (final PointerEvent event in interruptedVelocityEventData) { for (final PointerEvent event in interruptedVelocityEventData) {
...@@ -73,12 +75,12 @@ void main() { ...@@ -73,12 +75,12 @@ void main() {
} }
}); });
test('No data velocity estimation', () { testWidgetsWithLeakTracking('No data velocity estimation', (WidgetTester tester) async {
final VelocityTracker tracker = VelocityTracker.withKind(PointerDeviceKind.touch); final VelocityTracker tracker = VelocityTracker.withKind(PointerDeviceKind.touch);
expect(tracker.getVelocity(), Velocity.zero); expect(tracker.getVelocity(), Velocity.zero);
}); });
test('FreeScrollStartVelocityTracker.getVelocity throws when no points', () { testWidgetsWithLeakTracking('FreeScrollStartVelocityTracker.getVelocity throws when no points', (WidgetTester tester) async {
final IOSScrollViewFlingVelocityTracker tracker = IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch); final IOSScrollViewFlingVelocityTracker tracker = IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch);
AssertionError? exception; AssertionError? exception;
try { try {
...@@ -90,7 +92,7 @@ void main() { ...@@ -90,7 +92,7 @@ void main() {
expect(exception?.toString(), contains('at least 1 point')); expect(exception?.toString(), contains('at least 1 point'));
}); });
test('FreeScrollStartVelocityTracker.getVelocity throws when the new point precedes the previous point', () { testWidgetsWithLeakTracking('FreeScrollStartVelocityTracker.getVelocity throws when the new point precedes the previous point', (WidgetTester tester) async {
final IOSScrollViewFlingVelocityTracker tracker = IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch); final IOSScrollViewFlingVelocityTracker tracker = IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch);
AssertionError? exception; AssertionError? exception;
...@@ -105,7 +107,7 @@ void main() { ...@@ -105,7 +107,7 @@ void main() {
expect(exception?.toString(), contains('has a smaller timestamp')); expect(exception?.toString(), contains('has a smaller timestamp'));
}); });
test('Estimate does not throw when there are more than 1 point', () { testWidgetsWithLeakTracking('Estimate does not throw when there are more than 1 point', (WidgetTester tester) async {
final IOSScrollViewFlingVelocityTracker tracker = IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch); final IOSScrollViewFlingVelocityTracker tracker = IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch);
Offset position = Offset.zero; Offset position = Offset.zero;
Duration time = Duration.zero; Duration time = Duration.zero;
...@@ -127,7 +129,7 @@ void main() { ...@@ -127,7 +129,7 @@ void main() {
} }
}); });
test('Makes consistent velocity estimates with consistent velocity', () { testWidgetsWithLeakTracking('Makes consistent velocity estimates with consistent velocity', (WidgetTester tester) async {
final IOSScrollViewFlingVelocityTracker tracker = IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch); final IOSScrollViewFlingVelocityTracker tracker = IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch);
Offset position = Offset.zero; Offset position = Offset.zero;
Duration time = Duration.zero; Duration time = Duration.zero;
...@@ -144,4 +146,55 @@ void main() { ...@@ -144,4 +146,55 @@ void main() {
} }
} }
}); });
testWidgetsWithLeakTracking('Assume zero velocity when there are no recent samples - base VelocityTracker', (WidgetTester tester) async {
final VelocityTracker tracker = VelocityTracker.withKind(PointerDeviceKind.touch);
Offset position = Offset.zero;
Duration time = Duration.zero;
const Offset positionDelta = Offset(0, -1);
const Duration durationDelta = Duration(seconds: 1);
for (int i = 0; i < 10; i+=1) {
position += positionDelta;
time += durationDelta;
tracker.addPosition(time, position);
}
await tester.pumpAndSettle();
expect(tracker.getVelocity().pixelsPerSecond, Offset.zero);
});
testWidgetsWithLeakTracking('Assume zero velocity when there are no recent samples - IOS', (WidgetTester tester) async {
final IOSScrollViewFlingVelocityTracker tracker = IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch);
Offset position = Offset.zero;
Duration time = Duration.zero;
const Offset positionDelta = Offset(0, -1);
const Duration durationDelta = Duration(seconds: 1);
for (int i = 0; i < 10; i+=1) {
position += positionDelta;
time += durationDelta;
tracker.addPosition(time, position);
}
await tester.pumpAndSettle();
expect(tracker.getVelocity().pixelsPerSecond, Offset.zero);
});
testWidgetsWithLeakTracking('Assume zero velocity when there are no recent samples - MacOS', (WidgetTester tester) async {
final MacOSScrollViewFlingVelocityTracker tracker = MacOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch);
Offset position = Offset.zero;
Duration time = Duration.zero;
const Offset positionDelta = Offset(0, -1);
const Duration durationDelta = Duration(seconds: 1);
for (int i = 0; i < 10; i+=1) {
position += positionDelta;
time += durationDelta;
tracker.addPosition(time, position);
}
await tester.pumpAndSettle();
expect(tracker.getVelocity().pixelsPerSecond, Offset.zero);
});
} }
...@@ -8,9 +8,10 @@ import 'package:flutter/rendering.dart'; ...@@ -8,9 +8,10 @@ import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
void main() { void main() {
setUp(() => _GestureBindingSpy()); final TestWidgetsFlutterBinding binding = _GestureBindingSpy();
test('attach and detach correctly handle gesture', () { testWidgets('attach and detach correctly handle gesture', (_) async {
assert(WidgetsBinding.instance == binding);
final TextSelectionDelegate delegate = FakeEditableTextState(); final TextSelectionDelegate delegate = FakeEditableTextState();
final RenderEditable editable = RenderEditable( final RenderEditable editable = RenderEditable(
backgroundCursorColor: Colors.grey, backgroundCursorColor: Colors.grey,
......
...@@ -425,6 +425,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase ...@@ -425,6 +425,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
/// actual current wall-clock time. /// actual current wall-clock time.
Clock get clock; Clock get clock;
@override
SamplingClock? get debugSamplingClock => _TestSamplingClock(clock);
/// Triggers a frame sequence (build/layout/paint/etc), /// Triggers a frame sequence (build/layout/paint/etc),
/// then flushes microtasks. /// then flushes microtasks.
/// ///
...@@ -2149,6 +2152,18 @@ class TestViewConfiguration extends ViewConfiguration { ...@@ -2149,6 +2152,18 @@ class TestViewConfiguration extends ViewConfiguration {
String toString() => 'TestViewConfiguration'; String toString() => 'TestViewConfiguration';
} }
class _TestSamplingClock implements SamplingClock {
_TestSamplingClock(this._clock);
@override
DateTime now() => _clock.now();
@override
Stopwatch stopwatch() => _clock.stopwatch();
final Clock _clock;
}
const int _kPointerDecay = -2; const int _kPointerDecay = -2;
class _LiveTestPointerRecord { class _LiveTestPointerRecord {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment