Repository navigation
Conversation
| - **HCPP enabled (the default on master and beta):** Impeller presents through a `SurfaceControl` child layer on API 29+. Nothing reaches the `Surface`, so `getBitmap()` returns `null`, even with no platform views on screen. | ||
| - **Hybrid Composition:** Flutter UI drawn over a platform view lives in separate overlay surfaces, so it is dropped. | ||
|
|
||
| Android has no public API to read back those layers, so this approach cannot be made complete. |
There was a problem hiding this comment.
This is extremely unfortunate if true... Have we reached out to the Android team to ask if there are workarounds, or if they intend to add these APIs in the future?
This makes me rethink the decision to skip the screenshot embedder API. Assuming there is no workarounds, adding the screenshot embedder API would allow us to continue to support FlutterRenderer.getBitmap().
There was a problem hiding this comment.
As far as I can tell, this is true. I have not reached out to the Android team yet.
Even if we added the embedder API this would still have the same support story for platform views as a Dart framework call (which is to say, the same support story as the current getBitmap call - since they use the same code underneath the covers).
The biggest difference being the framework call does not block the main thread.
There was a problem hiding this comment.
@gmackall - FYI, https://b.corp.google.com/issues/494307996, I believe the same issue that prevents the Magnifier loupe from working also prevents use from getting the SurfaceControl pixels for screenshots.
There was a problem hiding this comment.
The biggest difference being the framework call does not block the main thread.
If we add a screenshot embedder API, I suspect that would be asynchronous. FlutterRenderer.getBitmap() would need to block to maintain backwards compatibility, but we could deprecate it in favor of an asynchronous alternative that doesn't block the platform thread.
|
|
||
| ### Phase 2: Remove | ||
|
|
||
| After the deprecation has shipped in at least one stable release, remove: |
There was a problem hiding this comment.
When the framework allowed breaking changes, we would wait 4 stable releases or 1 year, whichever is longer. I would recommend we wait at least as long.
There was a problem hiding this comment.
Oof. That's a long time, also considering the deprecation story for the Embedder API - that could push things back a bit.
There was a problem hiding this comment.
The unfortunate reality is this is public api in the Android embedder - almost all of the embedder is public api technically. We cannot sit for 4 releases - so I really think a single release deprecation period is ample considering the two actively maintained packages that use it could be updated (as well as any 1p partners) to this other method - have better performance and have the exact same support story.
…we'd still need to deprecate the old method though)
| - In AOSP, compositing and reading back a `SurfaceControl` subtree (`android.window.ScreenCapture.captureLayers`) is `@hide` / `@TestApi` and requires the signature `READ_FRAME_BUFFER` permission because child `SurfaceControl` layers can host cross-process `SurfaceControlViewHost` or `FLAG_SECURE` content. | ||
| - Even if a future Android version added a public `ASurfaceControl` readback API, `PixelCopy` would still fail on Android 10–16 (API 29–36), where HCPP is active. | ||
| - In AOSP, `AccessibilityInteractionController` composites a window's `SurfaceControl` subtree in-process by calling `android.window.ScreenCapture.captureLayers` on `ViewRootImpl.getSurfaceControl()` with `.setChildrenOnly(false).setUid(Process.myUid())`, but both APIs are `@hide` and blocked by Android's non-SDK interface restrictions (`blacklist`) for callers outside `/system/framework/framework.jar`. | ||
| - Android 17 (API 37) adds `AttachedSurfaceControl.createMirror()`, which lets an app mirror its window `SurfaceControl` hierarchy (filtering out layers from other UIDs) into a `VirtualDisplay` and `ImageReader`. However, that path is asynchronous, requires setting up a `VirtualDisplay` and waiting for a composition pass, and does not help on Android 10–16 (API 29–36), where HCPP is active. |
There was a problem hiding this comment.
hmm do you mean on 14-16? HCPP is only active on 34+
Or am I misreading this line
There was a problem hiding this comment.
Could be a bug.
CreateContextSettings forwards enable_surface_control (EnableHcpp=true, the default on master and beta) to ContextVK without the API 34 check, and swapchain_vk.cc activates AHBSwapchainVK ("ImpellerSurface") on API 29+ (surface_control.cc) at startup
AHBSwapchainVK::IsAvailableOnPlatform() checks api_level >= 29:
if (ContextVK::Cast(*context).GetShouldEnableSurfaceControlSwapchain() &&
AHBSwapchainVK::IsAvailableOnPlatform()) {
There was a problem hiding this comment.
Hmm yeah I think actually
https://github.com/flutter/flutter/blob/b98afb52407777ee1b2544b93b064335ed56e340/engine/src/flutter/impeller/renderer/backend/vulkan/swapchain/swapchain_vk.cc#L52
hcpp is actually 34+, but the AHB swapchain is still used sub 34, which is really the underlying reason the pixelcopy would fail (driving via surfacecontrol vs surface)
And seems intentional AFAICT
flutter/flutter#166943
There was a problem hiding this comment.
I was able to reproduce flutter/flutter#155877 on master with API 29 because EnableHcpp=true, so I think the bug was re-introduced and a proper api gate needs to be added.
There was a problem hiding this comment.
Hmm interesting, perhaps we should undo
flutter/flutter#166943
? (Or, at least the swapchain gating part)
I wonder why it was made?
A proposal to deprecate FlutterRenderer.getBitmap
Tracking issue: flutter/flutter#195720
Pre-launch Checklist
design doc.dart run bin/assign_rfc_number.dartlocally when instructed).