From 8e2d05fa95d7bd0f237789dd76ed33e6ec089793 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 3 Jul 2024 17:01:24 -0700 Subject: [PATCH] [Impeller] Re-enable fast blur path for elliptical rrects (#53704) Fixes https://github.com/flutter/flutter/issues/151034 Fast round rects were recently restricted to circular corners, but elliptical round rects can also go through the fast path. --- impeller/display_list/dl_golden_unittests.cc | 63 +++++++++++++++++++ impeller/display_list/skia_conversions.cc | 24 ++++--- .../skia_conversions_unittests.cc | 29 ++++++++- testing/impeller_golden_tests_output.txt | 3 + 4 files changed, 108 insertions(+), 11 deletions(-) diff --git a/impeller/display_list/dl_golden_unittests.cc b/impeller/display_list/dl_golden_unittests.cc index 506e3970da4c8..10edeafb5f53f 100644 --- a/impeller/display_list/dl_golden_unittests.cc +++ b/impeller/display_list/dl_golden_unittests.cc @@ -5,6 +5,7 @@ #include "impeller/display_list/dl_golden_unittests.h" #include "flutter/display_list/dl_builder.h" +#include "flutter/impeller/geometry/path_builder.h" #include "flutter/testing/testing.h" #include "gtest/gtest.h" @@ -182,6 +183,68 @@ TEST_P(DlGoldenTest, GaussianVsRRectBlurScaledRotated) { ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } +TEST_P(DlGoldenTest, FastVsGeneralGaussianMaskBlur) { + DisplayListBuilder builder; + builder.Scale(GetContentScale().x, GetContentScale().y); + builder.DrawColor(DlColor::kWhite(), DlBlendMode::kSrc); + + auto blur_sigmas = std::array{5.0f, 10.0f, 20.0f}; + auto blur_colors = std::array{ + DlColor::kBlue(), + DlColor::kGreen(), + DlColor::kMaroon(), + }; + + auto make_rrect_path = [](const SkRect& rect, DlScalar rx, + DlScalar ry) -> SkPath { + auto add_corner = [](SkPath& path, SkPoint rCorner, SkPoint rEnd) { + static const auto magic = impeller::PathBuilder::kArcApproximationMagic; + path.rCubicTo(rCorner.fX * (1.0f - magic), rCorner.fY * (1.0f - magic), + rCorner.fX + rEnd.fX * magic, rCorner.fY + rEnd.fY * magic, + rCorner.fX + rEnd.fX, rCorner.fY + rEnd.fY); + }; + + SkPath path; + path.moveTo(rect.fRight - rx, rect.fTop); + add_corner(path, {rx, 0.0f}, {0.0f, ry}); + path.lineTo(rect.fRight, rect.fBottom - ry); + add_corner(path, {0.0f, ry}, {-rx, 0.0f}); + path.lineTo(rect.fLeft + rx, rect.fBottom); + add_corner(path, {-rx, 0.0f}, {0.0f, -ry}); + path.lineTo(rect.fLeft, rect.fTop + ry); + add_corner(path, {0.0f, -ry}, {rx, 0.0f}); + path.close(); + return path; + }; + + for (size_t i = 0; i < blur_sigmas.size(); i++) { + auto rect = SkRect::MakeXYWH(i * 320.0f + 50.0f, 50.0f, 100.0f, 100.0f); + DlPaint paint = DlPaint() // + .setColor(blur_colors[i]) + .setMaskFilter(DlBlurMaskFilter::Make( + DlBlurStyle::kNormal, blur_sigmas[i])); + + builder.DrawRRect(SkRRect::MakeRectXY(rect, 10.0f, 10.0f), paint); + rect = rect.makeOffset(150.0f, 0.0f); + builder.DrawPath(make_rrect_path(rect, 10.0f, 10.0f), paint); + rect = rect.makeOffset(-150.0f, 0.0f); + + rect = rect.makeOffset(0.0f, 200.0f); + builder.DrawRRect(SkRRect::MakeRectXY(rect, 10.0f, 30.0f), paint); + rect = rect.makeOffset(150.0f, 0.0f); + builder.DrawPath(make_rrect_path(rect, 10.0f, 20.0f), paint); + rect = rect.makeOffset(-150.0f, 0.0f); + + rect = rect.makeOffset(0.0f, 200.0f); + builder.DrawRRect(SkRRect::MakeRectXY(rect, 30.0f, 10.0f), paint); + rect = rect.makeOffset(150.0f, 0.0f); + builder.DrawPath(make_rrect_path(rect, 20.0f, 10.0f), paint); + rect = rect.makeOffset(-150.0f, 0.0f); + } + + ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); +} + TEST_P(DlGoldenTest, DashedLinesTest) { Point content_scale = GetContentScale(); auto draw = [content_scale](DlCanvas* canvas, diff --git a/impeller/display_list/skia_conversions.cc b/impeller/display_list/skia_conversions.cc index 8ac11c81a2803..f53f4b1bb7100 100644 --- a/impeller/display_list/skia_conversions.cc +++ b/impeller/display_list/skia_conversions.cc @@ -9,18 +9,22 @@ namespace impeller { namespace skia_conversions { -bool IsNearlySimpleRRect(const SkRRect& rr) { - auto [a, b] = rr.radii(SkRRect::kUpperLeft_Corner); - auto [c, d] = rr.radii(SkRRect::kLowerLeft_Corner); - auto [e, f] = rr.radii(SkRRect::kUpperRight_Corner); - auto [g, h] = rr.radii(SkRRect::kLowerRight_Corner); +static inline bool SkScalarsNearlyEqual(SkScalar a, + SkScalar b, + SkScalar c, + SkScalar d) { return SkScalarNearlyEqual(a, b, kEhCloseEnough) && SkScalarNearlyEqual(a, c, kEhCloseEnough) && - SkScalarNearlyEqual(a, d, kEhCloseEnough) && - SkScalarNearlyEqual(a, e, kEhCloseEnough) && - SkScalarNearlyEqual(a, f, kEhCloseEnough) && - SkScalarNearlyEqual(a, g, kEhCloseEnough) && - SkScalarNearlyEqual(a, h, kEhCloseEnough); + SkScalarNearlyEqual(a, d, kEhCloseEnough); +} + +bool IsNearlySimpleRRect(const SkRRect& rr) { + auto [xa, ya] = rr.radii(SkRRect::kUpperLeft_Corner); + auto [xb, yb] = rr.radii(SkRRect::kLowerLeft_Corner); + auto [xc, yc] = rr.radii(SkRRect::kUpperRight_Corner); + auto [xd, yd] = rr.radii(SkRRect::kLowerRight_Corner); + return SkScalarsNearlyEqual(xa, xb, xc, xd) && + SkScalarsNearlyEqual(ya, yb, yc, yd); } Rect ToRect(const SkRect& rect) { diff --git a/impeller/display_list/skia_conversions_unittests.cc b/impeller/display_list/skia_conversions_unittests.cc index 257f2039d8ea1..e66226072328c 100644 --- a/impeller/display_list/skia_conversions_unittests.cc +++ b/impeller/display_list/skia_conversions_unittests.cc @@ -180,8 +180,35 @@ TEST(SkiaConversionsTest, IsNearlySimpleRRect) { SkRRect::MakeRectXY(SkRect::MakeLTRB(0, 0, 10, 10), 10, 10))); EXPECT_TRUE(skia_conversions::IsNearlySimpleRRect( SkRRect::MakeRectXY(SkRect::MakeLTRB(0, 0, 10, 10), 10, 9.999))); - EXPECT_FALSE(skia_conversions::IsNearlySimpleRRect( + EXPECT_TRUE(skia_conversions::IsNearlySimpleRRect( SkRRect::MakeRectXY(SkRect::MakeLTRB(0, 0, 10, 10), 10, 9))); + EXPECT_TRUE(skia_conversions::IsNearlySimpleRRect( + SkRRect::MakeRectXY(SkRect::MakeLTRB(0, 0, 10, 10), 10, 5))); + SkRect rect = SkRect::MakeLTRB(0, 0, 10, 10); + SkRRect rrect; + union { + SkPoint radii[4] = { + {10.0f, 9.0f}, + {10.0f, 9.0f}, + {10.0f, 9.0f}, + {10.0f, 9.0f}, + }; + SkScalar values[8]; + } test; + rrect.setRectRadii(rect, test.radii); + EXPECT_TRUE(skia_conversions::IsNearlySimpleRRect(rrect)); + for (int i = 0; i < 8; i++) { + auto save = test.values[i]; + test.values[i] -= kEhCloseEnough * 0.5f; + rrect.setRectRadii(rect, test.radii); + EXPECT_TRUE(skia_conversions::IsNearlySimpleRRect(rrect)) + << "values[" << i << "] == " << test.values[i]; + test.values[i] -= kEhCloseEnough * 2.0f; + rrect.setRectRadii(rect, test.radii); + EXPECT_FALSE(skia_conversions::IsNearlySimpleRRect(rrect)) + << "values[" << i << "] == " << test.values[i]; + test.values[i] = save; + } } } // namespace testing diff --git a/testing/impeller_golden_tests_output.txt b/testing/impeller_golden_tests_output.txt index 6a25f99892e82..f1b609c0f30eb 100644 --- a/testing/impeller_golden_tests_output.txt +++ b/testing/impeller_golden_tests_output.txt @@ -855,6 +855,9 @@ impeller_Play_DlGoldenTest_CanRenderImage_Vulkan.png impeller_Play_DlGoldenTest_DashedLinesTest_Metal.png impeller_Play_DlGoldenTest_DashedLinesTest_OpenGLES.png impeller_Play_DlGoldenTest_DashedLinesTest_Vulkan.png +impeller_Play_DlGoldenTest_FastVsGeneralGaussianMaskBlur_Metal.png +impeller_Play_DlGoldenTest_FastVsGeneralGaussianMaskBlur_OpenGLES.png +impeller_Play_DlGoldenTest_FastVsGeneralGaussianMaskBlur_Vulkan.png impeller_Play_DlGoldenTest_GaussianVsRRectBlurScaledRotated_Metal.png impeller_Play_DlGoldenTest_GaussianVsRRectBlurScaledRotated_OpenGLES.png impeller_Play_DlGoldenTest_GaussianVsRRectBlurScaledRotated_Vulkan.png