Include TextureViews in the output of Scene.toImage[Sync] #35527
Conversation
…ne into hack_for_texture_view
…ne into hack_for_texture_view
|
I don't think we can include a texture in a canvas, so we don't need to do anything for Picture.toImage, right? |
lib/ui/compositing.dart
Outdated
|
|
||
| @FfiNative<Handle Function(Pointer<Void>, Uint32, Uint32, Handle)>('Scene::toImage') | ||
| external String? _toImage(int width, int height, _Callback<_Image?> callback); | ||
| external String? _toImage(int width, int height, _Image outImage); |
There was a problem hiding this comment.
I'm not sure this is the right choice here.
The current API returns a host copy of the image every time, which can't get lost of GPU context gets lost and is safe to use in the background.
The new API fails in this scenario:
final image = await scene.toImage(40, 40);
// go to background
final data = await image.toByteData(...); // won't work.I think we should try to merge these in the C++ layer for now.
There was a problem hiding this comment.
So we could let the APIs diverge then, but I'm not sure of the exact mechanics here. Since the texture registry is only available on the raster thread, don't we need to both flatten and rasterize on that thread in order to retain the contents?
We can always punt on this for now, but I'd like to figure out what the path forward is.
There was a problem hiding this comment.
i.e. can I tweak the toImage path to flatten/rasterize on the raster thread and still be safe here?
There was a problem hiding this comment.
toImage already posts a task to the raster thread. We'd just need to move the flattening of the layer tree to that task.
I think the path forward would be to refactor Picture::RasterizeToImage so that its guts live on Scene::RasterizeToImage and the scene version knows how to convert a layer tree to a display list on the raster thread (but also can still take a display list if the call is coming from Picture, which already has one).
jonahwilliams
changed the title
| std::variant<sk_sp<DisplayList>, std::shared_ptr<LayerTree>> | ||
| display_list_or_layer_tree_; |
There was a problem hiding this comment.
Why not just keep this as a sk_sp<DisplayList>?
This makes the API a bit more confusing to read when we're just going to flatten it to a displaylist the first time we have to use it anyway.
There was a problem hiding this comment.
I see now. This makes it easier to return from the UI thread in the scene case.
I think we should just make SnapshotDisplayList take a std::shared_ptr<LayerTree> param. If that param is not null, it has to be flattened and set to the dispaly list, and that's only called as non-null from MakeFromLayerTree.
| // Only valid if constructed via MakeFromLayerTree. | ||
| uint32_t width_; | ||
| uint32_t height_; |
There was a problem hiding this comment.
We should be getting this from the image_info_ right?
lib/ui/compositing/scene.cc
Outdated
| const SkImageInfo image_info = SkImageInfo::Make( | ||
| width, height, kRGBA_8888_SkColorType, kPremul_SkAlphaType); | ||
| auto dl_image = DlDeferredImageGPU::MakeFromLayerTree( | ||
| image_info, std::move(layer_tree_), width, height, |
There was a problem hiding this comment.
width, height are redundant with what's stored in the SkImageInfo.
Right - the only way to work with |
jonahwilliams
changed the title
| fml::RefPtr<SkiaUnrefQueue> unref_queue); | ||
|
|
||
| void SnapshotDisplayList(); | ||
| void SnapshotDisplayList(std::shared_ptr<LayerTree> layer_tree = nullptr); |
There was a problem hiding this comment.
nit: add a doc comment about the parameter.
jonahwilliams
added
the
autosubmit
|
gold seems to be stuck, I'm going to give this a kick.... |
2 participants
Update Picture rasterization for toImage to accept a layer tree, optionally flattening on the raster thread if present.
Update Deferred GPU image for toImageSync to accept a layer tree, flattening it the first time it creates an image, storing the resulting display list.
This is also a performance fix for the zoom page transition, which currently does too much work on the UI thread on frame 1
Fixes flutter/flutter#107576