-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] Implements retry for Picture.toImage
#52470
Conversation
a787c8e
to
e4781b6
Compare
GetDelegate().GetAiksContext(); | ||
if (context) { | ||
context->GetContext()->StoreTaskForGPU( | ||
[context, sync_switch, display_list = std::move(display_list), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to store a weak_ptr to the context here and it didn't work. It looks like AiksContext doesn't have the proper lifespan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you be a bit more specific? Does the AiksContext die during backgrounding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it doesn't have to do with backgrounding. They don't seem to have a lifespan that is as long lived as the impeller context. I didn't look into what their lifespan was. I decided practically it was fine to have a strong retain on it while we don't have the GPU since that means we will not be executing anyways so the idea that the toImage
result will no longer will be relevant when the GPU comes back is low.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SG
GetDelegate().GetAiksContext(); | ||
if (context) { | ||
context->GetContext()->StoreTaskForGPU( | ||
[context, sync_switch, display_list = std::move(display_list), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you be a bit more specific? Does the AiksContext die during backgrounding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…147655) flutter/engine@0014e03...5129b49 2024-05-01 30870216+gaaclarke@users.noreply.github.com [Impeller] Implements retry for `Picture.toImage` (flutter/engine#52470) 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 aaclarke@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
fixes flutter/flutter#146990
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.