The size of the LTO build of the engine with the dylib compressed is as follows:
```sh
$ du -k libFlutter*
5236 libFlutter.dylib.tar.gz
4324 libFlutterSlimpeller.dylib.tar.gz
```
Sizes are in KiB. This represents a binary size reduction of 17.41% of the compressed artifacts. The compression ratios will likely differ based on the compression scheme.
Uncompressed, the sizes are:
```sh
$ du -k libFlutter*
16920 libFlutter.dylib
14044 libFlutterSlimpeller.dylib
```
This represents a binary size reduction of 16.99% which is in the same ballpark.
The really mucky bit was backing out the raster cache and persistent cache. I want to clean that up in a later patch so that those TUs are part of a separate submodule.
Opting out of Impeller will lead to a fatal log at startup saying the opt-out is disallowed.
Fixes https://github.com/flutter/flutter/issues/126606
Work towards part of https://github.com/flutter/flutter/issues/138798
Allow updating single glyphs in the glyph atlas, without replacing the entire bitmap. Required to efficiently append/update to large atlases.
This is a refactor that moves the `PointerDataPacketConverter` from `PlatformView` to `RuntimeController`.
This change is made for the following reasons:
- Currently, the pointer data conversion contains no platform specific logic (because the current converter's only responsibility is to make the event sequence conform Flutter's protocol). Therefore these logics should reside in a platform-independent place.
- The converter typically converts one event to many. It's better to have this conversion later than earlier.
- It removes a member from `PlatformView`, making it closer to a pure virtual class.
The reason to choose `RuntimeController` as the destination is because `RuntimeController` manages a map for views, which is required for the converter to implement a later patch https://github.com/flutter/engine/pull/51925.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
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
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.
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
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.
### Motivation of the change:
Both dart and flutter are using fairly outdated gn-sdk without properly maintained. Currently @hjfreyer is working on version'ed IDK / SDK libs which requires changes in gn-sdk to use the right version of the libs in fuchsia/sdk/obj/{arch}-api-{level} rather than the one in the fuchsia/sdk/arch. But current implementation does not support choosing the right version.
### Blocking issue:
The new gn-sdk (in flutter/tools/fuchsia/gn-sdk) generates multiple BUILD.gn files rather than a large BUILD.gn the previous version created. So most of the build rules need to switch from the old `fidl:{api}` build rule to `fidl/{api}` rule. The same change will happen in the dart/sdk, i.e. http://go/dart-reviews/356924. But since the two repos cannot have one single atomic change, changing either side first will cause flutter to break. E.g. the linkage error caused by duplicated symbols will happen if we change the dart/sdk first, since in flutter, it will still refer to the old build rules in the middle.
### Solutions:
Ideally we can create redirect rules in the current `build/fuchsia` buildroot tree to redirect the old rules into the new one, so we can make the change in the flutter first then dart/sdk. But creating the rules is not trivial and will only be used once.
So an alternative solution is
- pause the dart/sdk -> flutter roll
- submit dart/sdk change (http://go/dart-reviews/356924)
- update this change to manually bring the dart/sdk change, namely the `dart_revision` in the DEPS file and signatures in the ci/licences.
- resume the dart/sdk -> flutter roll.
But it requires this change itself to be reviewed first, and I'd like to know your opinion before moving forward.
See corresponding dart/sdk change at http://go/dart-reviews/356924.
### //build/fuchsia/ from buildroot should be removed after this change.
Bug: [b/40935282](https://issues.chromium.org/issues/40935282?pli=1&authuser=0)
FYI: @hjfreyer
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This is a prototype of the [PlatformIsolate
API](https://github.com/flutter/flutter/issues/136314).
**UPDATE (Jan 25):** The PR is ready for review. PTAL.
The `PlatformIsolate` creation flow is:
1. `PlatformIsolate.spawn` running on parent isolate
(platform_isolate.dart)
a. Create `isolateReadyPort`
b. `PlatformIsolateNativeApi::Spawn` (platform_isolate.cc)
c. `DartIsolate::CreatePlatformIsolate` (dart_isolate.cc)
d. Isolate created. Entry point invocation task dispatched to platform
thread
e. `PlatformIsolate.spawn` returns a `Future<Isolate>`
2. On the platform thread, `_platformIsolateMain` is invoked in the
platform isolate
a. Create `entryPointPort`
b. Send `Isolate.current` metadata and `entryPointPort` back to the
parent isolate via `isolateReadyPort`
3. Back in the parent isolate, `isolateReadyPort.handler` is invoked
a. Send the user's `entryPoint` and `message` to the platform isolate
via `entryPointPort`
b. Use received isolate metadata to create a new `Isolate` representing
the platform isolate and complete the `Future<Isolate>`
4. In the platform isolate, `entryPointPort.handler` is invoked
a. Run the user's `entryPoint(message)`
The engine shutdown flow is handled by `PlatformIsolateManager`, which
maintains a set of running platform isolates.
1. Makes it possible to implement Impeller Vulkan -> Skia fallback.
2. Fixes https://github.com/flutter/flutter/issues/143871
3. Fixes https://github.com/flutter/flutter/issues/143873
4. Work towards https://github.com/flutter/flutter/issues/137798
Combines AndroidRenderingAPI + enable_impeller into a 4 values enum that describes all combinations of rendering behavior: ImpellerVulkan, ImpellerOpenGLES, SkiaOpenGLES, Software.
Updates the fallback behavior to happen in flutter_main. This allows us to change the value of Settings.enable_impeller to support an Impeller -> Skia fallback.
@jason-simmons @jiahaog fyi
I need to figure out a test for this still but it seems to work. Hot restart is kind of a mess though, because `Engine::Restart` nulls out the asset manager it ends up reloading the font collection for every engine after a restart. But we separately need to fix hot restart, it's not very usable on the example app
Fixes https://github.com/flutter/flutter/issues/143701
Flutter implements the UI isolates message via posting to a UI task queue. That task queue has a primary and a secondary queue. The
* primary is used for running tasks resulated to framework (e.g. draw frame)
* secondary is used to run Dart event loop messages (e.g. received data on a socket)
The Dart semantics requires running microtasks before processing the next event loop message.
The way flutter implements by attaching an observer to the secondary queue. Every time a task from the queue is run, all observers are run and one of them is going to run pending microtasks.
In some situations the engine pauses the dart event loop. The terminology used in the code is "microtask" when in reality what it means is "event loop".
=> This PR changes this terminology to reflect what actually happens.
Reland of: https://github.com/flutter/engine/pull/50139
Metal does not seem to like it when we collect 50+ command buffers at once. Adjust the aiks context logic to regularly flush the cmd buffers.
Reverts flutter/engine#50139
Initiated by: jonahwilliams
This change reverts the following previous change:
Original Description:
The Impeller Vulkan backend benefits from batching submission to the vk graphics queue. Managing this automatically is non-trivial and adds surprising/fragile thread based behavior, see: https://github.com/flutter/engine/pull/49870
Instead, introduce an impeller::CommandQueue object that command buffers must be submitted to in lieu of CommandBuffer->Submit, which has been made private.
TLDR
old
```c++
buffer->Submit();
```
new
```c++
context.GetQueue()->Submit({buffer});
```
The Metal and GLES implementations internally just call the private CommandBuffer->Submit, though there may be future opportunities to simplify here. The Vulkan implementation is where the meat is.
Aiks takes advantage of this by storing all command buffers on the aiks context while rendering a frame, and then performing one submit in aiks_context render. I don't think this will introduce any thread safety problems, as we don't guarantee much about aiks context - nor do we use it in a multithreaded context as far as I know.
Other tasks such as image upload still just directly submit their command buffers via the queue.
Fixes https://github.com/flutter/flutter/issues/141123
The Impeller Vulkan backend benefits from batching submission to the vk graphics queue. Managing this automatically is non-trivial and adds surprising/fragile thread based behavior, see: https://github.com/flutter/engine/pull/49870
Instead, introduce an impeller::CommandQueue object that command buffers must be submitted to in lieu of CommandBuffer->Submit, which has been made private.
TLDR
old
```c++
buffer->Submit();
```
new
```c++
context.GetQueue()->Submit({buffer});
```
The Metal and GLES implementations internally just call the private CommandBuffer->Submit, though there may be future opportunities to simplify here. The Vulkan implementation is where the meat is.
Aiks takes advantage of this by storing all command buffers on the aiks context while rendering a frame, and then performing one submit in aiks_context render. I don't think this will introduce any thread safety problems, as we don't guarantee much about aiks context - nor do we use it in a multithreaded context as far as I know.
Other tasks such as image upload still just directly submit their command buffers via the queue.
Fixes https://github.com/flutter/flutter/issues/141123
Generated by https://github.com/flutter/engine/pull/48903 (`dart ./tools/header_guard_check/bin/main.dart --fix`).
As discussed with @cbracken and @jmagman, the guards are not technically needed on the Mac/iOS code, but they (a) do not hurt and (b) still provide value if for some reason `#include` is used instead of `#import` (though I suspect we could try to add that to the tool in the future as well).
Fixes https://github.com/flutter/flutter/issues/141571
Shell::Screenshot was impelemnted in a Skia-only way and would crash when invoked. Adds test coverage for the breaking path on iOS that ends up calling `FlutterView drawLayer`.
This keeps things consistent with other imports and fixes the internal breakage (b/320443454).
While the build system here is able to handle imports both with and without the flutter prefix, the internal mechanism that rewrites these imports is currently unable to handle the latter case.
This enables work ongoing by @derekxu16 to improve performance in flutter_tester when running multiple files from large test suites.
Specifically, it:
- Exposes a `Spawn` C symbol from flutter_tester that runs the current kernel in a new UI isolate with a different entrypoint and/or route name
- Exposes two symbols from flutter_tester to allow a test harness to more efficiently load particular kernel files or to lookup an entrypoint from an imported source file.
- Avoids re-loading the kernel file completely when spawning a new UI isolate
Googlers can look at go/flutter-tester-isolates for some more context. If anyone wants I'm happy to create a public version of that doc.
I tried to do this for Vulkan too but hit the limit of my patience for today on debugging why the Vulkan backend was segfaulting on shutdown.
Half of https://github.com/flutter/flutter/issues/140419
This is part of the work towards supporting OpenGLES and Vulkan for runtime stage shaders.
Removes some redundant work we had around SkSL. Now only bundles the shaders we actually ask for from the command line.
@bdero, we should figure out if this is the right approach for flutter_gpu.
With this change, the IPLR format goes from having a root table of shader related information to a root table of shader information per `sksl`, `metal`, `opengles`, and `vulkan` platforms.
This may end up allowing us to revert https://github.com/flutter/engine/pull/47278, but I'm not sure I understand all the implications of that at this point.
I have run some but not all tests locally.
This is one of a series of changes to reland https://github.com/flutter/engine/pull/47239.
This PR changes `Animator` so that if `Render` is not called after a `BeginFrame`, this call is ignored.
Note that this is slightly different from https://github.com/flutter/engine/pull/47239. Instead of saying that we should ultimately change this skip to an assertion, this PR aims to keep the skip as the final shape. This is because a while ago we (with @goderbauer and @loic-sharma) decided that `PlatformDispatcher` should contain as little logic as possible to allow testing, and instead serve as a minimal native function binding, which means that we should eventually move the code that validates calling convention to the engine.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This reverts commit 87171e73397f7edfd4c145fbaebdd7208954debf.
Internal performance test shows that this PR negatively affects startup time. I'll reland this PR in parts.
b/307872797
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
