Work towards https://github.com/flutter/flutter/issues/133569.
This PR is a proof of concept that shows we're able to use `package:test` in `flutter/engine` instead of `package:litetest`.
I think it also shows that, if we're going to continue to vend dependencies this way, we might want to re-think our strategy in terms of using `pub` as a management tool - it's quite unwieldy already. For example, here is every `pubspec.yaml` file in the repo:
```sh
$ find . -name 'pubspec.yaml' -exec sh -c 'echo "$0 $(wc -l < "$0")"' {} \;
# Some files omitted in third_party or similar.
./impeller/tessellator/dart/pubspec.yaml 11
./tools/const_finder/pubspec.yaml 35
./tools/api_check/pubspec.yaml 90
./tools/build_bucket_golden_scraper/pubspec.yaml 47
./tools/licenses/pubspec.yaml 53
./tools/path_ops/dart/pubspec.yaml 26
./tools/engine_tool/pubspec.yaml 76
./tools/dir_contents_diff/pubspec.yaml 19
./tools/compare_goldens/pubspec.yaml 3
./tools/golden_tests_harvester/pubspec.yaml 55
./tools/gen_web_locale_keymap/pubspec.yaml 37
./tools/githooks/pubspec.yaml 63
./tools/android_lint/pubspec.yaml 35
./tools/clang_tidy/pubspec.yaml 76
./tools/pkg/engine_repo_tools/pubspec.yaml 41
./tools/pkg/process_fakes/pubspec.yaml 36
./tools/pkg/engine_build_configs/pubspec.yaml 73
./tools/pkg/git_repo_tools/pubspec.yaml 60
./tools/header_guard_check/pubspec.yaml 70
./sky/packages/sky_engine/pubspec.yaml 8
./shell/vmservice/pubspec.yaml 8
./ci/pubspec.yaml 57
./testing/benchmark/pubspec.yaml 77
./testing/skia_gold_client/pubspec.yaml 66
./testing/pkg_test_demo/pubspec.yaml 116
./testing/smoke_test_failure/pubspec.yaml 31
./testing/dart/pubspec.yaml 71
./testing/android_background_image/pubspec.yaml 22
./testing/litetest/pubspec.yaml 33
./testing/symbols/pubspec.yaml 24
./testing/scenario_app/pubspec.yaml 67
./web_sdk/web_engine_tester/pubspec.yaml 14
./web_sdk/web_test_utils/pubspec.yaml 22
./web_sdk/pubspec.yaml 60
./lib/snapshot/pubspec.yaml 8
./lib/gpu/pubspec.yaml 14
./lib/web_ui/pubspec.yaml 60
./flutter_frontend_server/pubspec.yaml 39
```
I'll file a follow-up issue to discuss pub-package management in the engine.
Work towards https://github.com/flutter/flutter/issues/144835.
Doc (_sorry, internal only_): [go/flutter-engine-goldens-workflow](http://goto.google.com/flutter-engine-goldens-workflow).
This implements the majority of the proposed workflow, that is, optionally having a plain-text version at the root of the directory, and using it to apply a unique suffix we can review in release branches. As it stands, this is a NO-OP outside of tests (it will have no impact, and can be ignored).
What's missing before using this feature in release branches:
- Optimization work with the infra team (not sure if blocking or not):
https://github.com/flutter/flutter/issues/145842
- A dry-run of this with the release team to make sure it works as intended
@gaaclarke As implemented, I _think_ we don't need anything special for [`dir_contents_diff`](286169bb52/tools/dir_contents_diff), but maybe I'm wrong - I think only the _test_ names are being changed, not the names on disk.
/cc @zanderso as well.
Uses the mipmap implementation from https://vulkan-tutorial.com/Generating_Mipmaps .
Today we generate all mip levels from mip level 0, which results in lost data. Instead we need to use the previous mip level
This fixes https://github.com/flutter/flutter/issues/141639
Most of this was previously unimplemented. It turns out the reason for the hang described in the github issue was that there was a typo in the name of the `getMiterLimit` C function, so if the client actually called that method the Wasm module failed to compile, as it couldn't find an import with the misspelled name.
Previously the keyboard was initialized after the view is created. This used to be necessary as the keyboard & text input plugins were strongly tied to a view (and would crash in headless modes). This is no longer necessary, and the keyboard can now be initialized normally as part of the engine initialization.
Part of https://github.com/flutter/flutter/issues/142845
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Currently destroying a view also shuts down the engine. This makes sense as in single-view world where the view also always owns the engine. However, in a multi-view world the views will share the engine. Destroying one view shouldn't necessarily shut down the engine unless that view owns the engine.
Part of https://github.com/flutter/flutter/issues/142845
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This PR groups per-view information in `FlutterCompositor` into a private class, `ViewPresenter`. This makes it easier to manage per-view data and write view operations.
Part of https://github.com/flutter/flutter/issues/145874.
Currently, view presenters are never removed once created, since the macOS runner doesn't support removing views. This will be added in the future.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Closes https://github.com/flutter/flutter/issues/141641.
Basically, this should verify "it's possible to use VSCode+LSC, at least in theory".
I'm open to writing a test, but given it _is_ a test I'm less sure it's valuable. Feel free to push back.
Closes https://github.com/flutter/flutter/issues/144064.
From a conversation with @camsim99 on Discord:
> Ah this was to debug that missing emulator issue (https://github.com/flutter/flutter/issues/137947) that Ricardo and I were seeing. We wanted to see if it was specific to the API 34, and if so, escalate the issue internally. @ricardoamador Would you like to keep running the tests with API 33? You mentioned you have found a workaround, and the test runs seem to not have yielded any evidence against API 34.
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
I tested against a few example apps and found this case is actually very common. These full screen/pass clips render nothing to the depth buffer can be particularly troublesome in the presence of backdrop filters (due to clip replay).
While this case doesn't actually result in anything getting written to the depth buffer, we still end up overwriting the entire stencil buffer twice: Once for the preparation draw, and again for the depth buffer transfer/stencil cleanup draw.
I had failed to take position affinity into account when calculating word boundaries on skwasm. This brings skwasm's behavior in line with canvaskit's.
At some point, we inherited a change from the repo-level `analysis_options.yaml` that [allowed deprecated](https://github.com/flutter/engine/pull/50575) members to be used. We rely on that analyzer error to prevent `solo: true` from being committed in our tests ([example](c935c3ba36/lib/web_ui/test/ui/line_metrics_test.dart (L179)) of one that slipped recently).
This PR overrides the `deprecated_member_use` error to enable it inside the web engine.
This PR connects the view ID provided by the rasterizer to `EmbedderExternalViewEmbedder`'s presenting call (see `embedder_external_view_embedder.cc`).
This PR also contains a refactor (which actually takes the majority of LOC) that moves the specification of view ID from `PrepareFlutterView` to `SubmitFlutterView`. The `flutter_view_id` parameter in `PrepareFlutterView` was added in https://github.com/flutter/engine/pull/46169. It turns out that we don't need the view ID throughout the preparation phase, so we can delay the specification to submission, which makes it feel cleaner.
### Tests
Existing tests are changed to verify that `SubmitFlutterView` receive the correct view ID.
The feature that `EmbedderExternalViewEmbedder` sends the view ID to the presenting API is not tested for now, because `EmbedderExternalViewEmbedder` is typically tested in `embedder_unittests`, but it requires the embedder API `AddView`. It will be tested in later changes to `EmbedderExternalViewEmbedder`.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
We don't intend to make much progress on the Impeller OpenGLES backend in 2024. I won't remove the capability to run these tests locally (the plumbing is already done and doesn't cost anything), but we don't need to have these just sit/flake on CI consuming resources.
Partial work towards https://github.com/flutter/flutter/issues/144064.
Reland https://github.com/flutter/engine/pull/51698.
This reverts commit 7922740184.
Attempts to improve https://github.com/flutter/flutter/issues/145274.
Our new clipping technique paints walls on the depth buffer "in front" of the Entities that will be affected by said clips. So when an intersect clip is drawn (the common case), the clip will cover the whole framebuffer.
Depth is divvied up such that deeper clips get drawn _behind_ shallower clips, and so many clips actually don't end up drawing depth across the whole framebuffer. However, if the app does a lot of transitioning from a huge clips to a small clips, a lot of unnecessary depth churn occurs (very common in Flutter -- this happens with both the app bar and floating action button in the counter template, for example).
Since progressively deeper layers in the clip coverage stack always subset their parent layers, we can reduce waste for small intersect clips by setting the scissor to the clip coverage rect instead of drawing the clip to the whole screen.
Note that this change _does not_ help much with huge/fullscreen clips.
Also, we could potentially improve this further by computing much stricter bounds. Rather than just using clip coverage for the scissor, we could intersect it with the union of all draws affected by the clip at the cost of a bit more CPU churn per draw. I don't think that's enough juice for the squeeze though.
Before (`Play/AiksTest.CanRenderNestedClips/Metal`):
https://github.com/flutter/engine/assets/919017/7858400f-793a-4f7b-a0e4-fa3581198beb
After (`Play/AiksTest.CanRenderNestedClips/Metal`):
https://github.com/flutter/engine/assets/919017/b2f7c96d-a820-454d-91df-f5fae4976e91
Support for Android's predictive back feature on internal Flutter routes. Reports predictive back gestures to the framework (where supported by the system).
Fixes https://github.com/flutter/flutter/issues/145823
If we try to render a solid filled empty size path, the subpath division code creates an obscene number of vertices that hits overflow errors. Just No-op instead.
We have been transitioning the depth/stencil atttachment to general, even though it can just be in attachment optimal (we never use it as an input attachment).
Reverts: flutter/engine#51698
Initiated by: bdero
Reason for reverting: Golden breakage on framework tree -- https://flutter-gold.skia.org/search?issue=145855&crs=github&patchsets=4&corpus=flutter
Original PR Author: bdero
Reviewed By: {jonahwilliams}
This change reverts the following previous change:
Attempts to improve https://github.com/flutter/flutter/issues/145274.
Our new clipping technique paints walls on the depth buffer "in front" of the Entities that will be affected by said clips. So when an intersect clip is drawn (the common case), the clip will cover the whole framebuffer.
Depth is divvied up such that deeper clips get drawn _behind_ shallower clips, and so many clips actually don't end up drawing depth across the whole framebuffer. However, if the app does a lot of transitioning from a huge clips to a small clips, a lot of unnecessary depth churn occurs (very common in Flutter -- this happens with both the app bar and floating action button in the counter template, for example).
Since progressively deeper layers in the clip coverage stack always subset their parent layers, we can reduce waste for small intersect clips by setting the scissor to the clip coverage rect instead of drawing the clip to the whole screen.
Note that this change _does not_ help much with huge/fullscreen clips.
Also, we could potentially improve this further by computing much stricter bounds. Rather than just using clip coverage for the scissor, we could intersect it with the union of all draws affected by the clip at the cost of a bit more CPU churn per draw. I don't think that's enough juice for the squeeze though.
Before (`Play/AiksTest.CanRenderNestedClips/Metal`):
https://github.com/flutter/engine/assets/919017/7858400f-793a-4f7b-a0e4-fa3581198beb
After (`Play/AiksTest.CanRenderNestedClips/Metal`):
https://github.com/flutter/engine/assets/919017/b2f7c96d-a820-454d-91df-f5fae4976e91
Instead of just returning, if our paragraph builder has empty text, we still need to generate a set of line breaks for skia to use. Otherwise, it will actually cause subtle memory access errors.
Reverts: flutter/engine#51685
Initiated by: matanlurey
Reason for reverting: goldctl does not disambiguate negatives from untriaged images (see 9b9adad080/gold-client/cmd/goldctl/cmd_imgtest_test.go (L325)).
Original PR Author: matanlurey
Reviewed By: {mdebbar, gaaclarke}
This change reverts the following previous change:
`flutter/engine`-side fix for https://github.com/flutter/flutter/issues/145043.
- Before this PR, if a negative image was encountered, we'd silently pass pre-submit, merge, and turn the tree red.
- After this PR, a negative image both makes pre and post-submit red.
Added tests, and fixed up some unrelated tests that were accidentally setting `pid` instead of `exitCode`. Oops!
/cc @zanderso and @eyebrowsoffire (current engine sheriff).
Attempts to improve https://github.com/flutter/flutter/issues/145274.
Our new clipping technique paints walls on the depth buffer "in front" of the Entities that will be affected by said clips. So when an intersect clip is drawn (the common case), the clip will cover the whole framebuffer.
Depth is divvied up such that deeper clips get drawn _behind_ shallower clips, and so many clips actually don't end up drawing depth across the whole framebuffer. However, if the app does a lot of transitioning from a huge clips to a small clips, a lot of unnecessary depth churn occurs (very common in Flutter -- this happens with both the app bar and floating action button in the counter template, for example).
Since progressively deeper layers in the clip coverage stack always subset their parent layers, we can reduce waste for small intersect clips by setting the scissor to the clip coverage rect instead of drawing the clip to the whole screen.
Note that this change _does not_ help much with huge/fullscreen clips.
Also, we could potentially improve this further by computing much stricter bounds. Rather than just using clip coverage for the scissor, we could intersect it with the union of all draws affected by the clip at the cost of a bit more CPU churn per draw. I don't think that's enough juice for the squeeze though.
Before (`Play/AiksTest.CanRenderNestedClips/Metal`):
https://github.com/flutter/engine/assets/919017/7858400f-793a-4f7b-a0e4-fa3581198beb
After (`Play/AiksTest.CanRenderNestedClips/Metal`):
https://github.com/flutter/engine/assets/919017/b2f7c96d-a820-454d-91df-f5fae4976e91
