This repository was archived by the owner on Feb 25, 2025. It is now read-only.
[Impeller] Do not reference this in the submit callback for Metal GPU Surfaces
#50361
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
jason-simmons
suggested changes
Feb 5, 2024
| } | ||
| // Reset accumulated damage for current framebuffer | ||
| damage_[texture] = SkIRect::MakeEmpty(); | ||
| damage[texture] = SkIRect::MakeEmpty(); |
Member
There was a problem hiding this comment.
Previously this had been clearing the texture's entry in the damage_ map within the GPUSurfaceMetalImpeller
IIUC that still needs to happen.
Contributor
Author
There was a problem hiding this comment.
Made the damage map into a shared_ptr, which should also reduce some copying.
jason-simmons
approved these changes
Feb 5, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Feb 6, 2024
…or Metal GPU Surfaces (flutter/engine#50361)
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Feb 6, 2024
…142954) flutter/engine@9bd98bc...9c1b6c9 2024-02-05 skia-flutter-autoroll@skia.org Manual roll Dart SDK from 5a5d4c262200 to b62066b42af0 (5 revisions) (flutter/engine#50366) 2024-02-05 tessertaha@gmail.com Fix iOS password autofill prompt dismissal causes layout to resize (flutter/engine#50364) 2024-02-05 31859944+LongCatIsLooong@users.noreply.github.com `visiblePassword` uses ASCII keyboard on iOS (flutter/engine#50293) 2024-02-05 dnfield@google.com [Impeller] Do not reference `this` in the submit callback for Metal GPU Surfaces (flutter/engine#50361) 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 matanl@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
|
Hi @dnfield , will this fix be available at Flutter 3.22 or Flutter 3.19? |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes flutter/flutter#141351 (speculatively - I have not directly reproduced this in an application, but without this change the added test crashes with a segfault in the submit callback).
If the rasterizer gets torn down, the surface gets released and the submit callback may fire on a collected object. Capturing
thisisn't safe. I'm not quite sure how that could happen from the linked stack trace though, since the draw call and the teardown call should be happening on the raster thread, and if the surface was reset then the draw call should've failed earlier...The added test causes a segfault without the change.