[Impeller] keep imgui hostbuffer alive. #56409
Conversation
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
Code looks good and the fix is reasonable. I would like us to have an automated test for this. Can we just run a playgrounds test for N frames on CI?
|
+1 to trying to make an automated test |
|
I remember chinmay at some point implemented a flag for impeller_unittest that would run for N milliseconds. I thought we wired that up to be run on ci, but I guess not. |
|
Although, arguably, this issue doesn't affect the production code and it doesn't affect CI because we don't run playground in CI, so for the same reason that this PR was ignored by the "there shalt be tests!" bot, I don't think it's a big issue to have a test for this. |
|
I mean as long as we use interactive playground tests we should try to make sure they don't break since it costs developer time when they do break. If we don't want to try to assert they stay functional we should just delete them and rely on impeller_golden_tests. |
|
Expanded the interactive playground test suite to cover GLES and Metal. |
|
+1 to the thanks! |
|
The failure in 1DThreadgroupSizingIsCorrect looks real, investigating... |
jonahwilliams
added
the
autosubmit
…158297) flutter/engine@b36ca33...94dac95 2024-11-06 jonahwilliams@google.com [Impeller] keep imgui hostbuffer alive. (flutter/engine#56409) 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 jsimmons@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#158275 We are now counting on the content context to keep the host buffer alive. the imgui overlay does not use a content context, so it has to manage the lifetime of the host buffer correctly, keeping it alive for as many frames as needed and destroying it in the correct order (before context destruction).
3 participants
Fixes flutter/flutter#158275
We are now counting on the content context to keep the host buffer alive. the imgui overlay does not use a content context, so it has to manage the lifetime of the host buffer correctly, keeping it alive for as many frames as needed and destroying it in the correct order (before context destruction).