Fix some issues in the integration between EmbedderExternalViewEmbedder and Impeller#184905
Merged
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the embedder's external view rendering by moving the rendering logic from EmbedderExternalView to EmbedderExternalViewEmbedder and splitting it into distinct Skia and Impeller paths. EmbedderExternalView::Render now accepts a DlCanvas, and a frame_boundary flag is introduced to manage host buffer resets in Impeller. Additionally, a new Metal unit test and associated fixtures are added to verify Impeller platform view rendering. Review feedback recommends providing a cull rect to DisplayListBuilder for optimization and adding size validation assertions in the Impeller path for consistency.
Contributor
|
I think @knopp has better background for reviewing this? |
knopp
reviewed
May 13, 2026
| @@ -188,9 +101,6 @@ bool EmbedderExternalView::Render(const EmbedderRenderTarget& render_target, | |||
| slice_->render_into(&dl_canvas); | |||
| dl_canvas.RestoreToCount(restore_count); | |||
| dl_canvas.Flush(); | |||
Member
There was a problem hiding this comment.
Overall this looks good to me. But I'm wondering if we should flush the canvas after each slice. At least for Skia this submits the direct context, which only needs to be done per surface, not after each view.
Member
Author
There was a problem hiding this comment.
The flush has apparently been there since the first implementation of EmbedderExternalViewEmbedder (591f55b)
Not sure if it's obsolete now. But I'd prefer not to change it in this PR in order to avoid unintentionally introducing a regression.
Member
There was a problem hiding this comment.
I don't think it's obsolete, just that it only needs to be called for last view in the surface. I also don't think it should be changed in this PR.
Member
Author
There was a problem hiding this comment.
Looking at it some more, I think it should be safe to flush the Skia canvas after all of the EmbedderExternalViews in a Layer have been rendered to the canvas (instead of flushing for each EmbedderExternalView).
Changed the PR to move the DlCanvas::Flush call from EmbedderExternalView::Render to Layer::RenderFlutterContentsSkia.
Does that look correct?
Member
Author
…er and Impeller 1) Fix inaccurate Impeller rendering of embedder layers containing multiple Flutter content slices. All of the Flutter content slices within a layer will be merged into a single display list. The display list is then rendered with one call to impeller::RenderToTarget. Before this change, each Flutter slice in a layer made its own call to impeller::RenderToTarget. This caused earlier slices to be overwritten by the output of later slices. 2) Pass reset_host_buffer=true only for the frame's last call to impeller::RenderToTarget Setting the reset_host_buffer flag advances to the next device buffer within the impeller::HostBuffer associated with the Impeller context. This should only be done at a frame boundary after all of the frame's content has been rendered. See flutter#184091
knopp
approved these changes
May 21, 2026
jason-simmons
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
This was referenced May 22, 2026
This was referenced May 22, 2026
auto-submit Bot
pushed a commit
to flutter/packages
that referenced
this pull request
May 26, 2026
Roll Flutter from e03b91f1fe34 to f3a4b9897834 (63 revisions) flutter/flutter@e03b91f...f3a4b98 2026-05-26 [email protected] Update batch release doc to reflect latest workflow (flutter/flutter#186979) 2026-05-26 [email protected] Roll Skia from 0442274cc696 to 27a819894f7c (5 revisions) (flutter/flutter#187094) 2026-05-26 [email protected] [Tool Robustness] Gracefully handle asynchronous subprocess crashes and connection timeouts (flutter/flutter#186964) 2026-05-26 [email protected] [pubspec] Bump Dart SDK constraint to ^3.13.0 (flutter/flutter#186957) 2026-05-26 [email protected] Roll Dart SDK from 7eb54169841d to 00e625453c43 (1 revision) (flutter/flutter#187086) 2026-05-26 [email protected] [Impeller] Retire Y-coord-scale plumbing by flipping GLES at the vertex stage (flutter/flutter#186556) 2026-05-26 [email protected] Roll Skia from f4f294bdf98d to 0442274cc696 (2 revisions) (flutter/flutter#187079) 2026-05-26 [email protected] [flutter_tools] Fix version cache poisoning from git environment variables (flutter/flutter#186595) 2026-05-26 [email protected] [Tool] Handle DTD connection failures gracefully in widget-preview (flutter/flutter#186952) 2026-05-25 [email protected] Roll Skia from 9d1adb5f2427 to f4f294bdf98d (1 revision) (flutter/flutter#187056) 2026-05-25 [email protected] Roll Skia from 4dd78179e6ec to 9d1adb5f2427 (1 revision) (flutter/flutter#187048) 2026-05-25 [email protected] Roll Skia from 1f26101197bf to 4dd78179e6ec (4 revisions) (flutter/flutter#187044) 2026-05-24 [email protected] Roll Skia from bbe9ccc2bdbf to 1f26101197bf (1 revision) (flutter/flutter#187016) 2026-05-24 [email protected] Roll Fuchsia Linux SDK from nsgcNDlZOuweOvy3Q... to Itd2Jq_ZIABH2rW7B... (flutter/flutter#187032) 2026-05-23 [email protected] Roll Dart SDK from 7e0f28eb5315 to 7eb54169841d (1 revision) (flutter/flutter#187005) 2026-05-23 [email protected] Roll Dart SDK from 90e55fa88456 to 7e0f28eb5315 (1 revision) (flutter/flutter#186990) 2026-05-23 [email protected] Roll Fuchsia Linux SDK from 6T6BY9PTftoG3vP_1... to nsgcNDlZOuweOvy3Q... (flutter/flutter#186984) 2026-05-23 [email protected] iOS] Migrate VSyncClient to a pure Obj-C implementation (#186166) (flutter/flutter#186935) 2026-05-23 [email protected] Disables embedder_tests.cm for fuchsia (flutter/flutter#186969) 2026-05-23 [email protected] Roll Dart SDK from b8414c46f6c7 to 90e55fa88456 (2 revisions) (flutter/flutter#186977) 2026-05-22 [email protected] Roll Skia from 6fdb013d1953 to bbe9ccc2bdbf (1 revision) (flutter/flutter#186980) 2026-05-22 [email protected] [web] Fix cutoff text in WebParagraph (flutter/flutter#186819) 2026-05-22 [email protected] fix(web): Removes the iterative downscaling hack (flutter/flutter#186914) 2026-05-22 [email protected] opts the linux embedder into sdf rendering (flutter/flutter#186909) 2026-05-22 [email protected] Roll Skia from dae8778ca40d to 6fdb013d1953 (5 revisions) (flutter/flutter#186970) 2026-05-22 [email protected] Fix hooks inputs outputs rebuilt (flutter/flutter#186701) 2026-05-22 [email protected] adds linux impeller integration test for external textures (flutter/flutter#186759) 2026-05-22 [email protected] fix(flutter_tools): defensively catch DWDS unregistered service extension errors (flutter/flutter#186896) 2026-05-22 [email protected] [Impeller] Add golden harness support to the renderer test layer (flutter/flutter#186735) 2026-05-22 [email protected] [web] Remove image codecs from canvaskit_chromium (flutter/flutter#178133) 2026-05-22 [email protected] opts all macos into wide gamut (flutter/flutter#186277) 2026-05-22 [email protected] Roll Skia from 356185490a75 to dae8778ca40d (9 revisions) (flutter/flutter#186949) 2026-05-22 [email protected] Filter out SwiftPM schemes when fetching schemes (flutter/flutter#186006) 2026-05-22 [email protected] Roll Packages from 3754d04 to 69cf959 (1 revision) (flutter/flutter#186950) 2026-05-22 [email protected] Roll Dart SDK from eca46bec956d to b8414c46f6c7 (2 revisions) (flutter/flutter#186944) 2026-05-22 [email protected] Saves a DeviceHolderVK with the CommandPoolVK (flutter/flutter#186749) 2026-05-22 [email protected] Roll Dart SDK from e0d509fd676e to eca46bec956d (1 revision) (flutter/flutter#186922) 2026-05-22 [email protected] [ Tool ] Stop generating widget preview scaffold under $TMP (flutter/flutter#186476) 2026-05-21 [email protected] Fix typo in StretchingOverscrollIndicator docs (flutter/flutter#186897) 2026-05-21 [email protected] Roll Dart SDK from 28c7cb5a8e8d to e0d509fd676e (1 revision) (flutter/flutter#186903) 2026-05-21 [email protected] Fix some issues in the integration between EmbedderExternalViewEmbedder and Impeller (flutter/flutter#184905) 2026-05-21 [email protected] Fix a potential buffer overflow in the animated PNG decoder when parsing malformed fdAT chunks (flutter/flutter#186700) 2026-05-21 [email protected] Roll Skia from 2ff20950975d to 356185490a75 (5 revisions) (flutter/flutter#186892) 2026-05-21 [email protected] Add primitive shadows to rendering benchmark (flutter/flutter#186779) 2026-05-21 [email protected] Move prefetchSwiftPackages to be per platform (flutter/flutter#186468) 2026-05-21 [email protected] Upgrade iOS version (flutter/flutter#186889) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All of the Flutter content slices within a layer will be merged into a single display list. The display list is then rendered with one call to impeller::RenderToTarget.
Before this change, each Flutter slice in a layer made its own call to impeller::RenderToTarget. This caused earlier slices to be overwritten by the output of later slices.
Setting the reset_host_buffer flag advances to the next device buffer within the impeller::HostBuffer associated with the Impeller context. This should only be done at a frame boundary after all of the frame's content has been rendered.
See #184091