Nearly all parts of the Impeller rendering pipeline now rely on DlPath
and PathSource objects for their path description needs. The impeller
path only exists for impeller's unit tests and benchmarks to have a
local way to construct paths. We implement path construction for the
DlPath object via DlPathBuilder which makes the impeller family of path
objects functionally obsolete and so they are now deleted.
Currently stroking paths in Impeller goes through a few steps:
- [Once per path object, if reused] If necessary, convert a source Skia
path to an Impeller path
- Ask the Impeller path object to create a polyline, with only line
segments each potentially marked as "coming from a curve"
- Widen the polyline treating curves specially
- render the widened vertex list
This PR attempts to streamline that process by iterating the original
curve and widening the strokes from that. It will require no conversion
of the path from SkPath to impeller::Path (if it was created as an
SkPath as is done for all ui.Path objects). It also avoids the
intermediate polyline stage. It will also produce fewer outline vertices
since it attempts to recognize which vertices are produced by the prior
and future line segments and avoid duplicating them.
Currently Impeller asks incoming paths to convert themselves into the
impeller::Path class before tessellating their outlines. This wastes
conversion time when the path is only needed to iterate the path
segments. We now tessellate them directly from a path segment dispatcher
saving the conversion cost.
Now that Impeller performs high fidelity tessellation of Conic curves we
will no longer convert Flutter app's conic curves into approximated
quadratic curves.
The 2 enum classes `impeller::BlendMode` and `flutter::DlBlendMode` were
essentially identical so they were merged.
The names were consolidated to the ui.BlendMode/DlBlendMode versions and
the implementation is shared from Impeller to DisplayList via a `using`
directive. No conversion is needed any more to describe blend modes
between DisplayList and Impeller.
Delete remaining DlCanvas method overloads that were allowing the use of
Skia geometry objects in rendering code.
The only remaining uses of Skia classes in the DlCanvas interface are
SkImageInfo and SkTextBlob.
These cause ambiguous compiler reference errors on some GCC versions.
And, the errors seem right. But, newer versions of clang seem to resolve
that ambiguity by also checking if the references to the types are also
references to identical types.
Per the chat with Jim, also added the Dl prefix to those types.
No change in functionality. Just a rename of the types and removal of
the now unnecessary typedefs.
Fixes SkRSXform task in https://github.com/flutter/flutter/issues/161456
Removes (nearly) all uses of Skia SkRSXform object from DisplayList and
replaces it with a new Impeller RSTransform geometry object.
There are remaining uses in:
- Skia adapter code which needs to convert them back to SkRSXform when
using the Skia backend
- dl_rendering_tests which is waiting for a major conversion effort
- ?Fuchsia? code has an SkCanvas spy adapter used in its embedder code
(not DisplayList related)
- web_ui/skwasm
Address the first item in
https://github.com/flutter/flutter/issues/161456 (Unit tests in the
display_list/ directory)
Some new `DlPath::Make<Shape>` factories were added to make test writing
simpler.
`DlPath` is now bi-directional! You can construct one from either an
`SkPath` or an `impeller::Path` and it will auto-convert to the other as
needed. This allows unit tests with custom paths to rely on
`impeller::Path` for path construction instead of `SkPath` (as long as
only simple move/line/quad/curve verbs are needed).
`RoundRect` now normalizes the argument rect in all constructors to
match Flutter expectations and `SkRRect` legacy behavior. This behavior
was already being enforced in `ui.rrect` but the unit tests we have to
verify the behavior are written against the `RoundRect` object itself so
this was the simplest way to make the unit tests work right, while
ensuring that we maintain correct behavior for `ui` objects. Ideally
these issues would be tested at the `ui` native interface instead of as
unit tests on our internal objects and we should be allowed to decide
how we want our internal APIs to behave with regard to this concept.
Skia inverted path types are no longer allowed in `DlPath` and all use
of them should be eliminated in the engine (except to test if they crash
when used in a debug unit test)
A couple of unit tests for `DlOpSpy` and Impeller's interop package were
migrated here along for the ride even though this PR was focused
primarily on `display_list/` unit tests.
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.
A new source code/header structure was introduced when the DlColorSource and DlImageFilter objects were migrated to Impeller geometry classes. Even though the DlColorFilter objects did not depend on Skia geometry objects, they need to be updated to the new source layout for consistency.
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.
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.
Extracts backend-specific code in DlSurfaceProvider to separate translation units. In particular, this allows for less conditional header includes, and more specifically, allows code relating to the Metal backend to include headers that include ARC-managed Objective-C types. Today we cast these all to void* (and manage refcounting manually) since these headers are included in dl_surface_provider.cc, which is a pure C++ translation unit.
No test changes since 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
Since Metal code frequently uses Objective-C types like id<MTLTexture>
etc. this migrates Metal translation units to Obj-C++. In particular,
this allows transitively including headers that include such types. This
in turn helps with refactoring `EmbedderTest`, `TestMetalContext`,
`TestMetalSurface` to avoid specifying `void*` types in headers and
manually refcounting via ARC bridge casts.
Reformats the source, since Objective-C files are linted to 100 columns
rather than the 80 column limit we impose on C++ files.
This change introduces no semantic changes.
Issue: https://github.com/flutter/flutter/issues/137801
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
This is the beginning of the bulk of de-skia-fication work in the engine. All of the standard types in the DlCanvas API now have overloads that specify the corresponding Dl type, mainly for Dl*Rect and DlPoint types. This enables further work to switch from SkFoo types to DlFoo types in the various engine modules culminating in the elimination of the old methods that use the Sk types.
All of the former methods that used the basic Sk types are now implemented as inlinable translation overloads and the underlying implementations of DlCanvas now implement only the newer style interfaces so that they don't need to be further modified as we eliminate the old Skia types from the interface.
There are still a couple of Skia types remaining in the DlCanvas API without any DL type variants which will be handled in a future phase:
- SkRRect
- SkRSXform
- SkTextBlob (will be hidden behind a common interface along with TextFrame)
- SkImageInfo (only used in a few calling sites)
The blur ImageFilter has a tile mode that describes how to sample pixels near the edge of the source. When used as a BackdropFilter this behavior is important as the wrong tile mode can cause distracting flashing as app content is scrolled under foreground widgets that blur their background. Unfortunately the Skia backend used to default the tile mode for all backdrop filters to `clamp` mode with no way to update it and that mode was the one that produced the most distracting flashing.
Recently Skia opened up control over the tile mode used for backdrop filters and we now take advantage of that capability so that app developers can now set the tile mode to a nicer value.
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.
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.
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.
Experimental Canvas was getting depth assertion errors while trying to use the depth values supplied by DisplayList. This was mainly due to a difference in understanding as to how many depth values to allocate to a drawImageNine operation.
n case there are additional discrepancies, debugging code is added to assert the understanding of how many depth values the experimental canvas uses on each rendering operation. The depth debugging can be turned on and off with a #define
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...
We will be updating the DisplayList dispatching to support random access iteration through the records so we need a benchmark to set a baseline to track any impact on the default way to dispatch a DisplayList.
Fixes: https://github.com/flutter/flutter/issues/151850
Re-adding an optimization originally included in https://github.com/flutter/engine/pull/53642 that detects when ClipRRect and ClipPath operations are actually ovals and redirects them to the ClipOval path during recording to save space and reduce the need for dispatchers to do the same detections and optimizations.
Converts several AIKS golden tests to use DisplayList as the mechanism.
In order to convert some of the tests, new factory methods were added to DlColor and tested with new unit tests (an earlier golden test conversion PR had a version of this as well).
Also, a new DisplayList record op was created for ClipOval to handle the AIKS clipping golden tests, but this new recording op is not used from Flutter `ui` code (no plumbing to call it from `lib/ui/painting` or to convert any other DisplayList call to use the new record). An earlier attempt to add the new recording op caused a large number of golden changes upstream so this version will only be used for internal tests and support to use it from apps will follow in more targeted PRs to better manage golden changes. This PR should not result in any changes to goldens outside of internal engine tests.
Reverts flutter/engine#53642
This change causes 10k golden updates internally and we need to land this out of band (go/lssc). There is also an existing issue with one particular client screenshot test - see b/350129213 for more details.
Reverts flutter/engine#53622
There were some golden changes which might be minor, but they weren't expected. Also, I noticed a problem in reducing drawPath down to drawRect and drawOval - that should not be done if the path has the inverse fill flag set...
Impeller supports `ClipOval` and will detect oval paths and rrects and use that call instead when appropriate. Adding support for `ClipOval` to DisplayList allows that optimization code to be moved up into the recording process.
The Vertices objects are already allocated in a shared object by default so copying them inline into the recording buffer is usually a waste of time rather than reusing the memory allocated for the shared object by recording a reference. Note that the shared DlVertices objects already inline all of their data so we have good data locality as it is without further copying the data into the buffer.
Might help with https://github.com/flutter/flutter/issues/150513
With this minor addition to the DlCanvas/DisplayList API the code in the paragraph builder no longer needs to worry about PathEffect objects and their varying support on the backends.
At this point all PathEffect code in the engine is obsolete and can be deleted, but I'll leave that for a follow-on PR.
The only PathEffect related thing I did delete was support for rendering primitives with a PathEffect in the DL Rendering tests, both because it is a vestigial attribute and also because it would interfere with the new DrawDashedLine rendering test (a PathEffect on top of a PathEffect...).
This mechanism pulls some clip-reduction logic from Impeller Entities up into DisplayListBuilder to simplify the work that Impeller has to do and also amortize the work if the DisplayList is reused from frame to frame. Since the DisplayList does not have access to information such as surface sizes, there will still be cases that Impeller can catch that must be conservatively included by the recording process in the Builder, so this mechanism does not replace the same code in Impeller, it merely catches some cases earlier, while recording widget output, to avoid the same work on every frame down in Impeller.
In this first pass only the most conservative and straightforward cases are handled - a clip on a layer which has a previous clip that the new clip fully contains (and which was not already restored).
This limited approach is already enough to eliminate a couple of clip operations in the layout of many of the pages in the `new_gallery` benchmarks.
