Add `embedderId` to `PointerEvent` #60930

iskakaushik · 2020-07-06T21:41:34Z

This field is used to look-up the event in the platform that resuted a given PointerEvent. This is currently only used on Android, where the embedderId is set to be the motionEventId for a given MotionEvent.

Related Issues

#58837

Tests

@blasten and I will work on testing strategy for this change as follow-up PRs.

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.

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I signed the [CLA].
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read [Handling breaking changes].

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.

fluttergithubbot · 2020-07-06T21:41:37Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

goderbauer · 2020-07-06T22:29:43Z

packages/flutter/lib/src/gestures/events.dart

Is this a unique ID for each PointerEvent or can multiple PointerEvents have the same motionEventId?

Nope, this is unique per PointerEvent. Multiple PointerEvents can not have the same motionEventId.

goderbauer · 2020-07-06T22:33:09Z

packages/flutter/lib/src/gestures/events.dart

Can we document this in a more platform-independent way so we can re-use this for the other platforms as well?

Exact wording would depend on the answer to the question below. Does this id uniquely identify the event across framework and engine? Or (if there are multiple PointerEvent with this id) does it identify the native event that generated this framework-side PointerEvent uniquely on the engine side? Something like this should be included in the doc comment.

After that, we can also include a platform-specific paragraph that on android this maps to the ID of the underlying MotionEvent. Or something like that.

I will try to address this and the comment below here:

What do you think of embedderPointerEventIdentifier as a possible name for this? Given that this is different from pointerIdentifier -- which is used for hit testing, whereas embedderPointerEventIdentifier is used to tie back the current pointer event to the platform equivalent which resulted in this event.

If you agree with that, I can elaborate the dart-doc to explain the intent of this field along with the differences between this and pointerIdentifier.

Can we just shorten this to embedderId?

The doc should still include the explanation how it is different from the other IDs in this class.

goderbauer · 2020-07-06T22:34:56Z

packages/flutter/lib/src/gestures/events.dart

A name for this property that doesn't tie it to Android's MotionEvents (and hence can be reused for other platforms) would probably also be a good idea.

This change makes it so that we track all the motion events encountered by `FlutterView` and all of its subviews in the `MotionEventTracker` class, indexed by a unique `MotionEventId`. This identifier is then passed to the Flutter framework as seen in flutter/flutter#60930. Once the gestures take part in gesture disambiguation and are sent back to the engine, we look-up the original motion event using the `MotionEventId` and dispatch it to the platform. Bug: flutter/flutter#58837

iskakaushik · 2020-07-07T00:10:17Z

@goderbauer I plan on landing this change as part of a manual engine roll during off-peak hours. The engine commit that this change relies on is at: flutter/engine#19484.

goderbauer

LGTM

goderbauer · 2020-07-07T00:13:42Z

packages/flutter/lib/src/gestures/events.dart

... to the embedder event that created it?

goderbauer · 2020-07-07T00:14:10Z

packages/flutter/lib/src/gestures/events.dart

Maybe add a section about Android explaining that on Android this is the ID of the underlying MotionEvent.

This change makes it so that we track all the motion events encountered by `FlutterView` and all of its subviews in the `MotionEventTracker` class, indexed by a unique `MotionEventId`. This identifier is then passed to the Flutter framework as seen in flutter/flutter#60930. Once the gestures take part in gesture disambiguation and are sent back to the engine, we look-up the original motion event using the `MotionEventId` and dispatch it to the platform. Bug: flutter/flutter#58837

blasten

LGTM

This change makes it so that we track all the motion events encountered by `FlutterView` and all of its subviews in the `MotionEventTracker` class, indexed by a unique `MotionEventId`. This identifier is then passed to the Flutter framework as seen in flutter/flutter#60930. Once the gestures take part in gesture disambiguation and are sent back to the engine, we look-up the original motion event using the `MotionEventId` and dispatch it to the platform. Bug: flutter/flutter#58837

fluttergithubbot · 2020-07-07T08:14:02Z

This pull request is not suitable for automatic merging in its current state.

The status or check suite fuchsia_precache-linux has failed. Please fix the issues identified (or deflake) before re-applying this label.

This field is used to look-up the event in the platform that resuted a given `PointerEvent`. This is currently only used on Android, where the `embedderId` is set to be the `motionEventId` for a given `MotionEvent`.

fluttergithubbot · 2020-07-07T17:34:02Z

This pull request is not suitable for automatic merging in its current state.

The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

This reverts commit 2ab5099.

) This change makes it so that we track all the motion events encountered by `FlutterView` and all of its subviews in the `MotionEventTracker` class, indexed by a unique `MotionEventId`. This identifier is then passed to the Flutter framework as seen in flutter/flutter#60930. Once the gestures take part in gesture disambiguation and are sent back to the engine, we look-up the original motion event using the `MotionEventId` and dispatch it to the platform. Bug: flutter/flutter#58837

This field is used to look-up the event in the platform that resuted a given `PointerEvent`. This is currently only used on Android, where the `embedderId` is set to be the `motionEventId` for a given `MotionEvent`. Roll engine to d0d6a4c

…61010) This reverts commit 2ab5099.

fluttergithubbot added the framework flutter/packages/flutter repository. See also f: labels. label Jul 6, 2020

googlebot added the cla: yes label Jul 6, 2020

blasten requested review from blasten and goderbauer July 6, 2020 21:43

iskakaushik mentioned this pull request Jul 6, 2020

Track motion events for reuse post gesture disambiguation flutter/engine#19484

Merged

goderbauer reviewed Jul 6, 2020

View reviewed changes

iskakaushik force-pushed the preserve-motion-events branch from 338c4c2 to fc290a2 Compare July 7, 2020 00:07

iskakaushik changed the title ~~Android gestures will now contain motionEventId~~ Add embedderId to PointerEvent Jul 7, 2020

goderbauer approved these changes Jul 7, 2020

View reviewed changes

iskakaushik force-pushed the preserve-motion-events branch from fc290a2 to 4cb3686 Compare July 7, 2020 00:20

blasten approved these changes Jul 7, 2020

View reviewed changes

iskakaushik force-pushed the preserve-motion-events branch 2 times, most recently from 5dffc18 to ab9c057 Compare July 7, 2020 06:31

iskakaushik added the waiting for tree to go green label Jul 7, 2020

fluttergithubbot removed the waiting for tree to go green label Jul 7, 2020

iskakaushik force-pushed the preserve-motion-events branch from 446d655 to 32fc8c5 Compare July 7, 2020 16:09

iskakaushik added the waiting for tree to go green label Jul 7, 2020

Kaushik Iska added 2 commits July 7, 2020 09:28

Add embedderId to PointerEvent

b04bfe6

This field is used to look-up the event in the platform that resuted a given `PointerEvent`. This is currently only used on Android, where the `embedderId` is set to be the `motionEventId` for a given `MotionEvent`.

Roll engine to d0d6a4c

ee59f09

iskakaushik force-pushed the preserve-motion-events branch from 7a90f12 to ee59f09 Compare July 7, 2020 16:29

fluttergithubbot removed the waiting for tree to go green label Jul 7, 2020

iskakaushik merged commit 2ab5099 into flutter:master Jul 7, 2020

iskakaushik deleted the preserve-motion-events branch July 7, 2020 17:59

iskakaushik mentioned this pull request Jul 7, 2020

Synthesize gesture events in hybrid mode on Android #58837

Closed

iskakaushik pushed a commit to iskakaushik/flutter that referenced this pull request Jul 7, 2020

Revert "Add embedderId to PointerEvent (flutter#60930)"

ce66c14

This reverts commit 2ab5099.

iskakaushik added a commit that referenced this pull request Jul 7, 2020

Revert "Add embedderId to PointerEvent (#60930)" (#61010)

8ed8b06

This reverts commit 2ab5099.

mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020

Revert "Add embedderId to PointerEvent (flutter#60930)" (flutter#…

50d1d0b

…61010) This reverts commit 2ab5099.

github-actions bot locked as resolved and limited conversation to collaborators Jul 30, 2021

Add embedderId to PointerEvent #60930

Add embedderId to PointerEvent #60930

Uh oh!

Conversation

iskakaushik commented Jul 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

fluttergithubbot commented Jul 6, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iskakaushik commented Jul 7, 2020

Uh oh!

goderbauer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blasten left a comment

Choose a reason for hiding this comment

Uh oh!

fluttergithubbot commented Jul 7, 2020

Uh oh!

fluttergithubbot commented Jul 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Add `embedderId` to `PointerEvent` #60930

Add `embedderId` to `PointerEvent` #60930

iskakaushik commented Jul 6, 2020 •

edited

Loading