From 35386766c3f041a9b38fe349c4496ef54b1e69df Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 14 Jun 2024 17:02:57 -0700 Subject: [PATCH 1/4] [DisplayList] Create DrawDashedLine for paragraph code --- display_list/benchmarking/dl_complexity_gl.cc | 10 ++ display_list/benchmarking/dl_complexity_gl.h | 4 + .../benchmarking/dl_complexity_metal.cc | 10 ++ .../benchmarking/dl_complexity_metal.h | 4 + display_list/display_list.h | 1 + display_list/dl_builder.cc | 23 ++++ display_list/dl_builder.h | 11 ++ display_list/dl_canvas.h | 6 + display_list/dl_op_receiver.h | 4 + display_list/dl_op_records.h | 22 ++++ display_list/skia/dl_sk_canvas.cc | 11 ++ display_list/skia/dl_sk_canvas.h | 5 + display_list/skia/dl_sk_dispatcher.cc | 9 ++ display_list/skia/dl_sk_dispatcher.h | 4 + .../testing/dl_rendering_unittests.cc | 113 ++++++++---------- display_list/testing/dl_test_snippets.cc | 35 ++++++ display_list/utils/dl_receiver_utils.h | 4 + impeller/display_list/dl_dispatcher.cc | 23 ++++ impeller/display_list/dl_dispatcher.h | 12 +- shell/common/dl_op_spy.cc | 6 + shell/common/dl_op_spy.h | 4 + shell/common/dl_op_spy_unittests.cc | 21 ++++ testing/display_list_testing.cc | 11 ++ testing/display_list_testing.h | 4 + testing/mock_canvas.cc | 8 ++ testing/mock_canvas.h | 5 + third_party/txt/src/skia/paragraph_skia.cc | 58 +-------- third_party/txt/tests/paragraph_unittests.cc | 17 ++- 28 files changed, 327 insertions(+), 118 deletions(-) diff --git a/display_list/benchmarking/dl_complexity_gl.cc b/display_list/benchmarking/dl_complexity_gl.cc index a1f4c79de5d19..81143c85b40dd 100644 --- a/display_list/benchmarking/dl_complexity_gl.cc +++ b/display_list/benchmarking/dl_complexity_gl.cc @@ -100,6 +100,16 @@ void DisplayListGLComplexityCalculator::GLHelper::drawLine(const SkPoint& p0, AccumulateComplexity(complexity); } +void DisplayListGLComplexityCalculator::GLHelper::drawDashedLine( + const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) { + // Dashing is slightly more complex than a regular drawLine, but this + // op is so rare it is not worth measuring the difference. + drawLine(ToSkPoint(p0), ToSkPoint(p1)); +} + void DisplayListGLComplexityCalculator::GLHelper::drawRect(const SkRect& rect) { if (IsComplex()) { return; diff --git a/display_list/benchmarking/dl_complexity_gl.h b/display_list/benchmarking/dl_complexity_gl.h index c30926902ade3..5cb53c96c4268 100644 --- a/display_list/benchmarking/dl_complexity_gl.h +++ b/display_list/benchmarking/dl_complexity_gl.h @@ -40,6 +40,10 @@ class DisplayListGLComplexityCalculator const DlImageFilter* backdrop) override; void drawLine(const SkPoint& p0, const SkPoint& p1) override; + void drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) override; void drawRect(const SkRect& rect) override; void drawOval(const SkRect& bounds) override; void drawCircle(const SkPoint& center, SkScalar radius) override; diff --git a/display_list/benchmarking/dl_complexity_metal.cc b/display_list/benchmarking/dl_complexity_metal.cc index d24c1c643425e..a0e044093840b 100644 --- a/display_list/benchmarking/dl_complexity_metal.cc +++ b/display_list/benchmarking/dl_complexity_metal.cc @@ -111,6 +111,16 @@ void DisplayListMetalComplexityCalculator::MetalHelper::drawLine( AccumulateComplexity(complexity); } +void DisplayListMetalComplexityCalculator::MetalHelper::drawDashedLine( + const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) { + // Dashing is slightly more complex than a regular drawLine, but this + // op is so rare it is not worth measuring the difference. + drawLine(ToSkPoint(p0), ToSkPoint(p1)); +} + void DisplayListMetalComplexityCalculator::MetalHelper::drawRect( const SkRect& rect) { if (IsComplex()) { diff --git a/display_list/benchmarking/dl_complexity_metal.h b/display_list/benchmarking/dl_complexity_metal.h index 303e9c645d996..d5a5b77007ab8 100644 --- a/display_list/benchmarking/dl_complexity_metal.h +++ b/display_list/benchmarking/dl_complexity_metal.h @@ -40,6 +40,10 @@ class DisplayListMetalComplexityCalculator const DlImageFilter* backdrop) override; void drawLine(const SkPoint& p0, const SkPoint& p1) override; + void drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) override; void drawRect(const SkRect& rect) override; void drawOval(const SkRect& bounds) override; void drawCircle(const SkPoint& center, SkScalar radius) override; diff --git a/display_list/display_list.h b/display_list/display_list.h index e4cc58a32a37b..8658e35a63fb7 100644 --- a/display_list/display_list.h +++ b/display_list/display_list.h @@ -111,6 +111,7 @@ namespace flutter { V(DrawColor) \ \ V(DrawLine) \ + V(DrawDashedLine) \ V(DrawRect) \ V(DrawOval) \ V(DrawCircle) \ diff --git a/display_list/dl_builder.cc b/display_list/dl_builder.cc index 2a048c709aaf2..2ff8c9b5a1f82 100644 --- a/display_list/dl_builder.cc +++ b/display_list/dl_builder.cc @@ -1099,6 +1099,29 @@ void DisplayListBuilder::DrawLine(const SkPoint& p0, SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawLineFlags); drawLine(p0, p1); } +void DisplayListBuilder::drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) { + SkRect bounds = SkRect::MakeLTRB(p0.x, p0.y, p1.x, p1.y).makeSorted(); + DisplayListAttributeFlags flags = + (bounds.width() > 0.0f && bounds.height() > 0.0f) ? kDrawLineFlags + : kDrawHVLineFlags; + OpResult result = PaintResult(current_, flags); + if (result != OpResult::kNoEffect && AccumulateOpBounds(bounds, flags)) { + Push(0, p0, p1, on_length, off_length); + CheckLayerOpacityCompatibility(); + UpdateLayerResult(result); + } +} +void DisplayListBuilder::DrawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length, + const DlPaint& paint) { + SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawLineFlags); + drawDashedLine(p0, p1, on_length, off_length); +} void DisplayListBuilder::drawRect(const SkRect& rect) { DisplayListAttributeFlags flags = kDrawRectFlags; OpResult result = PaintResult(current_, flags); diff --git a/display_list/dl_builder.h b/display_list/dl_builder.h index 793856d64aad9..a6bf3fabab1f6 100644 --- a/display_list/dl_builder.h +++ b/display_list/dl_builder.h @@ -156,6 +156,12 @@ class DisplayListBuilder final : public virtual DlCanvas, const SkPoint& p1, const DlPaint& paint) override; // |DlCanvas| + void DrawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length, + const DlPaint& paint) override; + // |DlCanvas| void DrawRect(const SkRect& rect, const DlPaint& paint) override; // |DlCanvas| void DrawOval(const SkRect& bounds, const DlPaint& paint) override; @@ -419,6 +425,11 @@ class DisplayListBuilder final : public virtual DlCanvas, // |DlOpReceiver| void drawLine(const SkPoint& p0, const SkPoint& p1) override; // |DlOpReceiver| + void drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) override; + // |DlOpReceiver| void drawRect(const SkRect& rect) override; // |DlOpReceiver| void drawOval(const SkRect& bounds) override; diff --git a/display_list/dl_canvas.h b/display_list/dl_canvas.h index 9ba279a15b590..769deb3f582e9 100644 --- a/display_list/dl_canvas.h +++ b/display_list/dl_canvas.h @@ -8,6 +8,7 @@ #include "flutter/display_list/dl_blend_mode.h" #include "flutter/display_list/dl_paint.h" #include "flutter/display_list/dl_vertices.h" +#include "flutter/display_list/geometry/dl_geometry_types.h" #include "flutter/display_list/image/dl_image.h" #include "third_party/skia/include/core/SkM44.h" @@ -132,6 +133,11 @@ class DlCanvas { virtual void DrawLine(const SkPoint& p0, const SkPoint& p1, const DlPaint& paint) = 0; + virtual void DrawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length, + const DlPaint& paint) = 0; virtual void DrawRect(const SkRect& rect, const DlPaint& paint) = 0; virtual void DrawOval(const SkRect& bounds, const DlPaint& paint) = 0; virtual void DrawCircle(const SkPoint& center, diff --git a/display_list/dl_op_receiver.h b/display_list/dl_op_receiver.h index 5814c4cc6a52d..8896febc94e1b 100644 --- a/display_list/dl_op_receiver.h +++ b/display_list/dl_op_receiver.h @@ -343,6 +343,10 @@ class DlOpReceiver { virtual void drawColor(DlColor color, DlBlendMode mode) = 0; virtual void drawPaint() = 0; virtual void drawLine(const SkPoint& p0, const SkPoint& p1) = 0; + virtual void drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) = 0; virtual void drawRect(const SkRect& rect) = 0; virtual void drawOval(const SkRect& bounds) = 0; virtual void drawCircle(const SkPoint& center, SkScalar radius) = 0; diff --git a/display_list/dl_op_records.h b/display_list/dl_op_records.h index 2ce4d54a70f71..d3feba9b30e27 100644 --- a/display_list/dl_op_records.h +++ b/display_list/dl_op_records.h @@ -736,6 +736,28 @@ DEFINE_DRAW_2ARG_OP(Circle, SkPoint, center, SkScalar, radius) DEFINE_DRAW_2ARG_OP(DRRect, SkRRect, outer, SkRRect, inner) #undef DEFINE_DRAW_2ARG_OP +// 4 byte header + 24 byte payload packs into 32 bytes (4 bytes unused) +struct DrawDashedLineOp final : DrawOpBase { + static constexpr auto kType = DisplayListOpType::kDrawDashedLine; + + DrawDashedLineOp(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) + : p0(p0), p1(p1), on_length(on_length), off_length(off_length) {} + + const DlPoint p0; + const DlPoint p1; + const SkScalar on_length; + const SkScalar off_length; + + void dispatch(DispatchContext& ctx) const { + if (op_needed(ctx)) { + ctx.receiver.drawDashedLine(p0, p1, on_length, off_length); + } + } +}; + // 4 byte header + 28 byte payload packs efficiently into 32 bytes struct DrawArcOp final : DrawOpBase { static constexpr auto kType = DisplayListOpType::kDrawArc; diff --git a/display_list/skia/dl_sk_canvas.cc b/display_list/skia/dl_sk_canvas.cc index 07dc33f7063b4..244b8af704048 100644 --- a/display_list/skia/dl_sk_canvas.cc +++ b/display_list/skia/dl_sk_canvas.cc @@ -199,6 +199,17 @@ void DlSkCanvasAdapter::DrawLine(const SkPoint& p0, delegate_->drawLine(p0, p1, ToStrokedSk(paint)); } +void DlSkCanvasAdapter::DrawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length, + const DlPaint& paint) { + SkPaint dashed_paint = ToStrokedSk(paint); + SkScalar intervals[2] = {on_length, off_length}; + dashed_paint.setPathEffect(SkDashPathEffect::Make(intervals, 2, 0.0f)); + delegate_->drawLine(ToSkPoint(p0), ToSkPoint(p1), dashed_paint); +} + void DlSkCanvasAdapter::DrawRect(const SkRect& rect, const DlPaint& paint) { delegate_->drawRect(rect, ToSk(paint)); } diff --git a/display_list/skia/dl_sk_canvas.h b/display_list/skia/dl_sk_canvas.h index cb9ba4466b84c..9712a8acb152a 100644 --- a/display_list/skia/dl_sk_canvas.h +++ b/display_list/skia/dl_sk_canvas.h @@ -94,6 +94,11 @@ class DlSkCanvasAdapter final : public virtual DlCanvas { void DrawLine(const SkPoint& p0, const SkPoint& p1, const DlPaint& paint) override; + void DrawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length, + const DlPaint& paint) override; void DrawRect(const SkRect& rect, const DlPaint& paint) override; void DrawOval(const SkRect& bounds, const DlPaint& paint) override; void DrawCircle(const SkPoint& center, diff --git a/display_list/skia/dl_sk_dispatcher.cc b/display_list/skia/dl_sk_dispatcher.cc index e75ccd96da4b9..fdef98980970f 100644 --- a/display_list/skia/dl_sk_dispatcher.cc +++ b/display_list/skia/dl_sk_dispatcher.cc @@ -152,6 +152,15 @@ void DlSkCanvasDispatcher::drawColor(DlColor color, DlBlendMode mode) { void DlSkCanvasDispatcher::drawLine(const SkPoint& p0, const SkPoint& p1) { canvas_->drawLine(p0, p1, paint()); } +void DlSkCanvasDispatcher::drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) { + SkPaint dash_paint = paint(); + SkScalar intervals[] = {on_length, off_length}; + dash_paint.setPathEffect(SkDashPathEffect::Make(intervals, 2, 0.0f)); + canvas_->drawLine(ToSkPoint(p0), ToSkPoint(p1), dash_paint); +} void DlSkCanvasDispatcher::drawRect(const SkRect& rect) { canvas_->drawRect(rect, paint()); } diff --git a/display_list/skia/dl_sk_dispatcher.h b/display_list/skia/dl_sk_dispatcher.h index 0db4dce710d0f..17cb7440a7472 100644 --- a/display_list/skia/dl_sk_dispatcher.h +++ b/display_list/skia/dl_sk_dispatcher.h @@ -57,6 +57,10 @@ class DlSkCanvasDispatcher : public virtual DlOpReceiver, void drawPaint() override; void drawColor(DlColor color, DlBlendMode mode) override; void drawLine(const SkPoint& p0, const SkPoint& p1) override; + void drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) override; void drawRect(const SkRect& rect) override; void drawOval(const SkRect& bounds) override; void drawCircle(const SkPoint& center, SkScalar radius) override; diff --git a/display_list/testing/dl_rendering_unittests.cc b/display_list/testing/dl_rendering_unittests.cc index 6e9b23548297c..e55288b2172cd 100644 --- a/display_list/testing/dl_rendering_unittests.cc +++ b/display_list/testing/dl_rendering_unittests.cc @@ -1977,68 +1977,6 @@ class CanvasCompareTester { ctx.paint.setStrokeMiter(0.0); ctx.paint.setStrokeJoin(DlStrokeJoin::kMiter); })); - - { - const SkScalar test_dashes_1[] = {29.0, 2.0}; - const SkScalar test_dashes_2[] = {17.0, 1.5}; - auto dl_dash_effect = DlDashPathEffect::Make(test_dashes_1, 2, 0.0f); - auto sk_dash_effect = SkDashPathEffect::Make(test_dashes_1, 2, 0.0f); - { - RenderWith(testP, stroke_base_env, tolerance, - CaseParameters( - "PathEffect without forced stroking == Dash-29-2", - [=](const SkSetupContext& ctx) { - // Provide some non-trivial stroke size to get dashed - ctx.paint.setStrokeWidth(5.0); - ctx.paint.setPathEffect(sk_dash_effect); - }, - [=](const DlSetupContext& ctx) { - // Provide some non-trivial stroke size to get dashed - ctx.paint.setStrokeWidth(5.0); - ctx.paint.setPathEffect(dl_dash_effect); - })); - } - { - RenderWith(testP, stroke_base_env, tolerance, - CaseParameters( - "PathEffect == Dash-29-2", - [=](const SkSetupContext& ctx) { - // Need stroke style to see dashing properly - ctx.paint.setStyle(SkPaint::kStroke_Style); - // Provide some non-trivial stroke size to get dashed - ctx.paint.setStrokeWidth(5.0); - ctx.paint.setPathEffect(sk_dash_effect); - }, - [=](const DlSetupContext& ctx) { - // Need stroke style to see dashing properly - ctx.paint.setDrawStyle(DlDrawStyle::kStroke); - // Provide some non-trivial stroke size to get dashed - ctx.paint.setStrokeWidth(5.0); - ctx.paint.setPathEffect(dl_dash_effect); - })); - } - dl_dash_effect = DlDashPathEffect::Make(test_dashes_2, 2, 0.0f); - sk_dash_effect = SkDashPathEffect::Make(test_dashes_2, 2, 0.0f); - { - RenderWith(testP, stroke_base_env, tolerance, - CaseParameters( - "PathEffect == Dash-17-1.5", - [=](const SkSetupContext& ctx) { - // Need stroke style to see dashing properly - ctx.paint.setStyle(SkPaint::kStroke_Style); - // Provide some non-trivial stroke size to get dashed - ctx.paint.setStrokeWidth(5.0); - ctx.paint.setPathEffect(sk_dash_effect); - }, - [=](const DlSetupContext& ctx) { - // Need stroke style to see dashing properly - ctx.paint.setDrawStyle(DlDrawStyle::kStroke); - // Provide some non-trivial stroke size to get dashed - ctx.paint.setStrokeWidth(5.0); - ctx.paint.setPathEffect(dl_dash_effect); - })); - } - } } static void RenderWithTransforms(const TestParameters& testP, @@ -3003,6 +2941,57 @@ TEST_F(DisplayListRendering, DrawVerticalLine) { .set_vertical_line()); } +TEST_F(DisplayListRendering, DrawDiagonalDashedLines) { + SkPoint p1 = SkPoint::Make(kRenderLeft, kRenderTop); + SkPoint p2 = SkPoint::Make(kRenderRight, kRenderBottom); + SkPoint p3 = SkPoint::Make(kRenderLeft, kRenderBottom); + SkPoint p4 = SkPoint::Make(kRenderRight, kRenderTop); + // Adding some edge center to edge center diagonals to better fill + // out the RRect Clip so bounds checking sees less empty bounds space. + SkPoint p5 = SkPoint::Make(kRenderCenterX, kRenderTop); + SkPoint p6 = SkPoint::Make(kRenderRight, kRenderCenterY); + SkPoint p7 = SkPoint::Make(kRenderLeft, kRenderCenterY); + SkPoint p8 = SkPoint::Make(kRenderCenterX, kRenderBottom); + + // Full diagonals are 100x100 which are 140 in length + // Dashing them with 25 on, 5 off means that the last + // dash goes from 120 to 145 which means both ends of the + // diagonals will be in an "on" dash for maximum bounds + + // Edge to edge diagonals are 50x50 which are 70 in length + // Dashing them with 25 on, 5 off means that the last + // dash goes from 60 to 85 which means both ends of the + // edge diagonals will be in a dash segment + + CanvasCompareTester::RenderAll( // + TestParameters( + [=](const SkRenderContext& ctx) { // + // Skia requires kStroke style on horizontal and vertical + // lines to get the bounds correct. + // See https://bugs.chromium.org/p/skia/issues/detail?id=12446 + SkPaint p = ctx.paint; + p.setStyle(SkPaint::kStroke_Style); + SkScalar intervals[2] = {25.0f, 5.0f}; + p.setPathEffect(SkDashPathEffect::Make(intervals, 2.0f, 0.0f)); + ctx.canvas->drawLine(p1, p2, p); + ctx.canvas->drawLine(p3, p4, p); + ctx.canvas->drawLine(p5, p6, p); + ctx.canvas->drawLine(p7, p8, p); + }, + [=](const DlRenderContext& ctx) { // + ctx.canvas->DrawDashedLine(ToDlPoint(p1), ToDlPoint(p2), // + 25.0f, 5.0f, ctx.paint); + ctx.canvas->DrawDashedLine(ToDlPoint(p3), ToDlPoint(p4), // + 25.0f, 5.0f, ctx.paint); + ctx.canvas->DrawDashedLine(ToDlPoint(p5), ToDlPoint(p6), // + 25.0f, 5.0f, ctx.paint); + ctx.canvas->DrawDashedLine(ToDlPoint(p7), ToDlPoint(p8), // + 25.0f, 5.0f, ctx.paint); + }, + kDrawLineFlags) + .set_draw_line()); +} + TEST_F(DisplayListRendering, DrawRect) { // Bounds are offset by 0.5 pixels to induce AA CanvasCompareTester::RenderAll( // diff --git a/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc index 5e744519f67dc..a5c06eb357764 100644 --- a/display_list/testing/dl_test_snippets.cc +++ b/display_list/testing/dl_test_snippets.cc @@ -524,6 +524,10 @@ std::vector CreateAllRenderingOps() { [](DlOpReceiver& r) { r.drawLine({0, 0}, {10, 10}); }}, + {1, 24, 1, + [](DlOpReceiver& r) { + r.drawLine({1, 0}, {10, 10}); + }}, {1, 24, 1, [](DlOpReceiver& r) { r.drawLine({0, 1}, {10, 10}); @@ -537,6 +541,37 @@ std::vector CreateAllRenderingOps() { r.drawLine({0, 0}, {10, 20}); }}, }}, + {"DrawDashedLine", + { + {1, 32, 1, + [](DlOpReceiver& r) { + r.drawDashedLine({0, 0}, {10, 10}, 4.0f, 2.0f); + }}, + {1, 32, 1, + [](DlOpReceiver& r) { + r.drawDashedLine({1, 0}, {10, 10}, 4.0f, 2.0f); + }}, + {1, 32, 1, + [](DlOpReceiver& r) { + r.drawDashedLine({0, 1}, {10, 10}, 4.0f, 2.0f); + }}, + {1, 32, 1, + [](DlOpReceiver& r) { + r.drawDashedLine({0, 0}, {20, 10}, 4.0f, 2.0f); + }}, + {1, 32, 1, + [](DlOpReceiver& r) { + r.drawDashedLine({0, 0}, {10, 20}, 4.0f, 2.0f); + }}, + {1, 32, 1, + [](DlOpReceiver& r) { + r.drawDashedLine({0, 0}, {10, 10}, 5.0f, 2.0f); + }}, + {1, 32, 1, + [](DlOpReceiver& r) { + r.drawDashedLine({0, 0}, {10, 10}, 4.0f, 3.0f); + }}, + }}, {"DrawRect", { {1, 24, 1, diff --git a/display_list/utils/dl_receiver_utils.h b/display_list/utils/dl_receiver_utils.h index a3b5bb156de86..ada764ce61b3c 100644 --- a/display_list/utils/dl_receiver_utils.h +++ b/display_list/utils/dl_receiver_utils.h @@ -85,6 +85,10 @@ class IgnoreDrawDispatchHelper : public virtual DlOpReceiver { void drawColor(DlColor color, DlBlendMode mode) override {} void drawPaint() override {} void drawLine(const SkPoint& p0, const SkPoint& p1) override {} + void drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) override {} void drawRect(const SkRect& rect) override {} void drawOval(const SkRect& bounds) override {} void drawCircle(const SkPoint& center, SkScalar radius) override {} diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 6dbf67c41c669..a100ac5a3a6e2 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -802,6 +802,29 @@ void DlDispatcherBase::drawLine(const SkPoint& p0, const SkPoint& p1) { skia_conversions::ToPoint(p1), paint_); } +void DlDispatcherBase::drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) { + Scalar length = p0.GetDistance(p1); + if (length > 0.0f && on_length > 0.0f && off_length > 0.0f) { + Point delta = (p1 - p0) / length; // length > 0 already tested + Scalar consumed = 0.0f; + while (consumed < length) { + Scalar dash_end = consumed + on_length; + if (dash_end < length) { + GetCanvas().DrawLine(p0 + delta * consumed, p0 + delta * dash_end, + paint_); + } else { + GetCanvas().DrawLine(p0 + delta * consumed, p1, paint_); + } + consumed = dash_end + off_length; + } + } else { + drawLine(flutter::ToSkPoint(p0), flutter::ToSkPoint(p1)); + } +} + // |flutter::DlOpReceiver| void DlDispatcherBase::drawRect(const SkRect& rect) { GetCanvas().DrawRect(skia_conversions::ToRect(rect), paint_); diff --git a/impeller/display_list/dl_dispatcher.h b/impeller/display_list/dl_dispatcher.h index b089b1e0dc643..91ea7f94fbaaa 100644 --- a/impeller/display_list/dl_dispatcher.h +++ b/impeller/display_list/dl_dispatcher.h @@ -5,8 +5,9 @@ #ifndef FLUTTER_IMPELLER_DISPLAY_LIST_DL_DISPATCHER_H_ #define FLUTTER_IMPELLER_DISPLAY_LIST_DL_DISPATCHER_H_ -#include "display_list/utils/dl_receiver_utils.h" #include "flutter/display_list/dl_op_receiver.h" +#include "flutter/display_list/geometry/dl_geometry_types.h" +#include "flutter/display_list/utils/dl_receiver_utils.h" #include "fml/logging.h" #include "impeller/aiks/canvas.h" #include "impeller/aiks/experimental_canvas.h" @@ -16,6 +17,9 @@ namespace impeller { +using DlScalar = flutter::DlScalar; +using DlPoint = flutter::DlPoint; + class DlDispatcherBase : public flutter::DlOpReceiver { public: Picture EndRecordingAsPicture(); @@ -142,6 +146,12 @@ class DlDispatcherBase : public flutter::DlOpReceiver { // |flutter::DlOpReceiver| void drawLine(const SkPoint& p0, const SkPoint& p1) override; + // |flutter::DlOpReceiver| + void drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) override; + // |flutter::DlOpReceiver| void drawRect(const SkRect& rect) override; diff --git a/shell/common/dl_op_spy.cc b/shell/common/dl_op_spy.cc index f3995a45cf2f2..78ce2a3ada55d 100644 --- a/shell/common/dl_op_spy.cc +++ b/shell/common/dl_op_spy.cc @@ -44,6 +44,12 @@ void DlOpSpy::drawPaint() { void DlOpSpy::drawLine(const SkPoint& p0, const SkPoint& p1) { did_draw_ |= will_draw_; } +void DlOpSpy::drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) { + did_draw_ |= will_draw_; +} void DlOpSpy::drawRect(const SkRect& rect) { did_draw_ |= will_draw_; } diff --git a/shell/common/dl_op_spy.h b/shell/common/dl_op_spy.h index 438a2757bf631..2eeb6e66b5c36 100644 --- a/shell/common/dl_op_spy.h +++ b/shell/common/dl_op_spy.h @@ -46,6 +46,10 @@ class DlOpSpy final : public virtual DlOpReceiver, void drawColor(DlColor color, DlBlendMode mode) override; void drawPaint() override; void drawLine(const SkPoint& p0, const SkPoint& p1) override; + void drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) override; void drawRect(const SkRect& rect) override; void drawOval(const SkRect& bounds) override; void drawCircle(const SkPoint& center, SkScalar radius) override; diff --git a/shell/common/dl_op_spy_unittests.cc b/shell/common/dl_op_spy_unittests.cc index 6aecb4b5dbf5e..90de2eed6e180 100644 --- a/shell/common/dl_op_spy_unittests.cc +++ b/shell/common/dl_op_spy_unittests.cc @@ -182,6 +182,27 @@ TEST(DlOpSpy, DrawLine) { } } +TEST(DlOpSpy, DrawDashedLine) { + { // black + DisplayListBuilder builder; + DlPaint paint(DlColor::kBlack()); + builder.DrawDashedLine(DlPoint(0, 1), DlPoint(1, 2), 1.0f, 1.0f, paint); + sk_sp dl = builder.Build(); + DlOpSpy dl_op_spy; + dl->Dispatch(dl_op_spy); + ASSERT_DID_DRAW(dl_op_spy, dl); + } + { // transparent + DisplayListBuilder builder; + DlPaint paint(DlColor::kTransparent()); + builder.DrawDashedLine(DlPoint(0, 1), DlPoint(1, 2), 1.0f, 1.0f, paint); + sk_sp dl = builder.Build(); + DlOpSpy dl_op_spy; + dl->Dispatch(dl_op_spy); + ASSERT_NO_DRAW(dl_op_spy, dl); + } +} + TEST(DlOpSpy, DrawRect) { { // black DisplayListBuilder builder; diff --git a/testing/display_list_testing.cc b/testing/display_list_testing.cc index 170504373425f..93db4107d773f 100644 --- a/testing/display_list_testing.cc +++ b/testing/display_list_testing.cc @@ -776,6 +776,17 @@ void DisplayListStreamDispatcher::drawLine(const SkPoint& p0, const SkPoint& p1) { startl() << "drawLine(" << p0 << ", " << p1 << ");" << std::endl; } +void DisplayListStreamDispatcher::drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) { + startl() << "drawDashedLine(" + << p0 << ", " + << p1 << ", " + << on_length << ", " + << off_length + << ");" << std::endl; +} void DisplayListStreamDispatcher::drawRect(const SkRect& rect) { startl() << "drawRect(" << rect << ");" << std::endl; } diff --git a/testing/display_list_testing.h b/testing/display_list_testing.h index 369864176ca90..895056f97b445 100644 --- a/testing/display_list_testing.h +++ b/testing/display_list_testing.h @@ -125,6 +125,10 @@ class DisplayListStreamDispatcher final : public DlOpReceiver { void drawColor(DlColor color, DlBlendMode mode) override; void drawPaint() override; void drawLine(const SkPoint& p0, const SkPoint& p1) override; + void drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) override; void drawRect(const SkRect& rect) override; void drawOval(const SkRect& bounds) override; void drawCircle(const SkPoint& center, SkScalar radius) override; diff --git a/testing/mock_canvas.cc b/testing/mock_canvas.cc index 5966e24d857de..65ab1bb96f8dd 100644 --- a/testing/mock_canvas.cc +++ b/testing/mock_canvas.cc @@ -258,6 +258,14 @@ void MockCanvas::DrawLine(const SkPoint& p0, FML_DCHECK(false); } +void MockCanvas::DrawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length, + const DlPaint& paint) { + FML_DCHECK(false); +} + void MockCanvas::DrawPoints(PointMode, uint32_t, const SkPoint[], diff --git a/testing/mock_canvas.h b/testing/mock_canvas.h index de1b00690c184..7d303d466a498 100644 --- a/testing/mock_canvas.h +++ b/testing/mock_canvas.h @@ -218,6 +218,11 @@ class MockCanvas final : public DlCanvas { void DrawLine(const SkPoint& p0, const SkPoint& p1, const DlPaint& paint) override; + void DrawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length, + const DlPaint& paint) override; void DrawRect(const SkRect& rect, const DlPaint& paint) override; void DrawOval(const SkRect& bounds, const DlPaint& paint) override; void DrawCircle(const SkPoint& center, diff --git a/third_party/txt/src/skia/paragraph_skia.cc b/third_party/txt/src/skia/paragraph_skia.cc index 28c0f2ca0e446..4855e24c6d480 100644 --- a/third_party/txt/src/skia/paragraph_skia.cc +++ b/third_party/txt/src/skia/paragraph_skia.cc @@ -153,27 +153,16 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { SkScalar x1, SkScalar y1, const DecorationStyle& decor_style) override { - // We only support horizontal lines. - FML_DCHECK(y0 == y1); - - // This function is called for both solid and dashed lines. If we're drawing - // a dashed line, and we're using the Impeller backend, then we need to draw - // the line directly using the `drawLine` API instead of using a path effect - // (because Impeller does not support path effects). auto dash_path_effect = decor_style.getDashPathEffect(); -#ifdef IMPELLER_SUPPORTS_RENDERING - if (impeller_enabled_ && dash_path_effect) { - auto path = dashedLine(x0, x1, y0, *dash_path_effect); - builder_->DrawPath(path, toDlPaint(decor_style)); - return; - } -#endif // IMPELLER_SUPPORTS_RENDERING - auto paint = toDlPaint(decor_style); + if (dash_path_effect) { - setPathEffect(paint, *dash_path_effect); + builder_->DrawDashedLine(DlPoint(x0, y0), DlPoint(x1, y1), + dash_path_effect->fOnLength, + dash_path_effect->fOffLength, paint); + } else { + builder_->DrawLine(SkPoint::Make(x0, y0), SkPoint::Make(x1, y1), paint); } - builder_->DrawLine(SkPoint::Make(x0, y0), SkPoint::Make(x1, y1), paint); } void clipRect(const SkRect& rect) override { @@ -189,31 +178,6 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { void restore() override { builder_->Restore(); } private: - SkPath dashedLine(SkScalar x0, - SkScalar x1, - SkScalar y0, - const DashPathEffect& dash_path_effect) { - auto dx = 0.0; - auto path = SkPath(); - auto on = true; - auto length = x1 - x0; - while (dx < length) { - if (on) { - // Draw the on part of the dash. - path.moveTo(x0 + dx, y0); - dx += dash_path_effect.fOnLength; - path.lineTo(x0 + dx, y0); - } else { - // Skip the off part of the dash. - dx += dash_path_effect.fOffLength; - } - on = !on; - } - - path.close(); - return path; - } - bool ShouldRenderAsPath(const DlPaint& paint) const { FML_DCHECK(impeller_enabled_); // Text with non-trivial color sources should be rendered as a path when @@ -233,16 +197,6 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter { return paint; } - void setPathEffect(DlPaint& paint, const DashPathEffect& dash_path_effect) { - // Impeller does not support path effects, so we should never be setting. - FML_DCHECK(!impeller_enabled_); - - std::array intervals{dash_path_effect.fOnLength, - dash_path_effect.fOffLength}; - auto effect = DlDashPathEffect::Make(intervals.data(), intervals.size(), 0); - paint.setPathEffect(effect); - } - DisplayListBuilder* builder_; const std::vector& dl_paints_; const bool impeller_enabled_; diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index dd5446aa12ece..3a3f990af3098 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -43,6 +43,7 @@ class DlOpRecorder final : public virtual DlOpReceiver, private IgnoreTransformDispatchHelper { public: int lineCount() const { return lines_.size(); } + int dashedLineCount() const { return dashed_lines_.size(); } int rectCount() const { return rects_.size(); } int pathCount() const { return paths_.size(); } int textFrameCount() const { return text_frames_.size(); } @@ -54,6 +55,13 @@ class DlOpRecorder final : public virtual DlOpReceiver, lines_.emplace_back(p0, p1); } + void drawDashedLine(const DlPoint& p0, + const DlPoint& p1, + DlScalar on_length, + DlScalar off_length) override { + dashed_lines_.emplace_back(p0, p1, DlPoint(on_length, off_length)); + } + void drawTextFrame(const std::shared_ptr& text_frame, SkScalar x, SkScalar y) override { @@ -77,6 +85,7 @@ class DlOpRecorder final : public virtual DlOpReceiver, std::vector> text_frames_; std::vector> blobs_; std::vector> lines_; + std::vector> dashed_lines_; std::vector rects_; std::vector paths_; const DlPathEffect* path_effect_; @@ -201,8 +210,9 @@ TEST_F(PainterTest, DrawDashedLineSkia) { ->Dispatch(recorder); // Skia draws a dashed underline as a filled rectangle with a path effect. - EXPECT_EQ(recorder.lineCount(), 1); - EXPECT_TRUE(recorder.hasPathEffect()); + EXPECT_EQ(recorder.lineCount(), 0); + EXPECT_EQ(recorder.dashedLineCount(), 1); + EXPECT_FALSE(recorder.hasPathEffect()); } #ifdef IMPELLER_SUPPORTS_RENDERING @@ -227,7 +237,8 @@ TEST_F(PainterTest, DrawDashedLineImpeller) { ->Dispatch(recorder); // Impeller draws a dashed underline as a path. - EXPECT_EQ(recorder.pathCount(), 1); + EXPECT_EQ(recorder.pathCount(), 0); + EXPECT_EQ(recorder.dashedLineCount(), 1); EXPECT_FALSE(recorder.hasPathEffect()); } From 1c9b8c0f19420c5d76ae29b1501bb655bfcd6be7 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 14 Jun 2024 19:02:03 -0700 Subject: [PATCH 2/4] minor adjustment to validation logic and an impeller golden test --- impeller/display_list/dl_dispatcher.cc | 7 ++- impeller/display_list/dl_golden_unittests.cc | 50 ++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index a100ac5a3a6e2..9ccc88c882eb2 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -807,7 +807,12 @@ void DlDispatcherBase::drawDashedLine(const DlPoint& p0, DlScalar on_length, DlScalar off_length) { Scalar length = p0.GetDistance(p1); - if (length > 0.0f && on_length > 0.0f && off_length > 0.0f) { + // Reasons to defer to regular DrawLine: + // length is non-positive - drawLine will draw appropriate "dot" + // off_length is non-positive - no gaps, drawLine will draw it solid + // on_length is negative - invalid dashing + // Note that a 0 length "on" dash will draw "dot"s every "off" distance apart + if (length > 0.0f && on_length >= 0.0f && off_length > 0.0f) { Point delta = (p1 - p0) / length; // length > 0 already tested Scalar consumed = 0.0f; while (consumed < length) { diff --git a/impeller/display_list/dl_golden_unittests.cc b/impeller/display_list/dl_golden_unittests.cc index a0c3af88c04e9..65131989d1a4b 100644 --- a/impeller/display_list/dl_golden_unittests.cc +++ b/impeller/display_list/dl_golden_unittests.cc @@ -11,9 +11,11 @@ namespace flutter { namespace testing { +using impeller::Degrees; using impeller::PlaygroundBackend; using impeller::PlaygroundTest; using impeller::Point; +using impeller::Radians; using impeller::Scalar; INSTANTIATE_PLAYGROUND_SUITE(DlGoldenTest); @@ -180,5 +182,53 @@ TEST_P(DlGoldenTest, GaussianVsRRectBlurScaledRotated) { ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } +TEST_P(DlGoldenTest, DashedLinesTest) { + Point content_scale = GetContentScale(); + auto draw = [content_scale](DlCanvas* canvas, + const std::vector>& images) { + canvas->Scale(content_scale.x, content_scale.y); + canvas->DrawPaint(DlPaint().setColor(DlColor::kWhite())); + + auto draw_one = [canvas](DlStrokeCap cap, Scalar x, Scalar y, + Scalar dash_on, Scalar dash_off) { + Point center = Point(x, y); + Scalar inner = 20.0f; + Scalar outer = 100.0f; + DlPaint thick_paint = DlPaint() + .setColor(DlColor::kBlue()) + .setStrokeCap(cap) + .setStrokeWidth(8.0f); + DlPaint middle_paint = DlPaint() + .setColor(DlColor::kGreen()) + .setStrokeCap(cap) + .setStrokeWidth(5.0f); + DlPaint thin_paint = DlPaint() + .setColor(DlColor::kMagenta()) + .setStrokeCap(cap) + .setStrokeWidth(2.0f); + for (int degrees = 0; degrees < 360; degrees += 30) { + Point delta = Point(1.0f, 0.0f).Rotate(Degrees(degrees)); + canvas->DrawDashedLine(center + inner * delta, center + outer * delta, + dash_on, dash_off, thick_paint); + canvas->DrawDashedLine(center + inner * delta, center + outer * delta, + dash_on, dash_off, middle_paint); + canvas->DrawDashedLine(center + inner * delta, center + outer * delta, + dash_on, dash_off, thin_paint); + } + }; + + draw_one(DlStrokeCap::kButt, 150.0f, 150.0f, 15.0f, 10.0f); + draw_one(DlStrokeCap::kSquare, 400.0f, 150.0f, 15.0f, 10.0f); + draw_one(DlStrokeCap::kRound, 150.0f, 400.0f, 15.0f, 10.0f); + draw_one(DlStrokeCap::kRound, 400.0f, 400.0f, 0.0f, 11.0f); + }; + + DisplayListBuilder builder; + std::vector> images; + draw(&builder, images); + + ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); +} + } // namespace testing } // namespace flutter From 41023356444c0cddcff1df8c35980f1820e81065 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Mon, 17 Jun 2024 15:06:44 -0700 Subject: [PATCH 3/4] switch to one Path for the dashed line for better fidelity --- impeller/display_list/dl_dispatcher.cc | 17 ++++++++++++++--- impeller/display_list/dl_golden_unittests.cc | 12 ++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 9ccc88c882eb2..e377f7b5e4e38 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -814,17 +814,28 @@ void DlDispatcherBase::drawDashedLine(const DlPoint& p0, // Note that a 0 length "on" dash will draw "dot"s every "off" distance apart if (length > 0.0f && on_length >= 0.0f && off_length > 0.0f) { Point delta = (p1 - p0) / length; // length > 0 already tested + PathBuilder builder; + Scalar consumed = 0.0f; while (consumed < length) { + builder.MoveTo(p0 + delta * consumed); + Scalar dash_end = consumed + on_length; if (dash_end < length) { - GetCanvas().DrawLine(p0 + delta * consumed, p0 + delta * dash_end, - paint_); + builder.LineTo(p0 + delta * dash_end); } else { - GetCanvas().DrawLine(p0 + delta * consumed, p1, paint_); + builder.LineTo(p1); + // Should happen anyway due to the math, but let's make it explicit + // in case of bit errors. We're done with this line. + break; } + consumed = dash_end + off_length; } + + Paint stroke_paint = paint_; + stroke_paint.style = Paint::Style::kStroke; + GetCanvas().DrawPath(builder.TakePath(), stroke_paint); } else { drawLine(flutter::ToSkPoint(p0), flutter::ToSkPoint(p1)); } diff --git a/impeller/display_list/dl_golden_unittests.cc b/impeller/display_list/dl_golden_unittests.cc index 65131989d1a4b..506e3970da4c8 100644 --- a/impeller/display_list/dl_golden_unittests.cc +++ b/impeller/display_list/dl_golden_unittests.cc @@ -221,6 +221,18 @@ TEST_P(DlGoldenTest, DashedLinesTest) { draw_one(DlStrokeCap::kSquare, 400.0f, 150.0f, 15.0f, 10.0f); draw_one(DlStrokeCap::kRound, 150.0f, 400.0f, 15.0f, 10.0f); draw_one(DlStrokeCap::kRound, 400.0f, 400.0f, 0.0f, 11.0f); + + // Make sure the rendering op responds appropriately to clipping + canvas->Save(); + SkPath clip_path = SkPath(); + clip_path.moveTo(275.0f, 225.0f); + clip_path.lineTo(325.0f, 275.0f); + clip_path.lineTo(275.0f, 325.0f); + clip_path.lineTo(225.0f, 275.0f); + canvas->ClipPath(clip_path); + canvas->DrawColor(DlColor::kYellow()); + draw_one(DlStrokeCap::kRound, 275.0f, 275.0f, 15.0f, 10.0f); + canvas->Restore(); }; DisplayListBuilder builder; From 08b8a97c958c3b626bacb9ec56c5930cb71843e2 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Mon, 17 Jun 2024 15:09:04 -0700 Subject: [PATCH 4/4] add new golden files to output list --- testing/impeller_golden_tests_output.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testing/impeller_golden_tests_output.txt b/testing/impeller_golden_tests_output.txt index 1451a30eacc3b..7b4ead0b5c007 100644 --- a/testing/impeller_golden_tests_output.txt +++ b/testing/impeller_golden_tests_output.txt @@ -810,6 +810,9 @@ impeller_Play_DlGoldenTest_CanDrawPaint_Vulkan.png impeller_Play_DlGoldenTest_CanRenderImage_Metal.png impeller_Play_DlGoldenTest_CanRenderImage_OpenGLES.png 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_GaussianVsRRectBlurScaledRotated_Metal.png impeller_Play_DlGoldenTest_GaussianVsRRectBlurScaledRotated_OpenGLES.png impeller_Play_DlGoldenTest_GaussianVsRRectBlurScaledRotated_Vulkan.png