Without this, an opaque black surface may briefly flash onscreen when the platform view is shown (see internal issue b/332379081)
This matches the behavior used before to the introduction of PlatformViewRenderTarget in b3945f7706
That commit moved the clearing of the surface into SurfaceTexturePlatformViewRenderTarget. But that caused surface clearing to happen in scenarios where there was no PlatformViewWrapper, resulting in the issue seen in https://github.com/flutter/flutter/issues/141068
Later the surface clearing was removed from SurfaceTexturePlatformViewRenderTarget, causing the flashing black screen.
This enables the Windows embedder to render to multiple views using the new `FlutterEngineAddView` and `FlutterEngineRemoveView` embedder APIs. See: https://flutter.dev/go/multi-view-embedder-apis
Â
Prepares for https://github.com/flutter/flutter/issues/144810
Part of https://github.com/flutter/flutter/issues/142845
### Sync over async
Windows expects synchronous operations: windows are created, resized, and destroyed synchronously. However, Flutter native is asynchronous due to its [threading model](https://github.com/flutter/flutter/wiki/The-Engine-architecture#threading).
This change blocks the platform thread when a non-implicit view is added or removed. See: https://flutter.dev/go/multi-view-sync-over-async
### Synchronization
The embedder and engine have separate view states that they synchronize asynchronously. The engine can present a view on the raster thread while the embedder is destroying that same view on the platform thread. This change introduces a mutex to protect against this:
1. The platform thread acquires a **shared** lock whenever it needs to access the view.
2. The platform thread acquires an **exclusive** lock to add a view to the embedder, _before_ it notifies the engine of the view
3. The platform thread acquires an **exclusive** lock to remove a view from the embedder, *after* the engine has acknowledged the view's removal but *before* the embedder destroys the view.
4. The raster thread acquires a **shared** lock to present to a view. This lock is held for the entirety of the present operation, thereby blocking the platform thread from destroying the view.
The implicit view is an important corner case. The framework/engine believe the implicit view **always** exists, even if the app is in headless mode. The embedder does not notify the engine when it destroys the implicit view. In other words, the embedder must safely ignore presents to the implicit view when it does not exist.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Smart pointers support ARC as of https://github.com/flutter/engine/pull/47612, and the unit tests were migrated in https://github.com/flutter/engine/pull/48162.
Migrate `FlutterRestorationPlugin`, `FlutterTextureRegistryRelay`, and `UIViewController+FlutterScreenAndSceneIfLoaded` from MRC to ARC. These files do not themselves import any MRC files, making them leaf nodes in the dependency graph of MRC files.
Doing a few at a time to make the dependency graph manageable, and to easily revert if this causes retain cycles or other memory management issues.
Part of https://github.com/flutter/flutter/issues/137801.
Reverts: flutter/engine#51229
Initiated by: gmackall
Reason for reverting: blocking engine->framework roll (I missed some framework code referencing the v1 embedding).
Original PR Author: gmackall
Reviewed By: {matanlurey, reidbaker}
This change reverts the following previous change:
Fixes https://github.com/flutter/flutter/issues/143531
Also fixes a random typo I found
~TODO to test this~ (no more todo):
-~test the framework against this as well, probably with a dummy PR changing the engine commit to my branch if this is possible~ not possible, made a best effort removal of framework code in https://github.com/flutter/flutter/pull/144726.
-~figure out if the old embedding is used in g3 at all~ removed all uses
-~figure out exactly what the ShimPluginRegistry/ShimRegistrar are doing, and if fully deleting them was right~ (see https://github.com/flutter/engine/pull/51229#issuecomment-1981757743)
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Fixes https://github.com/flutter/flutter/issues/129085
## Changes to `FlutterMouseCursorPlugin`
- `none` cursor is not handled by hiding the cursor anymore. That was
too big of a hammer as it also affects other views (and platform views).
Instead empty cursor is created from empty `NSImage`.
- Cursor plugin now notifies the engine when cursor has changed. The
engine forwards the change to last `FlutterView` that handled mouse
event. This is necessary because on occasion `FlutterView` needs to be
able to restore cursor without framework being involved.
## Preventing PlatformView from changing cursor when it is obscured by
Flutter Content.
Generally in Cocoa cursor changes are done as response to `mouseMoved`
event, which is driven by a `NSTrackingArea`. The issue here is that
this is not affected by hit testing and tracking areas form a hierarchy
parallel to view hierarchy and are not affected by being obscured by
another view (or tracking area). This means that platform view will
receive `mouseMoved` event even when is obscured by Flutter content. To
work around this, the mutator view puts a tracking area above platform
view, which means it gets the mouseMove event first, and when it decides
that mouse is over Flutter content, it will prevent platform view from
changing the cursor for the rest of RunLoop turn (see
`NSCursor+IgnoreChange`).
## Actual hit testing
This part is rather straightforward, the area where FlutterContent
obscures mutator view is provided to the mutator view, which will return
`nil` from `hitTest:` when the point is in the obscured area.
## Example of hit testing
https://github.com/flutter/engine/assets/96958/bbac0cfd-8c44-44d3-addd-921c91a8a539
## Pre-launch Checklist
- [X] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [X] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [X] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [X] I listed at least one issue that this PR fixes in the description
above.
- [X] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I signed the [CLA].
- [X] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
Finishes part 2 of https://github.com/flutter/flutter/issues/145263.
This required fixing a python script to use the version of Java obtained
through the DEPS file rather than one assumed to exist in the
environment.
_This is a refactoring with no semantic changes._
The engine can start rendering into a view once it's received the view's initial window metrics. In a multi-view world, the initial window metrics will be sent by `FlutterEngineAddView`, which will be called by `FlutterWindowsEngine::CreateView`.
This change creates the view's EGL surface earlier, in `FlutterWindowsEngine::CreateView`, to prepare for multi-view rendering.
Prepares for https://github.com/flutter/flutter/issues/144810
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
Reverts: flutter/engine#51636
Initiated by: jmagman
Reason for reverting: This caused a slew of unexpected Scuba changes b/333039358
Original PR Author: jmagman
Reviewed By: {cbracken}
This change reverts the following previous change:
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)
Reverts: flutter/engine#51921
Initiated by: zanderso
Reason for reverting: This Dart roll is blocking the roll of the engine to the framework. Unblocking the rolls depends on addressing https://github.com/flutter/flutter/issues/146164.
Original PR Author: jason-simmons
Reviewed By: {zanderso, jonahwilliams}
This change reverts the following previous change:
The Dart SDK is now only building an AOT snapshot for the frontend server (see https://dart-review.googlesource.com/c/sdk/+/359100)
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
