Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@johnmccutchan
Copy link
Contributor

@johnmccutchan johnmccutchan commented Dec 7, 2023

The SurfaceProducer interface exposes only exposes a Surface to render on, abstracting away the consumer side of the external texture.

The ImageReaderSurfaceProducer implementation of this interface is included as well as a basic test of that code.

Also, a small refactor so that ImageTexture and ImageReaderSurfaceProducer can use the same native C++ code in the engine.

Subsequent CLs will need to address the following:

  • A SurfaceTextureSurfaceProducer (your eyes are probably bleeding from that name) implementation is needed so we can support GL based systems.
  • Update Platform Views to use this new SurfaceProducer type instead of the legacy types.
  • Deprecate SurfaceTexture and ImageTexture external texture types.

Related issue #139702

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, mostly nits on the Android API.

Copy link
Contributor

@matanlurey matanlurey left a comment

Choose a reason for hiding this comment

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

Thanks Jonah for doing such a strong initial review.

LGTM!

return;
}
onImage(image);
maybeCloseReader();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Maybe this should go in a finally block? I'm not sure of the API - maybe IllegalStateException is the only possible outcome and it's safe.

The SurfaceProducer interface exposes only exposes a Surface to render on,
abstracting away the consumer side of the external texture.

The `ImageReaderSurfaceProducer` implementation of this interface is included as
well as a basic test of that code.

Also, a small refactor so that ImageTexture and ImageReaderSurfaceProducer can use
the same native C++ code in the engine.

Subsequent CLs will need to address the following:

- A SurfaceTextureSurfaceProducer (your eyes are probably bleeding from that name) implementation is needed so we can support GL based systems.
- Update Platform Views to use this new SurfaceProducer texture type instead of the legacy types.
- Deprecate SurfaceTexture and ImageTexture external texture types.

Issue #139702
@johnmccutchan johnmccutchan merged commit 7dc51b8 into flutter:main Dec 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 8, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 8, 2023
flutter/engine@03c5f01...7dc51b8

2023-12-08 [email protected] Add a new external texture type to Android embedder (flutter/engine#48803)

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 [email protected],[email protected],[email protected] 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants