Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decide what parts of the Java embedder rendering APIs to deprecate #148557

Open
matanlurey opened this issue May 17, 2024 · 2 comments
Open

Decide what parts of the Java embedder rendering APIs to deprecate #148557

matanlurey opened this issue May 17, 2024 · 2 comments
Assignees
Labels
c: proposal A detailed proposal for a change to Flutter c: tech-debt Technical debt, code quality, testing, etc. platform-android Android applications specifically team-android Owned by Android platform team

Comments

@matanlurey
Copy link
Contributor

In Flutter 3.22, we've formally instructed users to stop using the {create|register}SurfaceTexture family of APIs and instead to use createSurfaceProducer, which, as an implementation detail, picks the right Surface-backed implementation. This is due to Impeller using Vulkan on newish Android APIs, and Vulkan not supporting OpenGL-based SurfaceTextures.

The next step in #139702 is deprecate APIs we don't expect users to need or use anymore (as of flutter/engine#52892 we issue error messages on the C++ engine side). For sake of scope, let's focus on io.flutter.view.TextureRegistry and known implementing classes.

Here is my proposal of APIs to deprecate, but I'd like feedback:

P1: Will no longer work in Impeller

We should do this before Impeller becomes the default.

API Action
.SurfaceTextureEntry Deprecate, we want users using SurfaceProducer.
.createSurfaceTexture Deprecate, we want users using SurfaceProducer.
.registerSurfaceTexture Deprecate, we want users using SurfaceProducer.

P2: Unused or no longer useful as part of the public API

These are nice-to-have cleanups, we should do the ones that make sense.

API Action
.GLTextureConsumer Remove. AFAICT this is unused code.
.ImageConsumer Make internal after registerImageTexture is no longer public.
.ImageTextureEntry Make internal after registerImageTexture is no longer public.
.OnFrameConsumedListener Remove or make @VisibleForTesting, 1 usage.
.OnTrimMemoryListener Make internal, used in one place internally only.
.TextureEntry Remove once there is only SurfaceProducer.
.createImageTexture Make internal only, 1 usage internally.
.onTrimMemory Make internal only.

Assigned @jonahwilliams and @johnmccutchan to review while I'm on vacation 😎

@matanlurey matanlurey added platform-android Android applications specifically c: proposal A detailed proposal for a change to Flutter c: tech-debt Technical debt, code quality, testing, etc. fyi-android For the attention of Android platform team team-engine Owned by Engine team labels May 17, 2024
@jonahwilliams jonahwilliams added team-android Owned by Android platform team and removed team-engine Owned by Engine team labels May 20, 2024
@flutter-triage-bot flutter-triage-bot bot removed the fyi-android For the attention of Android platform team label May 20, 2024
@flutter-triage-bot
Copy link

The fyi-android label is redundant with the team-android label.

@matanlurey matanlurey added fyi-engine For the attention of Engine team and removed fyi-engine For the attention of Engine team labels May 20, 2024
@johnmccutchan
Copy link
Contributor

.createImageTexture should be moved into the P0 group (the only internal use is easily replaced with SurfaceProducer).

We might need to make onTrimMemory and OnTrimMemoryListener part of the public API- as a work around for the Android 14 bug, we now destroy the ImageReader (and its underlying Surface) when the trim memory callback fires. Any users will need to grab a fresh Surface pointer and reconfigure their uses.

@reidbaker reidbaker self-assigned this May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: proposal A detailed proposal for a change to Flutter c: tech-debt Technical debt, code quality, testing, etc. platform-android Android applications specifically team-android Owned by Android platform team
Projects
None yet
Development

No branches or pull requests

5 participants