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

Conversation

@bdero
Copy link
Member

@bdero bdero commented Nov 16, 2023

Going light on the docstrings so we can get to experimenting faster.

Textures! They can be written to by the host with Texture.overwrite and used as a ui.Image with Texture.asImage.

Right now we need to capture the "texture coordinate system" concept in order for Impeller to handle drawing textures in the right orientation on a Canvas. The user of the API may switch from host authoring to render target usage at any time. Similar to Unity, users targeting GLES will need to use this information to handle the texture flip when sampling from shaders -- this will be important to document comprehensively.

Flutter GPU textures being drawn with a CustomPainter:

Screen.Recording.2023-11-16.at.1.58.03.AM.mov

@bdero bdero self-assigned this Nov 16, 2023
@bdero bdero marked this pull request as ready for review November 16, 2023 11:02
@bdero bdero requested a review from zanderso November 16, 2023 11:07
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

Lets get this moving!

Copy link
Contributor

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

I figure we will keep tinkering on this further but I think we should stick to the underlying API more closely with allocators, descriptors, etc.. This PR is also a fascinating look into the kind of friction we will run into with FFI and something to think more deeply about.


part of flutter_gpu;

base class Texture extends NativeFieldWrapperClass1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be more faithful to the the C++ API with the concept of descriptors and allocators obtaining texture handles?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I'm interested in hearing your thoughts around this. These breaks from Impeller are intentional, but indeed we'll keep tinkering and I'm super open to changing these things.

Will we ever need multiple device allocators? We ended up not using multiple for Entities, and most HALs seem to just tie allocations to the device. We did discuss the possibility of making contexts for multiple devices/having a device selection scheme in the future in the doc. There's a rough plan in case we need it, but those use cases are pretty eccentric.

The Flutter GPU doc removed a lot of descriptors from the beginning, opting for just using optional params in a lot of places instead. I think we should still have them for the more complicated config substructures like RenderPass attachments, but not sure we're gaining anything usability-wise by using them for texture and buffer creation?

@bdero bdero force-pushed the bdero/gpu-textures branch from 3908f7c to 4dffb08 Compare November 17, 2023 01:20
@bdero bdero force-pushed the bdero/gpu-textures branch from 4dffb08 to c0a0b75 Compare November 17, 2023 03:49
@bdero bdero merged commit 1fe3ae6 into flutter:main Nov 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 17, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 17, 2023
flutter/engine@90c3ada...141a01c

2023-11-17 dnfield@google.com Reland "[Impeller] Fail if software backend is chosen and Impeller is enabled on iOS." (flutter/engine#46275)
2023-11-17 matanlurey@users.noreply.github.com Make `impeller/geometry/...` compatible with `.clang-tidy`. (flutter/engine#48154)
2023-11-17 matanlurey@users.noreply.github.com Make `impeller/{archivist|compiler|core|entity}/...` compatible with � (flutter/engine#48153)
2023-11-17 109111084+yaakovschectman@users.noreply.github.com Assign mojom `kSwitch` role to switches (flutter/engine#48146)
2023-11-17 mdebbar@google.com [web] Move scene DOM management to DomManager (flutter/engine#47460)
2023-11-17 30870216+gaaclarke@users.noreply.github.com [Impeller] Unify around "transform" (flutter/engine#48184)
2023-11-17 skia-flutter-autoroll@skia.org Roll Dart SDK from a9c212f2f54b to 03cddb5d740d (1 revision) (flutter/engine#48182)
2023-11-17 matanlurey@users.noreply.github.com Actually make `status_or.h` compatible with `.clang-tidy`. (flutter/engine#48151)
2023-11-17 jonahwilliams@google.com [Impeller] Cleanups to geometry interfaces. (flutter/engine#48180)
2023-11-17 mdebbar@google.com [web] Move all DOM creation to DomManager (flutter/engine#48123)
2023-11-17 15619084+vashworth@users.noreply.github.com Reenable UnobstructedPlatformViewTests testMultiplePlatformViewsWithOverlays (flutter/engine#48139)
2023-11-17 skia-flutter-autoroll@skia.org Roll Dart SDK from 46e8b18047eb to a9c212f2f54b (1 revision) (flutter/engine#48176)
2023-11-17 skia-flutter-autoroll@skia.org Roll Skia from bcd22e8f95bc to 8e9e168418a0 (1 revision) (flutter/engine#48173)
2023-11-17 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from M0zM3CJLIrd5lb0u0... to Bcq9TZdt-vtTSL5YH... (flutter/engine#48172)
2023-11-17 skia-flutter-autoroll@skia.org Roll Skia from c8ee25282849 to bcd22e8f95bc (1 revision) (flutter/engine#48170)
2023-11-17 bdero@google.com [Flutter GPU] Add Textures. (flutter/engine#48118)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from M0zM3CJLIrd5 to Bcq9TZdt-vtT

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 jonahwilliams@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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants