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

Multi-view View Metrics #46174

Merged
merged 21 commits into from
Mar 14, 2024
Merged

Multi-view View Metrics #46174

merged 21 commits into from
Mar 14, 2024

Conversation

dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Sep 22, 2023

This PR adds FlutterWindowMetricsEvent.viewId to the embedder API.

This PR only tests the ability to send metrics event for the implicit view. Once multiple views can be added via embedder API, we should test the ability to send different IDs.

Part of flutter/flutter#144806
Part of flutter/flutter#142845

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.

@goderbauer
Copy link
Member

(triage) @dkwingsmt said last week that he will come back to this PR soon.

@dkwingsmt
Copy link
Contributor Author

Yes, I'll finish this once #46213 is merged, since they share the same structure and embedder API changes.

@dkwingsmt dkwingsmt marked this pull request as ready for review March 12, 2024 21:00
@dkwingsmt dkwingsmt requested a review from cbracken as a code owner March 12, 2024 21:00
@dkwingsmt dkwingsmt requested a review from chinmaygarde as a code owner March 12, 2024 21:00
@dkwingsmt dkwingsmt requested a review from loic-sharma March 12, 2024 21:02
Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM but please get approval from @cbracken or @chinmaygarde for the embedder.h ABI

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm for the embedder API change.

@dkwingsmt dkwingsmt merged commit 72ca2e1 into flutter:main Mar 14, 2024
23 checks passed
@dkwingsmt dkwingsmt deleted the mv-set-view-metrics branch March 14, 2024 00:00
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 14, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Mar 14, 2024
…145123)

flutter/engine@6710d10...599dbd7

2024-03-14 737941+loic-sharma@users.noreply.github.com [Embedder API] Add multi-view present callback (flutter/engine#51267)
2024-03-14 skia-flutter-autoroll@skia.org Roll Skia from a315e4572f4e to cac09f152d9b (10 revisions) (flutter/engine#51398)
2024-03-14 dkwingsmt@users.noreply.github.com Multi-view View Metrics (flutter/engine#46174)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC bdero@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants