Add platform screenshot provider to take screenshot (iOS) #11059
Conversation
cyanglaz
force-pushed
the
screenshot_provider
branch
from
e25673b to
2511ad1
Compare
| // matrix to identity. | ||
| SkMatrix root_surface_transformation; | ||
| root_surface_transformation.reset(); | ||
| // Prepare an image from the surface, this image may potentially be on the |
There was a problem hiding this comment.
Despite the big diff, here I only added
const auto screen_shot_provider = surface_->GetScreenShotProvider();
if (snapshot_surface == nullptr) { if (screen_shot_provider != nullptr) {
FML_LOG(ERROR) << "Screenshot: unable to create snapshot surface"; potentially_gpu_snapshot = screen_shot_provider->TakeScreenShot();
return nullptr;
}and put everything into an "else" block.
There was a problem hiding this comment.
I don't think I understood your approach with this interface. If you wanted to screenshot the entire view hierarchy with multiple interleaved levels of platform views and Flutter rendered views, you would have to to screenshot all the platform views as individual images and then composite them together with the Flutter overlays to form one image (the screenshot). As prototyped, the PlatformScreenShotProvider will return a single image. It isn't clear how you will manage interleaving of Flutter vs non-Flutter rendered contents.
Also, reading the iOS implementation, it seems like you will end up snapshotting the Flutter view, but that wont contain the contents rendered by Flutter using OpenGL because it doesn't have access to the GPU context.
| @@ -233,6 +233,11 @@ class ExternalViewEmbedder { | |||
|
|
|||
| }; // ExternalViewEmbedder | |||
|
|
|||
| class PlatformScreenShotProvider { | |||
There was a problem hiding this comment.
Nit: Please put new classes and interfaces in their own files. I realize the embedded_views.h is becoming sort of a a catchall for all things embedded platform views related, but we can separate this out in a later commit.
| @@ -233,6 +233,11 @@ class ExternalViewEmbedder { | |||
|
|
|||
| }; // ExternalViewEmbedder | |||
|
|
|||
| class PlatformScreenShotProvider { | |||
| public: | |||
| virtual sk_sp<SkImage> TakeScreenShot() = 0; | |||
There was a problem hiding this comment.
Can you document this in doxygen format please?
Also, we might need to rework this interface once overlays are not screen sized. We will need some sort of offset or ability to apply transformation. Not saying we should do this now, but just something to think about.
| sk_sp<SkImage> potentially_gpu_snapshot; | ||
| const auto screen_shot_provider = surface_->GetScreenShotProvider(); | ||
| if (screen_shot_provider != nullptr) { | ||
| potentially_gpu_snapshot = screen_shot_provider->TakeScreenShot(); |
There was a problem hiding this comment.
This can't be a GPU snapshot though. The overlays don't have access to a GPU context and cannot reference flutter managed resource. This has to be a raster (CPU) snapshot.
|
After offline discussion with @chinmaygarde, it looks like the approach of this PR will only work for simulator. Going to close this PR and rework a solution. |
Background:
For platforms use a PlatformViewLayer(e.g iOS) that composite the UI using native APIs, the screenshot by Skia won't take account of the native UI. For example, screenshot with a webview will show blank in the window that displays the webview on iOS.
We need to have a way to take screenshots with PlatformViewLayer in the scene to unblock screenshot testing for iOS platform views.
What does this PR do:
Introduced a
PlatformScreenshotProviderclass to handle the screenshot using APIs from the platform instead of Skia if necessary.Also added iOS implementation for the `PlatformScreenshotProvider.