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

Fix transforms for things with RenderPointerListeners (#32535)

This fixes #32525, because it now marks the compositing bits as needing to be recalculated if the mouse tracker changes its idea of whether or not a mouse is attached.

This bug occurred because the test framework was leaking state from one test to the next (the state about whether a mouse pointer was active), and so even though there was a "passing" test when run in order with the other tests in the file, when the test was run individually (or first), it would have failed and caught the bug.

This adds an assert to make sure that after each test there are no simulated mouse pointers connected, and now calls removePointer in all of the tests where this was a problem.
parent f5cf209a
......@@ -2495,6 +2495,7 @@ class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
onExit: _onPointerExit,
);
}
_mouseIsConnected = RendererBinding.instance.mouseTracker.mouseIsConnected;
}
/// Called when a pointer comes into contact with the screen (for touch
......@@ -2566,6 +2567,8 @@ class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
MouseTrackerAnnotation get hoverAnnotation => _hoverAnnotation;
void _updateAnnotations() {
assert(_hoverAnnotation == null || _onPointerEnter != _hoverAnnotation.onEnter || _onPointerHover != _hoverAnnotation.onHover || _onPointerExit != _hoverAnnotation.onExit,
"Shouldn't call _updateAnnotations if nothing has changed.");
bool changed = false;
final bool hadHoverAnnotation = _hoverAnnotation != null;
if (_hoverAnnotation != null && attached) {
......@@ -2594,9 +2597,16 @@ class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
}
}
bool _mouseIsConnected;
void _handleMouseTrackerChanged() {
if (attached)
markNeedsPaint();
final bool newState = RendererBinding.instance.mouseTracker.mouseIsConnected;
if (newState != _mouseIsConnected) {
_mouseIsConnected = newState;
if (_hoverAnnotation != null) {
markNeedsCompositingBitsUpdate();
markNeedsPaint();
}
}
}
@override
......@@ -2609,7 +2619,7 @@ class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
/// Attaches the annotation for this render object, if any.
///
/// This is called by [attach] to attach and new annotations.
/// This is called by [attach] to attach any new annotations.
///
/// This is also called by the [Listener]'s [Element] to tell this
/// [RenderPointerListener] that it will shortly be attached. That way,
......@@ -2641,10 +2651,7 @@ class RenderPointerListener extends RenderProxyBoxWithHitTestBehavior {
super.detach();
}
bool get _hasActiveAnnotation {
return _hoverAnnotation != null
&& RendererBinding.instance.mouseTracker.mouseIsConnected;
}
bool get _hasActiveAnnotation => _hoverAnnotation != null && _mouseIsConnected;
@override
bool get needsCompositing => _hasActiveAnnotation;
......
......@@ -1947,6 +1947,8 @@ void main() {
expect(controller.selection.baseOffset, testValue.indexOf('e'));
expect(controller.selection.extentOffset, testValue.indexOf('g'));
await gesture.removePointer();
});
testWidgets('Continuous dragging does not cause flickering', (WidgetTester tester) async {
......@@ -2002,6 +2004,8 @@ void main() {
expect(selectionChangedCount, 1);
expect(controller.selection.baseOffset, 2);
expect(controller.selection.extentOffset, 9);
await gesture.removePointer();
});
testWidgets(
......
......@@ -764,6 +764,8 @@ void main() {
// The cursor is placed just like a regular tap.
expect(controller.selection.baseOffset, eIndex);
expect(controller.selection.extentOffset, eIndex);
await gesture.removePointer();
});
testWidgets('enableInteractiveSelection = false, long-press', (WidgetTester tester) async {
......@@ -827,6 +829,8 @@ void main() {
expect(controller.selection.baseOffset, testValue.indexOf('e'));
expect(controller.selection.extentOffset, testValue.indexOf('g'));
await gesture.removePointer();
});
testWidgets('Continuous dragging does not cause flickering', (WidgetTester tester) async {
......@@ -879,6 +883,8 @@ void main() {
expect(selectionChangedCount, 1);
expect(controller.selection.baseOffset, 2);
expect(controller.selection.extentOffset, 9);
await gesture.removePointer();
});
testWidgets('Dragging in opposite direction also works', (WidgetTester tester) async {
......@@ -911,6 +917,8 @@ void main() {
expect(controller.selection.baseOffset, testValue.indexOf('e'));
expect(controller.selection.extentOffset, testValue.indexOf('g'));
await gesture.removePointer();
});
testWidgets('Slow mouse dragging also selects text', (WidgetTester tester) async {
......@@ -942,6 +950,8 @@ void main() {
expect(controller.selection.baseOffset, testValue.indexOf('e'));
expect(controller.selection.extentOffset, testValue.indexOf('g'));
await gesture.removePointer();
});
testWidgets('Can drag handles to change selection', (WidgetTester tester) async {
......
......@@ -134,6 +134,8 @@ void main() {
expect(enter, isNotNull);
expect(enter.position, equals(const Offset(400.0, 300.0)));
expect(exit, isNull);
await gesture.removePointer();
});
testWidgets('detects pointer exiting', (WidgetTester tester) async {
PointerEnterEvent enter;
......@@ -161,6 +163,8 @@ void main() {
expect(enter, isNull);
expect(exit, isNotNull);
expect(exit.position, equals(const Offset(1.0, 1.0)));
await gesture.removePointer();
});
testWidgets('detects pointer exit when widget disappears', (WidgetTester tester) async {
PointerEnterEvent enter;
......@@ -195,6 +199,8 @@ void main() {
expect(exit, isNotNull);
expect(exit.position, equals(const Offset(400.0, 300.0)));
expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener.hoverAnnotation), isFalse);
await gesture.removePointer();
});
testWidgets('Hover works with nested listeners', (WidgetTester tester) async {
final UniqueKey key1 = UniqueKey();
......@@ -276,6 +282,8 @@ void main() {
expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener1.hoverAnnotation), isTrue);
expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener2.hoverAnnotation), isTrue);
clearLists();
await gesture.removePointer();
});
testWidgets('Hover transfers between two listeners', (WidgetTester tester) async {
final UniqueKey key1 = UniqueKey();
......@@ -379,6 +387,8 @@ void main() {
expect(exit2, isEmpty);
expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener1.hoverAnnotation), isFalse);
expect(tester.binding.mouseTracker.isAnnotationAttached(renderListener2.hoverAnnotation), isFalse);
await gesture.removePointer();
});
testWidgets('works with transform', (WidgetTester tester) async {
......@@ -422,9 +432,9 @@ void main() {
final Offset bottomLeft = tester.getBottomLeft(find.byKey(key));
expect(topRight.dx - topLeft.dx, scaleFactor * localWidth);
expect(bottomLeft.dy - topLeft.dy, scaleFactor * localHeight);
print('Rect: ${tester.getRect(find.byKey(key))}');
final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
await gesture.addPointer();
await gesture.moveTo(topLeft - const Offset(1, 1));
await tester.pump();
expect(events, isEmpty);
......@@ -446,6 +456,8 @@ void main() {
await tester.pump();
expect(events.single, isA<PointerExitEvent>());
events.clear();
await gesture.removePointer();
});
testWidgets('needsCompositing updates correctly and is respected', (WidgetTester tester) async {
......@@ -479,7 +491,7 @@ void main() {
),
);
expect(listener.needsCompositing, isTrue);
// Composting is required, therefore a dedicated TransformLayer for
// Compositing is required, therefore a dedicated TransformLayer for
// `Transform.scale` is added.
expect(tester.layers.whereType<TransformLayer>(), hasLength(2));
......@@ -495,6 +507,8 @@ void main() {
// TransformLayer for `Transform.scale` is removed again as transform is
// executed directly on the canvas.
expect(tester.layers.whereType<TransformLayer>(), hasLength(1));
await gesture.removePointer();
});
testWidgets("Callbacks aren't called during build", (WidgetTester tester) async {
......@@ -524,6 +538,8 @@ void main() {
await tester.pump();
expect(HoverClientState.numEntries, equals(2));
expect(HoverClientState.numExits, equals(1));
await gesture.removePointer();
});
testWidgets("Listener activate/deactivate don't duplicate annotations", (WidgetTester tester) async {
......@@ -553,6 +569,8 @@ void main() {
await tester.pump();
expect(HoverClientState.numEntries, equals(2));
expect(HoverClientState.numExits, equals(2));
await gesture.removePointer();
});
});
}
......@@ -316,6 +316,8 @@ void main() {
expect(tapCount, 1);
expect(singleTapUpCount, 1);
expect(singleLongTapStartCount, 0);
await gesture.removePointer();
});
testWidgets('a touch drag is not recognized for text selection', (WidgetTester tester) async {
......@@ -335,6 +337,8 @@ void main() {
expect(dragStartCount, 0);
expect(dragUpdateCount, 0);
expect(dragEndCount, 0);
await gesture.removePointer();
});
testWidgets('a mouse drag is recognized for text selection', (WidgetTester tester) async {
......@@ -354,6 +358,8 @@ void main() {
expect(dragStartCount, 1);
expect(dragUpdateCount, 1);
expect(dragEndCount, 1);
await gesture.removePointer();
});
testWidgets('a slow mouse drag is still recognized for text selection', (WidgetTester tester) async {
......@@ -371,5 +377,7 @@ void main() {
expect(dragStartCount, 1);
expect(dragUpdateCount, 1);
expect(dragEndCount, 1);
await gesture.removePointer();
});
}
......@@ -695,6 +695,10 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
_pendingExceptionDetails = null;
_parentZone = null;
buildOwner.focusManager = FocusManager();
assert(!RendererBinding.instance.mouseTracker.mouseIsConnected,
'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.');
}
}
......
......@@ -102,6 +102,7 @@ class TestPointer {
return PointerDownEvent(
timeStamp: timeStamp,
kind: kind,
device: _device,
pointer: pointer,
position: location,
);
......@@ -125,6 +126,7 @@ class TestPointer {
return PointerMoveEvent(
timeStamp: timeStamp,
kind: kind,
device: _device,
pointer: pointer,
position: newLocation,
delta: delta,
......@@ -143,6 +145,7 @@ class TestPointer {
return PointerUpEvent(
timeStamp: timeStamp,
kind: kind,
device: _device,
pointer: pointer,
position: location,
);
......@@ -160,6 +163,7 @@ class TestPointer {
return PointerCancelEvent(
timeStamp: timeStamp,
kind: kind,
device: _device,
pointer: pointer,
position: location,
);
......@@ -225,8 +229,8 @@ class TestPointer {
return PointerHoverEvent(
timeStamp: timeStamp,
kind: kind,
position: newLocation,
device: _device,
position: newLocation,
delta: delta,
);
}
......@@ -246,6 +250,7 @@ class TestPointer {
return PointerScrollEvent(
timeStamp: timeStamp,
kind: kind,
device: _device,
position: location,
scrollDelta: scrollDelta,
);
......
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