While recently updating the DlColorSource sources I noticed some questionably implementation choices in the Color variant of the color sources.
I then realized that there was no public use of these classes (other than mostly their own unit tests) and so they should be deleted to focus on implementing the variants that are actually used by Flutter.
Changes the following shell callbacks to flush the dart event loop:
* OnPlatformViewSetViewportMetrics
* OnPlatformViewDispatchPointerDataPacket
* OnPlatformViewDispatchPlatformMessage
* OnPlatformViewSetSemanticsEnabled
* OnPlatformViewSetAccessibilityFeatures
Using a new TaskRunner API RunNowAndFlushMessages. If the task runner can run tasks on the current thread, this will immediately invoke a callback and then post an empty task to the event loop to ensure dart listeners fire.
Unlike https://github.com/flutter/engine/pull/56738, does not touch the vsync API - which looks like it depends on scheduling behavior today, at least for iOS.
Reverts: flutter/engine#56738
Initiated by: jonahwilliams
Reason for reverting: speculative revert for framework failures.
Original PR Author: jonahwilliams
Reviewed By: {jason-simmons}
This change reverts the following previous change:
Changes the following shell callbacks to flush the dart event loop:
* OnPlatformViewSetViewportMetrics
* OnPlatformViewDispatchPointerDataPacket
* OnPlatformViewDispatchPlatformMessage
* OnPlatformViewSetSemanticsEnabled
* OnPlatformViewSetAccessibilityFeatures
Using a new TaskRunner API RunNowAndFlushMessages. If the task runner can run tasks on the current thread, this will immediately invoke a callback and then post an empty task to the event loop to ensure dart listeners fire.
This also updates the vsync waiter to use RunNowOrPostTask, so that we start vsync events as early as possible.
The Skia snapshot controller will activate the delegate's surface render context on the current thread. If the delegate has no surface, then it will use the snapshot surface producer to create a temporary surface.
The Impeller snapshot controller needs to do the same in order to support OpenGL/GLES scenarios where the thread does not currently have an EGL context.
The DlColorSource code uses Skia geometry classes for its internal computations. This PR switches those implementations to use the Impeller geometry classes for consistency and 3rd party header file independence.
Changes the following shell callbacks to flush the dart event loop:
* OnPlatformViewSetViewportMetrics
* OnPlatformViewDispatchPointerDataPacket
* OnPlatformViewDispatchPlatformMessage
* OnPlatformViewSetSemanticsEnabled
* OnPlatformViewSetAccessibilityFeatures
Using a new TaskRunner API RunNowAndFlushMessages. If the task runner can run tasks on the current thread, this will immediately invoke a callback and then post an empty task to the event loop to ensure dart listeners fire.
This also updates the vsync waiter to use RunNowOrPostTask, so that we start vsync events as early as possible.
Moves code specific to each graphics backend into the (existing) translation unit associated with that backend.
Previously, we could not include any Objective-C types in `shell_test_platform_view_metal.h`, since that file was included into `shell_test_platform_view.cc`, which is pure C++. To work around this, we had encapsulated Objective-C Metal types in a `DarwinContextMetal` struct hidden in the implementation file with a pointer to it forward-declared in the header.
We now use Metal types directly in the header, without the workarounds.
Issue: https://github.com/flutter/flutter/issues/158998
Issue: https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
`kDefaultBackendType` is intended to make life easier for authors of tests, but in any switch statement where it's used (currently just a single location), we rely on ordering it first and `#ifdef`ing out all backends that aren't available.
Instead, we define a static function that returns the default that callers can invoke instead. This avoids relinace on case ordering and fallthrough.
In https://github.com/flutter/engine/pull/56722 we split backends out into separate translation units, and ideally should remove the `#ifdef`s, which means we can't rely on this trick anymore.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This PR introduces a `NativeAssetsManifest.json` next to the `AssetManifest.json` and `FontManifest.json`. This removes the need for embedding the native assets mapping inside the kernel file and will enable decoupling native assets building and bundling from the kernel compilation in flutter tools. This will then allow us to remove dry-run from the build hook protocol.
(It also means all isolate groups will have the same native assets. However, since Flutter does not support `Isolate.spawnUri` from kernel files anyways, this is not a regression.)
This manifest is parsed eagerly on startup by the engine in a manner similar to how the font manifest is parsed. The manifest contents need to be available in the callback for resolving assets, which does not have access to the engine. Therefore the parsed manifest is `NativeAssetsManager` stored in the `IsolateGroupData`. The engine passes it in on isolate group creation, and the FFI callbacks access it from the isolate group data.
Issue:
* https://github.com/flutter/flutter/issues/154425
Related PRs:
* https://dart-review.googlesource.com/c/sdk/+/388161
Follow up work:
* This PR does not yet remove the engine callbacks registered via the dart_api that rely on kernel embedding. If we were to do that in this PR, it would require a manual roll of the engine into flutter/flutter with the PR that switches flutter_tools to emit the native assets manifest instead of embedding in kernel, and a manual roll into g3 to switch emitting a manifest instead of embedding in kernel. A TODO is left in the code for those callbacks to be removed.
## Testing
Most of this PR cannot be tested in isolation. The code in this PR is heavily exercised in the follow up flutter_tools PR which creates the `NativeAssetsManifest.json` and removes the embedding of native assets in kernel files.
* This PR adds a unit test for parsing the JSON manifest.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
The DlImageFilter code uses Skia geometry classes for its internal computations. This PR switches those implementations to use the Impeller geometry classes for consistency and 3rd party header file independence.
Enables the `-fobjc-arc` compiler flag for Objective-C and Objective-C++
translation units.
Eliminates the flutter_cflags_objc[c]_arc settings, since they're now
redundant.
All Obj-C/Obj-C++ code in our codebase has now been migrated to ARC.
Issue: https://github.com/flutter/flutter/issues/137801
Flutter_tester has a task observer that checks whether the test's Dart code has finished execution. If Dart no longer has live ports but does have pending microtasks, then flutter_tester should continue running and force a drain of the microtask queue.
Fixes https://github.com/flutter/flutter/issues/158129
Ensure that all Objective-C code in the codebase is being built with the standard set of Flutter Objective-C compiler flags with ARC enabled.
Also bumps the cflags config up to the top of the first block within each target in which Objective-C sources appear, so that the location is consistent.
Migrates The following targets to ARC, which had been missed in previous passes since they didn't declare the standard Flutter Obj-C[++] cflags:
* `//flutter/fml:fml_unittests`
* `//flutter/impeller/golden_tests:metal_screenshot`
* `//flutter/impeller/playground:playground`
* `//flutter/impeller/backend/metal:metal`
* `//flutter/impeller/backend/metal:metal_unittests`
* `//shell/gpu:gpu_surface_metal_unittests`
* `//flutter/shell/platform/embedder:embedder_unittests`
This patch includes no semantic changes.
Issue: https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Migrates DarwinContextMetal to a plain C struct, eliminating the need for constructor, getters, etc. since it's only used in this translation unit. The fields themselves cannot be inlined as fields on ShellTestPlatformViewMetal because the header in which that class is defined is included in plain C++ (non-Obj-C++) translation units and therefore cannot contain Obj-C types.
This simplifies the code and simultaneously fixes complicated ARC behaviour in which the const "DarwinContextMetal::context()" getter caused retainCount to be incremented on the underlying context_ pointer, but not decremented even if never unassigned. In particular the line
```objc
FML_CHECK([metal_context->context() mainContext]));
```
appeared to cause refcount to be incremented but never decremented.
Regardless of the ARC issue, this significantly simplifies the code.
This also eliminates the last remaining use of fml::scoped_nsobject in Flutter's codebase. That class will be removed in a followup PR.
No test changes since there is no change to semantics.
Issue: https://github.com/flutter/flutter/issues/137801
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Impeller geometry class analogous to SkRRect - soon to be the standard round rectangle class throughout most of the engine that doesn't talk directly to Skia.
More unit tests are being written, but this PR will enable developers to look at the API of the new class and comment as well as test its effect on any goldens (there should be none).
Introduces a mechanism to allow backdrop filters to 1) share backdrop inputs and 2) fuse filter applications for faster blurs.
This is a proposed solution to https://github.com/flutter/flutter/issues/131568
Implemented:
* Developer can specify a "backdrop id" which indicates that a backdrop layer should share the input texture and potentially cached filter for a layer.
* Removes second save layer for each backdrop filter
* Removes save layer trace event for backdrop filter
* Can fuse backdrop filters if there is more than one identical filter
TBD:
* Adjust heruristic to avoid applying bdf filter to entire screen
Suggestions: applying a bdf should be a distinct operation from a save layer in the DL builder/dispatcher. The saveLayer implmenentation in the impeller dispatcher is super convoluted because it needs to handle both.
### Video
Video starts with normal bdf then I hot reload to specify that the bdfs share inputs/filters. This is running on a pixel 8 pro
Change to the macrobenchmark app is just:
```dart
Widget build(BuildContext context) {
Widget addBlur(Widget child, bool shouldBlur) {
if (shouldBlur) {
return ClipRect(
child: BackdropFilter(
filter: ImageFilter.blur(sigmaX: 5, sigmaY: 5),
backdropId: 1, // Added ID
child: child,
),
);
} else {
return child;
}
}
```
https://github.com/user-attachments/assets/22707f97-5825-43f1-91b4-1a02a43437f5
Requires framework changes in https://github.com/jonahwilliams/flutter/pull/new/backdrop_id
Shell::OnPlatformViewDispatchPlatformMessage is called on the platform thread and must invoke the message handler on the UI thread. If the platform and UI threads are merged, then the handler can be run immediately. The shell can then post an empty task to the UI thread to make the UI message loop run the task observer that drains the microtask queue.
Fixes https://github.com/flutter/flutter/issues/156595
Addresses https://github.com/flutter/flutter/issues/155541
This does **not** remove Skia itself from the build. I'll stage that in a followup. Want to keep the scope of this patch small.
This is the new behavior:
* Impeller is the default on iOS devices **and** simulators.
* On iOS devices **only**, all flags and plist options are ignored.
* On iOS simulators **only**, Flutter will used Impeller's Metal backend by default and fallback to the Null backend if Metal device access is not available.
* On iOS simulators **only**, Flutter can pick Skia using the command line flags or plist flags. This is to allow users one more month to migrate as communicated by @zanderso.
The settings objects `enable_impeller` field is now `static constexpr` to avoid accidentally disabling the flag while it still exists.
The MockCanvas API was originally used to test the output of rendering methods to match them against a reference set of "MockData" structures. This API has been superseded by just using a DisplayListBuilder to record reference operations and then to compare the resulting DisplayList objects.
This PR deletes the last remaining uses of MockCanvas as well as the mechanism itself.
Aiks color source is more or less a one to one copy of the impeller::DisplayListColorSource. We can rip it out, instead of flutter::DisplayList::ColorSource -> impeller::Aiks::ColorSource -> impelller:Entity::ColorSourceContents, just go directly flutter::DisplayList::ColorSource -> impelller:Entity::ColorSourceContents.
Part of https://github.com/flutter/flutter/issues/142054
Reverts: flutter/engine#55393
Initiated by: matanlurey
Reason for reverting: Postsubmit turned red (clang tidy and build).
Original PR Author: jonahwilliams
Reviewed By: {chinmaygarde}
This change reverts the following previous change:
Hoist the content context up to the ios_surface so that overlay layers don't recreate a bunch of expensive machinery.
Fixes https://github.com/flutter/flutter/issues/154836
Aiks has been gutted, and most of the reming types need to be merged with dl/entity types and deleted. this is difficult to do with impeller/display_list and aiks as separate gn targets as it requires circular deps.
Lets merge them and remove the aiks directory.
Fixes https://github.com/flutter/flutter/issues/155133
Dl dispatching still relies on cull rects computed during that dispatch process. Make sure that the text frame dispatcher doesn't populate text frames that are way offscreen.
This culling is more conservative than the rendering dispatcher. We'd need to do some refactoring so the logic isn't repeated multiple times.
ui.Canvas and ui.SceneBuilder now use the DlPath object directly from the ui.Path object. This results in increased sharing of the wrapper objects which then increases the sharing of both the converted Impeller paths and Skia's volatile flag.
The VolatilePathTracker mechanism is deleted and rather than count the number of frames that a path is stable for, instead we count the number of times it is used for rendering. If a path is used 100 times in a single frame, it will become non-volatile and start being cached almost immediately. The cached Impeller paths are now also tracked for all instances of the same path, rather than for each call site that originated from a DisplayList dispatch.
I've found a few instances where Vulkan worked correctly but surface control did not. lets add a debugging flag we can ask folks to try to narrow down the issue.
Switch from using the clumsy manual CacheablePath object to a more automatic DlPath object for holding paths in DisplayLists and dispatching them to either Skia or Impeller with auto-conversion.
For now DlPath is just a wrapper around SkPath with an auto-generating Impeller Path object which is very similar in design from what was done with the CacheablePath object except that it manages the caching of the Impeller path internally without extra burden on Impeller or Skia. There is also no need to communicate with the Dispatch method as to which type of path you prefer, they're all "auto-converting" DlPath objects now.
For now, ui.Path still generates an SkPath and so we wrap it when we record it into a DisplayList, just like the former CacheablePath mechanism. It will be a simple conversion to create the DlPath wrapper in ui.Path, though, so as to maintain the cached Impeller paths across frames even if the DisplayList itself is not preserved.
Eventually DlPath will take on more of a role of hiding the construction and internal representation of the paths so that we could be using SkPath, impeller::Path, or some other internal storage. For now, SkPath will likely remain primary storage for a while so that we can deal with PathOps.
This is a re-land of https://github.com/flutter/engine/pull/55006, except that we special case the navigation channel to immediately dispatch its message if the isolate is not yet running.
This preserves the existing behavior relied upon by several iOS add2app tests, as well as the still used embedder v1 - and potentially undicovered future embedders.
Reverts: flutter/engine#55006
Initiated by: jonahwilliams
Reason for reverting: still failling mac module test
Original PR Author: jonahwilliams
Reviewed By: {jason-simmons}
This change reverts the following previous change:
Reland of https://github.com/flutter/engine/pull/54975
Fixes the initial route behavior for iOS. Previously the initial route setting would _always_ be posted as a task, which after merging the threads would fire after isolate creation, thus it would not actually update the initial route setting. Fixed Engine constructor so that it reads the initial route from the settings.
Reland of https://github.com/flutter/engine/pull/54975
Fixes the initial route behavior for iOS. Previously the initial route setting would _always_ be posted as a task, which after merging the threads would fire after isolate creation, thus it would not actually update the initial route setting. Fixed Engine constructor so that it reads the initial route from the settings.
Wean the DlOpReceiver interface and implementations off of using the SkScalar, Sk[I]Rect, and SkPoint objects in favor of our own DL/Impeller versions.
The start of an ongoing effort to eventually compartmentalize all use of Skia interfaces into a single backend rendering module that is one of 2 semi-pluggable renderers.
Reverts: flutter/engine#54975
Initiated by: jtmcdole
Reason for reverting: I believe this change caused flutter/flutter to break in an engine roll. It was one of two changes - the other being a webui change.
Tests broken:
module_test_ios: testDualCold
router test:
```
[2024-09-05 17:43:20.837343] [STDOUT] stderr: [ +135 ms] VMServiceFlutterDriver: Connected to Flutter application.
[2024-09-05 17:43:20.841927] [STDOUT] stdout: [ +4 ms] 00:00 [32m+0[0m:
Original PR Author: jonahwilliams
Reviewed By: {jason-simmons, johnmccutchan}
This change reverts the following previous change:
If we use runNowOrPostTask on platform channel responses, then we may not wake up the dart message loop. If nothing else wakes it up, then the embedder may hang on platform channel responses.
This fixes several google3 integration tests when run in merged thread mode.
If we use runNowOrPostTask on platform channel responses, then we may not wake up the dart message loop. If nothing else wakes it up, then the embedder may hang on platform channel responses.
This fixes several google3 integration tests when run in merged thread mode.
Fixes https://github.com/flutter/flutter/issues/153623
Fixes https://github.com/flutter/flutter/issues/153788
* Avoids slow CPU resizer
* Avoids us having to write any code at all to resize these images
* Avoids reading from the GPU staging buffers that are not read cached (similar to Glyph cache performance issues).
Also changes the single frame codec upload to either use the device private storage path or to defer upload until the GPU returns. This allows us to ensure that mips are always constructed and makes sure that we don't need to keep around the CPU texture reisze that depends on the Skia software backend.
Separately, I updated the deferred task system to allow separate success/failure tasks to make it easier to track the final state.
Make sure the old dispatcher cannot be used if the new dispatcher is enabled. Migrate tests using old canvas to new canvas, mostly to make deleting the old one easier...
The first part of switching Impeller/Aiks to using the display list instead of re-recording rendering operations. This should eventually let us cut CPU overhead of the raster thread for complex applications, though it should have no impact on GPU performance.
This does introduce a GLES only rendering bug that I haven't had luck tracking down, but is almost certainly due to switching to DL computed depth values. I'd like to handle this as a follow up when we prioritize GLES. https://github.com/flutter/flutter/issues/153504
Part of https://github.com/flutter/flutter/issues/142054
Reverts flutter/engine#54453
Not quite ready to remove Impeller Scene yet because I'm still porting
the animation functionality. Keeping it around allows me to switch back
and forth to compare without having to recompile the engine.
Over a month ago I said something like "we can revert this a couple of
weeks from now" in one of the Impeller meetings. But for better or worse
(better, I think), I ended up spending a ton of time trying to make the
PBR good first (which doesn't exist in this C++ version).
