Hook ImageReaderSurfaceProducer to the onTrimMemory listener interface #50792
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
johnmccutchan
force-pushed
the
image_reader_trim_memory
branch
2 times, most recently
from
99feb46
to
35556a1
Compare
| imageReaderQueue.clear(); | ||
| } | ||
| createNewReader = true; | ||
| } |
There was a problem hiding this comment.
This code looks similar to the code below in releaseInternal, but maybe more complete. Can/should we combine these?
Is this something we want to do on all trim levels, or only critical/UI hidden?
There was a problem hiding this comment.
I've factored the common code out. As a separate change we might want to relax this and only purge this on specific trim levels- I can't reproduce the issue though. Maybe you could grab a dump of the sequence of trim levels on the Samsung phone?
There was a problem hiding this comment.
Going to the background will always give the https://developer.android.com/reference/android/content/ComponentCallbacks2#TRIM_MEMORY_UI_HIDDEN
We could do it for the RUNNING_CRITICAL level out of politeness :). The other ones are probably fine to leave, but we can do this as a follow up.
johnmccutchan
force-pushed
the
image_reader_trim_memory
branch
from
35556a1
to
0627d4b
Compare
Added test. |
johnmccutchan
force-pushed
the
image_reader_trim_memory
branch
2 times, most recently
from
3095383
to
b25d118
Compare
| @@ -607,7 +611,15 @@ PerImage dequeueImage() { | |||
| lastDequeueTime = System.nanoTime(); | |||
| } | |||
| } | |||
| // We have dequeued the first image from reader. | |||
| if (lastDequeuedImage != null) { | |||
There was a problem hiding this comment.
How do we know that we're done presenting the image when we dequeueImage?
There was a problem hiding this comment.
This is the contract between image_external_texture* and this class. Making this more explicit is probably a good cleanup to do.
- Close all ImageReaders and Images when we get an onTrimMemory callback. - Remove the first frame fix based around caching the last image displayed because it isn't safe to do on some platforms. Leave a TODO to revisit this.
johnmccutchan
force-pushed
the
image_reader_trim_memory
branch
from
b25d118
to
e73b0f9
Compare
…143875) flutter/engine@52ffcaa...bf5c003 2024-02-21 skia-flutter-autoroll@skia.org Roll Skia from 9d86359b5fe8 to 8fa858855820 (15 revisions) (flutter/engine#50827) 2024-02-21 matanlurey@users.noreply.github.com Add the `scenario_app` `'solid_blue'` golden to the Android test suite (flutter/engine#50801) 2024-02-21 matanlurey@users.noreply.github.com Ignore EOF newline characters and added tests to `dir_contents_diff` tool (flutter/engine#50805) 2024-02-21 jason-simmons@users.noreply.github.com Make the GL context current in EmbedderSurfaceGLImpeller before creating the GPU surface (flutter/engine#50807) 2024-02-21 matanlurey@users.noreply.github.com Fail engine post-submit on skia-gold comparions. (flutter/engine#50826) 2024-02-21 34871572+gmackall@users.noreply.github.com Remove WindowManager reflection in SingleViewPresentation.java (flutter/engine#49996) 2024-02-21 30870216+gaaclarke@users.noreply.github.com [Impeller] applied the lerp hack to blur (roughly 2x speedup?) (flutter/engine#50790) 2024-02-21 jason-simmons@users.noreply.github.com Migrate the Fuchsia embedder to the Dart_RecordTimelineEvent API (flutter/engine#50823) 2024-02-21 john@johnmccutchan.com Hook ImageReaderSurfaceProducer to the onTrimMemory listener interface (flutter/engine#50792) 2024-02-21 matanlurey@users.noreply.github.com Fix the local-only lint errors due to an unexpected `GeneratedPluginRegistrant.java` (flutter/engine#50795) 2024-02-21 jonahwilliams@google.com [Impeller] cache onscreen render targets. (flutter/engine#50751) 2024-02-21 zanderso@users.noreply.github.com Use 'et format' in CI. Check formatting of all files in CI (flutter/engine#50810) 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 jimgraham@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
We have seen some reports of platform views not working after an application is backgrounded and then resumed. According to Android GPU folks ImageReader/Image/HardwareBuffers should be valid after an application has been resumed. However on Samsung we know that isn't the case and there are (unconfirmed) reports of it also impacting Pixel devices.
Should fix flutter/flutter#142978 and flutter/flutter#139039
Also fixes flutter/flutter#143720