Transform PointerEvents to the local coordinate system of the event receiver#31222
Transform PointerEvents to the local coordinate system of the event receiver#31222goderbauer wants to merge 21 commits intoflutter:masterfrom
Conversation
goderbauer
added
the
framework
|
|
||
| import 'events.dart'; | ||
|
|
||
| typedef HitTest = bool Function(HitTestResult result, Offset position); |
There was a problem hiding this comment.
this is only RenderBox's hitTest type... I don't think the gestures library should hard-code this.
| } | ||
| _pushTransform(transform); | ||
| } | ||
| final Offset transformedPosition = position == null ? position : PointerEvent.transformPosition(transform, position); |
There was a problem hiding this comment.
Oh, I see. This is why this has Cartesian knowledge...
There was a problem hiding this comment.
maybe all this belongs in BoxHitTestEntry
There was a problem hiding this comment.
Yeah, the problem is that this needs to live in the HitTestResult, though, and not in an individual HitTestEntry. (There are no individual HitTestResult classes for Box or Sliver.)
My "excuse" for hard-coding the cartesian knowledge here is that the gesture library already has this cartesian knowledge baked into the events it dispatched: All PointerEvents in the gesture library have a (cartesian) Offset as position:
flutter/packages/flutter/lib/src/gestures/events.dart
Lines 171 to 173 in 8e7c7fc
From that I generalized that the entire hit-testing kinda happens in the cartesian system. There is a small escape hatch for non-cartesian components: If you don't care about the transformed position, a caller can just provide null. This works pretty well in the sliver case, see https://github.com/flutter/flutter/pull/31222/files#diff-4b5ed324ca3ddab0cd1942fc98a42573R1526.
I am not entirely sure how we can move this to BoxHitTesting - we also need to collect the transformation matrix for non-box ancestors (e.g. slivers) to calculate the correct local position for pointer events.
One thing we could do is remove the Offset stuff from the gesture library and have the call-site do the position transforms if they need it. As outlined above, we'd still need to keep the transform matrix, though. I am currently not sure how "cartesian" a transform matrix is.
Let me know if you have a better idea...
There was a problem hiding this comment.
Can you elaborate on "this needs to live in the HitTestResult"?
There was a problem hiding this comment.
Talked offline. We will try to untangle this by implementing separate HitTestResults for Box, Sliver, and other coordinate systems.
|
@goderbauer will this also address #31958? |
Yep, it does - I patched this in and checked 😄 |
|
@tvolkert Thanks for checking. I added the issue to the PR description and will add a test for the specific case. |
|
I am closing this PR to continue this work in #32192. |
4 participants
This PR has been updated and re-posted as #32192.
Description
Before this change,
PointerEvents would always be expressed in the global coordinate system of the screen. The change addslocalPositionandlocalDeltatoPointerEvents, which are expressed in the local coordinate system of the event receiver. This means - among other things - that scrolling now sticks to your finger even if the Scrollable has been scaled up or down. Furthermore, a rotated scrollable can now be scrolled by moving the finger in the (rotated) direction of the scrollview.Furthermore, this change also fixes hit testing for perspective transforms. For example, if a view is rotated along its x axis, buttons now continue to react to touch events:
Related Issues
Fixes #29916
Fixes #18408
Fixes #31958
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?