Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make pointer events ignore invalid views #51925

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Apr 5, 2024

With this PR, Flutter will now ignore pointer events for an invalid view, i.e. views that have not been add or have been removed.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@dkwingsmt dkwingsmt marked this pull request as draft April 5, 2024 18:58
@flutter-dashboard
Copy link

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

@dkwingsmt dkwingsmt force-pushed the pointer-data-ignore-invalid-views branch from 45fc3df to 3a79047 Compare April 8, 2024 23:47
dkwingsmt added a commit that referenced this pull request Apr 11, 2024
…I doc (#52003)

This PR moves the methods to add or remove views from `Shell` to
`PlatformView`. By design, the `Shell` is supposed to be a messenger
that glues classes together, while `PlatformView` is the operator that
embedders that do not use the embedder API should operate on. The
current design was made due to lack of knowledge to this design.

This also makes `PlatformView` aware of views, which might be a
prerequisite to #51925.

This PR also adds some details to embedder API `AddView` and
`RemoveView`.

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I signed the [CLA].
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
auto-submit bot pushed a commit that referenced this pull request Apr 26, 2024
…roller` (#51952)

This is a refactor that moves the `PointerDataPacketConverter` from `PlatformView` to `RuntimeController`. 

This change is made for the following reasons:
- Currently, the pointer data conversion contains no platform specific logic (because the current converter's only responsibility is to make the event sequence conform Flutter's protocol). Therefore these logics should reside in a platform-independent place.
- The converter typically converts one event to many. It's better to have this conversion later than earlier.
- It removes a member from `PlatformView`, making it closer to a pure virtual class.

The reason to choose `RuntimeController` as the destination is because `RuntimeController` manages a map for views, which is required for the converter to implement a later patch #51925.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
@dkwingsmt dkwingsmt marked this pull request as ready for review June 3, 2024 05:31
@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 4, 2024
Copy link
Contributor

auto-submit bot commented Jun 4, 2024

auto label is removed for flutter/engine/51925, due to - The status or check suite Windows windows_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

  • The status or check suite Windows windows_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Windows windows_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac mac_ios_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac mac_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac mac_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux mac_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux clangd has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux linux_license has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux linux_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux linux_host_desktop_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux linux_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux linux_arm_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux linux_clang_tidy has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux linux_fuchsia has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Windows windows_arm_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux linux_android_emulator_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux local_engine_builds has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants