From ac9142c96c94c78a4377d151dab74d6e83866959 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Sat, 10 Jul 2021 00:46:02 -0700 Subject: [PATCH] encode DPR for shadows into DisplayList (#27289) --- flow/display_list.cc | 47 +++++++++++++++------------ flow/display_list.h | 9 +++-- flow/display_list_canvas.cc | 5 +-- flow/display_list_canvas.h | 3 +- flow/display_list_canvas_unittests.cc | 27 +++++++++++++-- flow/display_list_unittests.cc | 11 ++++--- flow/display_list_utils.cc | 9 ++--- flow/display_list_utils.h | 3 +- lib/ui/painting/canvas.cc | 13 ++++---- 9 files changed, 83 insertions(+), 44 deletions(-) diff --git a/flow/display_list.cc b/flow/display_list.cc index 0e45f3574f..2de6809531 100644 --- a/flow/display_list.cc +++ b/flow/display_list.cc @@ -796,24 +796,28 @@ struct DrawTextBlobOp final : DLOp { }; // 4 byte header + 28 byte payload packs evenly into 32 bytes -struct DrawShadowOp final : DLOp { - static const auto kType = DisplayListOpType::kDrawShadow; - - DrawShadowOp(const SkPath& path, - SkColor color, - SkScalar elevation, - bool occludes) - : color(color), elevation(elevation), occludes(occludes), path(path) {} - - const SkColor color; - const SkScalar elevation; - const bool occludes; - const SkPath path; - - void dispatch(Dispatcher& dispatcher) const { - dispatcher.drawShadow(path, color, elevation, occludes); - } -}; +#define DEFINE_DRAW_SHADOW_OP(name, occludes) \ + struct Draw##name##Op final : DLOp { \ + static const auto kType = DisplayListOpType::kDraw##name; \ + \ + Draw##name##Op(const SkPath& path, \ + SkColor color, \ + SkScalar elevation, \ + SkScalar dpr) \ + : color(color), elevation(elevation), dpr(dpr), path(path) {} \ + \ + const SkColor color; \ + const SkScalar elevation; \ + const SkScalar dpr; \ + const SkPath path; \ + \ + void dispatch(Dispatcher& dispatcher) const { \ + dispatcher.drawShadow(path, color, elevation, occludes, dpr); \ + } \ + }; +DEFINE_DRAW_SHADOW_OP(Shadow, false) +DEFINE_DRAW_SHADOW_OP(ShadowOccludes, true) +#undef DEFINE_DRAW_SHADOW_OP #pragma pack(pop, DLOp_Alignment) @@ -1351,8 +1355,11 @@ void DisplayListBuilder::drawTextBlob(const sk_sp blob, void DisplayListBuilder::drawShadow(const SkPath& path, const SkColor color, const SkScalar elevation, - bool occludes) { - Push(0, path, color, elevation, occludes); + bool occludes, + SkScalar dpr) { + occludes // + ? Push(0, path, color, elevation, dpr) + : Push(0, path, color, elevation, dpr); } } // namespace flutter diff --git a/flow/display_list.h b/flow/display_list.h index 2388e27821..22858511b4 100644 --- a/flow/display_list.h +++ b/flow/display_list.h @@ -144,7 +144,8 @@ namespace flutter { V(DrawDisplayList) \ V(DrawTextBlob) \ \ - V(DrawShadow) + V(DrawShadow) \ + V(DrawShadowOccludes) #define DL_OP_TO_ENUM_VALUE(name) k##name, enum class DisplayListOpType { FOR_EACH_DISPLAY_LIST_OP(DL_OP_TO_ENUM_VALUE) }; @@ -318,7 +319,8 @@ class Dispatcher { virtual void drawShadow(const SkPath& path, const SkColor color, const SkScalar elevation, - bool occludes) = 0; + bool occludes, + SkScalar dpr) = 0; }; // The primary class used to build a display list. The list of methods @@ -431,7 +433,8 @@ class DisplayListBuilder final : public virtual Dispatcher, public SkRefCnt { void drawShadow(const SkPath& path, const SkColor color, const SkScalar elevation, - bool occludes) override; + bool occludes, + SkScalar dpr) override; sk_sp Build(); diff --git a/flow/display_list_canvas.cc b/flow/display_list_canvas.cc index 1da919fba0..7e37e8f61d 100644 --- a/flow/display_list_canvas.cc +++ b/flow/display_list_canvas.cc @@ -176,9 +176,10 @@ void DisplayListCanvasDispatcher::drawTextBlob(const sk_sp blob, void DisplayListCanvasDispatcher::drawShadow(const SkPath& path, const SkColor color, const SkScalar elevation, - bool occludes) { + bool occludes, + SkScalar dpr) { flutter::PhysicalShapeLayer::DrawShadow(canvas_, path, color, elevation, - occludes, 1.0); + occludes, dpr); } DisplayListCanvasRecorder::DisplayListCanvasRecorder(const SkRect& bounds) diff --git a/flow/display_list_canvas.h b/flow/display_list_canvas.h index f118c345cb..76e49615de 100644 --- a/flow/display_list_canvas.h +++ b/flow/display_list_canvas.h @@ -110,7 +110,8 @@ class DisplayListCanvasDispatcher : public virtual Dispatcher, void drawShadow(const SkPath& path, const SkColor color, const SkScalar elevation, - bool occludes) override; + bool occludes, + SkScalar dpr) override; private: SkCanvas* canvas_; diff --git a/flow/display_list_canvas_unittests.cc b/flow/display_list_canvas_unittests.cc index 41eb47f635..a7419769a5 100644 --- a/flow/display_list_canvas_unittests.cc +++ b/flow/display_list_canvas_unittests.cc @@ -1350,7 +1350,7 @@ TEST(DisplayListCanvas, DrawShadow) { 1.0); }, [=](DisplayListBuilder& builder) { // - builder.drawShadow(path, color, elevation, false); + builder.drawShadow(path, color, elevation, false, 1.0); }); CanvasCompareTester::UsingShadows = false; } @@ -1373,7 +1373,30 @@ TEST(DisplayListCanvas, DrawOccludingShadow) { 1.0); }, [=](DisplayListBuilder& builder) { // - builder.drawShadow(path, color, elevation, true); + builder.drawShadow(path, color, elevation, true, 1.0); + }); + CanvasCompareTester::UsingShadows = false; +} + +TEST(DisplayListCanvas, DrawShadowDpr) { + CanvasCompareTester::UsingShadows = true; + SkPath path; + path.moveTo(RenderCenterX, RenderTop); + path.lineTo(RenderRight, RenderBottom); + path.lineTo(RenderLeft, RenderCenterY); + path.lineTo(RenderRight, RenderCenterY); + path.lineTo(RenderLeft, RenderBottom); + path.close(); + const SkColor color = SK_ColorDKGRAY; + const SkScalar elevation = 10; + + CanvasCompareTester::RenderNoAttributes( + [=](SkCanvas* canvas, SkPaint& paint) { // + PhysicalShapeLayer::DrawShadow(canvas, path, color, elevation, false, + 2.5); + }, + [=](DisplayListBuilder& builder) { // + builder.drawShadow(path, color, elevation, false, 2.5); }); CanvasCompareTester::UsingShadows = false; } diff --git a/flow/display_list_unittests.cc b/flow/display_list_unittests.cc index 465663579e..151a4f21ef 100644 --- a/flow/display_list_unittests.cc +++ b/flow/display_list_unittests.cc @@ -663,11 +663,12 @@ std::vector allGroups = { // See: https://bugs.chromium.org/p/skia/issues/detail?id=12125 { "DrawShadow", { // cv shadows are turned into an opaque ShadowRec which is not exposed - {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 1.0, false);}}, - {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath2, SK_ColorGREEN, 1.0, false);}}, - {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorBLUE, 1.0, false);}}, - {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 2.0, false);}}, - {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 1.0, true);}}, + {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 1.0, false, 1.0);}}, + {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath2, SK_ColorGREEN, 1.0, false, 1.0);}}, + {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorBLUE, 1.0, false, 1.0);}}, + {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 2.0, false, 1.0);}}, + {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 1.0, true, 1.0);}}, + {1, 32, -1, 32, [](DisplayListBuilder& b) {b.drawShadow(TestPath1, SK_ColorGREEN, 1.0, false, 2.5);}}, } }, }; diff --git a/flow/display_list_utils.cc b/flow/display_list_utils.cc index c8282bf1ef..72780fa1fb 100644 --- a/flow/display_list_utils.cc +++ b/flow/display_list_utils.cc @@ -335,7 +335,8 @@ void DisplayListBoundsCalculator::drawTextBlob(const sk_sp blob, void DisplayListBoundsCalculator::drawShadow(const SkPath& path, const SkColor color, const SkScalar elevation, - bool occludes) { + bool occludes, + SkScalar dpr) { // Constants from physical_shape_layer.cc const SkScalar kLightHeight = 600; const SkScalar kLightRadius = 800; @@ -348,9 +349,9 @@ void DisplayListBoundsCalculator::drawShadow(const SkPath& path, SkScalar shadow_y = bounds.top() - 600.0f; SkRect shadow_bounds; SkShadowUtils::GetLocalBounds( - matrix(), path, SkPoint3::Make(0, 0, elevation), - SkPoint3::Make(shadow_x, shadow_y, kLightHeight), kLightRadius, flags, - &shadow_bounds); + matrix(), path, SkPoint3::Make(0, 0, dpr * elevation), + SkPoint3::Make(shadow_x, shadow_y, dpr * kLightHeight), + dpr * kLightRadius, flags, &shadow_bounds); accumulateRect(shadow_bounds); } diff --git a/flow/display_list_utils.h b/flow/display_list_utils.h index f399bfa5de..bba718e388 100644 --- a/flow/display_list_utils.h +++ b/flow/display_list_utils.h @@ -308,7 +308,8 @@ class DisplayListBoundsCalculator final void drawShadow(const SkPath& path, const SkColor color, const SkScalar elevation, - bool occludes) override; + bool occludes, + SkScalar dpr) override; SkRect getBounds() { return accumulator_->getBounds(); } diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index fee3127f06..cd5492f0b6 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -499,6 +499,11 @@ void Canvas::drawShadow(const CanvasPath* path, ToDart("Canvas.drawShader called with non-genuine Path.")); return; } + SkScalar dpr = UIDartState::Current() + ->platform_configuration() + ->get_window(0) + ->viewport_metrics() + .device_pixel_ratio; if (display_list_recorder_) { // The DrawShadow mechanism results in non-public operations to be // performed on the canvas involving an SkDrawShadowRec. Since we @@ -507,13 +512,9 @@ void Canvas::drawShadow(const CanvasPath* path, // that situation we bypass the canvas interface and inject the // shadow parameters directly into the underlying DisplayList. // See: https://bugs.chromium.org/p/skia/issues/detail?id=12125 - builder()->drawShadow(path->path(), color, elevation, transparentOccluder); + builder()->drawShadow(path->path(), color, elevation, transparentOccluder, + dpr); } else { - SkScalar dpr = UIDartState::Current() - ->platform_configuration() - ->get_window(0) - ->viewport_metrics() - .device_pixel_ratio; flutter::PhysicalShapeLayer::DrawShadow( canvas_, path->path(), color, elevation, transparentOccluder, dpr); } -- GitLab