From 248b86a64b5a445b332d74f5fc577dbbcf7d55eb Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Thu, 26 Sep 2024 17:34:17 +0000 Subject: [PATCH] Reverts "Reverts "Add `SurfaceProducer#onSurfaceAvailable`, deprecate `onSurfaceCreated`. (#55418)" (#55450)" (flutter/engine#55463) Reverts: flutter/engine#55450 Initiated by: matanlurey Reason for reverting: Fixed forward in https://github.com/flutter/packages/pull/7712. Original PR Author: auto-submit[bot] Reviewed By: {fluttergithubbot} This change reverts the following previous change: Reverts: flutter/engine#55418 Initiated by: bdero Reason for reverting: [Engine->Framework roll breakage](https://github.com/flutter/flutter/issues/155727#issuecomment-2375489803) Original PR Author: matanlurey Reviewed By: {jonahwilliams} This change reverts the following previous change: Closes https://github.com/flutter/flutter/issues/155131. Not only did I rename the method, but I also changed the contract slightly - now `onSurfaceAvailable` is _only_ invoked _after_ `onSurfaceDestroyed` has been called. The cost is a single `boolean`, and it honestly makes the API make a lot more sense than someone having to track this themselves. /cc @johnmccutchan (OOO), and @flutter/android-reviewers. --- .../engine/renderer/FlutterRenderer.java | 13 +++- .../io/flutter/view/TextureRegistry.java | 65 ++++++++++++++++--- .../engine/renderer/FlutterRendererTest.java | 5 +- 3 files changed, 71 insertions(+), 12 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java index 7edaf4932f..1f4c601d98 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java @@ -108,8 +108,9 @@ public class FlutterRenderer implements TextureRegistry { public void onResume(@NonNull LifecycleOwner owner) { Log.v(TAG, "onResume called; notifying SurfaceProducers"); for (ImageReaderSurfaceProducer producer : imageReaderProducers) { - if (producer.callback != null) { - producer.callback.onSurfaceCreated(); + if (producer.callback != null && producer.notifiedDestroy) { + producer.notifiedDestroy = false; + producer.callback.onSurfaceAvailable(); } } } @@ -462,6 +463,13 @@ public class FlutterRenderer implements TextureRegistry { // will be produced at that size. private boolean createNewReader = true; + /** + * Stores whether {@link Callback#onSurfaceDestroyed()} was previously invoked. + * + *
Used to avoid signaling {@link Callback#onSurfaceAvailable()} unnecessarily. + */ + private boolean notifiedDestroy = false; + // State held to track latency of various stages. private long lastDequeueTime = 0; private long lastQueueTime = 0; @@ -689,6 +697,7 @@ public class FlutterRenderer implements TextureRegistry { cleanup(); createNewReader = true; if (this.callback != null) { + notifiedDestroy = true; this.callback.onSurfaceDestroyed(); } } diff --git a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java index c9c9836bf2..23991b0981 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java @@ -96,8 +96,8 @@ public interface TextureRegistry { /** * Sets a callback that is notified when a previously created {@link Surface} returned by {@link - * SurfaceProducer#getSurface()} is no longer valid, either due to being destroyed or being - * changed. + * SurfaceProducer#getSurface()} is no longer valid due to being destroyed, or a new surface is + * now available (after the previous one was destroyed) for rendering. * * @param callback The callback to notify, or null to remove the callback. */ @@ -106,18 +106,65 @@ public interface TextureRegistry { /** Callback invoked by {@link #setCallback(Callback)}. */ interface Callback { /** - * Invoked when a previous surface is now invalid and a new surface is now available. + * An alias for {@link Callback#onSurfaceAvailable()} with a less accurate name. * - *
Typically plugins will use this callback as a signal to redraw, such as due to the - * texture being resized, the format being changed, or the application being resumed after - * being suspended in the background. + * @deprecated Override and use {@link Callback#onSurfaceAvailable()} instead. */ - void onSurfaceCreated(); + @Deprecated(since = "Flutter 3.27", forRemoval = true) + default void onSurfaceCreated() {} /** - * Invoked when a previous surface is now invalid. + * Invoked when an Android application is resumed after {@link Callback#onSurfaceDestroyed()}. * - *
Typically plugins will use this callback as a signal to release resources. + *
Applications should now call {@link SurfaceProducer#getSurface()} to get a new + * {@link Surface}, as the previous one was destroyed and released as a result of a low memory + * event from the Android OS. + * + *
+ * {@code + * void example(SurfaceProducer producer) { + * producer.setCallback(new SurfaceProducer.Callback() { + * @override + * public void onSurfaceAvailable() { + * Surface surface = producer.getSurface(); + * redrawOrUse(surface); + * } + * + * // ... + * }); + * } + * } + *+ */ + default void onSurfaceAvailable() { + this.onSurfaceCreated(); + } + + /** + * Invoked when a {@link Surface} returned by {@link SurfaceProducer#getSurface()} is invalid. + * + *
In a low memory environment, the Android OS will signal to Flutter to release resources, + * such as surfaces, that are not currently in use, such as when the application is in the + * background, and this method is subsequently called to notify a plugin author to stop + * using or rendering to the last surface. + * + *
Use {@link Callback#onSurfaceAvailable()} to be notified to resume rendering. + * + *
+ * {@code + * void example(SurfaceProducer producer) { + * producer.setCallback(new SurfaceProducer.Callback() { + * @override + * public void onSurfaceDestroyed() { + * // Store information about the last frame, if necessary. + * // Potentially release other dependent resources. + * } + * + * // ... + * }); + * } + * } + **/ void onSurfaceDestroyed(); } diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java index 7ab861ab40..fe4699de9f 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java @@ -785,7 +785,7 @@ public class FlutterRendererTest { TextureRegistry.SurfaceProducer.Callback callback = new TextureRegistry.SurfaceProducer.Callback() { @Override - public void onSurfaceCreated() { + public void onSurfaceAvailable() { latch.countDown(); } @@ -794,6 +794,9 @@ public class FlutterRendererTest { }; producer.setCallback(callback); + // Trim memory. + ((FlutterRenderer.ImageReaderSurfaceProducer) producer).onTrimMemory(40); + // Trigger a resume. ((LifecycleRegistry) ProcessLifecycleOwner.get().getLifecycle()) .setCurrentState(Lifecycle.State.RESUMED);