Skip to content

Commit

Permalink
Reland VelocityTracker update (again) (flutter#138843)
Browse files Browse the repository at this point in the history 
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 flutter#137381 which attempted to reland flutter#132291
Fixes flutter#97761

The original change was reverted due to flakiness it introduced in tests that use fling gestures.
* flutter#135728

It was reverted again due to a change in the leak tracking tests that broke it.
  • Loading branch information
@Piinks @caseycrogers
Piinks authored and caseycrogers committed Dec 29, 2023
1 parent 3192365 commit 43c9990
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 43 deletions.
23 changes: 14 additions & 9 deletions packages/flutter/lib/src/gestures/binding.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H

if (resamplingEnabled) {
_resampler.addOrDispatch(event);
_resampler.sample(samplingOffset, _samplingClock);
_resampler.sample(samplingOffset, samplingClock);
return;
}

Expand Down Expand Up @@ -505,24 +505,29 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H
_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() {
if (!locked) {
if (resamplingEnabled) {
_resampler.sample(samplingOffset, _samplingClock);
_resampler.sample(samplingOffset, samplingClock);
}
else {
_resampler.stop();
}
}
}

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();
assert(() {
final SamplingClock? debugValue = debugSamplingClock;
Expand Down
44 changes: 43 additions & 1 deletion packages/flutter/lib/src/gestures/velocity_tracker.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import 'package:flutter/foundation.dart';

import 'binding.dart';
import 'events.dart';
import 'lsq_solver.dart';

Expand Down Expand Up @@ -149,12 +150,21 @@ class VelocityTracker {
/// The kind of pointer this tracker is for.
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.
final List<_PointAtTime?> _samples = List<_PointAtTime?>.filled(_historySize, null);
int _index = 0;

/// Adds a position as the given time to the tracker.
void addPosition(Duration time, Offset position) {
_sinceLastSample.start();
_sinceLastSample.reset();
_index += 1;
if (_index == _historySize) {
_index = 0;
Expand All @@ -169,6 +179,16 @@ class VelocityTracker {
///
/// Returns null if there is no data on which to base an estimate.
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> y = <double>[];
final List<double> w = <double>[];
Expand All @@ -195,7 +215,7 @@ class VelocityTracker {
final double age = (newestSample.time - sample.time).inMicroseconds.toDouble() / 1000;
final double delta = (sample.time - previousSample.time).inMicroseconds.abs().toDouble() / 1000;
previousSample = sample;
if (age > _horizonMilliseconds || delta > _assumePointerMoveStoppedMilliseconds) {
if (age > _horizonMilliseconds || delta > VelocityTracker._assumePointerMoveStoppedMilliseconds) {
break;
}

Expand Down Expand Up @@ -288,6 +308,8 @@ class IOSScrollViewFlingVelocityTracker extends VelocityTracker {

@override
void addPosition(Duration time, Offset position) {
_sinceLastSample.start();
_sinceLastSample.reset();
assert(() {
final _PointAtTime? previousPoint = _touchSamples[_index];
if (previousPoint == null || previousPoint.time <= time) {
Expand Down Expand Up @@ -326,6 +348,16 @@ class IOSScrollViewFlingVelocityTracker extends VelocityTracker {

@override
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
// 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
Expand Down Expand Up @@ -387,6 +419,16 @@ class MacOSScrollViewFlingVelocityTracker extends IOSScrollViewFlingVelocityTrac

@override
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
// scroll velocity of a macOS scroll view at the moment the user touch was
// released.
Expand Down
24 changes: 3 additions & 21 deletions packages/flutter/test/gestures/gesture_binding_resample_event_on_widget_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,17 @@

import 'dart:ui' as ui;

import 'package:clock/clock.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.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() {
final TestWidgetsFlutterBinding binding = TestResampleEventFlutterBinding();
testWidgetsWithLeakTracking('PointerEvent resampling on a widget', (WidgetTester tester) async {
assert(WidgetsBinding.instance == binding);
Duration currentTestFrameTime() => Duration(milliseconds: binding.clock.now().millisecondsSinceEpoch);
Duration currentTestFrameTime() => Duration(
milliseconds: TestWidgetsFlutterBinding.instance.clock.now().millisecondsSinceEpoch,
);
void requestFrame() => SchedulerBinding.instance.scheduleFrameCallback((_) {});
final Duration epoch = currentTestFrameTime();
final ui.PointerDataPacket packet = ui.PointerDataPacket(
Expand Down
4 changes: 2 additions & 2 deletions packages/flutter/test/gestures/gesture_tester.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import 'package:fake_async/fake_async.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';

class GestureTester {
Expand All @@ -26,7 +26,7 @@ typedef GestureTest = void Function(GestureTester tester);

@isTest
void testGesture(String description, GestureTest callback) {
test(description, () {
testWidgetsWithLeakTracking(description, (_) async {
FakeAsync().run((FakeAsync async) {
callback(GestureTester._(async));
});
Expand Down
69 changes: 61 additions & 8 deletions packages/flutter/test/gestures/velocity_tracker_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

import 'package:flutter/gestures.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';

import 'velocity_tracker_data.dart';

bool _withinTolerance(double actual, double expected) {
Expand Down Expand Up @@ -34,7 +36,7 @@ void main() {
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);
int i = 0;
for (final PointerEvent event in velocityEventData) {
Expand All @@ -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 velocity2 = Velocity(pixelsPerSecond: Offset(12.0, 0.0));
expect(velocity1, equals(const Velocity(pixelsPerSecond: Offset(7.0, 0.0))));
Expand All @@ -60,7 +62,7 @@ void main() {
expect(velocity1, hasOneLineDescription);
});

test('Interrupted velocity estimation', () {
testWidgetsWithLeakTracking('Interrupted velocity estimation', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/pull/7510
final VelocityTracker tracker = VelocityTracker.withKind(PointerDeviceKind.touch);
for (final PointerEvent event in interruptedVelocityEventData) {
Expand All @@ -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);
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);
AssertionError? exception;
try {
Expand All @@ -90,7 +92,7 @@ void main() {
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);
AssertionError? exception;

Expand All @@ -105,7 +107,7 @@ void main() {
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);
Offset position = Offset.zero;
Duration time = Duration.zero;
Expand All @@ -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);
Offset position = Offset.zero;
Duration time = Duration.zero;
Expand All @@ -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);
});
}
5 changes: 3 additions & 2 deletions packages/flutter/test/rendering/editable_gesture_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart';

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 RenderEditable editable = RenderEditable(
backgroundCursorColor: Colors.grey,
Expand Down
15 changes: 15 additions & 0 deletions packages/flutter_test/lib/src/binding.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
/// actual current wall-clock time.
Clock get clock;

@override
SamplingClock? get debugSamplingClock => _TestSamplingClock(clock);

/// Triggers a frame sequence (build/layout/paint/etc),
/// then flushes microtasks.
///
Expand Down Expand Up @@ -2149,6 +2152,18 @@ class TestViewConfiguration extends ViewConfiguration {
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;

class _LiveTestPointerRecord {
Expand Down

0 comments on commit 43c9990

Please sign in to comment.