I am guessing this just either served it's purpose (https://github.com/flutter/flutter/issues/26654, https://github.com/flutter/flutter/issues/26728) or didn't and was never followed up. I can't find any examples of it running on CI - I suspect if we want a test like this, we're better off just adding it to `scenario_app` versus creating more 1-off Android integration test environments.
Work towards https://github.com/flutter/flutter/issues/145988.
Should be a NO-OP in behavior, but hopefully make some of the false negatives less confusing (i.e. getting "missing X outputted files when the test is about to fail anyway".
Part 1 of https://github.com/flutter/flutter/issues/145263
This PR updates the names of builds outside of `local_engine.json` to be
prefixed with the string `ci/` (or `ci\` on Windows). For better or
worse, the "name" field of a build is used to construct a path used as
the source directory of a copy operation (I think the CAS archive
step?). Because of that, changing the name of a build also requires
updating the build output directory of the ninja build.
This PR also adds tests to make sure the naming of these builds remains
consistent.
Closes https://github.com/flutter/flutter/issues/145957.
As @jonahwilliams and @johnmccutchan and I discussed (https://github.com/flutter/flutter/issues/144407), the functionality that was being tested was actually _Android's_ ability to rotate and crop `SurfaceTexture`-backed textures. This same functionality doesn't even exist in the `ImageReader`-based textures (read: modern Android devices):
> Due to an oversight by Android, ImageReader backed surfaces do not respect metadata applied to the surface (rotation & crop). Rotation information is not available at all and crop information is corrupted by the ImageReader (only the width/height is propagated the origin offset is not).
We might decide to re-add this functionality in the Dart `Texture` widget, but given we'll be migrating our plugins to `SurfaceProducer` (again, read: using `ImageTexture` for most Android phones), it's pointless to test this (and isn't even testing Flutter's code).
This reduces our test suite significantly (8 tests down to 2), which should also help with runtime and flakiness.
/cc @zanderso who I'm sure will be stoked.
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.
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
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
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).
Resolves https://github.com/flutter/flutter/issues/144333. (Specifically, this [follow-up issue](https://github.com/flutter/flutter/issues/144333#issuecomment-2002399870))
When drawing objects with a perspective transform, the rasterizer applies perspective correction for interpolated vertex attributes. By absorbing W and replacing Z with a discrete value, we were essentially removing all perspective information and disabling perspective correction.
Instead, we can manipulate the Entity transform to remap the output Z range to fit within the small depth slices that each Entity is allotted.
The golden draws a clip sandwich:
1. Draw and restore a difference clip _before_ drawing the airplane image. This clip will get drawn to the depth buffer behind the airplane image.
2. Draw an oval clip that applies to the airplane image. This clip will get drawn in front of the airplane image on the depth buffer.
3. Draw the airplane image with a 3D rotation and perspective transform.
4. Draw a semi-translucent blue circle atop all previous draws.
Before:
https://github.com/flutter/engine/assets/919017/c2a7d012-714e-4234-83ac-61c792172f30
After:
https://github.com/flutter/engine/assets/919017/de3b78ff-00bf-4bc9-8821-8e86b9a9e6bf
`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).
The previous PR https://github.com/flutter/flutter/issues/143420 rounds out the layers and rounds in the platform views. This results in missing pixel on the edge of the intersection when there's fractional coordinate (as shown in the screenshot below), because platform view is below the layers.
It turns out that we have to round out both platform view and layers, because:
- rounding in platform view rects will result in missing pixels on the edge of the intersection.
- rounding in layer rects will result in missing pixels on the edge of the layer that's on top of the platform view.
This PR simply skips the single (or partial) pixel on the edge, which is a special case, while still preserve the `roundOut` behavior for general non-edge cases.
Before the fix, notice a very thin gray line cutting through the purple box:
<img src="https://github.com/flutter/engine/assets/41930132/1482d81a-337e-4841-ac08-eff08bbc71ef" height="500">
Then after the fix, the gray line is gone:
<img src="https://github.com/flutter/engine/assets/41930132/0eddae69-ab62-4de6-8932-c67cc5aced73" height="500">
*List which issues are fixed by this PR. You must list at least one issue.*
https://github.com/flutter/flutter/issues/143420
*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
Address the performance of platform view due to an extra overlay. This overlay was added due to the following rounding problem:
> For example, if we interleave flutter widget and platform view in a list, and let's say we have a flutter widget (top = 0, bottom = 100.1), and a platform view below that widget (top = 100.1, bottom = 200). They are NOT supposed to be overlapping. However, after rounding out, we will get flutter widget (top = 0, bottom = 101), and platform view (top = 100, bottom 200). This will result in 1 pixel overlap as long as there's a floating point in the coord.
(Quote myself from the discussion below).
*List which issues are fixed by this PR. You must list at least one issue.*
Fixes https://github.com/flutter/flutter/issues/143420
*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
Sets up rules to create an APK that is comprised of solely native code. Existing executable targets (like GTests) can then use this to run on Android devices while having access to activities, windows, etc.. This allows for broader test coverage. Basically, anything that needed an ANativeWindow could only be tested in an integration test.
Executables that need access to the native activity must provide an implementation of `NativeActivityMain` that returns a custom subclass of `flutter::NativeActivity`. The `native_activity_apk` reads like an `executable` or `shared_library` target. Just one that packages that executable in an APK.
The APK is built using the Android Tools and does not use Gradle. Creating a new APK after invalidating some code takes ~200ms on my machine. The edit, compile, run cycle for only a tiny bit worse than testing on the host.
Builds on top of this new infrastructure to create a `GTestActivity` that runs an existing test suites. This works really well except the GTest suite logs to `STDOUT` whereas the engine logs to `logcat`. To quickly work around this, a custom test status listener has been wired up. This only displays the test results to logcat today but a similar mechanism can be used to talk to the test runner in the host. I will wire this up in an upcoming patch as there is no hooks into this from CI right now.
Creates an APK variant of the `impeller_toolkit_android_unittests` harness.
This reverts commit 037aa6b4caa6a3a726e67e519324b2c0a1ec274a.
The original cause of the revert was a flake introduced because the unit-test could request a frame from the Choreographer if it ran long enough. The availability checks in the choreographer were inaccurate after we intentionally backed out of using the Callback32 variant on 32 bit platforms.
The new tests didn't catch it because of an unrelated issue. In the first version of the patch for review, the proc table was only supposed to run on API levels 29 and above. When @dnfield requested we also get rid of the NDK helpers, the choreographer and additional utilities were added. But the API level gate in the new test harness wasn't removed. This made the tests be skipped. That gate has been removed entirely now. The error that cause the revert because of flakiness will now be a reliable failure.
Work towards https://github.com/flutter/flutter/issues/145219.
Previously all logging would be silent in the case that `test_foo` uploads a digest that is considered "untriaged", and we'd be entirely reliant on the `flutter-gold` check to pick this up asynchronously.
As part of debugging https://github.com/flutter/flutter/issues/145219 (but probably to keep this code indefinitely, it's not harmful), we now unconditionally log the swallowed failures to `stderr` so they will show up in our LUCI logs.
/cc @gaaclarke @jonahwilliams
Only available on Android device API levels >= 29. Proc table is setup has versioning checks. All handles are type safe. Collection of handles takes into account cleanup tasks (like reparenting surface controls). The proc table contains code duplicated in ndk_helpers and I will remove that in favor of this in a subsequent patch.
Part of https://github.com/flutter/engine/pull/51213 being chopped up.
Previously the DisplayListBuilder would only pass along bounds for a saveLayer when they were supplied by the caller that was building the DisplayList. This would require Impeller to use post-processing of the EntityPass lists to compute them on its own.
DisplayList can now compute those bounds as it builds the DisplayList to save dispatch clients from having to do so on their own. It will also provide an indicator in the case when the caller supplied bounds that ended up being too small to capture all of the content, causing clipping by the layer render target.
Closes https://github.com/flutter/flutter/issues/144365.
Wasn't able to delete as much as I wanted (it's used by the video-rendering code path), but it should fix the problem we're seeing. I expect this change to remove the bottom (Android-rendered) image from our golden files for `ExternalTexturesTests`.
Reverts: flutter/engine#51429
Initiated by: bdero
Reason for reverting: Engine tree breakage
```
+ /b/s/w/ir/cache/builder/src/out/host_release/display_list_region_benchmarks --benchmark_format=json
/b/s/w/ir/cache/builder/src/flutter/testing/benchmark/generate_metrics.sh: line 17: /b/s/w/ir/cache/builder/src/out/host_release/display_list_region_benchmarks: No such file or directory
```
Original PR Author: flar
Reviewed By: {godofredoc}
This change reverts the following previous change:
These benchmark results aren't shown in Skia perf because they were never added to the CI tasks to run.
- Replaced manual `StringBuffer()..writeln('stdout: ...')` with a single
`SkiaGoldProcessError` constructor.
- Updated tests to make sure it's working.
_/cc @dnfield @jonahwilliams FYI only._
