Fix onExit calling when the mouse is removed. (#33477)
This PR solves two problems: currently, the onExit is called for a mouse pointer the moment the removal message is received, except that by the time it actually calls it, there is no _lastEvent for it in the mouse tracker (it's already been removed), resulting in an event being passed to the onExit that contains nulls for the position. Also, removePointer events don't actually get created with a position, although they easily could be, so that even the the _lastEvent in the mouse tracker were still populated, it would still give a null position and delta.
This PR adds support for the position and delta in a PointerRemovedEvent, and populates them. In addition, when a remove event is received, it doesn't actually remove the pointer until the mouse position check that gets scheduled actually happens.
diff --git a/packages/flutter/lib/src/gestures/converter.dart b/packages/flutter/lib/src/gestures/converter.dart
index b1f6173..324c33e 100644
--- a/packages/flutter/lib/src/gestures/converter.dart
+++ b/packages/flutter/lib/src/gestures/converter.dart
@@ -386,8 +386,8 @@
kind: kind,
device: datum.device,
position: position,
- buttons: datum.buttons,
delta: state.deltaTo(position),
+ buttons: datum.buttons,
obscured: datum.obscured,
pressureMin: datum.pressureMin,
pressureMax: datum.pressureMax,
@@ -408,6 +408,7 @@
timeStamp: timeStamp,
kind: kind,
device: datum.device,
+ position: position,
obscured: datum.obscured,
pressureMin: datum.pressureMin,
pressureMax: datum.pressureMax,
diff --git a/packages/flutter/lib/src/gestures/events.dart b/packages/flutter/lib/src/gestures/events.dart
index d3f8f81..7b8ec05 100644
--- a/packages/flutter/lib/src/gestures/events.dart
+++ b/packages/flutter/lib/src/gestures/events.dart
@@ -240,7 +240,7 @@
final Offset position;
/// Distance in logical pixels that the pointer moved since the last
- /// [PointerMoveEvent].
+ /// [PointerMoveEvent] or [PointerHoverEvent].
///
/// This value is always 0.0 for down, up, and cancel events.
final Offset delta;
@@ -477,6 +477,7 @@
Duration timeStamp = Duration.zero,
PointerDeviceKind kind = PointerDeviceKind.touch,
int device = 0,
+ Offset position = Offset.zero,
bool obscured = false,
double pressureMin = 1.0,
double pressureMax = 1.0,
@@ -487,7 +488,7 @@
timeStamp: timeStamp,
kind: kind,
device: device,
- position: null,
+ position: position,
obscured: obscured,
pressure: 0.0,
pressureMin: pressureMin,
diff --git a/packages/flutter/lib/src/gestures/mouse_tracking.dart b/packages/flutter/lib/src/gestures/mouse_tracking.dart
index 77b35e5..cc97a12 100644
--- a/packages/flutter/lib/src/gestures/mouse_tracking.dart
+++ b/packages/flutter/lib/src/gestures/mouse_tracking.dart
@@ -125,7 +125,9 @@
final _TrackedAnnotation trackedAnnotation = _findAnnotation(annotation);
for (int deviceId in trackedAnnotation.activeDevices) {
if (annotation.onExit != null) {
- annotation.onExit(PointerExitEvent.fromMouseEvent(_lastMouseEvent[deviceId]));
+ final PointerEvent event = _lastMouseEvent[deviceId] ?? _pendingRemovals[deviceId];
+ assert(event != null);
+ annotation.onExit(PointerExitEvent.fromMouseEvent(event));
}
}
_trackedAnnotations.remove(annotation);
@@ -153,11 +155,13 @@
}
final int deviceId = event.device;
if (event is PointerAddedEvent) {
+ // If we are adding the device again, then we're not removing it anymore.
+ _pendingRemovals.remove(deviceId);
_addMouseEvent(deviceId, event);
return;
}
if (event is PointerRemovedEvent) {
- _removeMouseEvent(deviceId);
+ _removeMouseEvent(deviceId, event);
// If the mouse was removed, then we need to schedule one more check to
// exit any annotations that were active.
_scheduleMousePositionCheck();
@@ -205,7 +209,9 @@
void collectMousePositions() {
void exitAnnotation(_TrackedAnnotation trackedAnnotation, int deviceId) {
if (trackedAnnotation.annotation?.onExit != null && trackedAnnotation.activeDevices.contains(deviceId)) {
- trackedAnnotation.annotation.onExit(PointerExitEvent.fromMouseEvent(_lastMouseEvent[deviceId]));
+ final PointerEvent event = _lastMouseEvent[deviceId] ?? _pendingRemovals[deviceId];
+ assert(event != null);
+ trackedAnnotation.annotation.onExit(PointerExitEvent.fromMouseEvent(event));
trackedAnnotation.activeDevices.remove(deviceId);
}
}
@@ -219,75 +225,89 @@
}
}
- // This indicates that all mouse pointers were removed, or none have been
- // connected yet. If no mouse is connected, then we want to make sure that
- // all active annotations are exited.
- if (!mouseIsConnected) {
- _trackedAnnotations.values.forEach(exitAllDevices);
- return;
- }
-
- for (int deviceId in _lastMouseEvent.keys) {
- final PointerEvent lastEvent = _lastMouseEvent[deviceId];
- final Iterable<MouseTrackerAnnotation> hits = annotationFinder(lastEvent.position);
-
- // No annotations were found at this position for this deviceId, so send an
- // exit to all active tracked annotations, since none of them were hit.
- if (hits.isEmpty) {
- // Send an exit to all tracked animations tracking this deviceId.
- for (_TrackedAnnotation trackedAnnotation in _trackedAnnotations.values) {
- exitAnnotation(trackedAnnotation, deviceId);
- }
- continue;
+ try {
+ // This indicates that all mouse pointers were removed, or none have been
+ // connected yet. If no mouse is connected, then we want to make sure that
+ // all active annotations are exited.
+ if (!mouseIsConnected) {
+ _trackedAnnotations.values.forEach(exitAllDevices);
+ return;
}
- final Set<_TrackedAnnotation> hitAnnotations = hits.map<_TrackedAnnotation>((MouseTrackerAnnotation hit) => _findAnnotation(hit)).toSet();
- for (_TrackedAnnotation hitAnnotation in hitAnnotations) {
- if (!hitAnnotation.activeDevices.contains(deviceId)) {
- // A tracked annotation that just became active and needs to have an enter
- // event sent to it.
- hitAnnotation.activeDevices.add(deviceId);
- if (hitAnnotation.annotation?.onEnter != null) {
- hitAnnotation.annotation.onEnter(PointerEnterEvent.fromMouseEvent(lastEvent));
+ for (int deviceId in _lastMouseEvent.keys) {
+ final PointerEvent lastEvent = _lastMouseEvent[deviceId];
+ final Iterable<MouseTrackerAnnotation> hits = annotationFinder(lastEvent.position);
+
+ // No annotations were found at this position for this deviceId, so send an
+ // exit to all active tracked annotations, since none of them were hit.
+ if (hits.isEmpty) {
+ // Send an exit to all tracked animations tracking this deviceId.
+ for (_TrackedAnnotation trackedAnnotation in _trackedAnnotations.values) {
+ exitAnnotation(trackedAnnotation, deviceId);
}
- }
- if (hitAnnotation.annotation?.onHover != null && lastEvent is PointerHoverEvent) {
- hitAnnotation.annotation.onHover(lastEvent);
+ continue;
}
- // Tell any tracked annotations that weren't hit that they are no longer
- // active.
- for (_TrackedAnnotation trackedAnnotation in _trackedAnnotations.values) {
- if (hitAnnotations.contains(trackedAnnotation)) {
- continue;
- }
- if (trackedAnnotation.activeDevices.contains(deviceId)) {
- if (trackedAnnotation.annotation?.onExit != null) {
- trackedAnnotation.annotation.onExit(PointerExitEvent.fromMouseEvent(lastEvent));
+ final Set<_TrackedAnnotation> hitAnnotations = hits.map<_TrackedAnnotation>((MouseTrackerAnnotation hit) => _findAnnotation(hit)).toSet();
+ for (_TrackedAnnotation hitAnnotation in hitAnnotations) {
+ if (!hitAnnotation.activeDevices.contains(deviceId)) {
+ // A tracked annotation that just became active and needs to have an enter
+ // event sent to it.
+ hitAnnotation.activeDevices.add(deviceId);
+ if (hitAnnotation.annotation?.onEnter != null) {
+ hitAnnotation.annotation.onEnter(PointerEnterEvent.fromMouseEvent(lastEvent));
}
- trackedAnnotation.activeDevices.remove(deviceId);
+ }
+ if (hitAnnotation.annotation?.onHover != null && lastEvent is PointerHoverEvent) {
+ hitAnnotation.annotation.onHover(lastEvent);
+ }
+
+ // Tell any tracked annotations that weren't hit that they are no longer
+ // active.
+ for (_TrackedAnnotation trackedAnnotation in _trackedAnnotations.values) {
+ if (hitAnnotations.contains(trackedAnnotation)) {
+ continue;
+ }
+ if (trackedAnnotation.activeDevices.contains(deviceId)) {
+ if (trackedAnnotation.annotation?.onExit != null) {
+ trackedAnnotation.annotation.onExit(PointerExitEvent.fromMouseEvent(lastEvent));
+ }
+ trackedAnnotation.activeDevices.remove(deviceId);
+ }
}
}
}
+ } finally {
+ _pendingRemovals.clear();
}
}
void _addMouseEvent(int deviceId, PointerEvent event) {
final bool wasConnected = mouseIsConnected;
+ if (event is PointerAddedEvent) {
+ // If we are adding the device again, then we're not removing it anymore.
+ _pendingRemovals.remove(deviceId);
+ }
_lastMouseEvent[deviceId] = event;
if (mouseIsConnected != wasConnected) {
notifyListeners();
}
}
- void _removeMouseEvent(int deviceId) {
+ void _removeMouseEvent(int deviceId, PointerEvent event) {
final bool wasConnected = mouseIsConnected;
+ assert(event is PointerRemovedEvent);
+ _pendingRemovals[deviceId] = event;
_lastMouseEvent.remove(deviceId);
if (mouseIsConnected != wasConnected) {
notifyListeners();
}
}
+ // A list of device IDs that should be removed and notified when scheduling a
+ // mouse position check.
+ final Map<int, PointerRemovedEvent> _pendingRemovals = <int, PointerRemovedEvent>{};
+
/// The most recent mouse event observed for each mouse device ID observed.
///
/// May be null if no mouse is connected, or hasn't produced an event yet.
diff --git a/packages/flutter/test/gestures/mouse_tracking_test.dart b/packages/flutter/test/gestures/mouse_tracking_test.dart
index 687ad97..b22da89 100644
--- a/packages/flutter/test/gestures/mouse_tracking_test.dart
+++ b/packages/flutter/test/gestures/mouse_tracking_test.dart
@@ -91,9 +91,11 @@
kind: PointerDeviceKind.mouse,
),
]);
- const ui.PointerDataPacket packet3 = ui.PointerDataPacket(data: <ui.PointerData>[
+ final ui.PointerDataPacket packet3 = ui.PointerDataPacket(data: <ui.PointerData>[
ui.PointerData(
change: ui.PointerChange.remove,
+ physicalX: 1.0 * ui.window.devicePixelRatio,
+ physicalY: 201.0 * ui.window.devicePixelRatio,
kind: PointerDeviceKind.mouse,
),
]);
@@ -101,7 +103,7 @@
ui.PointerData(
change: ui.PointerChange.hover,
physicalX: 1.0 * ui.window.devicePixelRatio,
- physicalY: 201.0 * ui.window.devicePixelRatio,
+ physicalY: 301.0 * ui.window.devicePixelRatio,
kind: PointerDeviceKind.mouse,
),
]);
@@ -109,7 +111,7 @@
ui.PointerData(
change: ui.PointerChange.hover,
physicalX: 1.0 * ui.window.devicePixelRatio,
- physicalY: 301.0 * ui.window.devicePixelRatio,
+ physicalY: 401.0 * ui.window.devicePixelRatio,
kind: PointerDeviceKind.mouse,
device: 1,
),
@@ -144,20 +146,20 @@
expect(enter.length, equals(0), reason: 'enter contains $enter');
expect(move.length, equals(0), reason: 'move contains $move');
expect(exit.length, equals(1), reason: 'exit contains $exit');
- expect(exit.first.position, isNull);
- expect(exit.first.device, isNull);
+ expect(exit.first.position, equals(const Offset(1.0, 201.0)));
+ expect(exit.first.device, equals(0));
expect(exit.first.runtimeType, equals(PointerExitEvent));
clear();
ui.window.onPointerDataPacket(packet4);
tracker.collectMousePositions();
expect(enter.length, equals(1), reason: 'enter contains $enter');
- expect(enter.first.position, equals(const Offset(1.0, 201.0)));
+ expect(enter.first.position, equals(const Offset(1.0, 301.0)));
expect(enter.first.device, equals(0));
expect(enter.first.runtimeType, equals(PointerEnterEvent));
expect(exit.length, equals(0), reason: 'exit contains $exit');
expect(move.length, equals(1), reason: 'move contains $move');
- expect(move.first.position, equals(const Offset(1.0, 201.0)));
+ expect(move.first.position, equals(const Offset(1.0, 301.0)));
expect(move.first.device, equals(0));
expect(move.first.runtimeType, equals(PointerHoverEvent));
@@ -166,15 +168,15 @@
ui.window.onPointerDataPacket(packet5);
tracker.collectMousePositions();
expect(enter.length, equals(1), reason: 'enter contains $enter');
- expect(enter.first.position, equals(const Offset(1.0, 301.0)));
+ expect(enter.first.position, equals(const Offset(1.0, 401.0)));
expect(enter.first.device, equals(1));
expect(enter.first.runtimeType, equals(PointerEnterEvent));
expect(exit.length, equals(0), reason: 'exit contains $exit');
expect(move.length, equals(2), reason: 'move contains $move');
- expect(move.first.position, equals(const Offset(1.0, 201.0)));
+ expect(move.first.position, equals(const Offset(1.0, 301.0)));
expect(move.first.device, equals(0));
expect(move.first.runtimeType, equals(PointerHoverEvent));
- expect(move.last.position, equals(const Offset(1.0, 301.0)));
+ expect(move.last.position, equals(const Offset(1.0, 401.0)));
expect(move.last.device, equals(1));
expect(move.last.runtimeType, equals(PointerHoverEvent));
});
@@ -228,7 +230,7 @@
expect(exit.first.device, equals(0));
expect(exit.first.runtimeType, equals(PointerExitEvent));
- // Actually detatch annotation. Shouldn't receive hit.
+ // Actually detach annotation. Shouldn't receive hit.
tracker.detachAnnotation(annotation);
clear();
isInHitRegionOne = false;
@@ -274,9 +276,11 @@
kind: PointerDeviceKind.mouse,
),
]);
- const ui.PointerDataPacket packet2 = ui.PointerDataPacket(data: <ui.PointerData>[
+ final ui.PointerDataPacket packet2 = ui.PointerDataPacket(data: <ui.PointerData>[
ui.PointerData(
change: ui.PointerChange.remove,
+ physicalX: 1.0 * ui.window.devicePixelRatio,
+ physicalY: 201.0 * ui.window.devicePixelRatio,
kind: PointerDeviceKind.mouse,
),
]);
@@ -288,15 +292,18 @@
tracker.collectMousePositions();
expect(enter.length, equals(1), reason: 'enter contains $enter');
expect(enter.first.position, equals(const Offset(1.0, 101.0)));
+ expect(enter.first.delta, equals(const Offset(1.0, 101.0)));
expect(enter.first.device, equals(0));
expect(enter.first.runtimeType, equals(PointerEnterEvent));
expect(move.length, equals(1), reason: 'move contains $move');
expect(move.first.position, equals(const Offset(1.0, 101.0)));
+ expect(move.first.delta, equals(const Offset(1.0, 101.0)));
expect(move.first.device, equals(0));
expect(move.first.runtimeType, equals(PointerHoverEvent));
expect(exit.length, equals(1), reason: 'exit contains $exit');
- expect(exit.first.position, isNull);
- expect(exit.first.device, isNull);
+ expect(exit.first.position, equals(const Offset(1.0, 201.0)));
+ expect(exit.first.delta, equals(const Offset(0.0, 0.0)));
+ expect(exit.first.device, equals(0));
expect(exit.first.runtimeType, equals(PointerExitEvent));
});
test('handles mouse down and move', () {
diff --git a/packages/flutter/test/widgets/listener_test.dart b/packages/flutter/test/widgets/listener_test.dart
index b4da4b5..6fcd88a 100644
--- a/packages/flutter/test/widgets/listener_test.dart
+++ b/packages/flutter/test/widgets/listener_test.dart
@@ -594,5 +594,50 @@
await gesture.removePointer();
});
+ testWidgets('Exit event when unplugging mouse should have a position', (WidgetTester tester) async {
+ final List<PointerEnterEvent> enter = <PointerEnterEvent>[];
+ final List<PointerHoverEvent> hover = <PointerHoverEvent>[];
+ final List<PointerExitEvent> exit = <PointerExitEvent>[];
+
+ await tester.pumpWidget(
+ Center(
+ child: Listener(
+ onPointerEnter: (PointerEnterEvent e) => enter.add(e),
+ onPointerHover: (PointerHoverEvent e) => hover.add(e),
+ onPointerExit: (PointerExitEvent e) => exit.add(e),
+ child: Container(
+ height: 100.0,
+ width: 100.0,
+ ),
+ ),
+ ),
+ );
+
+ // Plug-in a mouse and move it to the center of the container.
+ final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
+ await gesture.addPointer();
+ await gesture.moveTo(tester.getCenter(find.byType(Container)));
+ await tester.pumpAndSettle();
+
+ expect(enter.length, 1);
+ expect(enter.single.position, const Offset(400.0, 300.0));
+ expect(hover.length, 1);
+ expect(hover.single.position, const Offset(400.0, 300.0));
+ expect(exit.length, 0);
+
+ enter.clear();
+ hover.clear();
+ exit.clear();
+
+ // Unplug the mouse.
+ await gesture.removePointer();
+ await tester.pumpAndSettle();
+
+ expect(enter.length, 0);
+ expect(hover.length, 0);
+ expect(exit.length, 1);
+ expect(exit.single.position, const Offset(400.0, 300.0));
+ expect(exit.single.delta, const Offset(0.0, 0.0));
+ });
});
}
diff --git a/packages/flutter_test/lib/src/test_pointer.dart b/packages/flutter_test/lib/src/test_pointer.dart
index 609c6bc..f957f77 100644
--- a/packages/flutter_test/lib/src/test_pointer.dart
+++ b/packages/flutter_test/lib/src/test_pointer.dart
@@ -209,35 +209,36 @@
///
/// By default, the time stamp on the event is [Duration.zero]. You can give a
/// specific time stamp by passing the `timeStamp` argument.
- ///
- /// [isDown] must be false, since hover events can't be sent when the pointer
- /// is up.
PointerAddedEvent addPointer({
Duration timeStamp = Duration.zero,
+ Offset location,
}) {
assert(timeStamp != null);
+ _location = location ?? _location;
return PointerAddedEvent(
timeStamp: timeStamp,
kind: kind,
device: _device,
+ position: _location ?? Offset.zero,
);
}
- /// Create a [PointerRemovedEvent] with the kind the pointer was created with.
+ /// Create a [PointerRemovedEvent] with the [PointerDeviceKind] the pointer
+ /// was created with.
///
/// By default, the time stamp on the event is [Duration.zero]. You can give a
/// specific time stamp by passing the `timeStamp` argument.
- ///
- /// [isDown] must be false, since hover events can't be sent when the pointer
- /// is up.
PointerRemovedEvent removePointer({
Duration timeStamp = Duration.zero,
+ Offset location,
}) {
assert(timeStamp != null);
+ _location = location ?? _location;
return PointerRemovedEvent(
timeStamp: timeStamp,
kind: kind,
device: _device,
+ position: _location ?? Offset.zero,
);
}
@@ -374,14 +375,14 @@
/// In a test, send a pointer add event for this pointer.
Future<void> addPointer({ Duration timeStamp = Duration.zero }) {
return TestAsyncUtils.guard<void>(() {
- return _dispatcher(_pointer.addPointer(timeStamp: timeStamp), null);
+ return _dispatcher(_pointer.addPointer(timeStamp: timeStamp, location: _pointer.location), null);
});
}
/// In a test, send a pointer remove event for this pointer.
- Future<void> removePointer({ Duration timeStamp = Duration.zero }) {
+ Future<void> removePointer({ Duration timeStamp = Duration.zero}) {
return TestAsyncUtils.guard<void>(() {
- return _dispatcher(_pointer.removePointer(timeStamp: timeStamp), null);
+ return _dispatcher(_pointer.removePointer(timeStamp: timeStamp, location: _pointer.location), null);
});
}
