Conversation
…/1pFgFYzAKrIFEQQfSz6NbOvKXG6K3oksPQqhKSQZ7t68 TAG=agy CONV=0c48c67f-eb59-40f8-9424-1a74c8da8e46
…ate BUILD.gn line links
| - [`//flutter/shell/platform/embedder:embedder_headers`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/BUILD.gn#L210) and [`//flutter/shell/platform/embedder:embedder_as_internal_library`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/BUILD.gn#L206) (with `defines = [ "FLUTTER_ENGINE_NO_PROTOTYPES" ]` enforcing dynamic `FlutterEngineProcTable` resolution) | ||
| - [`//flutter/fml`](https://github.com/flutter/flutter/tree/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/fml) | ||
| - [`//flutter/common`](https://github.com/flutter/flutter/tree/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/common) | ||
| - [`//flutter/assets`](https://github.com/flutter/flutter/tree/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/assets) |
There was a problem hiding this comment.
Just curious, why is this needed? I would think we could remove this dependency as well. I believe desktop platforms do not depend on this library. Not a big deal either ways, asking for my understanding.
| - Establish an enforced architectural boundary between Android embedder and the engine core. | ||
| - Reduce direct internal engine header inclusions in [`shell/platform/android/`](https://github.com/flutter/flutter/tree/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android) from 98 to 0. | ||
| - Enable independent compilation and stable ABI boundaries for Android embeddings. | ||
| - Maintain complete behavioral correctness, performance parity, and accessibility support across all supported Android API levels and rendering backends (Impeller Vulkan, Impeller OpenGL, and legacy Skia). |
There was a problem hiding this comment.
Maybe consider adding another goal since it is a motivation for this work:
Ensure the embedder API exposes the features needed to create an out-of-tree embedder that is as feature-complete as Flutter Android.
|
|
||
| - Fragile refactoring: Any change to private engine internals (such as Impeller backend refactors or Flow mutator stack updates) breaks the Android embedder. | ||
| - Prolonged rebuild times: Compiling `libflutter.so` requires rebuilding internal engine translation units even when only platform-specific code changes. | ||
| - Lack of API contract: No stable boundary prevents Android embedder code from reaching into private engine internals, which inhibits engine modularization. |
There was a problem hiding this comment.
Another potential maintenance problem:
- Feature parity lag: Because Android and iOS bypass the embedder API, public engine capabilities often lag behind engine internals, which prevents out-of-tree platforms from achieving feature parity with mobile platforms.
|
|
||
| Because `embedder.h` requires a concrete renderer configuration struct (`FlutterRendererConfig` selecting `kOpenGL` or `kVulkan`) at `FlutterEngineInitialize` time, the embedder resolves the graphics backend during initialization before passing renderer pointers to the engine. | ||
|
|
||
| Performing synchronous Vulkan driver queries during `FlutterLoader.ensureInitializationComplete` on Android's main thread introduces a 15 to 30 ms cold-startup delay on budget devices (such as MediaTek or older Mali chipsets) due to driver library loading and physical device property enumeration. |
There was a problem hiding this comment.
Are these Vulkan driver queries needed to choose between Vulkan vs OpenGL backend? Or are these queries used to initialize Vulkan, after we've picked Vulkan as our backend?
I ask as if it's the former, I suspect FlutterOpenGLRendererConfig.setup_callback and FlutterVulkanRendererConfig.setup_callback aren't sufficient since I imagine these callbacks are invoked after we've picked our rendering backend.
| - **Texture Layer Hybrid Composition (TLHC) and Virtual Display (VD) via External Textures**: Both [`configureForTextureLayerComposition`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java#L652) and [`configureForVirtualDisplay`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java#L596) allocate a `PlatformViewRenderTarget` from `TextureRegistry` (`SurfaceProducer` / `AHardwareBuffer` or `SurfaceTexture`) in Java and return its texture ID to the Dart framework `Texture` widget. At the native engine boundary, both TLHC and VD operate strictly through the C-API external texture callbacks (`FlutterEngineRegisterExternalTexture`, `FlutterEngineMarkExternalTextureFrameAvailable`, `FlutterVulkanExternalTextureFrameCallback`, `FlutterHardwareBufferExternalTextureFrameCallback`, and `FlutterOpenGLTexture`) without creating `FlutterPlatformView` layers. | ||
| - **Hybrid Composition (HC) and Hybrid Composition++ (HCPP) via `FlutterCompositor`**: The engine's internal [`EmbedderExternalViewEmbedder`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder_external_view_embedder.h) implements `flutter::ExternalViewEmbedder` behind the public [`FlutterCompositor`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder.h#L2117) interface (`create_backing_store_callback`, `collect_backing_store_callback`, and `present_layers_callback` / `present_view_callback`). `AndroidCompositor` and `AndroidPlatformViewsController` drive both HC and HCPP over `FlutterCompositor` without including internal `//flutter/flow` or `fml::RasterThreadMerger` headers: | ||
|
|
||
| 1. **Backing Store Allocation (`create_backing_store_callback`)**: For the primary Flutter layer (`platform_views_count == 0`), `AndroidCompositor::CreateBackingStore` returns the main surface framebuffer (`GetFBO()`). When a Hybrid Composition platform view splits the scene and Flutter renders UI layers above the platform view, subsequent `CreateBackingStore` calls within the same frame allocate offscreen framebuffers (`AcquireOffscreenFBO`). |
There was a problem hiding this comment.
For the primary Flutter layer (
platform_views_count == 0),AndroidCompositor::CreateBackingStorereturns the main surface framebuffer (GetFBO()).
Does the prototype already do this? I'd be curious how this works. This seems like a good optimization, but AFAICT FlutterBackingStoreConfig does not provide enough information in the create_backing_store_callback to determine if you can render to the main surface framebuffer directly.
| 2. **Raster-Before-Acquire Ordering and Synchronous Platform-Thread Latch**: | ||
| In the legacy architecture, `AndroidExternalViewEmbedder` merged the raster thread onto the platform thread via `fml::RasterThreadMerger` only because it called JNI view hierarchy mutations inline during `SubmitFlutterView`. In `AndroidCompositor::PresentLayers`: | ||
| - On the raster thread, `surface_manager_->Present()` and `surface_manager_->BlitAndSwapOverlaySurface(...)` first complete `eglSwapBuffers` / `vkQueuePresentKHR` into the root `FlutterImageView` and `PlatformOverlayView` `ImageReader` `ANativeWindow` queues for Frame $N$. | ||
| - `AndroidCompositor` packages Frame $N$'s platform view geometry (`offset`, `size`), value-copied `AndroidMutatorsStack`, and overlay rectangles into a single atomic platform-thread frame transaction (`onBeginFrame` $\rightarrow$ `onDisplayPlatformView` $\rightarrow$ `onDisplayOverlaySurface` $\rightarrow$ `onEndFrame`). |
There was a problem hiding this comment.
... single atomic platform-thread frame transaction...
Could you expand on this? I don't follow this bit. Is this the synchronization_fence_fd mechanism described below, or something else?
|
|
||
| At the C-API boundary (`embedder.h`), the four composition modes partition into two external-texture modes and two compositor-layer modes: | ||
|
|
||
| - **Texture Layer Hybrid Composition (TLHC) and Virtual Display (VD) via External Textures**: Both [`configureForTextureLayerComposition`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java#L652) and [`configureForVirtualDisplay`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android/io/flutter/plugin/platform/PlatformViewsController.java#L596) allocate a `PlatformViewRenderTarget` from `TextureRegistry` (`SurfaceProducer` / `AHardwareBuffer` or `SurfaceTexture`) in Java and return its texture ID to the Dart framework `Texture` widget. At the native engine boundary, both TLHC and VD operate strictly through the C-API external texture callbacks (`FlutterEngineRegisterExternalTexture`, `FlutterEngineMarkExternalTextureFrameAvailable`, `FlutterVulkanExternalTextureFrameCallback`, `FlutterHardwareBufferExternalTextureFrameCallback`, and `FlutterOpenGLTexture`) without creating `FlutterPlatformView` layers. |
There was a problem hiding this comment.
FlutterHardwareBufferExternalTextureFrameCallback
Is this explained somewhere? It seems like this is a new API - if so, could you add an API proposal for this one too?
|
|
||
| The discriminated union is what makes this workable on Android. [`SurfaceProducer`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android/io/flutter/view/TextureRegistry.java#L21) hands out [`AHardwareBuffer`](https://developer.android.com/ndk/reference/group/a-hardware-buffer) handles rather than [`VkImage`](https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkImage.html) handles, and `ycbcr_conversion_info` carries the external format identifier and component swizzle that camera and video decoder buffers require. Vendor buffers with an external format cannot be sampled correctly without it. | ||
|
|
||
| For legacy [`SurfaceTexture`](https://developer.android.com/reference/android/graphics/SurfaceTexture) instances, the existing [`FlutterEngineRegisterExternalTexture`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder.h#L3227) API with `kFlutterExternalTextureTypeOpenGL` is used. However, the existing [`FlutterOpenGLTexture`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder.h#L1121) struct only specifies `target`, `name`, `format`, `width`, and `height`; it lacks a UV coordinate transformation matrix. On Android, [`SurfaceTexture.getTransformMatrix()`](https://developer.android.com/reference/android/graphics/SurfaceTexture#getTransformMatrix(float[])) provides a 4x4 matrix accounting for camera sensor rotation, video decoder padding, and coordinate inversion. In the existing architecture, [`SurfaceTextureExternalTexture::Update()`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android/surface_texture_external_texture.cc#L134-L143) queries this matrix through [`PlatformViewAndroidJNIImpl::SurfaceTextureGetTransformMatrix`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc#L1626-L1645) and returns an [`SkM44`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/third_party/skia/include/core/SkM44.h) matrix directly: |
There was a problem hiding this comment.
it lacks a UV coordinate transformation matrix.
What scenarios do we need the UV coordinate transformation matrix? Is it only for the gl_external_texture_frame_callback, or more than just that?
I'm wondering if we should rename FlutterOpenGLTexture2 to something like FlutterOpenGLExternalTexture, which would better mirror FlutterMetalExternalTexture.
Also, do we need to deprecate any APIs that use FlutterOpenGLTexture?
| kFlutterPathVerbConic, | ||
| kFlutterPathVerbCubic, | ||
| kFlutterPathVerbClose, | ||
| } FlutterPathVerb; |
There was a problem hiding this comment.
Maybe consider renaming to FlutterPathType to better mirror other unions.
| kFlutterPathVerbClose, | ||
| } FlutterPathVerb; | ||
|
|
||
| typedef struct { |
There was a problem hiding this comment.
Are we 100% sure we will never add more path types? If no, I would add a struct_size member here.
| FlutterPathFillType fill_type; | ||
| size_t segments_count; | ||
| /// Valid only for the duration of the frame presentation callback. | ||
| const FlutterPathSegment* segments; |
There was a problem hiding this comment.
Are we 100% sure we will never add more path types? If no, this should be FlutterPathSegment** so that adding more members to FlutterPathSegment does not break the ABI of this array.
| The array is an array of pointers rather than an array of values. Indexing strides by pointer width instead of `sizeof(FlutterPlatformViewMutation)`, so a future mutation type that widens the union does not shift the stride out from under an already-compiled embedder. This is the same technique [`FlutterSemanticsUpdate2`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder.h#L1868-L1881) uses. | ||
|
|
||
| > [!NOTE] Mutator Simplification: Dropping Complex Path Clipping | ||
| > If legacy Hybrid Composition under Skia is dropped, or if platform view clipping is restricted to standard geometric bounds, `kFlutterPlatformViewMutationTypeClipPath` and the whole `FlutterPath` family become dead weight. Mutations would then convey only affine transformations, bounding rectangles, and corner radii, all of which map directly to Android `SurfaceControl` or `View` properties. |
There was a problem hiding this comment.
macOS doesn't support path clips because we currently have no way to pass it through embedder api. but there isn't a fundamental problem why path clips wouldn't work on macOS.
| The legacy [`FlutterSemanticsUpdate`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder.h#L1855) (v1) stored nodes in a flat array of structs by value (`FlutterSemanticsNode* nodes`). In C, indexing `nodes[i]` strides by `sizeof(FlutterSemanticsNode)`, so adding members changed the struct size and broke older binaries. | ||
|
|
||
| [`FlutterSemanticsUpdate2`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder.h#L1868-L1881) resolved this by replacing the flat array with an array of pointers: | ||
|
|
||
| ```c | ||
| typedef struct { | ||
| size_t struct_size; | ||
| size_t node_count; | ||
| FlutterSemanticsNode2** nodes; // Array of pointers | ||
| size_t custom_action_count; | ||
| FlutterSemanticsCustomAction2** custom_actions; // Array of pointers | ||
| FlutterViewId view_id; | ||
| } FlutterSemanticsUpdate2; | ||
| ``` | ||
|
|
||
| Because `nodes` is an array of pointers, pointer striding is fixed (`sizeof(FlutterSemanticsNode2*)`). Each [`FlutterSemanticsNode2`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder.h#L1730-L1783) begins with `size_t struct_size`. This design allows `FlutterSemanticsNode2` to be extended in place: | ||
|
|
||
| 1. **Precedent**: The engine team has already appended members directly to [`FlutterSemanticsNode2`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder.h#L1730-L1783) without introducing a v3 struct (for example, adding `heading_level` in engine commit `59d8010db51`, and adding `identifier` in engine commit `f01fa5950c0`). | ||
| 2. **Missing Android Accessibility Members**: The remaining fields required by Android's `AccessibilityBridge` are appended to the end of `FlutterSemanticsNode2`: |
There was a problem hiding this comment.
I would shorten this section, I don't think reviewers need all this glorious detail. Maybe something like:
The legacy
FlutterSemanticsUpdateandFlutterSemanticsNodeAPIs are frozen as updates would break ABI guarantees. These were deprecated in favor ofFlutterSemanticsUpdate2andFlutterSemanticsNode2. New fields will be added to the en ofFlutterSemanticsNode2:
| double max_value; | ||
| int32_t traversal_parent; | ||
| FlutterTransformation hit_test_transform; | ||
| ``` |
There was a problem hiding this comment.
These LGTM, but we should get these new members reviewed by @chunhtai and @hannah-hyj
| /// @param[in] user_data The user data provided in `FlutterProjectArgs`. | ||
| typedef void (*FlutterRequestDartDeferredLibraryCallback)( | ||
| intptr_t loading_unit_id, | ||
| void* user_data); |
There was a problem hiding this comment.
For clarity, could you also add the new FlutterProjectArgs member for this callback?
| typedef struct { | ||
| size_t struct_size; | ||
| const uint8_t* mapping; | ||
| size_t size; |
There was a problem hiding this comment.
For clarity, consider:
| size_t size; | |
| size_t mapping_size; |
| const uint8_t* mapping; | ||
| size_t size; | ||
| void* user_data; | ||
| VoidCallback release_callback; |
There was a problem hiding this comment.
So far the embedder API has called everything destruction_callback, I'd consider sticking with that terminology here and in FlutterCustomAssetResolver below.
| FlutterAssetResolverGetAsMappingCallback get_as_mapping_callback; | ||
| // Indicates if the resolver remains valid after the underlying Android | ||
| // AAssetManager instance changes or is recreated. | ||
| bool is_valid_after_asset_manager_change; |
There was a problem hiding this comment.
Does Android actually need this? I was under the impression APK assets remain valid after restart. If Android doesn't need this today, we could punt this and add it later when we do need it.
| const FlutterAssetResolverRegistrationInfo* info); | ||
| ``` | ||
|
|
||
| A `FlutterCustomAssetResolver` passes into `FlutterProjectArgs.custom_asset_resolver` (or registers dynamically via `FlutterEngineRegisterAssetResolver`) so that the engine streams assets directly from the APK. During hot restart or dynamic feature installation, the engine calls [`UpdateResolverByType`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/assets/asset_manager.h#L67) via `FlutterEngineUpdateAssetResolverByType` to reload asset manifests and updated Dart bytecode bundles without restarting the native engine instance. |
There was a problem hiding this comment.
Could you also include the new FlutterProjectArgs member for the custom asset resolvers?
Also, do we want to support multiple asset resolvers, instead of just one?
| const FlutterAssetResolverRegistrationInfo* info); | ||
| ``` | ||
|
|
||
| A `FlutterCustomAssetResolver` passes into `FlutterProjectArgs.custom_asset_resolver` (or registers dynamically via `FlutterEngineRegisterAssetResolver`) so that the engine streams assets directly from the APK. During hot restart or dynamic feature installation, the engine calls [`UpdateResolverByType`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/assets/asset_manager.h#L67) via `FlutterEngineUpdateAssetResolverByType` to reload asset manifests and updated Dart bytecode bundles without restarting the native engine instance. |
There was a problem hiding this comment.
the engine calls
UpdateResolverByTypeviaFlutterEngineUpdateAssetResolverByTypeto reload asset manifests and updated Dart bytecode bundles without restarting the native engine instance.
Could you expand on why this is necessary? Why can't the engine call get_as_mapping_callback on hot restart instead?
| void* user_data; | ||
| const FlutterProjectArgs* project_args; | ||
| const FlutterRendererConfig* renderer_config; | ||
| } FlutterEngineSpawnConfig; |
There was a problem hiding this comment.
Consider renaming to FlutterEngineSpawnInfo to mirror other info struct pattern.
| ``` | ||
|
|
||
| > [!IMPORTANT] [`FlutterEngineSetGpuAvailability`](#renderer-availability-multi-window-lifecycle-and-flutterviewid-keying) is proposed, not existing API | ||
| > `FlutterEngineNotifyCreated` and `FlutterEngineNotifyDestroyed` are already in `embedder.h`. `FlutterEngineSetGpuAvailability` is not. It is proposed here as a thin exposure of [`Shell::SetGpuAvailability`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/common/shell.h#L404), which the engine already implements and which the iOS embedder already drives. The proposal adds no new engine behavior; it gives the C-API access to behavior that exists. |
There was a problem hiding this comment.
FlutterEngineNotifyCreated
andFlutterEngineNotifyDestroyedare already inembedder.h`.
AFAIK these don't exist today. Does Flutter Android need this? If not, I'd remove these from the design.
|
|
||
| #### Implicit Single-View vs. Explicit Multi-View Surface Lifecycle | ||
|
|
||
| In `embedder.h`, `FlutterEngineNotifyCreated` and `FlutterEngineNotifyDestroyed` manage the surface lifecycle of the implicit default view (`kFlutterImplicitViewId = 0`). When an Android application attaches secondary `FlutterView` instances to the same `FlutterEngine` (such as foldable dual-screen postures, secondary [`android.app.Presentation`](https://developer.android.com/reference/android/app/Presentation) external displays, or multi-surface Add-to-App hosts), the embedder registers and removes secondary views via [`FlutterEngineAddView`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder.h#L3048) and [`FlutterEngineRemoveView`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/embedder/embedder.h#L3062): |
There was a problem hiding this comment.
AFAIK, Android does not call FlutterEngineAddView or FlutterEngineRemoveView yet.
| - On Android, attempting GPU operations after window detachment risks driver timeouts (`VK_ERROR_DEVICE_LOST`) or memory corruption when the OS evicts application GPU memory. | ||
| - `FlutterEngineSetGpuAvailability` provides a unified, cross-platform C-API abstraction that satisfies both Android and iOS operational requirements. | ||
|
|
||
| ### Hybrid Composition++ (HCPP), SurfaceControl Compositing, and Per-View Frame State |
There was a problem hiding this comment.
Could we move this content up to other platform view design bits? That would improve readability.
|
|
||
| ### Process-Scoped Utilities, Image Decoding, and Diagnostics C-APIs | ||
|
|
||
| Several Android JNI entry points in `FlutterMain` and `FlutterJNI` interact with engine utilities outside the scope of a single frame or require decoding and diagnostic primitives that previously pulled `//flutter/lib/ui`, `//flutter/txt`, `//flutter/runtime`, and `//flutter/skia` headers into `shell/platform/android/`. To sever those dependencies completely, `embedder.h` exposes five focused C-API groups: |
There was a problem hiding this comment.
FYI, this section proposes several new APIs, but they seem to be missing the API proposal themselves. Could you add those in?
| 2. **Hardware-Accelerated Image Decoding (`//flutter/lib/ui/painting/image_generator.h` and `//flutter/skia` Severance)**: Android API 28+ supports hardware-accelerated image decoding via `ImageDecoder`. To register platform image decoders without subclassing internal C++ `flutter::ImageGenerator` or `SkImageGenerator` types, `embedder.h` adds `FlutterEngineRegisterImageGenerator` (`FlutterImageGeneratorRegistrationInfo`, `FlutterImageGeneratorFactoryCallback`, `FlutterImageGeneratorGetInfoCallback`, and `FlutterImageGeneratorDecodeCallback`) backed by an internal `EmbedderImageLRU` cache inside `shell/platform/embedder/`. | ||
| 3. **Font Manager Prefetching (`//flutter/txt` Severance)**: During `FlutterMain.init`, Android warms up the default system font manager on a background worker. `FlutterEnginePrefetchDefaultFontManager()` exposes this one-time initialization in `embedder.h` so `flutter_main.cc` no longer includes `flutter/txt/src/txt/platform.h`. | ||
| 4. **VM Service URI Discovery (`//flutter/runtime/dart_service_isolate.h` Severance)**: Tooling and integration tests discover the Dart VM Service observatory URL via `FlutterJNI.getVMServiceUri`. `FlutterEngineRegisterVMServiceUriCallback` and `FlutterEngineDeregisterVMServiceUriCallback` deliver the URI string asynchronously via a `FlutterVMServiceUriCallback` handle, removing the last `//flutter/runtime` header from `shell/platform/android/`. | ||
| 5. **Synchronous Rasterizer Screenshots**: `FlutterEngineScreenshot` and `FlutterEngineFreeScreenshot` capture uncompressed RGBA pixel buffers (`FlutterEngineScreenshotData`) from the active rasterizer to service `FlutterJNI.takeScreenshot` without reaching into `flutter::Rasterizer::Screenshot`. |
There was a problem hiding this comment.
Did you check if the Android embedder could do this itself, without the engine exposing an API for this? That would likely be preferable. (I suspect the engine screenshot API doesn't support platform views for example).
|
|
||
| This configuration field satisfies both mobile embedder targets: | ||
|
|
||
| - **Zero-Initialization Safety**: Existing desktop embedders (Linux, macOS, Windows) zero-initialize `FlutterProjectArgs`, which defaults the field to `true` and preserves their single-threaded platform message assumptions. |
There was a problem hiding this comment.
I believe this is incorrect, C++ zero initialization defaults booleans to false. I think we need to invert the field such that false is today's behavior.
| size_t display_features_count; | ||
|
|
||
| /// Array of physical display features (foldable hinges, cutouts). | ||
| const FlutterDisplayFeature* display_features; |
There was a problem hiding this comment.
This should be ** so that adding new members to FlutterDisplayFeature does not break the ABI of indexing this array.
| const FlutterDisplayFeature* display_features; | |
| const FlutterDisplayFeature** display_features; |
| double top; | ||
| double right; | ||
| double bottom; | ||
| } FlutterRectInsets; |
There was a problem hiding this comment.
Maybe consider renaming this to FlutterEdgeInsets to better mirror the framework's name for this.
| double display_corner_radius_top_left; | ||
| double display_corner_radius_top_right; | ||
| double display_corner_radius_bottom_right; | ||
| double display_corner_radius_bottom_left; |
There was a problem hiding this comment.
Should we introduce a FlutterBorderRadius struct for this? I don't feel strongly.
|
|
||
| To support non-linear scaling without synchronous engine queries or new C-APIs, the existing `flutter/settings` channel ([`SettingsChannel.java`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/SettingsChannel.java)) is extended: | ||
|
|
||
| - During startup and configuration changes, the embedder extracts curve sample points from Android's [`FontScaleConverter`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android/io/flutter/view/FontScaleConverter.java) and includes them in the `flutter/settings` message payload: |
There was a problem hiding this comment.
Could you add more information on how this would work? What would be the logic for PlatformDispatcher.scaleFontSize?
I imagine PlatformDispatcher._updateUserSettingsData will need to be updated to invoke PlatformDispatcher.onTextScaleFactorChanged if fontScaleCurves changes.
We'll want to get @LongCatIsLooong's feedback here.
|
|
||
| ### Platform Channel Messaging, Background Handlers, and Task Queues | ||
|
|
||
| In the existing Android architecture, [`PlatformMessageHandlerAndroid::DoesHandlePlatformMessageOnPlatformThread`](https://github.com/flutter/flutter/blob/4762dec4fc9072cd9269814ef36364f35a9a66dc/engine/src/flutter/shell/platform/android/platform_message_handler_android.h#L23-L25) returns `false` to permit message dispatch from arbitrary threads: |
There was a problem hiding this comment.
I believe this is incorrect. Here's the only use of DoesHandlePlatformMessageOnPlatformThread:
TLDR: DoesHandlePlatformMessageOnPlatformThread is only used at startup to do some task hopping to ensure messages wait until handlers have been registered.
Gemini explanation...
DoesHandlePlatformMessageOnPlatformThread() says whether a handler always sends messages to the platform thread itself:
| Handler | Returns | Behavior |
|---|---|---|
| iOS / Android | false | Looks up the channel's handler right away on the UI thread, then runs it on the main queue or on a background task queue. |
| Embedder (Windows, Linux, macOS, custom embedders) | true | HandlePlatformMessage posts the message to the platform thread itself. |
The problem it solves
Host apps often do this in one platform-thread event:
run engine → register method channel handlers
Before background channels, every message from Dart went UI → platform thread. Even if the Dart isolate sent a message right away, it waited in the platform queue until the
current event finished, and by then the handlers were registered.
With iOS/Android's direct dispatch, the UI thread can look up the handler before the app registers it. In that case the iOS handler just calls CompleteEmpty() (line 88–91),
so the message is lost.
How the fix works
- The flag: Shell::Setup (which runs on the platform thread) sets route_messages_through_platform_thread_ = true. It also posts a task to the platform thread that sets it
back to false. That task can only run after the current platform event (the one creating the shell and registering handlers) finishes. - The bounce: In OnEngineHandlePlatformMessage, while the flag is true and the handler is a direct-dispatch one (false), the message takes a round trip:
• UI → platform: a task is posted to the platform task runner. It runs after the current event, so the handlers have been registered by then.
• platform → UI: that task posts back to the UI runner, because HandlePlatformMessage` is meant to be called on the UI thread.
• A weak_ptr to the handler is captured so nothing crashes if the shell is destroyed in the meantime. - Steady state: once the flag is cleared, messages go straight to platform_message_handler_->HandlePlatformMessage(...) on the UI thread. This is the fast path that
background channels rely on.
Why DoesHandlePlatformMessageOnPlatformThread() skips the bounce
If the handler returns true, it already queues every message on the platform thread, so the old ordering holds automatically. Bouncing would just add two extra thread hops
for nothing.
|
|
||
| #### Architectural Rationale for Off-Platform-Thread Dispatch | ||
|
|
||
| Returning `false` provides two critical architectural capabilities on mobile platforms: |
There was a problem hiding this comment.
The embedder API's FlutterProjectArgs.platform_message_callback - the callback the engine uses to notify the embedder of messages - states it is invoked on the platform thread. We'll want to keep that behavior by default for backwards compatibility, especially for custom embedders that haven't merged the UI / platform threads yet.
Instead, I think we might want to introduce a new embedder API, like FlutterProjectArgs.enable_direct_platform_message_dispatch.
PlatformViewEmbedder::EmbedderPlatformMessageHandler could then have the following behavior:
DoesHandlePlatformMessageOnPlatformThreadreturnsfalseifFlutterProjectArgs.enable_direct_platform_message_dispatchistruePlatformViewEmbedder::EmbedderPlatformMessageHandlerwould only post a task to the platform thread ifFlutterProjectArgs.enable_direct_platform_message_dispatchisfalse.
And we'd need to update FlutterProjectArgs.platform_message_callback to call out its threading behavior depends on FlutterProjectArgs.enable_direct_platform_message_dispatch.
| Returning `false` provides two critical architectural capabilities on mobile platforms: | ||
|
|
||
| 1. **Zero Main-Thread Contention for Background Channels**: Flutter plugins handling high-bandwidth or compute-heavy workloads (such as camera stream ingestion, local database operations via SQLite, network responses, and cryptography) register background task queues via `BinaryMessenger.makeBackgroundTaskQueue()`. Because message dispatch does not force an intermediate hop to the platform thread, this traffic bypasses Android's main [`Looper`](https://developer.android.com/reference/android/os/Looper) entirely. The platform main thread remains dedicated to view layout, input processing, and frame presentation. | ||
| 2. **Elimination of Latency and Head-of-Line Blocking**: Forcing a hop from the engine UI thread through the platform thread before dispatching to a background worker pool introduces an extra event loop dispatch cycle and thread context switch. If the Android main thread is blocked performing expensive view inflation or layout measurement passes, background channel execution stalls behind that work, which negates the advantage of offloading work to background workers. |
There was a problem hiding this comment.
I suspect these are over stated. If we didn't do this, background tasks would still go to a background thread, we would just do an unnecessary platform task in the middle.
Android Embedder Migration RFC
Tracking issue: flutter/flutter#193309
Pre-launch Checklist
design doc.dart run bin/assign_rfc_number.dartlocally when instructed).