This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Pass SurfaceFrame to SubmitFrame #18709
Merged
Conversation
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
chinmaygarde
reviewed
Jun 1, 2020
shell/common/rasterizer.cc
Outdated
| external_view_embedder->FinishFrame(); | ||
| std::move(frame)); | ||
| // The frame must be submitted at this point. | ||
| FML_DCHECK(frame->IsSubmitted()); |
There was a problem hiding this comment.
This is a use after move. This check is passing because of undefined behavior.
shell/common/surface_frame.cc
Outdated
| SurfaceFrame::SurfaceFrame(sk_sp<SkSurface> surface, | ||
| bool supports_readback, | ||
| const SubmitCallback& submit_callback) | ||
| : submitted_(false), |
There was a problem hiding this comment.
Initialize these in the header please. That way, if/when new ctors are added, they don't all have to remember to initialize all ivars.
There was a problem hiding this comment.
I see you just moved this TU. I bet this is an old TU. I'd recommend updating it nonetheless.
| @@ -141,6 +142,18 @@ source_set("common") { | |||
| public_configs = [ "//flutter:config" ] | |||
| } | |||
|
|
|||
| source_set("surface_frame") { | |||
There was a problem hiding this comment.
Why is this in its own target? The only place this is being included (flow) also includes common.
There was a problem hiding this comment.
To workaround a circular dependency. :common depends on //flutter/flow, but //flutter/flow depends on :surface_frame.
chinmaygarde
approved these changes
Jun 2, 2020
added 3 commits
June 1, 2020 21:09
blasten
force-pushed
the
refactor_external_view_embedder
branch
from
f2be027 to
0fc6235
Compare
added 3 commits
June 2, 2020 11:09
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 3, 2020
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 4, 2020
This was referenced Jun 4, 2020
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 4, 2020
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 4, 2020
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 4, 2020
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 4, 2020
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jun 5, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This PR refactors the
ExternalViewEmbedderinterface, such that SubmitFrame takes aSurfaceFrameinstead of just the surface'sSkCanvas. As the name implies,SubmitFrameshould be responsible for submitting the frame.After taking this direction, I realized that
FinishFramecan be removed, since the use case (submittingCATransitions) be handled inSubmitFrame.This is required for hybrid composition on Android.
Fixes flutter/flutter#58286