Unverified Commit 8ba459ce authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Update VelocityTracker (4) (#139166)

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

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

1. The original change was reverted due to flakiness it introduced in tests that use fling gestures.
  * Using a mocked clock through the test binding fixes this now
2. It was reverted a second time because a change at tip of tree broke it, exposing memory leaks, but it was not rebased before landing. 
  * These leaks are now fixed
3. It was reverted a third time, because we were so excellently quick to revert those other times, that we did not notice the broken benchmark that only runs in postsubmit.
  * The benchmark is now fixed
parent 133711ba
......@@ -3,6 +3,7 @@
// found in the LICENSE file.
import 'package:flutter/gestures.dart';
import 'package:flutter_test/flutter_test.dart';
import '../common.dart';
import 'data/velocity_tracker_data.dart';
......@@ -16,38 +17,43 @@ class TrackerBenchmark {
final String name;
}
void main() {
Future<void> main() async {
assert(false, "Don't run benchmarks in debug mode! Use 'flutter run --release'.");
final BenchmarkResultPrinter printer = BenchmarkResultPrinter();
final List<TrackerBenchmark> benchmarks = <TrackerBenchmark>[
TrackerBenchmark(name: 'velocity_tracker_iteration', tracker: VelocityTracker.withKind(PointerDeviceKind.touch)),
TrackerBenchmark(name: 'velocity_tracker_iteration_ios_fling', tracker: IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch)),
TrackerBenchmark(name: 'velocity_tracker_iteration',
tracker: VelocityTracker.withKind(PointerDeviceKind.touch)),
TrackerBenchmark(name: 'velocity_tracker_iteration_ios_fling',
tracker: IOSScrollViewFlingVelocityTracker(PointerDeviceKind.touch)),
];
final Stopwatch watch = Stopwatch();
for (final TrackerBenchmark benchmark in benchmarks) {
print('${benchmark.name} benchmark...');
final VelocityTracker tracker = benchmark.tracker;
watch.reset();
watch.start();
for (int i = 0; i < _kNumIters; i += 1) {
for (final PointerEvent event in velocityEventData) {
if (event is PointerDownEvent || event is PointerMoveEvent) {
tracker.addPosition(event.timeStamp, event.position);
}
if (event is PointerUpEvent) {
tracker.getVelocity();
await benchmarkWidgets((WidgetTester tester) async {
for (final TrackerBenchmark benchmark in benchmarks) {
print('${benchmark.name} benchmark...');
final VelocityTracker tracker = benchmark.tracker;
watch.reset();
watch.start();
for (int i = 0; i < _kNumIters; i += 1) {
for (final PointerEvent event in velocityEventData) {
if (event is PointerDownEvent || event is PointerMoveEvent) {
tracker.addPosition(event.timeStamp, event.position);
}
if (event is PointerUpEvent) {
tracker.getVelocity();
}
}
}
watch.stop();
printer.addResult(
description: 'Velocity tracker: ${tracker.runtimeType}',
value: watch.elapsedMicroseconds / _kNumIters,
unit: 'µs per iteration',
name: benchmark.name,
);
}
watch.stop();
printer.addResult(
description: 'Velocity tracker: ${tracker.runtimeType}',
value: watch.elapsedMicroseconds / _kNumIters,
unit: 'µs per iteration',
name: benchmark.name,
);
}
});
printer.printToStdout();
}
......@@ -373,7 +373,7 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H
if (resamplingEnabled) {
_resampler.addOrDispatch(event);
_resampler.sample(samplingOffset, _samplingClock);
_resampler.sample(samplingOffset, samplingClock);
return;
}
......@@ -512,16 +512,10 @@ 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();
......@@ -529,7 +523,18 @@ mixin GestureBinding on BindingBase implements HitTestable, HitTestDispatcher, H
}
}
SamplingClock get _samplingClock {
/// Overrides the sampling clock 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;
......
......@@ -5,6 +5,7 @@
import 'package:flutter/foundation.dart';
import 'binding.dart';
import 'events.dart';
import 'lsq_solver.dart';
......@@ -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;
......@@ -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 > _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>[];
......@@ -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) {
......@@ -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
......@@ -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.
......
......@@ -33,6 +33,7 @@ void main() {
setUp(() {
tap = DoubleTapGestureRecognizer();
addTearDown(tap.dispose);
doubleTapRecognized = false;
tap.onDoubleTap = () {
......@@ -156,6 +157,7 @@ void main() {
final DoubleTapGestureRecognizer tapSecondary = DoubleTapGestureRecognizer(
allowedButtonsFilter: (int buttons) => buttons == kSecondaryButton,
);
addTearDown(tapSecondary.dispose);
tapSecondary.onDoubleTap = () {
doubleTapRecognized = true;
};
......@@ -545,6 +547,7 @@ void main() {
final DoubleTapGestureRecognizer tapPrimary = DoubleTapGestureRecognizer(
allowedButtonsFilter: (int buttons) => buttons == kPrimaryButton,
);
addTearDown(tapPrimary.dispose);
tapPrimary.onDoubleTap = () {
doubleTapRecognized = true;
};
......@@ -647,14 +650,17 @@ void main() {
..onTapDown = (TapDownDetails details) {
recognized.add('tapPrimary');
};
addTearDown(tapPrimary.dispose);
tapSecondary = TapGestureRecognizer()
..onSecondaryTapDown = (TapDownDetails details) {
recognized.add('tapSecondary');
};
addTearDown(tapSecondary.dispose);
doubleTap = DoubleTapGestureRecognizer()
..onDoubleTap = () {
recognized.add('doubleTap');
};
addTearDown(doubleTap.dispose);
});
tearDown(() {
......@@ -692,6 +698,7 @@ void main() {
..onDoubleTap = () {
recognized.add('primary');
};
addTearDown(doubleTap.dispose);
// Down/up pair 7: normal tap sequence close to pair 6
const PointerDownEvent down7 = PointerDownEvent(
......@@ -730,6 +737,7 @@ void main() {
..onDoubleTap = () {
recognized.add('primary');
};
addTearDown(doubleTap.dispose);
// Down/up pair 7: normal tap sequence close to pair 6
const PointerDownEvent down7 = PointerDownEvent(
......@@ -765,8 +773,10 @@ void main() {
int tapCount = 0;
final DoubleTapGestureRecognizer doubleTap = DoubleTapGestureRecognizer()
..onDoubleTap = () {};
addTearDown(doubleTap.dispose);
final TapGestureRecognizer tap = TapGestureRecognizer()
..onTap = () => tapCount++;
addTearDown(tap.dispose);
// Open a arena with 2 members and holding.
doubleTap.addPointer(down1);
......
......@@ -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(
......
......@@ -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 {
......@@ -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));
});
......
......@@ -4,6 +4,7 @@
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) {
......@@ -34,7 +35,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) {
......@@ -48,7 +49,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))));
......@@ -60,7 +61,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) {
......@@ -73,12 +74,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 {
......@@ -90,7 +91,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;
......@@ -105,7 +106,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;
......@@ -127,7 +128,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;
......@@ -144,4 +145,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';
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 {
expect(WidgetsBinding.instance, binding);
final TextSelectionDelegate delegate = FakeEditableTextState();
final RenderEditable editable = RenderEditable(
backgroundCursorColor: Colors.grey,
......
......@@ -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.
///
......@@ -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 {
......
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