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.
Update SaveLayerConsolidation unit tests to allow 1px differences due to rounding differences when Skia elides image filter passes and can calculate the CF and opacity in full float without an intermediate conversion to 8-bit color.
This is the last remaining use of the staging that will allow closing https://g-issues.skia.org/issues/40042615.
*List which issues are fixed by this PR. You must list at least one issue.*
*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
The DisplayListBuilder now tracks the blend mode(s) used for its contents and whether it contains a child SaveLayer that uses a backdrop filter - both conditions that could require the graphics engine to use a different type of layer to satisfy the requests.
blend modes are tracked as the "highest" blend mode enum used by any content (defaults to kClear) as the enum values tend to be ordered so that larger values will tend to require more complicated render-target accesses.
The root layer of the DisplayList can be queried for both conditions on the root layer using methods on the DisplayList class.
Reverts https://github.com/flutter/engine/pull/52725
This commit makes some long-needed internal improvements to the way that the DisplayListBuilder manages its per-save/saveLayer data. The information for layer bounds and matrix/clips is now maintained in the layer info structure itself rather than shared across a number of stack structures which required careful alignment of the 3 different stacks and made it more difficult to compare and update adjacent layers during save and restore operations.
The new code stores all information for a layer within a single structure and the save and restore operations can be more clear about which information they are getting or setting in the current and parent layers.
In addition, the layer bounds accumulations were updated to have a more flexible algorithm for detecting overlap of rendering operations for the opacity peephole optimization. Previously, more than one rendering op on a layer would prevent opacity peephole optimizations, but now the condition will be recognized until the first rendering op that overlaps the bounds of the previous rendering operations. This will help for some potentially common cases of 2 non-overlapping ops or even a list of rendering operations laid out in a row.
The DisplayList will now track the "render depth" of all rendering ops and provide the total content depth for DisplayList and save[Layer]/restore pairs to enable depth clipping in the back ends.
The depth accounting process is described in the class comment for `DlOpReceiver`
Relands https://github.com/flutter/engine/pull/51589
The fix is in 74397bc171c74d2bfb24e82b47f2aa29d70c1711. I couldn't
figure out how to get a test in the engine to cover it. The test is in
the devicelab.
Here's what I attempted:
```c++
TEST_P(AiksTest, BlendModePlusAlphaColorFilterAlphaWideGamut) {
if (GetParam() != PlaygroundBackend::kMetal) {
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
}
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
PixelFormat::kR16G16B16A16Float);
Canvas canvas;
canvas.Scale(GetContentScale());
canvas.DrawPaint({.color = Color(0.1, 0.2, 0.1, 0.5)});
canvas.SaveLayer({
.color_filter = ColorFilter::MakeBlend(BlendMode::kPlus,
Color(Vector4{1, 0, 0, 0.5})),
});
Paint paint;
paint.color = Color(1, 0, 0, 0.5);
canvas.DrawRect(Rect::MakeXYWH(100, 100, 400, 400), paint);
paint.color = Color::White();
canvas.Restore();
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}
```
## 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 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
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[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
