Reduce number of surfaces required when presenting platform views #43301

knopp · 2023-06-28T12:47:59Z

Fixes flutter/flutter#129710
Fixes flutter/flutter#138936

Implements https://flutter.dev/go/optimized-platform-view-layers

Example

This scene would normally require 5 surfaces, but with the PR it comes down to 2 (when drawing over platform views) and 1 when all drawing is outside of platform views).

optimized-platform-view-layers.mov

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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 Hixie said 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.

flutter-dashboard · 2023-08-15T12:01:48Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

chinmaygarde · 2023-08-31T20:19:48Z

cc @cbracken @knopp Are we going to make progress on this?

knopp · 2023-08-31T20:23:01Z

#42960 just got merged today. I'll rebase this tomorrow, retest and remove the wip.

knopp · 2023-09-03T14:48:28Z

I'm not sure why verifyb143464703_soft_noxform fails. I tried running it locally but I get all golden failures, even through images seemingly look same. Maybe a slight difference in antialiasing?

chinmaygarde · 2023-09-07T20:14:04Z

The failure should point to both the expectation and the actual image. Can you paste both of them here. If its AA, we should be able to reason about it.

shell/platform/embedder/embedder_render_target_cache.cc

shell/platform/embedder/embedder_external_view_embedder.cc

cbracken · 2023-11-26T20:26:28Z

shell/platform/embedder/embedder_external_view_embedder.cc

+    for (auto i = params->mutatorsStack().Begin();
+         i != params->mutatorsStack().End(); ++i) {
+      const auto& m = *i;
+      if (m->GetType() == MutatorType::kTransform) {


Use a switch statement here; it's wordier, but helps with listing.

cbracken

lgtm with mostly nits.

shell/platform/embedder/embedder_external_view_embedder.cc

… views (flutter/engine#43301)

…139063) flutter/engine@6f499ec...9b610ec 2023-11-27 matej.knopp@gmail.com Reduce number of surfaces required when presenting platform views (flutter/engine#43301) 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 jacksongardner@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

…utter#43301) Fixes flutter/flutter#129710 Fixes flutter/flutter#138936 Implements https://flutter.dev/go/optimized-platform-view-layers ## Example This scene would normally require 5 surfaces, but with the PR it comes down to 2 (when drawing over platform views) and 1 when all drawing is outside of platform views). https://github.com/flutter/flutter/assets/96958/091da832-bfbc-44a2-8da5-d55d84024c96 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [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

renanyoy · 2023-12-06T07:55:21Z

this PR remove the ability to use Opacity(..) around an AppKitView() also it seems that each time a new platform view is created the UI blink during one or 2 frames.

knopp · 2023-12-06T08:48:51Z

this PR remove the ability to use Opacity(..) around an AppKitView() also it seems that each time a new platform view is created the UI blink during one or 2 frames.

I can't reproduce that even with your example. This is AppKitView wrapped with 0.1 opacity:

That said, once opacity is enabled, I can definitely reproduce the flicker. I have reopened flutter/flutter#138936.

…lutter#139063) flutter/engine@6f499ec...9b610ec 2023-11-27 matej.knopp@gmail.com Reduce number of surfaces required when presenting platform views (flutter/engine#43301) 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 jacksongardner@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

knopp requested review from cbracken and chinmaygarde as code owners June 28, 2023 12:47

github-actions bot added platform-android platform-ios platform-macos labels Jun 28, 2023

knopp added the Work in progress (WIP) Not ready (yet) for review! label Jun 28, 2023

knopp force-pushed the platform_views_optimized_layers branch from 1fc99b2 to 4ad904e Compare June 28, 2023 12:52

knopp added affects: desktop and removed platform-ios platform-android labels Jun 28, 2023

knopp force-pushed the platform_views_optimized_layers branch from 4ad904e to e6beedb Compare June 28, 2023 13:26

github-actions bot added platform-android platform-ios labels Jun 28, 2023

knopp force-pushed the platform_views_optimized_layers branch from e6beedb to b3be259 Compare June 28, 2023 13:30

lucalooz mentioned this pull request Jun 28, 2023

Embedder uses too many surfaces for platform views flutter/flutter#129710

Closed

chinmaygarde requested review from gaaclarke and removed request for chinmaygarde and gaaclarke July 6, 2023 20:21

knopp force-pushed the platform_views_optimized_layers branch from b3be259 to 0677c2c Compare July 26, 2023 12:00

cbracken mentioned this pull request Jul 28, 2023

[macOS] Implement unobstructed platform views #42960

Merged

8 tasks

knopp force-pushed the platform_views_optimized_layers branch from 0677c2c to 1f2d059 Compare September 1, 2023 16:38

github-actions bot removed affects: desktop platform-macos labels Sep 1, 2023

knopp changed the title ~~WIP: Reduce number of surfaces when presenting platform views~~ Reduce number of surfaces when presenting platform views Sep 1, 2023

knopp force-pushed the platform_views_optimized_layers branch from 7facd2e to 63a45e2 Compare September 2, 2023 15:45

knopp added platform-macos affects: desktop labels Nov 24, 2023

github-actions bot added platform-ios platform-android and removed affects: desktop platform-macos labels Nov 24, 2023

knopp changed the title ~~Reduce number of surfaces when presenting platform views~~ Reduce number of surfaces required when presenting platform views Nov 24, 2023

knopp force-pushed the platform_views_optimized_layers branch from f4de691 to f77dc51 Compare November 25, 2023 15:32

cbracken reviewed Nov 26, 2023

View reviewed changes

shell/platform/embedder/embedder_render_target_cache.cc Outdated Show resolved Hide resolved

cbracken reviewed Nov 26, 2023

View reviewed changes

shell/platform/embedder/embedder_external_view_embedder.cc Outdated Show resolved Hide resolved

cbracken reviewed Nov 26, 2023

View reviewed changes

cbracken approved these changes Nov 26, 2023

View reviewed changes

shell/platform/embedder/embedder_external_view_embedder.cc Outdated Show resolved Hide resolved

shell/platform/embedder/embedder_external_view_embedder.cc Outdated Show resolved Hide resolved

shell/platform/embedder/embedder_external_view_embedder.cc Show resolved Hide resolved

knopp added 5 commits November 27, 2023 12:06

Reduce number of surfaces when presenting platform views

a1ebf7e

Update golden image

ca58a79

Add comments

f098cf8

wording

3d3e803

Nits

26b0c66

knopp force-pushed the platform_views_optimized_layers branch from d58f1be to 26b0c66 Compare November 27, 2023 11:06

knopp merged commit 9b610ec into flutter:main Nov 27, 2023
27 checks passed

knopp deleted the platform_views_optimized_layers branch November 27, 2023 13:07

engine-flutter-autoroll mentioned this pull request Nov 27, 2023

Roll Flutter Engine from 6f499ecea657 to 9b610ecda771 (1 revision) flutter/flutter#139063

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 27, 2023

9b610ecda Reduce number of surfaces required when presenting platform…

dc7bec0

… views (flutter/engine#43301)

jmagman mentioned this pull request Feb 16, 2024

[iOS] What is the point of the transform mutators? flutter/flutter#143566

Open

knopp mentioned this pull request Mar 20, 2024

[web] explicit platform view compositing modes flutter/flutter#145360

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce number of surfaces required when presenting platform views #43301

Reduce number of surfaces required when presenting platform views #43301

knopp commented Jun 28, 2023 •

edited

flutter-dashboard bot commented Aug 15, 2023

chinmaygarde commented Aug 31, 2023

knopp commented Aug 31, 2023

knopp commented Sep 3, 2023 •

edited

chinmaygarde commented Sep 7, 2023

cbracken Nov 26, 2023

cbracken left a comment

renanyoy commented Dec 6, 2023 •

edited

knopp commented Dec 6, 2023

Reduce number of surfaces required when presenting platform views #43301

Reduce number of surfaces required when presenting platform views #43301

Conversation

knopp commented Jun 28, 2023 • edited

Example

Pre-launch Checklist

flutter-dashboard bot commented Aug 15, 2023

chinmaygarde commented Aug 31, 2023

knopp commented Aug 31, 2023

knopp commented Sep 3, 2023 • edited

chinmaygarde commented Sep 7, 2023

cbracken Nov 26, 2023

Choose a reason for hiding this comment

cbracken left a comment

Choose a reason for hiding this comment

renanyoy commented Dec 6, 2023 • edited

knopp commented Dec 6, 2023

knopp commented Jun 28, 2023 •

edited

knopp commented Sep 3, 2023 •

edited

renanyoy commented Dec 6, 2023 •

edited