This PR relands https://github.com/flutter/engine/pull/44473.
The previous PR was immediately reverted after merging because we found that the PR could cause illegal renders to be skipped on debug builds but crash the app on release builds. This PR makes the `Animator::Render` skip illegal renders as well. This should not be the final shape of this feature, and thus a TODO is added.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
As far as I can tell, this functionality has _always_ been falling back to software rendering with Skia when Impeller was enabled. This is at best extremely misleading. Since I started putting Impeller specific objects in the display list it introduced crashes, and must be disabled.
Fixes https://github.com/flutter/flutter/issues/136847
This PR refactors `Rasterizer` so that it's more suitable for multi-view.
Design doc: [flutter.dev/go/multi-view-pipeline-and-rasterizer](http://flutter.dev/go/multi-view-pipeline-and-rasterizer)
With this change, `Rasterizer::DrawToSurfaces` has a structure that can handle drawing to multiple views, although the lack of some functionality still blocks it, mostly related to `ExternalViewEmbedder` and `RasterCache`.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Due to https://github.com/flutter/flutter/issues/127500 , we can get in a state where enable-impeller is true but we're using Skia. We need to either fall back completely to Skia, make this configuration fatal, or remote the check
----------------
Conversion of SkTextBlobs to impeller::TextFrame objects is one of the most expensive operations in display list dispatching. While the rest of the engine and framework makes a reasonable attempt to cache the SkTextBlobs generated during paragraph construction, the design of the dl dispatcher means that these the Impeller backend will always reconstruct all text frames on each frame - even if the display list/picture that contained those text frames was unchanged.
Removing this overhead is one of the goals of https://github.com/flutter/engine/pull/45386 , however this patch is also fairly risky and will be difficult to land. As a more incremental solution, we can instead construct the impeller::TextFrame objects when performing paragraph painting and record them in the display list. This both moves the text frame construction to the UI thread and allows the framework/engine to cache unchanged text frames.
This also does not conflict with the dl_aiks_canvas patch directly, and is fine to land before or after it does. (though I'd argue we should land this first).
To compare the current performance levels, I ran the complex_layout_scroll perf test, since this is fairly text filled. On a Pixel 6 pro. Across several runs this is a fairly consistent ~1ms raster time improvement.
Fixes https://github.com/flutter/flutter/issues/133204
Closes https://github.com/flutter/flutter/issues/135057.
This is a fair bit more involved than previous changes, just due to the sheer number of implicit conversions.
Highlights:
- Made `public uint32_t argb` `private uint32_t argb_`, and added `argb()` instead.
- Added `ToSk(DlColor)` instead of using implicit conversions.
There were a bunch of places where I had to make a judgement call (particularly in tests) to keep the code a bit "messy", i.e. `DlColor(SK_RED)`, just to make the diff as small as possible and to prevent silly copy and paste bugs. I'd be open to filing a follow-up issue to reduce unnecessary wrapping.
Reverts flutter/engine#45418
Some google3 tests are hitting the CHECK I added in the DlSkCanvasDispatcher::drawTextFrame, which indicates that the SkParagraph code likely thinks impeller is enabled, whereas other code might be running with Skia.
Perhaps this could happen if its software rendering? It should be a fatal error on startup so we can track this down.
Conversion of SkTextBlobs to impeller::TextFrame objects is one of the most expensive operations in display list dispatching. While the rest of the engine and framework makes a reasonable attempt to cache the SkTextBlobs generated during paragraph construction, the design of the dl dispatcher means that these the Impeller backend will always reconstruct all text frames on each frame - even if the display list/picture that contained those text frames was unchanged.
Removing this overhead is one of the goals of https://github.com/flutter/engine/pull/45386 , however this patch is also fairly risky and will be difficult to land. As a more incremental solution, we can instead construct the impeller::TextFrame objects when performing paragraph painting and record them in the display list. This both moves the text frame construction to the UI thread and allows the framework/engine to cache unchanged text frames.
This also does not conflict with the dl_aiks_canvas patch directly, and is fine to land before or after it does. (though I'd argue we should land this first).
To compare the current performance levels, I ran the complex_layout_scroll perf test, since this is fairly text filled. On a Pixel 6 pro. Across several runs this is a fairly consistent ~1ms raster time improvement.
### Skia
```
"average_frame_build_time_millis": 1.497333333333333,
"90th_percentile_frame_build_time_millis": 2.038,
"99th_percentile_frame_build_time_millis": 17.686,
"worst_frame_build_time_millis": 23.095,
"missed_frame_build_budget_count": 3,
"average_frame_rasterizer_time_millis": 5.5078589743589745,
"stddev_frame_rasterizer_time_millis": 2.226343414420338,
"90th_percentile_frame_rasterizer_time_millis": 7.481,
"99th_percentile_frame_rasterizer_time_millis": 19.11,
"worst_frame_rasterizer_time_millis": 79.799,
"missed_frame_rasterizer_budget_count": 7,
"frame_count": 234,
"frame_rasterizer_count": 234,
"new_gen_gc_count": 10,
"old_gen_gc_count": 2,
```
### Impeller (ToT)
```
"average_frame_build_time_millis": 1.431575000000001,
"90th_percentile_frame_build_time_millis": 2.196,
"99th_percentile_frame_build_time_millis": 14.486,
"worst_frame_build_time_millis": 23.728,
"missed_frame_build_budget_count": 2,
"average_frame_rasterizer_time_millis": 6.536087499999999,
"stddev_frame_rasterizer_time_millis": 1.9902712500000004,
"90th_percentile_frame_rasterizer_time_millis": 9.705,
"99th_percentile_frame_rasterizer_time_millis": 14.727,
"worst_frame_rasterizer_time_millis": 17.838,
"missed_frame_rasterizer_budget_count": 1,
"frame_count": 240,
"frame_rasterizer_count": 240,
"new_gen_gc_count": 10,
"old_gen_gc_count": 2,
```
### Impeller (Patched)
```
"average_frame_build_time_millis": 1.4500167364016743,
"90th_percentile_frame_build_time_millis": 2.478,
"99th_percentile_frame_build_time_millis": 14.883,
"worst_frame_build_time_millis": 18.782,
"missed_frame_build_budget_count": 1,
"average_frame_rasterizer_time_millis": 5.023033333333336,
"stddev_frame_rasterizer_time_millis": 1.6445388888888894,
"90th_percentile_frame_rasterizer_time_millis": 7.814,
"99th_percentile_frame_rasterizer_time_millis": 13.497,
"worst_frame_rasterizer_time_millis": 15.008,
"missed_frame_rasterizer_budget_count": 0,
"frame_count": 239,
"frame_rasterizer_count": 240,
"new_gen_gc_count": 8,
"old_gen_gc_count": 0,
```
Fixes https://github.com/flutter/flutter/issues/133377
The default allocation of RasterCacheItems in the layer tree was showing up in profiles of apps running on Impeller which don't actually use the raster cache.
In order to eliminate the overhead of those allocations, RasterCacheItems are now lazily allocated only when the layers encounter an actual raster_cache during the Preroll phase.
Relands https://github.com/flutter/engine/pull/45131
Fixes https://github.com/flutter/flutter/issues/132416
Differences from last time:
- Some minor merge conflict fixes
- Use the RTree to get the bounds instead of recalculating the bounds
- Make the iOS platform view controller implementation use the impeller-aware slices instead of the display list ones. This has been fixed for Android and the desktop embedding, but I missed iOS. The unit tests weren't actually running before I branched for my PR, @zanderso fixed them up separately and this resulted in catching the failures on post submit last time.
Closes https://github.com/flutter/flutter/issues/126009.
One major change, as-per @jonahwilliams's feedback, is that I created a helper class called `DlVertexPainter`, which provides a `DrawRect`-like API, but just builds a buffer of vertices, resulting into just a single:
```cc
// Actually draw.
canvas->DrawVertices(painter.IntoVertices(), DlBlendMode::kSrc, paint);
```
Also, added test for it, since there is no way to screenshot test to Impeller overlay yet.
# Impeller
Reverts flutter/engine#45131
This is failing the Impeller variants of the unobstructed platform views tests:
https://ci.chromium.org/ui/p/flutter/builders/prod/Mac%20Production%20Engine%20Drone/132249/overview
```
Failing tests:
-[UnobstructedPlatformViewTests testPlatformViewsMaxOverlays]
-[UnobstructedPlatformViewTests testOneOverlay]
-[UnobstructedPlatformViewTests testOneOverlayPartialIntersection]
-[UnobstructedPlatformViewTests testTwoIntersectingOverlays]
-[UnobstructedPlatformViewTests testOneOverlayAndTwoIntersectingOverlays]
** TEST FAILED **
```
We've retried it a few times so I suspect this isn't a flake.
This is a reland of https://github.com/flutter/engine/pull/44248
Fixes https://github.com/flutter/flutter/issues/132416
Changes from last time:
- The `drawPoints` benchmark was failing to render anything meaningful because of an error in `AiksLayer` that resulted in an infinitely sized bounding rectangle poisoning the bounds with `NaN` (this happens, for example, with a `drawPaint` call, which that benchmark happens to use). Added a test covering this and filed https://github.com/flutter/flutter/issues/132770 to explore ways to avoid this in the future.
- There was a bug in `DlAiksCanvas::SaveLayer` where a `nullptr` `paint` but non-`nullptr` `backdrop` was failing to actually save the layer. This resulted in incorrect rendering.
- There was a bug in `impeller::Canvas::DrawPicture` that resulted in incorrect stencil depth counting. That was fixed separately by @bdero, but was the cause of incorrect rendering in some Wonderous screens.
- I've added a simple implementation for `AiksLayer::IsReplacing`. It does not currently compare as deeply as the `DisplayListLayer` version potentially does, but it is good enough to avoid the regression noted in https://github.com/flutter/flutter/issues/132071. That regression was on a benchmark that greatly benefits from partial repaint. With the new implementation, it still gains partial repaint where it previously did not. There is more work that can be done here, filed https://github.com/flutter/flutter/issues/133361 to track that work.
I merged but did not fully integrate the `DisplayListBuilder`/`CanvasToReceiver` work that @flar has done. I have a local experiment with that, but would prefer to see this land and run through the device lab so we get some better comparison numbers for which one performs better.
## Description
This PR completes the documentation for `RasterStatus::kSkipAndRetry` in the `compositor_context.h` file. The previous comment ended abruptly, leaving ambiguity regarding the behavior when the thread merger is disabled.
### Changes:
- Added clarification on the behavior when the thread merger is disabled.
- Explained the potential performance implications when threads are not merged.
## Related Issues
https://github.com/flutter/flutter/issues/131814
## Tests
No tests were added as this PR only involves documentation changes.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
DisplayListBuilder grew over time from a class that implemented a developer-unfriendly stateful API into one that implemented both the developer-friendly DlCanvas API as well as its original stateful API. Over time, the stateful API was buried under a "testing only" facade.
In the meantime, the optimization features that it applies to the DlCanvas calls before it records the operations in a DlOp store are useful without the recording process and so I've been wanting to break those into 2 parts for a while with the goal of removing all stateful APIs from DisplayListBuilder (see https://github.com/flutter/flutter/issues/108303).
This PR takes a major step along that direction by splitting DisplayListBuilder into essentially a convenience class that marries 2 new classes together to achieve its old functionality:
- `DlCanvasToReceiver` - a class that implements DlCanvas, optimizes common situations, and then sends commands to any object that implements `DlOpReceiver`
- `DlOpRecorder` - an implementation of DlOpReceiver that records the operations in a buffer from which to create a `DisplayList` object
- `DisplayListBuilder` now inherits from DlCanvasToReceiver to get the optimizations and provides it with an instance of `DlOpRecorder` as the receiver that it will send its results to
- Similarly, a `DlCanvasToReceiver` instance could be directed to an `impeller:DlDispatcher` to achieve a more straight-through path from the DlCanvas interface to impeller Pictures.
Fixes https://github.com/flutter/flutter/issues/130141
The primary goal of this patch is to move dispatching of `dart:ui` `Canvas` commands to the UI thread.
Before this patch, the architecture is something like:
## UI Thread
- `dart:ui` talks to `DisplayListBuilder`, a `DlCanvas` implementation.
- `DisplayListBuilder` does some clip/bounds tracking and creates a `DisplayList` object that is held by `dart:ui`'s `Picture` objects.
- `DisplayList`s are added to `DisplayListLayer`s in `flow`.
## Raster Thread
- `flow` flattens the various operations into a single `DisplayList` via another `DisplayListBuilder`.
- A `DlOpReceiver`implementation converts that `DisplayList` into an `Aiks` `Canvas`/`Picture`.
After this patch, the architecture instead looks like:
## UI Thread
- No change for Skia.
- If Impeller, use a new `DlCanvasImplementation` that talks to `Aiks`'s `Canvas`.
- If Impeller, `dart:ui` Picture's now hold an `Aiks` `Picture`, which get shared into `AiksLayer`s in `flow`.
## Raster thread
- No change for Skia, but some light refactoring for places that assumed a `DisplayListBuilder` where they really just needed a `DlCanvas`.
- The `Aiks` `Picture`s are combined using new API on `DlCanvas` and still backed by `Aiks`.
These changes show significant improvement on raster times on Android and only very small regressions on UI times in local testing, see https://gist.github.com/dnfield/26528090194c9f5abdbac13cdcbf4f79 for old gallery transition perf numbers.
Many of the other changes in this patch are related to the following:
- Making `DlRTree` usable for Impeller.
- It would be nice to have a version of DlRTree that speaks `impeller::Rect`.
- Creating the requisite classes to support `EmbeddedViews` so that Desktop works.
This patch does not remove the `impeller::DlDispatcher`, which now would only be used in tests.
Stop using the special fuchsia headers, as they are contributing to
undefined behavior when building with LTO.
They are also no longer needed. All relevant fuchsia extensions were
upstreamed some time ago.
In the process, engage in some cleanup which means the "vulkan_config"
is also no longer needed. Rely on GN "public_deps" to provide the vulkan
include paths to various source sets.
See b/291462180 for more details on why this fix was neccesary.
Fixes https://github.com/flutter/flutter/issues/130775
On the Skia backend, antiAliasWithSaveLayer is the highest fidelity clipping option. In the Impeller backend, there isn't any difference in how we clip, since the stencil buffer is always used. Nevertheless we were still inserting the save layer, which results in an extra offscreen texture and is wasteful.
Track if impeller is enabled in the diff/preroll/paint context and avoid inserting a save layer.
Fixes https://github.com/flutter/flutter/issues/129862
This reverts commit cd52c0ce82b37bc9d16bea26fbd1753174c49e42.
These changes were exposing an underlying bug in the DisplayListMatrixClipTracker that has now been fixed independently (https://github.com/flutter/engine/pull/43664). There are no changes to the commit being relanded here, it has been tested on the Wondrous app which demonstrated the regression before the NOP culling feature was reverted and it now works fine due to the fix of the underlying cause.
These sources have long been obsolete for most of the engine, except for the Fuchsia sources. They do not need to be built for any other platform so this PR moves them into a Fuchsia platform-specific build directory so that they are only present in the Fuchsia binaries.
Most of the #include directives for SkPicture are removed except where they are still functional. Many comments rewritten to no longer be SkPicture-centric.
- DL unit tests still use it for consistency testing
- rasterizer/engine still use it for screen shot support
- Fuchsia still uses it extensively
