Skip to content

Conversation

@matanlurey
Copy link
Contributor

Closes #163561.

If this is the right approach, I'll clean it up a bit and add a few unit tests.

@github-actions github-actions bot added platform-android Android applications specifically engine flutter/engine related. See also e: labels. labels Feb 20, 2025
manual,

/**
* The surface may be reset if the app enters the background.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to be will and not may, because if the image is cleared and the surface is not reset, we'll get the bug you're trying to fix.

jlong texture_id,
jobject image_texture_entry) {
jobject image_texture_entry,
jboolean reset_on_gr_context_destroyed) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: call this something unrelated to gr_context which is a Skia concept only

const fml::jni::ScopedJavaGlobalRef<jobject>& image_texture_entry,
const std::shared_ptr<PlatformViewAndroidJNI>& jni_facade);
const std::shared_ptr<PlatformViewAndroidJNI>& jni_facade,
const ImageLifecycle lifecycle = ImageLifecycle::kReset);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and elsewhere I would probably leave this without a default value. its important that we don't ever forget to pass it, as IIRC there is not a safe default: that is, not disposing the image when the surface is an error and disposing the image when the surface is retained leads to a blank screen.

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM w/ nits

@jonahwilliams
Copy link
Contributor

Oh and tests too, naturally :)

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using request changes to indicate I would like changes made

@matanlurey matanlurey closed this Mar 23, 2025
github-merge-queue bot pushed a commit that referenced this pull request Mar 28, 2025
Fixes #163561
Fixes #156488
Fixes #156489
Fixes #163520

Forked from #163692


Only release background image readers on Android 14. I believe reader
disposal is the ultimate cause of
#162147 , where older android
devices don't seem to handle backgrounding the same way we expect on
newer devices. The result of this is a crash in HWUI, which is
unexpected.

Since we only ever did the background release to work around an ANdroid
14 bug, and because it breaks other functionality like background
playback - we should remove it for all targets besides 14.
zhangyuang pushed a commit to zhangyuang/flutter-fork that referenced this pull request Jun 9, 2025
…er#165942)

Fixes flutter#163561
Fixes flutter#156488
Fixes flutter#156489
Fixes flutter#163520

Forked from flutter#163692


Only release background image readers on Android 14. I believe reader
disposal is the ultimate cause of
flutter#162147 , where older android
devices don't seem to handle backgrounding the same way we expect on
newer devices. The result of this is a crash in HWUI, which is
unexpected.

Since we only ever did the background release to work around an ANdroid
14 bug, and because it breaks other functionality like background
playback - we should remove it for all targets besides 14.
romanejaquez pushed a commit to romanejaquez/flutter that referenced this pull request Aug 14, 2025
…er#165942)

Fixes flutter#163561
Fixes flutter#156488
Fixes flutter#156489
Fixes flutter#163520

Forked from flutter#163692


Only release background image readers on Android 14. I believe reader
disposal is the ultimate cause of
flutter#162147 , where older android
devices don't seem to handle backgrounding the same way we expect on
newer devices. The result of this is a crash in HWUI, which is
unexpected.

Since we only ever did the background release to work around an ANdroid
14 bug, and because it breaks other functionality like background
playback - we should remove it for all targets besides 14.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine flutter/engine related. See also e: labels. platform-android Android applications specifically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ImageExternalTexture should fallback to previous image if next image is unavailable

2 participants