Perform device to host texture transfer on the IO thread.

[chinmaygarde]

Capturing a snapshot of a scene is extremely expensive (multiple frame intervals worth of time) and needs to happen on the GPU thread within a frame workload. Because of this, operations like Scene.toImage and Picture.toImage can potentially cause extreme amounts of jank in a Flutter application.

The sequence of operations that need to be performed to realize a host snapshot allocation are as follows:

• The application on the UI thread calls Scene.toImage or Picture.toImage. The scene first needs to be converted into a device image before moving the device image allocation to the host.
• Since the scene itself may reference other device images, the realization of the device image for the snapshot can only happen on a thread with a GPU context. The UI threads does not have such a context. It must therefore be submitted to either the GPU or IO threads. The GPU and IO thread contexts are in the same sharegroup and OpenGL image handles can be shared between the same. However, Skia maintains state along with the handle that is not safe to access from multiple threads concurrently. The restriction means that while texture backed Skia images can be used on either the GPU or IO thread contexts, the images may not be used simultaneously on both. While the engine would ideally like to realize the device image for the snapshot on the IO thread, it cannot do so because the GPU thread may be using that image to render the same onscreen. So, the engine is forced to do this step on the GPU task runner.
• While having to realize device image for the snapshot on the GPU thread is unfortunate, it is nowhere near the bottleneck when it comes to the total time taken on the CPU. That step is the device to host transfer. Ideally, the texture used as the color attachment of the framebuffer could be stolen as a Skia image and sent to IO thread where the device to host transfer can happen. However, the only available API for the creation of a snapshot from a surface (the Skia abstraction for a render target / framebuffer) returns a regular SkImage (SkSurface::makeImageSnapshot). Only images created using SkImage::MakeCrossContextFromPixmap may be shared between contexts. This makes the resulting image unsuitable for cross context sharing for purposes of device to host transfer. The engine also cannot steal the GrBackendRender target from the SkSurface and create a cross context image from the same because the only constructor for the creation of a cross context image is a pixmap. The entire point of the dance is to get a pixmap. Out of options, the engine uses the SkSurface::makeImageSnapshot call on the GPU followed immediately by the SkImage::makeRasterImage call to perform the device to host transfer. All frames rendered by the Flutter application are now susceptible to jank.

To fix the performance issues mentioned above, the entire series of operations were moved to the IO thread. This was because the concurrent use restrictions was not apparent to engine authors. Once issues related to this violation were surfaced in https://github.com/flutter/flutter/issues/43085, the patch was reverted in https://github.com/flutter/engine/pull/13467.

To get back the performance improvements while maintaining threading correctness, one of the following API enhancements from Skia would be necessary:

• The ability to create cross context images from backend textures or backend render targets. The engine can then steal the color attachment of the framebuffer and use that to create a cross context texture on the GPU thread. The image can then be sent to the IO thread for device to host transfer.
• Make the SkSurface::makeImageSnapshot return a cross context image directly.
• Internally, Skia fences access to the data structures so that concurrent read access to the texture is possible.

[chinmaygarde]

cc @gaaclarke @jason-simmons @liyuqian

[gaaclarke]

However, the only available API for the creation of a snapshot from a surface (the Skia abstraction for a render target / framebuffer) returns a regular SkImage

It seems like if we were taking advantage of texture caches we wouldn't have the issue with shared contexts since we'd just be able to get an address in memory we could memcpy from.

Maybe the Metal renderer doesn't have this issue? This seems like a very OpenGL problem.

[liyuqian]

Also CC @brianosman as this seems to be an ask for Skia.

[brianosman]

There is a way to move the texture from an SkImage across thread boundaries that (I think) will be sufficient for this case. SkImage::MakeBackendTextureFromSkImage (https://cs.chromium.org/chromium/src/third_party/skia/include/core/SkImage.h?q=MakeBackendTextureFromSk+file:%5Esrc/third_party/skia/+package:%5Echromium$&g=0&l=1095) lets you take a texture-backed SkImage and extract the GrBackendTexture from it. If the SkImage has no other references at the time (eg, you std::move the last reference into that function), then it avoids making a copy and destroys the SkImage, so the GrBackendTexture becomes the sole owner of the texture.

At that point, you can move the GrBackendTexture across the thread boundary, and re-import it on the IO thread with SkImage::MakeFromTexture. You can use the IO thread's context to do the read-back, and then do the final cleanup.

[cmkweber]

Is there an ETA for this issue? This is a significant blocker in our application: https://github.com/flutter/flutter/issues/40990

Ideally this would address screenshots taken onscreen with toImage (what this issue is referencing), but also an offscreen canvas (as mentioned in the linked issue).

[cmkweber]

@chinmaygarde - Any chance this could be looked at following Metal integration?

[joshualitt]

Just to give a brief update on this issue, we spoke with the Skia team offline. They were no longer comfortable recommending the approach mentioned above. Instead, they suggested trying an asynchronous read pixels(#81239). However, ideally we'd do both, an asynchronous read pixels to avoid blocking the CPU, and performing the device to host texture transfer on the IO thread. I'm going to leave this bug open, and focus on the async read pixels. Hopefully we can return to this later, but we may need a new API first.

[flutter-triage-bot[bot]]

This issue is assigned but has had no recent status updates. Please consider unassigning this issue if it is not going to be addressed in the near future. This allows people to have a clearer picture of what work is actually planned. Thanks!