Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 0c5504e

Browse files
authored
[impeller] adds test for catching shimmer in gaussian blur (#54116)
fixes flutter/flutter#152195 Part of a series of gaussian blur changes: 1) #54148 1) #54116 1) #54150 1) #54149 Note: this test won't pass until #54148 lands. ## 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
1 parent a18b6e1 commit 0c5504e

9 files changed

+188
-26
lines changed

impeller/display_list/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ template("display_list_unittests_component") {
9191
deps += [
9292
":display_list",
9393
"../playground:playground_test",
94+
"//flutter/impeller/golden_tests:screenshot",
9495
"//flutter/impeller/scene",
9596
"//flutter/impeller/typographer/backends/stb:typographer_stb_backend",
9697
"//flutter/third_party/txt",

impeller/display_list/dl_golden_blur_unittests.cc

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include "flutter/display_list/dl_builder.h"
88
#include "flutter/display_list/effects/dl_mask_filter.h"
9+
#include "flutter/impeller/golden_tests/screenshot.h"
910
#include "flutter/testing/testing.h"
1011
#include "gtest/gtest.h"
1112
#include "impeller/typographer/backends/skia/text_frame_skia.h"
@@ -110,5 +111,120 @@ TEST_P(DlGoldenTest, TextBlurMaskFilterDisrespectCTM) {
110111
ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
111112
}
112113

114+
namespace {
115+
double CalculateDistance(const uint8_t* left, const uint8_t* right) {
116+
double diff[4] = {
117+
static_cast<double>(left[0]) - right[0], //
118+
static_cast<double>(left[1]) - right[1], //
119+
static_cast<double>(left[2]) - right[2], //
120+
static_cast<double>(left[3]) - right[3] //
121+
};
122+
return sqrt((diff[0] * diff[0]) + //
123+
(diff[1] * diff[1]) + //
124+
(diff[2] * diff[2]) + //
125+
(diff[3] * diff[3]));
126+
}
127+
128+
double RMSE(const impeller::testing::Screenshot* left,
129+
const impeller::testing::Screenshot* right) {
130+
FML_CHECK(left);
131+
FML_CHECK(right);
132+
FML_CHECK(left->GetWidth() == right->GetWidth());
133+
FML_CHECK(left->GetHeight() == right->GetHeight());
134+
135+
int64_t samples = left->GetWidth() * left->GetHeight();
136+
double tally = 0;
137+
138+
const uint8_t* left_ptr = left->GetBytes();
139+
const uint8_t* right_ptr = right->GetBytes();
140+
for (int64_t i = 0; i < samples; ++i, left_ptr += 4, right_ptr += 4) {
141+
double distance = CalculateDistance(left_ptr, right_ptr);
142+
tally += distance * distance;
143+
}
144+
145+
return sqrt(tally / static_cast<double>(samples));
146+
}
147+
} // namespace
148+
149+
// This is a test to make sure that we don't regress "shimmering" in the
150+
// gaussian blur. Shimmering is abrupt changes in signal when making tiny
151+
// changes to the blur parameters.
152+
//
153+
// See also:
154+
// - https://github.com/flutter/flutter/issues/152195
155+
TEST_P(DlGoldenTest, ShimmerTest) {
156+
impeller::Point content_scale = GetContentScale();
157+
auto draw = [&](DlCanvas* canvas, const std::vector<sk_sp<DlImage>>& images,
158+
float sigma) {
159+
canvas->DrawColor(DlColor(0xff111111));
160+
canvas->Scale(content_scale.x, content_scale.y);
161+
162+
DlPaint paint;
163+
canvas->DrawImage(images[0], SkPoint::Make(10.135, 10.36334),
164+
DlImageSampling::kLinear, &paint);
165+
166+
SkRect save_layer_bounds = SkRect::MakeLTRB(0, 0, 1024, 768);
167+
DlBlurImageFilter blur(sigma, sigma, DlTileMode::kDecal);
168+
canvas->ClipRect(SkRect::MakeLTRB(11.125, 10.3737, 911.25, 755.3333));
169+
canvas->SaveLayer(&save_layer_bounds, /*paint=*/nullptr, &blur);
170+
canvas->Restore();
171+
};
172+
173+
std::vector<sk_sp<DlImage>> images;
174+
images.emplace_back(CreateDlImageForFixture("boston.jpg"));
175+
176+
auto make_screenshot = [&](float sigma) {
177+
DisplayListBuilder builder;
178+
draw(&builder, images, sigma);
179+
180+
std::unique_ptr<impeller::testing::Screenshot> screenshot =
181+
MakeScreenshot(builder.Build());
182+
return screenshot;
183+
};
184+
185+
float start_sigma = 10.0f;
186+
std::unique_ptr<impeller::testing::Screenshot> left =
187+
make_screenshot(start_sigma);
188+
if (!left) {
189+
GTEST_SKIP() << "making screenshots not supported.";
190+
}
191+
192+
double average_rmse = 0.0;
193+
const int32_t sample_count = 200;
194+
for (int i = 1; i <= sample_count; ++i) {
195+
float sigma = start_sigma + (i / 2.f);
196+
std::unique_ptr<impeller::testing::Screenshot> right =
197+
make_screenshot(sigma);
198+
double rmse = RMSE(left.get(), right.get());
199+
average_rmse += rmse;
200+
201+
// To debug this output the frames can be written out to disk then
202+
// transformed to a video with ffmpeg.
203+
//
204+
// ## save images command
205+
// std::stringstream ss;
206+
// ss << "_" << std::setw(3) << std::setfill('0') << (i - 1);
207+
// SaveScreenshot(std::move(left), ss.str());
208+
//
209+
// ## ffmpeg command
210+
// ```
211+
// ffmpeg -framerate 30 -pattern_type glob -i '*.png' \
212+
// -c:v libx264 -pix_fmt yuv420p out.mp4
213+
// ```
214+
left = std::move(right);
215+
}
216+
217+
average_rmse = average_rmse / sample_count;
218+
219+
// This is a somewhat arbitrary threshold. It could be increased if we wanted.
220+
// In the problematic cases previously we should values like 28. Before
221+
// increasing this you should manually inspect the behavior in
222+
// `AiksTest.GaussianBlurAnimatedBackdrop`. Average RMSE is a able to catch
223+
// shimmer but it isn't perfect.
224+
EXPECT_TRUE(average_rmse < 1.0) << "average_rmse: " << average_rmse;
225+
// An average rmse of 0 would mean that the blur isn't blurring.
226+
EXPECT_TRUE(average_rmse >= 0.0) << "average_rmse: " << average_rmse;
227+
}
228+
113229
} // namespace testing
114230
} // namespace flutter

impeller/display_list/dl_playground.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ bool DlPlayground::OpenPlaygroundHere(DisplayListPlaygroundCallback callback) {
7171
});
7272
}
7373

74+
std::unique_ptr<testing::Screenshot> DlPlayground::MakeScreenshot(
75+
const sk_sp<flutter::DisplayList>& list) {
76+
return nullptr;
77+
}
78+
7479
SkFont DlPlayground::CreateTestFontOfSize(SkScalar scalar) {
7580
static constexpr const char* kTestFontFixture = "Roboto-Regular.ttf";
7681
auto mapping = flutter::testing::OpenFixtureAsSkData(kTestFontFixture);

impeller/display_list/dl_playground.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include "flutter/display_list/display_list.h"
99
#include "flutter/display_list/dl_builder.h"
10+
#include "flutter/impeller/golden_tests/screenshot.h"
1011
#include "impeller/playground/playground_test.h"
1112
#include "third_party/skia/include/core/SkFont.h"
1213

@@ -27,6 +28,9 @@ class DlPlayground : public PlaygroundTest {
2728

2829
bool OpenPlaygroundHere(DisplayListPlaygroundCallback callback);
2930

31+
std::unique_ptr<testing::Screenshot> MakeScreenshot(
32+
const sk_sp<flutter::DisplayList>& list);
33+
3034
SkFont CreateTestFontOfSize(SkScalar scalar);
3135

3236
SkFont CreateTestFont();

impeller/golden_tests/BUILD.gn

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ impeller_component("golden_playground_test") {
1212

1313
deps = [
1414
":digest",
15+
":screenshot",
1516
"//flutter/fml",
1617
"//flutter/impeller/aiks",
1718
"//flutter/impeller/display_list:display_list",
@@ -44,6 +45,11 @@ impeller_component("digest") {
4445
deps = [ "//flutter/fml" ]
4546
}
4647

48+
impeller_component("screenshot") {
49+
testonly = true
50+
sources = [ "screenshot.h" ]
51+
}
52+
4753
if (is_mac) {
4854
impeller_component("metal_screenshot") {
4955
testonly = true
@@ -53,13 +59,13 @@ if (is_mac) {
5359
"metal_screenshot.mm",
5460
"metal_screenshotter.h",
5561
"metal_screenshotter.mm",
56-
"screenshot.h",
5762
"screenshotter.h",
5863
"vulkan_screenshotter.h",
5964
"vulkan_screenshotter.mm",
6065
]
6166

6267
deps = [
68+
":screenshot",
6369
"//flutter/impeller/aiks",
6470
"//flutter/impeller/display_list",
6571
"//flutter/impeller/playground",

impeller/golden_tests/golden_playground_test.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "flutter/display_list/display_list.h"
1111
#include "flutter/display_list/image/dl_image.h"
1212
#include "flutter/impeller/aiks/aiks_context.h"
13+
#include "flutter/impeller/golden_tests/screenshot.h"
1314
#include "flutter/impeller/playground/playground.h"
1415
#include "flutter/impeller/renderer/render_target.h"
1516
#include "flutter/testing/testing.h"
@@ -51,6 +52,12 @@ class GoldenPlaygroundTest
5152

5253
bool OpenPlaygroundHere(const sk_sp<flutter::DisplayList>& list);
5354

55+
std::unique_ptr<testing::Screenshot> MakeScreenshot(
56+
const sk_sp<flutter::DisplayList>& list);
57+
58+
static bool SaveScreenshot(std::unique_ptr<testing::Screenshot> screenshot,
59+
const std::string& postfix = "");
60+
5461
static bool ImGuiBegin(const char* name,
5562
bool* p_open,
5663
ImGuiWindowFlags flags);

impeller/golden_tests/golden_playground_test_mac.cc

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,17 +140,20 @@ std::string GetTestName() {
140140
return result;
141141
}
142142

143-
std::string GetGoldenFilename() {
144-
return GetTestName() + ".png";
143+
std::string GetGoldenFilename(const std::string& postfix) {
144+
return GetTestName() + postfix + ".png";
145145
}
146+
} // namespace
146147

147-
bool SaveScreenshot(std::unique_ptr<testing::Screenshot> screenshot) {
148+
bool GoldenPlaygroundTest::SaveScreenshot(
149+
std::unique_ptr<testing::Screenshot> screenshot,
150+
const std::string& postfix) {
148151
if (!screenshot || !screenshot->GetBytes()) {
149152
FML_LOG(ERROR) << "Failed to collect screenshot for test " << GetTestName();
150153
return false;
151154
}
152155
std::string test_name = GetTestName();
153-
std::string filename = GetGoldenFilename();
156+
std::string filename = GetGoldenFilename(postfix);
154157
testing::GoldenDigest::Instance()->AddImage(
155158
test_name, filename, screenshot->GetWidth(), screenshot->GetHeight());
156159
if (!screenshot->WriteToPNG(
@@ -161,8 +164,6 @@ bool SaveScreenshot(std::unique_ptr<testing::Screenshot> screenshot) {
161164
return true;
162165
}
163166

164-
} // namespace
165-
166167
struct GoldenPlaygroundTest::GoldenPlaygroundTestImpl {
167168
std::unique_ptr<PlaygroundImpl> test_vulkan_playground;
168169
std::unique_ptr<PlaygroundImpl> test_opengl_playground;
@@ -396,4 +397,19 @@ fml::Status GoldenPlaygroundTest::SetCapabilities(
396397
return pimpl_->screenshotter->GetPlayground().SetCapabilities(capabilities);
397398
}
398399

400+
std::unique_ptr<testing::Screenshot> GoldenPlaygroundTest::MakeScreenshot(
401+
const sk_sp<flutter::DisplayList>& list) {
402+
AiksContext renderer(GetContext(), typographer_context_);
403+
404+
DlDispatcher dispatcher;
405+
list->Dispatch(dispatcher);
406+
Picture picture = dispatcher.EndRecordingAsPicture();
407+
408+
std::unique_ptr<testing::Screenshot> screenshot =
409+
pimpl_->screenshotter->MakeScreenshot(renderer, picture,
410+
pimpl_->window_size);
411+
412+
return screenshot;
413+
}
414+
399415
} // namespace impeller

impeller/golden_tests/golden_playground_test_stub.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,9 @@ fml::Status GoldenPlaygroundTest::SetCapabilities(
8989
"GoldenPlaygroundTest-Stub doesn't support SetCapabilities.");
9090
}
9191

92+
std::unique_ptr<testing::Screenshot> GoldenPlaygroundTest::MakeScreenshot(
93+
const sk_sp<flutter::DisplayList>& list) {
94+
return nullptr;
95+
}
96+
9297
} // namespace impeller

impeller/golden_tests/metal_screenshotter.mm

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,30 +37,32 @@
3737
std::unique_ptr<Screenshot> MetalScreenshotter::MakeScreenshot(
3838
AiksContext& aiks_context,
3939
const std::shared_ptr<Texture> texture) {
40-
id<MTLTexture> metal_texture =
41-
std::static_pointer_cast<TextureMTL>(texture)->GetMTLTexture();
40+
@autoreleasepool {
41+
id<MTLTexture> metal_texture =
42+
std::static_pointer_cast<TextureMTL>(texture)->GetMTLTexture();
4243

43-
CGColorSpaceRef color_space = CGColorSpaceCreateDeviceRGB();
44-
CIImage* ciImage = [[CIImage alloc]
45-
initWithMTLTexture:metal_texture
46-
options:@{kCIImageColorSpace : (__bridge id)color_space}];
47-
CGColorSpaceRelease(color_space);
48-
FML_CHECK(ciImage);
44+
CGColorSpaceRef color_space = CGColorSpaceCreateDeviceRGB();
45+
CIImage* ciImage = [[CIImage alloc]
46+
initWithMTLTexture:metal_texture
47+
options:@{kCIImageColorSpace : (__bridge id)color_space}];
48+
CGColorSpaceRelease(color_space);
49+
FML_CHECK(ciImage);
4950

50-
std::shared_ptr<Context> context = playground_->GetContext();
51-
std::shared_ptr<ContextMTL> context_mtl =
52-
std::static_pointer_cast<ContextMTL>(context);
53-
CIContext* cicontext =
54-
[CIContext contextWithMTLDevice:context_mtl->GetMTLDevice()];
55-
FML_CHECK(context);
51+
std::shared_ptr<Context> context = playground_->GetContext();
52+
std::shared_ptr<ContextMTL> context_mtl =
53+
std::static_pointer_cast<ContextMTL>(context);
54+
CIContext* cicontext =
55+
[CIContext contextWithMTLDevice:context_mtl->GetMTLDevice()];
56+
FML_CHECK(context);
5657

57-
CIImage* flipped = [ciImage
58-
imageByApplyingOrientation:kCGImagePropertyOrientationDownMirrored];
58+
CIImage* flipped = [ciImage
59+
imageByApplyingOrientation:kCGImagePropertyOrientationDownMirrored];
5960

60-
CGImageRef cgImage = [cicontext createCGImage:flipped
61-
fromRect:[ciImage extent]];
61+
CGImageRef cgImage = [cicontext createCGImage:flipped
62+
fromRect:[ciImage extent]];
6263

63-
return std::unique_ptr<MetalScreenshot>(new MetalScreenshot(cgImage));
64+
return std::unique_ptr<MetalScreenshot>(new MetalScreenshot(cgImage));
65+
}
6466
}
6567

6668
} // namespace testing

0 commit comments

Comments
 (0)