With this PR, backing stores are labeled with view IDs. When the engine requests the embedder to create a backing store, the engine will promise that it will only be used for a specific view.
This follows the design doc http://flutter.dev/go/backing-stores-for-multi-view-partial-repaint, so that backing stores can be used as a front surface that retains its content last frame.
The engine will create a render target cache for each view to cache backing stores separately.
### Alternative design
The separate render target cache for each view is not needed to implement the design doc, since all usages described in the design doc avoids the engine cache. Instead, we can make the engine still only manage one render target cache for all views, and backing stores in it are interchangeable across views. We might describe the behavior in this way:
* In general, the view ID is just provided to the creating callback as information. (This is how it is seen when the engine cache is avoided.)
* If the engine cache is used, then the created backing store might not be immediately collected, and might be reused for different views.
But it's really hard to explain the mechanism and the result is really confusing (as can be seen from my attempt). Why is there a view ID but it's not used, and if you enable the engine cache it's not even followed?
That's why I chose the current approach. Feel free to suggest otherwise for this.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
All physical iOS devices Flutter supports (iOS 12+) can run Metal. The only time Flutter doesn't use Metal afaik is for iOS simulators running < iOS 13, which is covered by a few `if (@available(iOS METAL_IOS_VERSION_BASELINE, *))` checks.
aef5775087/shell/platform/darwin/ios/rendering_api_selection.h (L37-L41)
Remove hardware checks for physical devices.
Remove `shell_enable_metal` from the gn files and the `#if SHELL_ENABLE_METAL` checks in the iOS embedder.
I limited this PR to just iOS, but I imagine it's safe to remove `shell_enable_metal` everywhere?
97b286ca62/shell/platform/darwin/macos/BUILD.gn (L18)97b286ca62/tools/gn (L673-L679)
Part of https://github.com/flutter/flutter/issues/144439
This does two things:
* Logs a warning when the embedder requests a non-Impeller preference when creating a shell.
* Makes the iOS embedder request a warning be logged when Impeller is not used.
I decided to put the warning logs in the shell so that as we get more opinionated about Impeller on other platforms, those platforms can just flip a flag with common log origin.
The Linux embedder is rendering to a FlutterOpenGLFramebuffer that specifies an OpenGL FBO. This API allows the embedder to create a color attachment that wraps the FBO so that the render pass will bind to that FBO during encoding.
This fixes the `WindowsTest.EngineCanTransitionToHeadless` flakiness reported by @matanlurey.
EGL surfaces can only be used by a single thread at a time. Concurrent operations are unsafe.
Previously, the EGL surface was destroyed on the platform thread. This was safe as this always happened after the engine was shutdown and the raster thread was stopped. However, in a multi-view world a view can be destroyed while the engine is running. There may be pending raster tasks that operate on the render surface. Thus, the EGL surfaces should be destroyed on the raster thread when it is available.
This bug was introduced by https://github.com/flutter/engine/pull/51681
Part of https://github.com/flutter/flutter/issues/142845
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Previously the keyboard was initialized after the view is created. This used to be necessary as the keyboard & text input plugins were strongly tied to a view (and would crash in headless modes). This is no longer necessary, and the keyboard can now be initialized normally as part of the engine initialization.
Part of https://github.com/flutter/flutter/issues/142845
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Currently destroying a view also shuts down the engine. This makes sense as in single-view world where the view also always owns the engine. However, in a multi-view world the views will share the engine. Destroying one view shouldn't necessarily shut down the engine unless that view owns the engine.
Part of https://github.com/flutter/flutter/issues/142845
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This PR groups per-view information in `FlutterCompositor` into a private class, `ViewPresenter`. This makes it easier to manage per-view data and write view operations.
Part of https://github.com/flutter/flutter/issues/145874.
Currently, view presenters are never removed once created, since the macOS runner doesn't support removing views. This will be added in the future.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This PR connects the view ID provided by the rasterizer to `EmbedderExternalViewEmbedder`'s presenting call (see `embedder_external_view_embedder.cc`).
This PR also contains a refactor (which actually takes the majority of LOC) that moves the specification of view ID from `PrepareFlutterView` to `SubmitFlutterView`. The `flutter_view_id` parameter in `PrepareFlutterView` was added in https://github.com/flutter/engine/pull/46169. It turns out that we don't need the view ID throughout the preparation phase, so we can delay the specification to submission, which makes it feel cleaner.
### Tests
Existing tests are changed to verify that `SubmitFlutterView` receive the correct view ID.
The feature that `EmbedderExternalViewEmbedder` sends the view ID to the presenting API is not tested for now, because `EmbedderExternalViewEmbedder` is typically tested in `embedder_unittests`, but it requires the embedder API `AddView`. It will be tested in later changes to `EmbedderExternalViewEmbedder`.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Support for Android's predictive back feature on internal Flutter routes. Reports predictive back gestures to the framework (where supported by the system).
The previous PR https://github.com/flutter/flutter/issues/143420 rounds out the layers and rounds in the platform views. This results in missing pixel on the edge of the intersection when there's fractional coordinate (as shown in the screenshot below), because platform view is below the layers.
It turns out that we have to round out both platform view and layers, because:
- rounding in platform view rects will result in missing pixels on the edge of the intersection.
- rounding in layer rects will result in missing pixels on the edge of the layer that's on top of the platform view.
This PR simply skips the single (or partial) pixel on the edge, which is a special case, while still preserve the `roundOut` behavior for general non-edge cases.
Before the fix, notice a very thin gray line cutting through the purple box:
<img src="https://github.com/flutter/engine/assets/41930132/1482d81a-337e-4841-ac08-eff08bbc71ef" height="500">
Then after the fix, the gray line is gone:
<img src="https://github.com/flutter/engine/assets/41930132/0eddae69-ab62-4de6-8932-c67cc5aced73" height="500">
*List which issues are fixed by this PR. You must list at least one issue.*
https://github.com/flutter/flutter/issues/143420
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
No semantic changes, just consolidates static functions in FlutterViewController.mm local to the translation unit into a single contiguous block at the top.
Also includes three minor (non-semantic) changes:
* Corrects static function naming to UpperCamelCase
* Adds doc comments for `OnKeyboardLayoutChanged`
* Adds a mark to help highlight the static data/function block at the top of the file in Xcode.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This doesn't actually seem to be helping. For the most part, the frame time of the banners is steady with occassional spikes. Adding the framerate cap doesn't help the spikes, since those are in the 30+ ms frame time, and just results in everything else looking janky as well (especially if we dance around the cap, then we go 120 - 80)
Since this is based on an arbitrary number, from an arbitrary app, on an arbitrary point in time we should remove it and instead try to fix the performance problems
Reverts flutter/engine#51391
Reason for revert: b/330184547 - I believe there is a good chance that the investigation on that issue was not really adequate to justify this revert, but this change can be easily relanded if reverting this was the wrong choice.
Adds `FlutterDesktopEngineCreateViewController` to the Windows embedder's C API. Example usage:
```cpp
// Create the engine.
FlutterDesktopEngineProperties engine_properties = {};
auto engine = FlutterDesktopEngineCreate(&engine_properties);
// Create a views controller.
FlutterDesktopViewControllerProperties properties = {};
properties.width = 400;
properties.height = 400;
auto controller = FlutterDesktopEngineCreateViewController(engine, &properties);
// Run app...
// Destroy the view controller. This does not destroy the engine.
FlutterDesktopViewControllerDestroy(controller);
// Destroy the engine.
FlutterDesktopEngineDestroy(engine);
```
This new API creates a view controller that does **not** own the engine, unlike the existing `FlutterDesktopEngineCreateViewController`. In the future, this new API will allow a Windows app to have multiple views that share a single engine.
This API is in the `flutter_windows_internal.h`, making it "pubternal". This API is not in the public Windows header files, however, one can link against this API for testing purposes. This API will be promoted to the public Windows header files once the Windows embedder is ready to have multiple views.
Relevant design documents:
1. https://flutter.dev/go/desktop-multi-view-runner-apis
1. http://flutter.dev/go/windows-multi-view-ownership-updates
Relevant issues:
1. https://github.com/flutter/flutter/issues/143767
1. https://github.com/flutter/flutter/issues/142845
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Some embedders use a custom platform task runner that does not support fml::MessageLoopTaskQueues and may not have an fml::MessageLoop.
To support those embedders, DartIsolate::SetMessageHandlingTaskRunner will not use fml::MessageLoopTaskQueues for platform isolates.
See https://github.com/flutter/engine/pull/48551#issuecomment-1962190896
Consider this scenario: In an add-to-app context, where multiple Flutter activities share the same engine, a situation occurs. When navigating from FlutterActivity1 to FlutterActivity2, the Flutter view associated with FlutterActivity1 is detached from the engine. Then, the Flutter view of FlutterActivity2 is attached. Upon navigating back to FlutterActivity1, its Flutter view is re-attached to the shared engine.
The expected behavior is: When a Flutter view detaches from the shared engine, the associated surface should be released. When the Flutter view re-attaches, a new surface should be created.
After #47358, no new surface is created when the Flutter view is attached again. This results in the Flutter view having no underlying surface, which causes the page to appear blank or freeze without responding.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Address the performance of platform view due to an extra overlay. This overlay was added due to the following rounding problem:
> For example, if we interleave flutter widget and platform view in a list, and let's say we have a flutter widget (top = 0, bottom = 100.1), and a platform view below that widget (top = 100.1, bottom = 200). They are NOT supposed to be overlapping. However, after rounding out, we will get flutter widget (top = 0, bottom = 101), and platform view (top = 100, bottom 200). This will result in 1 pixel overlap as long as there's a floating point in the coord.
(Quote myself from the discussion below).
*List which issues are fixed by this PR. You must list at least one issue.*
Fixes https://github.com/flutter/flutter/issues/143420
*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This reverts commit 037aa6b4caa6a3a726e67e519324b2c0a1ec274a.
The original cause of the revert was a flake introduced because the unit-test could request a frame from the Choreographer if it ran long enough. The availability checks in the choreographer were inaccurate after we intentionally backed out of using the Callback32 variant on 32 bit platforms.
The new tests didn't catch it because of an unrelated issue. In the first version of the patch for review, the proc table was only supposed to run on API levels 29 and above. When @dnfield requested we also get rid of the NDK helpers, the choreographer and additional utilities were added. But the API level gate in the new test harness wasn't removed. This made the tests be skipped. That gate has been removed entirely now. The error that cause the revert because of flakiness will now be a reliable failure.
Only available on Android device API levels >= 29. Proc table is setup has versioning checks. All handles are type safe. Collection of handles takes into account cleanup tasks (like reparenting surface controls). The proc table contains code duplicated in ndk_helpers and I will remove that in favor of this in a subsequent patch.
Part of https://github.com/flutter/engine/pull/51213 being chopped up.
Previously the DisplayListBuilder would only pass along bounds for a saveLayer when they were supplied by the caller that was building the DisplayList. This would require Impeller to use post-processing of the EntityPass lists to compute them on its own.
DisplayList can now compute those bounds as it builds the DisplayList to save dispatch clients from having to do so on their own. It will also provide an indicator in the case when the caller supplied bounds that ended up being too small to capture all of the content, causing clipping by the layer render target.
