Unverified Commit 05097916 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Fix mouse hover to not schedule a frame for every mouse move. (#41014)

This fixes the mouse hover code to not schedule frames with every mouse move.

Before this, it would schedule a post frame callback, and then schedule a frame immediately, even if there was nothing that needed to be updated. Now it will schedule checks for mouse position updates synchronously, unless there's a new annotation, and skip scheduling a new frame in all cases. It has to be async in the case of a new annotation (i.e. a new MouseRegion is added), since when the annotation is added, it hasn't yet painted, and it can't hit test against the new layer until after the paint, so in that case it schedules a post frame callback, but since it's already building a frame when it does that, it doesn't need to schedule a frame.

The code also used to do mouse position checks for all mice if only one mouse changed position. I fixed this part too, so that it will only check position for the mouse that changed.
parent 75552684
......@@ -92,12 +92,21 @@ class MouseTracker extends ChangeNotifier {
/// Creates a mouse tracker to keep track of mouse locations.
///
/// All of the parameters must not be null.
MouseTracker(PointerRouter router, this.annotationFinder)
: assert(router != null),
MouseTracker(this._router, this.annotationFinder)
: assert(_router != null),
assert(annotationFinder != null) {
router.addGlobalRoute(_handleEvent);
_router.addGlobalRoute(_handleEvent);
}
@override
void dispose() {
super.dispose();
_router.removeGlobalRoute(_handleEvent);
}
// The pointer router that the mouse tracker listens to for events.
final PointerRouter _router;
/// Used to find annotations at a given logical coordinate.
final MouseDetectorAnnotationFinder annotationFinder;
......@@ -111,16 +120,19 @@ class MouseTracker extends ChangeNotifier {
/// annotation has been added to the layer tree.
void attachAnnotation(MouseTrackerAnnotation annotation) {
_trackedAnnotations[annotation] = _TrackedAnnotation(annotation);
// Schedule a check so that we test this new annotation to see if the mouse
// is currently inside its region.
_scheduleMousePositionCheck();
// Schedule a check so that we test this new annotation to see if any mouse
// is currently inside its region. It has to happen after the frame is
// complete so that the annotation layer has been added before the check.
if (mouseIsConnected) {
_scheduleMousePositionCheck();
}
}
/// Stops tracking an annotation, indicating that it has been removed from the
/// layer tree.
///
/// If the associated layer is not removed, and receives a hit, then
/// [collectMousePositions] will assert the next time it is called.
/// [sendMouseNotifications] will assert the next time it is called.
void detachAnnotation(MouseTrackerAnnotation annotation) {
final _TrackedAnnotation trackedAnnotation = _findAnnotation(annotation);
for (int deviceId in trackedAnnotation.activeDevices) {
......@@ -133,18 +145,19 @@ class MouseTracker extends ChangeNotifier {
_trackedAnnotations.remove(annotation);
}
bool _postFrameCheckScheduled = false;
bool _scheduledPostFramePositionCheck = false;
// Schedules a position check at the end of this frame for those annotations
// that have been added.
void _scheduleMousePositionCheck() {
// If we're not tracking anything, then there is no point in registering a
// frame callback or scheduling a frame. By definition there are no active
// annotations that need exiting, either.
if (_trackedAnnotations.isNotEmpty && !_postFrameCheckScheduled) {
_postFrameCheckScheduled = true;
SchedulerBinding.instance.addPostFrameCallback((Duration _) {
_postFrameCheckScheduled = false;
collectMousePositions();
if (_trackedAnnotations.isNotEmpty && !_scheduledPostFramePositionCheck) {
_scheduledPostFramePositionCheck = true;
SchedulerBinding.instance.addPostFrameCallback((Duration duration) {
sendMouseNotifications(_lastMouseEvent.keys);
_scheduledPostFramePositionCheck = false;
});
SchedulerBinding.instance.scheduleFrame();
}
}
......@@ -158,21 +171,24 @@ class MouseTracker extends ChangeNotifier {
// If we are adding the device again, then we're not removing it anymore.
_pendingRemovals.remove(deviceId);
_addMouseEvent(deviceId, event);
sendMouseNotifications(<int>{deviceId});
return;
}
if (event is PointerRemovedEvent) {
_removeMouseEvent(deviceId, event);
// If the mouse was removed, then we need to schedule one more check to
// exit any annotations that were active.
_scheduleMousePositionCheck();
sendMouseNotifications(<int>{deviceId});
} else {
if (event is PointerMoveEvent || event is PointerHoverEvent || event is PointerDownEvent) {
if (!_lastMouseEvent.containsKey(deviceId) || _lastMouseEvent[deviceId].position != event.position) {
final PointerEvent lastEvent = _lastMouseEvent[deviceId];
_addMouseEvent(deviceId, event);
if (lastEvent == null ||
lastEvent is PointerAddedEvent || lastEvent.position != event.position) {
// Only schedule a frame if we have our first event, or if the
// location of the mouse has changed, and only if there are tracked annotations.
_scheduleMousePositionCheck();
sendMouseNotifications(<int>{deviceId});
}
_addMouseEvent(deviceId, event);
}
}
}
......@@ -206,14 +222,18 @@ class MouseTracker extends ChangeNotifier {
/// This function is only public to allow for proper testing of the
/// MouseTracker. Do not call in other contexts.
@visibleForTesting
void collectMousePositions() {
void sendMouseNotifications(Iterable<int> deviceIds) {
if (_trackedAnnotations.isEmpty) {
return;
}
void exitAnnotation(_TrackedAnnotation trackedAnnotation, int deviceId) {
if (trackedAnnotation.annotation?.onExit != null && trackedAnnotation.activeDevices.contains(deviceId)) {
final PointerEvent event = _lastMouseEvent[deviceId] ?? _pendingRemovals[deviceId];
assert(event != null);
trackedAnnotation.annotation.onExit(PointerExitEvent.fromMouseEvent(event));
trackedAnnotation.activeDevices.remove(deviceId);
}
trackedAnnotation.activeDevices.remove(deviceId);
}
void exitAllDevices(_TrackedAnnotation trackedAnnotation) {
......@@ -234,8 +254,9 @@ class MouseTracker extends ChangeNotifier {
return;
}
for (int deviceId in _lastMouseEvent.keys) {
for (int deviceId in deviceIds) {
final PointerEvent lastEvent = _lastMouseEvent[deviceId];
assert(lastEvent != null);
final Iterable<MouseTrackerAnnotation> hits = annotationFinder(lastEvent.position);
// No annotations were found at this position for this deviceId, so send an
......@@ -311,7 +332,6 @@ class MouseTracker extends ChangeNotifier {
/// 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.
/// Will not be updated unless there is at least one tracked annotation.
final Map<int, PointerEvent> _lastMouseEvent = <int, PointerEvent>{};
/// Whether or not a mouse is connected and has produced events.
......
......@@ -43,7 +43,7 @@ mixin RendererBinding on BindingBase, ServicesBinding, SchedulerBinding, Gesture
_handleSemanticsEnabledChanged();
assert(renderView != null);
addPersistentFrameCallback(_handlePersistentFrameCallback);
_mouseTracker = _createMouseTracker();
initMouseTracker();
}
/// The current [RendererBinding], if one has been created.
......@@ -238,10 +238,14 @@ mixin RendererBinding on BindingBase, ServicesBinding, SchedulerBinding, Gesture
SemanticsHandle _semanticsHandle;
// Creates a [MouseTracker] which manages state about currently connected
// mice, for hover notification.
MouseTracker _createMouseTracker() {
return MouseTracker(pointerRouter, renderView.hitTestMouseTrackers);
/// Creates a [MouseTracker] which manages state about currently connected
/// mice, for hover notification.
///
/// Used by testing framework to reinitialize the mouse tracker between tests.
@visibleForTesting
void initMouseTracker([MouseTracker tracker]) {
_mouseTracker?.dispose();
_mouseTracker = tracker ?? MouseTracker(pointerRouter, renderView.hitTestMouseTrackers);
}
void _handleSemanticsEnabledChanged() {
......
......@@ -5833,7 +5833,7 @@ class MouseRegion extends SingleChildRenderObjectWidget {
final PointerExitEventListener onExit;
@override
_ListenerElement createElement() => _ListenerElement(this);
_MouseRegionElement createElement() => _MouseRegionElement(this);
@override
RenderMouseRegion createRenderObject(BuildContext context) {
......@@ -5866,20 +5866,20 @@ class MouseRegion extends SingleChildRenderObjectWidget {
}
}
class _ListenerElement extends SingleChildRenderObjectElement {
_ListenerElement(SingleChildRenderObjectWidget widget) : super(widget);
class _MouseRegionElement extends SingleChildRenderObjectElement {
_MouseRegionElement(SingleChildRenderObjectWidget widget) : super(widget);
@override
void activate() {
super.activate();
final RenderMouseRegion renderMouseListener = renderObject;
renderMouseListener.postActivate();
final RenderMouseRegion renderMouseRegion = renderObject;
renderMouseRegion.postActivate();
}
@override
void deactivate() {
final RenderMouseRegion renderMouseListener = renderObject;
renderMouseListener.preDeactivate();
final RenderMouseRegion renderMouseRegion = renderObject;
renderMouseRegion.preDeactivate();
super.deactivate();
}
}
......
......@@ -388,7 +388,7 @@ void main() {
final TestGesture gesture = await tester.createGesture(
kind: PointerDeviceKind.mouse,
);
await gesture.addPointer();
await gesture.addPointer(location: Offset.zero);
addTearDown(gesture.removePointer);
await gesture.moveTo(center);
await tester.pumpAndSettle();
......
......@@ -147,6 +147,7 @@ void main() {
TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
addTearDown(() => gesture?.removePointer());
await tester.pumpAndSettle();
await gesture.moveTo(tester.getCenter(find.byType(FloatingActionButton)));
await tester.pumpAndSettle();
......
......@@ -8,7 +8,6 @@ import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter/gestures.dart';
// The tests in this file are moved from listener_test.dart, which tests several
// deprecated APIs. The file should be removed once these parameters are.
......@@ -82,7 +81,7 @@ void main() {
// onPointer{Enter,Hover,Exit} are removed. They were kept for compatibility,
// and the tests have been copied to mouse_region_test.
// https://github.com/flutter/flutter/issues/36085
setUp((){
setUp(() {
HoverClientState.numExits = 0;
HoverClientState.numEntries = 0;
});
......@@ -91,19 +90,22 @@ void main() {
PointerEnterEvent enter;
PointerHoverEvent move;
PointerExitEvent exit;
await tester.pumpWidget(Center(
child: Listener(
child: Container(
color: const Color.fromARGB(0xff, 0xff, 0x00, 0x00),
width: 100.0,
height: 100.0,
await tester.pumpWidget(
Center(
child: Listener(
child: Container(
color: const Color.fromARGB(0xff, 0xff, 0x00, 0x00),
width: 100.0,
height: 100.0,
),
onPointerEnter: (PointerEnterEvent details) => enter = details,
onPointerHover: (PointerHoverEvent details) => move = details,
onPointerExit: (PointerExitEvent details) => exit = details,
),
onPointerEnter: (PointerEnterEvent details) => enter = details,
onPointerHover: (PointerHoverEvent details) => move = details,
onPointerExit: (PointerExitEvent details) => exit = details,
),
));
);
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer(location: Offset.zero);
addTearDown(gesture.removePointer);
await gesture.moveTo(const Offset(400.0, 300.0));
await tester.pump();
......@@ -117,20 +119,22 @@ void main() {
PointerEnterEvent enter;
PointerHoverEvent move;
PointerExitEvent exit;
await tester.pumpWidget(Center(
child: Listener(
child: Container(
width: 100.0,
height: 100.0,
await tester.pumpWidget(
Center(
child: Listener(
child: Container(
width: 100.0,
height: 100.0,
),
onPointerEnter: (PointerEnterEvent details) => enter = details,
onPointerHover: (PointerHoverEvent details) => move = details,
onPointerExit: (PointerExitEvent details) => exit = details,
),
onPointerEnter: (PointerEnterEvent details) => enter = details,
onPointerHover: (PointerHoverEvent details) => move = details,
onPointerExit: (PointerExitEvent details) => exit = details,
),
));
);
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer(location: const Offset(400.0, 300.0));
addTearDown(gesture.removePointer);
await gesture.moveTo(const Offset(400.0, 300.0));
await tester.pump();
move = null;
enter = null;
......@@ -145,24 +149,25 @@ void main() {
PointerEnterEvent enter;
PointerHoverEvent move;
PointerExitEvent exit;
await tester.pumpWidget(Center(
child: Listener(
child: Container(
width: 100.0,
height: 100.0,
await tester.pumpWidget(
Center(
child: Listener(
child: Container(
width: 100.0,
height: 100.0,
),
onPointerEnter: (PointerEnterEvent details) => enter = details,
onPointerHover: (PointerHoverEvent details) => move = details,
onPointerExit: (PointerExitEvent details) => exit = details,
),
onPointerEnter: (PointerEnterEvent details) => enter = details,
onPointerHover: (PointerHoverEvent details) => move = details,
onPointerExit: (PointerExitEvent details) => exit = details,
),
));
);
final RenderMouseRegion renderListener = tester.renderObject(find.byType(MouseRegion));
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer(location: const Offset(400.0, 300.0));
addTearDown(gesture.removePointer);
await gesture.moveTo(const Offset(400.0, 300.0));
await tester.pump();
expect(move, isNotNull);
expect(move.position, equals(const Offset(400.0, 300.0)));
expect(move, isNull);
expect(enter, isNotNull);
expect(enter.position, equals(const Offset(400.0, 300.0)));
expect(exit, isNull);
......@@ -197,7 +202,7 @@ void main() {
await tester.pumpWidget(Container());
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer);
await gesture.moveTo(const Offset(400.0, 0.0));
await gesture.addPointer(location: const Offset(400.0, 0.0));
await tester.pump();
await tester.pumpWidget(
Column(
......@@ -430,9 +435,8 @@ void main() {
expect(bottomLeft.dy - topLeft.dy, scaleFactor * localHeight);
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.addPointer(location: topLeft - const Offset(1, 1));
addTearDown(gesture.removePointer);
await gesture.moveTo(topLeft - const Offset(1, 1));
await tester.pump();
expect(events, isEmpty);
......@@ -458,7 +462,7 @@ void main() {
testWidgets('needsCompositing updates correctly and is respected', (WidgetTester tester) async {
// Pretend that we have a mouse connected.
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.addPointer(location: Offset.zero);
addTearDown(gesture.removePointer);
await tester.pumpWidget(
......@@ -507,7 +511,7 @@ void main() {
testWidgets("Callbacks aren't called during build", (WidgetTester tester) async {
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.addPointer(location: Offset.zero);
addTearDown(gesture.removePointer);
await tester.pumpWidget(
......@@ -538,7 +542,7 @@ void main() {
testWidgets("Listener activate/deactivate don't duplicate annotations", (WidgetTester tester) async {
final GlobalKey feedbackKey = GlobalKey();
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.addPointer(location: Offset.zero);
addTearDown(gesture.removePointer);
await tester.pumpWidget(
......@@ -586,11 +590,8 @@ void main() {
// Plug-in a mouse and move it to the center of the container.
TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
addTearDown(() async {
if (gesture != null)
return gesture.removePointer();
});
await gesture.addPointer(location: Offset.zero);
addTearDown(() => gesture?.removePointer());
await gesture.moveTo(tester.getCenter(find.byType(Container)));
await tester.pumpAndSettle();
......@@ -613,7 +614,7 @@ void main() {
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));
expect(exit.single.delta, Offset.zero);
});
});
}
......@@ -805,6 +805,10 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
'The MouseTracker thinks that there is still a mouse connected, which indicates that a '
'test has not removed the mouse pointer which it added. Call removePointer on the '
'active mouse gesture to remove the mouse pointer.');
// ignore: invalid_use_of_visible_for_testing_member
RendererBinding.instance.initMouseTracker();
// ignore: invalid_use_of_visible_for_testing_member
PointerEventConverter.clearPointers();
}
}
......
......@@ -373,16 +373,16 @@ class TestGesture {
}
/// In a test, send a pointer add event for this pointer.
Future<void> addPointer({ Duration timeStamp = Duration.zero }) {
Future<void> addPointer({ Duration timeStamp = Duration.zero, Offset location }) {
return TestAsyncUtils.guard<void>(() {
return _dispatcher(_pointer.addPointer(timeStamp: timeStamp, location: _pointer.location), null);
return _dispatcher(_pointer.addPointer(timeStamp: timeStamp, location: 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, Offset location }) {
return TestAsyncUtils.guard<void>(() {
return _dispatcher(_pointer.removePointer(timeStamp: timeStamp, location: _pointer.location), null);
return _dispatcher(_pointer.removePointer(timeStamp: timeStamp, location: location ?? _pointer.location), null);
});
}
......
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