### 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
During some new development work that might make transform resets more expensive we realized that the resets were mostly coming from the calls to snap the transform to a pixel translate value and many of those were NOPs since the transform was already on a pixel translate value. This PR will avoid those trivially unnecessary reset operations.
This PR changes the format check on CI to use the command added in
https://github.com/flutter/engine/pull/50747.
Additionally, while making this change, I noticed that the CI check was
not checking the formatting of all files, and that as a result, files
were present in the repo with incorrect formatting. I have fixed the
formatting and fixed the check to always check all files.
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
This reverts commit d5d8b5de90f40644f23d309c0f68d03e299334eb.
The flutter/tests/skp_generator test (running in flutter_tester on
Linux) was failing, along with some customer tests.
In https://g-issues.skia.org/issues/305780908 Skia is removing the
default SkFontMgr. Previous work consolidated all references to
txt/platform.h and this replaces those last references. I attempted to
mirror the existing functionality, which still responds to GN flags and
the target platform.
After this PR, Flutter should not be depending on the default fontmgr
(and the defines in flutter_defines.gni) will maintain that behavior
until the legacy functions/methods are deleted from Skia. There were a
few tests that I missed on an earlier PR which relied on the default
font (helper added in #47493). These tests were failing because they
were making some assertions related to TextBlobs, which didn't work if
the (now-empty) Typeface they loaded had no glyphs. Thus, I added a few
extra asserts to make sure these textblobs *had* glyphs which make the
failing tests less mysterious, should this issue crop up again.
I cleaned up Flutter's BUILD.gn file for Skia a bit, deleting unused
targets related to the font managers. This involved fixing an implicit
dependency from //third_party/glfw/ to `Gdi32.lib` on Windows.
## 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.
- [ ] 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
This PR adds multiview support for `ExternalViewEmbedder`.
## Nomenclature
The term **view** can be ambiguous in `ExternalViewEmbedder`, and therefore the following terms are used:
* A **native view** refers to the final drawing surface that to composite layers to. It is the "view" used in other places of the engine, such as `Shell::AddView`.
* A **platform view** refers a platform view, a layer that holds content to be embedded.
## Change
The lifecycle of `ExternalViewEmbedder` is changed:
<table>
<thead>
<tr>
<th>Before PR</th>
<th>After PR</th>
<th>How it's called</th>
</tr>
</thead>
<tbody>
<tr>
<td rowspan=2>BeginFrame</td>
<td>BeginFrame</td>
<td>Once per frame</td>
</tr>
<tr>
<td>PrepareFlutterView</td>
<td>Once per flutter view</td>
</tr>
<tr>
<td>SubmitFrame</td>
<td>SubmitFlutterView (renamed)</td>
<td>Once per flutter view</td>
</tr>
<tr>
<td>EndFrame</td>
<td>EndFrame (unchanged)</td>
<td>Once per frame</td>
</tr>
</tbody>
</table>
* `BeginFrame` should perform per-frame actions, such as merge-unmerging threads.
* `PrepareView` should perform per-native-view preparations, such as recording the view ID and view size.
This change is necessary because some actions in `PrepareView` needs to be refreshed at the beginning of drawing every native view.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
The root slice of a surface frame was requesting the construction of an RTree from its DisplayList when it was built from a layer tree, but since the layer tree already does branch culling, it would not benefit from any further culling during `DisplayList::Dispatch`. Further, if there are platform views present in the layer tree then they will need an RTree to accurately convey "pixel ownership" information to the platform, but the root slice lives below any and all platform views, so it is the only slice that doesn't need an RTree for that case.
The cost of having an RTree in that slice was the accumulation of information and lists of rects that would never prove useful.
With the current stopwatch design, we request the frame rate multiple times per frame. On Android this calls into JNI which is pretty slow. If we cache the value at construction of the StopwatchVisualizer, then we will only compute it once per frame (because the StopwatchVisualizer is reconstructed each frame).
Fixes https://github.com/flutter/flutter/issues/137797
So the issue isn't that we're checking the fresh rate every frame, its that we were checking N times on each frame.
The new `BUILD.gn` files in the Engine tree can't go under
`build/secondary` because Skia still has its own, and they'd be selected
first. So, this PR puts the new `BUILD.gn` files under `flutter/skia`.
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.
