From 1f80c5f7302e25e530ff1a9e7823495d0cb9c27d Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Mon, 9 Sep 2024 17:05:15 -0700 Subject: [PATCH 1/2] [DisplayList] DlPath object provides auto-conversion from Skia to Impeller --- display_list/BUILD.gn | 3 + display_list/benchmarking/dl_complexity_gl.cc | 4 +- display_list/benchmarking/dl_complexity_gl.h | 4 +- .../benchmarking/dl_complexity_helper.h | 3 +- .../benchmarking/dl_complexity_metal.cc | 4 +- .../benchmarking/dl_complexity_metal.h | 4 +- display_list/display_list_unittests.cc | 108 ++--------- display_list/dl_builder.cc | 34 ++-- display_list/dl_builder.h | 22 ++- display_list/dl_canvas.h | 16 ++ display_list/dl_op_receiver.h | 46 +---- display_list/dl_op_records.h | 115 +++++------- display_list/geometry/dl_geometry_types.h | 8 + display_list/geometry/dl_path.cc | 149 +++++++++++++++ display_list/geometry/dl_path.h | 49 +++++ display_list/geometry/dl_path_unittests.cc | 171 ++++++++++++++++++ display_list/skia/dl_sk_dispatcher.cc | 13 +- display_list/skia/dl_sk_dispatcher.h | 6 +- display_list/testing/dl_test_snippets.h | 18 +- display_list/utils/dl_receiver_utils.h | 6 +- impeller/display_list/dl_dispatcher.cc | 69 ++----- impeller/display_list/dl_dispatcher.h | 30 +-- impeller/display_list/skia_conversions.cc | 91 ---------- impeller/display_list/skia_conversions.h | 5 - shell/common/dl_op_spy.cc | 4 +- shell/common/dl_op_spy.h | 4 +- testing/display_list_testing.cc | 13 +- testing/display_list_testing.h | 13 +- third_party/txt/tests/paragraph_unittests.cc | 4 +- 29 files changed, 577 insertions(+), 439 deletions(-) create mode 100644 display_list/geometry/dl_path.cc create mode 100644 display_list/geometry/dl_path.h create mode 100644 display_list/geometry/dl_path_unittests.cc diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn index 781e5b028303b..dbb81d46cd626 100644 --- a/display_list/BUILD.gn +++ b/display_list/BUILD.gn @@ -60,6 +60,8 @@ source_set("display_list") { "effects/dl_runtime_effect.cc", "effects/dl_runtime_effect.h", "geometry/dl_geometry_types.h", + "geometry/dl_path.cc", + "geometry/dl_path.h", "geometry/dl_region.cc", "geometry/dl_region.h", "geometry/dl_rtree.cc", @@ -119,6 +121,7 @@ if (enable_unittests) { "effects/dl_image_filter_unittests.cc", "effects/dl_mask_filter_unittests.cc", "geometry/dl_geometry_types_unittests.cc", + "geometry/dl_path_unittests.cc", "geometry/dl_region_unittests.cc", "geometry/dl_rtree_unittests.cc", "skia/dl_sk_conversions_unittests.cc", diff --git a/display_list/benchmarking/dl_complexity_gl.cc b/display_list/benchmarking/dl_complexity_gl.cc index 28ca569e0cc7e..44f658bb99146 100644 --- a/display_list/benchmarking/dl_complexity_gl.cc +++ b/display_list/benchmarking/dl_complexity_gl.cc @@ -332,7 +332,7 @@ void DisplayListGLComplexityCalculator::GLHelper::drawDRRect( AccumulateComplexity(complexity); } -void DisplayListGLComplexityCalculator::GLHelper::drawPath(const SkPath& path) { +void DisplayListGLComplexityCalculator::GLHelper::drawPath(const DlPath& path) { if (IsComplex()) { return; } @@ -655,7 +655,7 @@ void DisplayListGLComplexityCalculator::GLHelper::drawTextFrame( DlScalar y) {} void DisplayListGLComplexityCalculator::GLHelper::drawShadow( - const SkPath& path, + const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/display_list/benchmarking/dl_complexity_gl.h b/display_list/benchmarking/dl_complexity_gl.h index 53849599db0ba..d0fcbb98a3bf4 100644 --- a/display_list/benchmarking/dl_complexity_gl.h +++ b/display_list/benchmarking/dl_complexity_gl.h @@ -49,7 +49,7 @@ class DisplayListGLComplexityCalculator void drawCircle(const DlPoint& center, DlScalar radius) override; void drawRRect(const SkRRect& rrect) override; void drawDRRect(const SkRRect& outer, const SkRRect& inner) override; - void drawPath(const SkPath& path) override; + void drawPath(const DlPath& path) override; void drawArc(const DlRect& oval_bounds, DlScalar start_degrees, DlScalar sweep_degrees, @@ -76,7 +76,7 @@ class DisplayListGLComplexityCalculator void drawTextFrame(const std::shared_ptr& text_frame, DlScalar x, DlScalar y) override; - void drawShadow(const SkPath& path, + void drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/display_list/benchmarking/dl_complexity_helper.h b/display_list/benchmarking/dl_complexity_helper.h index 8393151d32113..1303ff3a25633 100644 --- a/display_list/benchmarking/dl_complexity_helper.h +++ b/display_list/benchmarking/dl_complexity_helper.h @@ -214,11 +214,12 @@ class ComplexityCalculatorHelper inline unsigned int Ceiling() { return ceiling_; } inline unsigned int CurrentComplexityScore() { return complexity_score_; } - unsigned int CalculatePathComplexity(const SkPath& path, + unsigned int CalculatePathComplexity(const DlPath& dl_path, unsigned int line_verb_cost, unsigned int quad_verb_cost, unsigned int conic_verb_cost, unsigned int cubic_verb_cost) { + const SkPath& path = dl_path.GetSkPath(); int verb_count = path.countVerbs(); std::vector verbs(verb_count); path.getVerbs(verbs.data(), verbs.size()); diff --git a/display_list/benchmarking/dl_complexity_metal.cc b/display_list/benchmarking/dl_complexity_metal.cc index 6f69eb8a2d7a5..82f641e6cc123 100644 --- a/display_list/benchmarking/dl_complexity_metal.cc +++ b/display_list/benchmarking/dl_complexity_metal.cc @@ -329,7 +329,7 @@ void DisplayListMetalComplexityCalculator::MetalHelper::drawDRRect( } void DisplayListMetalComplexityCalculator::MetalHelper::drawPath( - const SkPath& path) { + const DlPath& path) { if (IsComplex()) { return; } @@ -599,7 +599,7 @@ void DisplayListMetalComplexityCalculator::MetalHelper::drawTextFrame( DlScalar y) {} void DisplayListMetalComplexityCalculator::MetalHelper::drawShadow( - const SkPath& path, + const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/display_list/benchmarking/dl_complexity_metal.h b/display_list/benchmarking/dl_complexity_metal.h index 8c428cd571737..d11d0ff2a7b02 100644 --- a/display_list/benchmarking/dl_complexity_metal.h +++ b/display_list/benchmarking/dl_complexity_metal.h @@ -49,7 +49,7 @@ class DisplayListMetalComplexityCalculator void drawCircle(const DlPoint& center, DlScalar radius) override; void drawRRect(const SkRRect& rrect) override; void drawDRRect(const SkRRect& outer, const SkRRect& inner) override; - void drawPath(const SkPath& path) override; + void drawPath(const DlPath& path) override; void drawArc(const DlRect& oval_bounds, DlScalar start_degrees, DlScalar sweep_degrees, @@ -76,7 +76,7 @@ class DisplayListMetalComplexityCalculator void drawTextFrame(const std::shared_ptr& text_frame, DlScalar x, DlScalar y) override; - void drawShadow(const SkPath& path, + void drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 8d67422f00c91..9838573cb1435 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -3618,98 +3618,6 @@ TEST_F(DisplayListTest, NopOperationsOmittedFromRecords) { }); } -TEST_F(DisplayListTest, ImpellerPathPreferenceIsHonored) { - class Tester : public virtual DlOpReceiver, - public IgnoreClipDispatchHelper, - public IgnoreDrawDispatchHelper, - public IgnoreAttributeDispatchHelper, - public IgnoreTransformDispatchHelper { - public: - explicit Tester(bool prefer_impeller_paths) - : prefer_impeller_paths_(prefer_impeller_paths) {} - - bool PrefersImpellerPaths() const override { - return prefer_impeller_paths_; - } - - void drawPath(const SkPath& path) override { skia_draw_path_calls_++; } - - void drawPath(const CacheablePath& cache) override { - impeller_draw_path_calls_++; - } - - void clipPath(const SkPath& path, ClipOp op, bool is_aa) override { - skia_clip_path_calls_++; - } - - void clipPath(const CacheablePath& cache, ClipOp op, bool is_aa) override { - impeller_clip_path_calls_++; - } - - virtual void drawShadow(const SkPath& sk_path, - const DlColor color, - const SkScalar elevation, - bool transparent_occluder, - SkScalar dpr) override { - skia_draw_shadow_calls_++; - } - - virtual void drawShadow(const CacheablePath& cache, - const DlColor color, - const SkScalar elevation, - bool transparent_occluder, - SkScalar dpr) override { - impeller_draw_shadow_calls_++; - } - - int skia_draw_path_calls() const { return skia_draw_path_calls_; } - int skia_clip_path_calls() const { return skia_draw_path_calls_; } - int skia_draw_shadow_calls() const { return skia_draw_path_calls_; } - int impeller_draw_path_calls() const { return impeller_draw_path_calls_; } - int impeller_clip_path_calls() const { return impeller_draw_path_calls_; } - int impeller_draw_shadow_calls() const { return impeller_draw_path_calls_; } - - private: - const bool prefer_impeller_paths_; - int skia_draw_path_calls_ = 0; - int skia_clip_path_calls_ = 0; - int skia_draw_shadow_calls_ = 0; - int impeller_draw_path_calls_ = 0; - int impeller_clip_path_calls_ = 0; - int impeller_draw_shadow_calls_ = 0; - }; - - DisplayListBuilder builder; - builder.DrawPath(SkPath::Rect(SkRect::MakeLTRB(0, 0, 100, 100)), DlPaint()); - builder.ClipPath(SkPath::Rect(SkRect::MakeLTRB(0, 0, 100, 100)), - ClipOp::kIntersect, true); - builder.DrawShadow(SkPath::Rect(SkRect::MakeLTRB(20, 20, 80, 80)), - DlColor::kBlue(), 1.0f, true, 1.0f); - auto display_list = builder.Build(); - - { - Tester skia_tester(false); - display_list->Dispatch(skia_tester); - EXPECT_EQ(skia_tester.skia_draw_path_calls(), 1); - EXPECT_EQ(skia_tester.skia_clip_path_calls(), 1); - EXPECT_EQ(skia_tester.skia_draw_shadow_calls(), 1); - EXPECT_EQ(skia_tester.impeller_draw_path_calls(), 0); - EXPECT_EQ(skia_tester.impeller_clip_path_calls(), 0); - EXPECT_EQ(skia_tester.impeller_draw_shadow_calls(), 0); - } - - { - Tester impeller_tester(true); - display_list->Dispatch(impeller_tester); - EXPECT_EQ(impeller_tester.skia_draw_path_calls(), 0); - EXPECT_EQ(impeller_tester.skia_clip_path_calls(), 0); - EXPECT_EQ(impeller_tester.skia_draw_shadow_calls(), 0); - EXPECT_EQ(impeller_tester.impeller_draw_path_calls(), 1); - EXPECT_EQ(impeller_tester.impeller_clip_path_calls(), 1); - EXPECT_EQ(impeller_tester.impeller_draw_shadow_calls(), 1); - } -} - class SaveLayerBoundsExpector : public virtual DlOpReceiver, public IgnoreAttributeDispatchHelper, public IgnoreClipDispatchHelper, @@ -4616,7 +4524,7 @@ TEST_F(DisplayListTest, DrawDisplayListForwardsBackdropFlag) { #define CLIP_EXPECTOR(name) ClipExpector name(__FILE__, __LINE__) struct ClipExpectation { - std::variant shape; + std::variant shape; bool is_oval; ClipOp clip_op; bool is_aa; @@ -4628,7 +4536,7 @@ struct ClipExpectation { case 1: return "SkRRect"; case 2: - return "SkPath"; + return "DlPath"; default: return "Unknown"; } @@ -4648,7 +4556,7 @@ ::std::ostream& operator<<(::std::ostream& os, const ClipExpectation& expect) { os << std::get(expect.shape); break; case 2: - os << std::get(expect.shape); + os << std::get(expect.shape).GetSkPath(); break; case 3: os << "Unknown"; @@ -4713,7 +4621,7 @@ class ClipExpector : public virtual DlOpReceiver, return *this; } - ClipExpector& addExpectation(const SkPath& path, + ClipExpector& addExpectation(const DlPath& path, ClipOp clip_op = ClipOp::kIntersect, bool is_aa = false) { clip_expectations_.push_back({ @@ -4725,6 +4633,12 @@ class ClipExpector : public virtual DlOpReceiver, return *this; } + ClipExpector& addExpectation(const SkPath& path, + ClipOp clip_op = ClipOp::kIntersect, + bool is_aa = false) { + return addExpectation(DlPath(path), clip_op, is_aa); + } + void clipRect(const DlRect& rect, DlCanvas::ClipOp clip_op, bool is_aa) override { @@ -4740,7 +4654,7 @@ class ClipExpector : public virtual DlOpReceiver, bool is_aa) override { check(rrect, clip_op, is_aa); } - void clipPath(const SkPath& path, + void clipPath(const DlPath& path, DlCanvas::ClipOp clip_op, bool is_aa) override { check(path, clip_op, is_aa); diff --git a/display_list/dl_builder.cc b/display_list/dl_builder.cc index d1f3707ca80d1..c5329c64b9495 100644 --- a/display_list/dl_builder.cc +++ b/display_list/dl_builder.cc @@ -1044,30 +1044,30 @@ void DisplayListBuilder::ClipRRect(const SkRRect& rrect, break; } } -void DisplayListBuilder::ClipPath(const SkPath& path, +void DisplayListBuilder::ClipPath(const DlPath& path, ClipOp clip_op, bool is_aa) { if (current_info().is_nop) { return; } - if (!path.isInverseFillType()) { + if (!path.IsInverseFillType()) { SkRect rect; - if (path.isRect(&rect)) { + if (path.IsSkRect(&rect)) { ClipRect(rect, clip_op, is_aa); return; } - if (path.isOval(&rect)) { + if (path.IsSkOval(&rect)) { ClipOval(rect, clip_op, is_aa); return; } SkRRect rrect; - if (path.isRRect(&rrect)) { + if (path.IsSkRRect(&rrect)) { ClipRRect(rrect, clip_op, is_aa); return; } } - global_state().clipPath(path, clip_op, is_aa); - layer_local_state().clipPath(path, clip_op, is_aa); + global_state().clipPath(path.GetSkPath(), clip_op, is_aa); + layer_local_state().clipPath(path.GetSkPath(), clip_op, is_aa); if (global_state().is_cull_rect_empty() || layer_local_state().is_cull_rect_empty()) { current_info().is_nop = true; @@ -1234,13 +1234,14 @@ void DisplayListBuilder::DrawDRRect(const SkRRect& outer, SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawDRRectFlags); drawDRRect(outer, inner); } -void DisplayListBuilder::drawPath(const SkPath& path) { +void DisplayListBuilder::drawPath(const DlPath& path) { DisplayListAttributeFlags flags = kDrawPathFlags; OpResult result = PaintResult(current_, flags); if (result != OpResult::kNoEffect) { - bool is_visible = path.isInverseFillType() - ? AccumulateUnbounded() - : AccumulateOpBounds(path.getBounds(), flags); + bool is_visible = + path.IsInverseFillType() + ? AccumulateUnbounded() + : AccumulateOpBounds(ToSkRect(path.GetBounds()), flags); if (is_visible) { Push(0, path); CheckLayerOpacityHairlineCompatibility(); @@ -1249,6 +1250,11 @@ void DisplayListBuilder::drawPath(const SkPath& path) { } } void DisplayListBuilder::DrawPath(const SkPath& path, const DlPaint& paint) { + SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawPathFlags); + DlPath dl_path(path); + drawPath(dl_path); +} +void DisplayListBuilder::DrawPath(const DlPath& path, const DlPaint& paint) { SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawPathFlags); drawPath(path); } @@ -1731,15 +1737,15 @@ void DisplayListBuilder::DrawTextFrame( drawTextFrame(text_frame, x, y); } -void DisplayListBuilder::DrawShadow(const SkPath& path, +void DisplayListBuilder::DrawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, DlScalar dpr) { OpResult result = PaintResult(DlPaint(color)); if (result != OpResult::kNoEffect) { - SkRect shadow_bounds = - DlCanvas::ComputeShadowBounds(path, elevation, dpr, GetTransform()); + SkRect shadow_bounds = DlCanvas::ComputeShadowBounds( + path.GetSkPath(), elevation, dpr, GetTransform()); if (AccumulateOpBounds(shadow_bounds, kDrawShadowFlags)) { transparent_occluder // ? Push(0, path, color, elevation, diff --git a/display_list/dl_builder.h b/display_list/dl_builder.h index f6a7069033385..ed2a5a8759ad6 100644 --- a/display_list/dl_builder.h +++ b/display_list/dl_builder.h @@ -126,6 +126,12 @@ class DisplayListBuilder final : public virtual DlCanvas, bool is_aa = false) override; // |DlCanvas| void ClipPath(const SkPath& path, + ClipOp clip_op = ClipOp::kIntersect, + bool is_aa = false) override { + ClipPath(DlPath(path), clip_op, is_aa); + } + // |DlCanvas| + void ClipPath(const DlPath& path, ClipOp clip_op = ClipOp::kIntersect, bool is_aa = false) override; @@ -181,6 +187,8 @@ class DisplayListBuilder final : public virtual DlCanvas, // |DlCanvas| void DrawPath(const SkPath& path, const DlPaint& paint) override; // |DlCanvas| + void DrawPath(const DlPath& path, const DlPaint& paint) override; + // |DlCanvas| void DrawArc(const SkRect& bounds, DlScalar start, DlScalar sweep, @@ -245,6 +253,14 @@ class DisplayListBuilder final : public virtual DlCanvas, // |DlCanvas| void DrawShadow(const SkPath& path, + const DlColor color, + const DlScalar elevation, + bool transparent_occluder, + DlScalar dpr) override { + DrawShadow(DlPath(path), color, elevation, transparent_occluder, dpr); + } + // |DlCanvas| + void DrawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, @@ -412,7 +428,7 @@ class DisplayListBuilder final : public virtual DlCanvas, ClipRRect(rrect, clip_op, is_aa); } // |DlOpReceiver| - void clipPath(const SkPath& path, ClipOp clip_op, bool is_aa) override { + void clipPath(const DlPath& path, ClipOp clip_op, bool is_aa) override { ClipPath(path, clip_op, is_aa); } @@ -440,7 +456,7 @@ class DisplayListBuilder final : public virtual DlCanvas, // |DlOpReceiver| void drawDRRect(const SkRRect& outer, const SkRRect& inner) override; // |DlOpReceiver| - void drawPath(const SkPath& path) override; + void drawPath(const DlPath& path) override; // |DlOpReceiver| void drawArc(const DlRect& bounds, DlScalar start, @@ -492,7 +508,7 @@ class DisplayListBuilder final : public virtual DlCanvas, DlScalar x, DlScalar y) override; // |DlOpReceiver| - void drawShadow(const SkPath& path, + void drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/display_list/dl_canvas.h b/display_list/dl_canvas.h index f21e54a1013bd..43771e39661df 100644 --- a/display_list/dl_canvas.h +++ b/display_list/dl_canvas.h @@ -9,6 +9,7 @@ #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/geometry/dl_path.h" #include "flutter/display_list/image/dl_image.h" #include "third_party/skia/include/core/SkM44.h" @@ -114,6 +115,11 @@ class DlCanvas { virtual void ClipPath(const SkPath& path, ClipOp clip_op = ClipOp::kIntersect, bool is_aa = false) = 0; + virtual void ClipPath(const DlPath& path, + ClipOp clip_op = ClipOp::kIntersect, + bool is_aa = false) { + ClipPath(path.GetSkPath(), clip_op, is_aa); + } /// Conservative estimate of the bounds of all outstanding clip operations /// measured in the coordinate space within which this DisplayList will @@ -151,6 +157,9 @@ class DlCanvas { const SkRRect& inner, const DlPaint& paint) = 0; virtual void DrawPath(const SkPath& path, const DlPaint& paint) = 0; + virtual void DrawPath(const DlPath& path, const DlPaint& paint) { + DrawPath(path.GetSkPath(), paint); + } virtual void DrawArc(const SkRect& bounds, DlScalar start, DlScalar sweep, @@ -222,6 +231,13 @@ class DlCanvas { const DlScalar elevation, bool transparent_occluder, DlScalar dpr) = 0; + virtual void DrawShadow(const DlPath& path, + const DlColor color, + const DlScalar elevation, + bool transparent_occluder, + DlScalar dpr) { + DrawShadow(path.GetSkPath(), color, elevation, transparent_occluder, dpr); + } virtual void Flush() = 0; diff --git a/display_list/dl_op_receiver.h b/display_list/dl_op_receiver.h index 6f0ebae917907..091ed6f655ec6 100644 --- a/display_list/dl_op_receiver.h +++ b/display_list/dl_op_receiver.h @@ -96,46 +96,6 @@ class DlOpReceiver { // MaxDrawPointsCount * sizeof(DlPoint) must be less than 1 << 32 static constexpr int kMaxDrawPointsCount = ((1 << 29) - 1); - // --------------------------------------------------------------------- - // The CacheablePath forms of the drawPath, clipPath, and drawShadow - // methods are only called if the DlOpReceiver indicates that it prefers - // impeller paths by returning true from |PrefersImpellerPaths|. - // Note that we pass in both the SkPath and (a place to cache the) - // impeller::Path forms of the path since the SkPath version can contain - // information about the type of path that lets the receiver optimize - // the operation (and potentially avoid the need to cache it). - // It is up to the receiver to convert the path to Impeller form and - // cache it to avoid needing to do a lot of Impeller-specific processing - // inside the DisplayList code. - - virtual bool PrefersImpellerPaths() const { return false; } - - struct CacheablePath { - explicit CacheablePath(const SkPath& path) : sk_path(path) {} - - const SkPath sk_path; - mutable impeller::Path cached_impeller_path; - - bool operator==(const CacheablePath& other) const { - return sk_path == other.sk_path; - } - }; - - virtual void clipPath(const CacheablePath& cache, - ClipOp clip_op, - bool is_aa) { - FML_UNREACHABLE(); - } - virtual void drawPath(const CacheablePath& cache) { FML_UNREACHABLE(); } - virtual void drawShadow(const CacheablePath& cache, - const DlColor color, - const DlScalar elevation, - bool transparent_occluder, - DlScalar dpr) { - FML_UNREACHABLE(); - } - // --------------------------------------------------------------------- - // The following methods are nearly 1:1 with the methods on DlPaint and // carry the same meanings. Each method sets a persistent value for the // attribute for the rest of the display list or until it is reset by @@ -330,7 +290,7 @@ class DlOpReceiver { virtual void clipRect(const DlRect& rect, ClipOp clip_op, bool is_aa) = 0; virtual void clipOval(const DlRect& bounds, ClipOp clip_op, bool is_aa) = 0; virtual void clipRRect(const SkRRect& rrect, ClipOp clip_op, bool is_aa) = 0; - virtual void clipPath(const SkPath& path, ClipOp clip_op, bool is_aa) = 0; + virtual void clipPath(const DlPath& path, ClipOp clip_op, bool is_aa) = 0; // The following rendering methods all take their rendering attributes // from the last value set by the attribute methods above (regardless @@ -351,7 +311,7 @@ class DlOpReceiver { virtual void drawCircle(const DlPoint& center, DlScalar radius) = 0; virtual void drawRRect(const SkRRect& rrect) = 0; virtual void drawDRRect(const SkRRect& outer, const SkRRect& inner) = 0; - virtual void drawPath(const SkPath& path) = 0; + virtual void drawPath(const DlPath& path) = 0; virtual void drawArc(const DlRect& oval_bounds, DlScalar start_degrees, DlScalar sweep_degrees, @@ -395,7 +355,7 @@ class DlOpReceiver { const std::shared_ptr& text_frame, DlScalar x, DlScalar y) = 0; - virtual void drawShadow(const SkPath& path, + virtual void drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/display_list/dl_op_records.h b/display_list/dl_op_records.h index 9bc5385195543..f14025b48b5b0 100644 --- a/display_list/dl_op_records.h +++ b/display_list/dl_op_records.h @@ -510,30 +510,25 @@ DEFINE_CLIP_SHAPE_OP(Oval, DlRect, Difference) DEFINE_CLIP_SHAPE_OP(RRect, SkRRect, Difference) #undef DEFINE_CLIP_SHAPE_OP -#define DEFINE_CLIP_PATH_OP(clipop) \ - struct Clip##clipop##PathOp final : TransformClipOpBase { \ - static constexpr auto kType = DisplayListOpType::kClip##clipop##Path; \ - \ - Clip##clipop##PathOp(const SkPath& path, bool is_aa) \ - : is_aa(is_aa), cached_path(path) {} \ - \ - const bool is_aa; \ - const DlOpReceiver::CacheablePath cached_path; \ - \ - void dispatch(DlOpReceiver& receiver) const { \ - if (receiver.PrefersImpellerPaths()) { \ - receiver.clipPath(cached_path, DlCanvas::ClipOp::k##clipop, is_aa); \ - } else { \ - receiver.clipPath(cached_path.sk_path, DlCanvas::ClipOp::k##clipop, \ - is_aa); \ - } \ - } \ - \ - DisplayListCompare equals(const Clip##clipop##PathOp* other) const { \ - return is_aa == other->is_aa && cached_path == other->cached_path \ - ? DisplayListCompare::kEqual \ - : DisplayListCompare::kNotEqual; \ - } \ +#define DEFINE_CLIP_PATH_OP(clipop) \ + struct Clip##clipop##PathOp final : TransformClipOpBase { \ + static constexpr auto kType = DisplayListOpType::kClip##clipop##Path; \ + \ + Clip##clipop##PathOp(const DlPath& path, bool is_aa) \ + : is_aa(is_aa), path(path) {} \ + \ + const bool is_aa; \ + const DlPath path; \ + \ + void dispatch(DlOpReceiver& receiver) const { \ + receiver.clipPath(path, DlCanvas::ClipOp::k##clipop, is_aa); \ + } \ + \ + DisplayListCompare equals(const Clip##clipop##PathOp* other) const { \ + return is_aa == other->is_aa && path == other->path \ + ? DisplayListCompare::kEqual \ + : DisplayListCompare::kNotEqual; \ + } \ }; DEFINE_CLIP_PATH_OP(Intersect) DEFINE_CLIP_PATH_OP(Difference) @@ -596,21 +591,17 @@ DEFINE_DRAW_1ARG_OP(RRect, SkRRect, rrect) struct DrawPathOp final : DrawOpBase { static constexpr auto kType = DisplayListOpType::kDrawPath; - explicit DrawPathOp(const SkPath& path) : cached_path(path) {} + explicit DrawPathOp(const DlPath& path) : path(path) {} - const DlOpReceiver::CacheablePath cached_path; + const DlPath path; - void dispatch(DlOpReceiver& receiver) const { - if (receiver.PrefersImpellerPaths()) { - receiver.drawPath(cached_path); - } else { - receiver.drawPath(cached_path.sk_path); - } + void dispatch(DlOpReceiver& receiver) const { // + receiver.drawPath(path); } DisplayListCompare equals(const DrawPathOp* other) const { - return cached_path == other->cached_path ? DisplayListCompare::kEqual - : DisplayListCompare::kNotEqual; + return path == other->path ? DisplayListCompare::kEqual + : DisplayListCompare::kNotEqual; } }; @@ -1005,37 +996,31 @@ struct DrawTextFrameOp final : DrawOpBase { }; // 4 byte header + 140 byte payload packs evenly into 140 bytes -#define DEFINE_DRAW_SHADOW_OP(name, transparent_occluder) \ - struct Draw##name##Op final : DrawOpBase { \ - static constexpr auto kType = DisplayListOpType::kDraw##name; \ - \ - Draw##name##Op(const SkPath& path, \ - DlColor color, \ - DlScalar elevation, \ - DlScalar dpr) \ - : color(color), elevation(elevation), dpr(dpr), cached_path(path) {} \ - \ - const DlColor color; \ - const DlScalar elevation; \ - const DlScalar dpr; \ - const DlOpReceiver::CacheablePath cached_path; \ - \ - void dispatch(DlOpReceiver& receiver) const { \ - if (receiver.PrefersImpellerPaths()) { \ - receiver.drawShadow(cached_path, color, elevation, \ - transparent_occluder, dpr); \ - } else { \ - receiver.drawShadow(cached_path.sk_path, color, elevation, \ - transparent_occluder, dpr); \ - } \ - } \ - \ - DisplayListCompare equals(const Draw##name##Op* other) const { \ - return color == other->color && elevation == other->elevation && \ - dpr == other->dpr && cached_path == other->cached_path \ - ? DisplayListCompare::kEqual \ - : DisplayListCompare::kNotEqual; \ - } \ +#define DEFINE_DRAW_SHADOW_OP(name, transparent_occluder) \ + struct Draw##name##Op final : DrawOpBase { \ + static constexpr auto kType = DisplayListOpType::kDraw##name; \ + \ + Draw##name##Op(const DlPath& path, \ + DlColor color, \ + DlScalar elevation, \ + DlScalar dpr) \ + : color(color), elevation(elevation), dpr(dpr), path(path) {} \ + \ + const DlColor color; \ + const DlScalar elevation; \ + const DlScalar dpr; \ + const DlPath path; \ + \ + void dispatch(DlOpReceiver& receiver) const { \ + receiver.drawShadow(path, color, elevation, transparent_occluder, dpr); \ + } \ + \ + DisplayListCompare equals(const Draw##name##Op* other) const { \ + return color == other->color && elevation == other->elevation && \ + dpr == other->dpr && path == other->path \ + ? DisplayListCompare::kEqual \ + : DisplayListCompare::kNotEqual; \ + } \ }; DEFINE_DRAW_SHADOW_OP(Shadow, false) DEFINE_DRAW_SHADOW_OP(ShadowTransparentOccluder, true) diff --git a/display_list/geometry/dl_geometry_types.h b/display_list/geometry/dl_geometry_types.h index d50cb367e4f1e..0825ebe0bc02f 100644 --- a/display_list/geometry/dl_geometry_types.h +++ b/display_list/geometry/dl_geometry_types.h @@ -51,6 +51,10 @@ inline const DlIRect& ToDlIRect(const SkIRect& rect) { return *reinterpret_cast(&rect); } +inline DlRect* ToDlRect(SkRect* rect) { + return rect == nullptr ? nullptr : reinterpret_cast(rect); +} + inline const DlRect* ToDlRect(const SkRect* rect) { return rect == nullptr ? nullptr : reinterpret_cast(rect); } @@ -100,6 +104,10 @@ inline const SkRect* ToSkRect(const DlRect* rect) { return rect == nullptr ? nullptr : reinterpret_cast(rect); } +inline SkRect* ToSkRect(DlRect* rect) { + return rect == nullptr ? nullptr : reinterpret_cast(rect); +} + inline const SkRect* ToSkRects(const DlRect* rects) { return rects == nullptr ? nullptr : reinterpret_cast(rects); } diff --git a/display_list/geometry/dl_path.cc b/display_list/geometry/dl_path.cc new file mode 100644 index 0000000000000..768c958e1123f --- /dev/null +++ b/display_list/geometry/dl_path.cc @@ -0,0 +1,149 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/display_list/geometry/dl_path.h" + +#include "flutter/display_list/geometry/dl_geometry_types.h" +#include "flutter/impeller/geometry/path_builder.h" + +namespace flutter { + +const SkPath& DlPath::GetSkPath() const { + return sk_path_; +} + +impeller::Path DlPath::GetPath() const { + if (path_.IsEmpty() && !sk_path_.isEmpty()) { + path_ = ConvertToImpellerPath(sk_path_); + } + + return path_; +} + +bool DlPath::IsInverseFillType() const { + return sk_path_.isInverseFillType(); +} + +bool DlPath::IsRect(DlRect* rect, bool* is_closed) const { + return sk_path_.isRect(ToSkRect(rect), is_closed); +} + +bool DlPath::IsOval(DlRect* bounds) const { + return sk_path_.isOval(ToSkRect(bounds)); +} + +bool DlPath::IsSkRect(SkRect* rect, bool* is_closed) const { + return sk_path_.isRect(rect, is_closed); +} + +bool DlPath::IsSkOval(SkRect* bounds) const { + return sk_path_.isOval(bounds); +} + +bool DlPath::IsSkRRect(SkRRect* rrect) const { + return sk_path_.isRRect(rrect); +} + +SkRect DlPath::GetSkBounds() const { + return sk_path_.getBounds(); +} + +DlRect DlPath::GetBounds() const { + return ToDlRect(sk_path_.getBounds()); +} + +bool DlPath::operator==(const DlPath& other) const { + return sk_path_ == other.sk_path_; +} + +using Path = impeller::Path; +using PathBuilder = impeller::PathBuilder; +using FillType = impeller::FillType; +using Convexity = impeller::Convexity; + +Path DlPath::ConvertToImpellerPath(const SkPath& path, const DlPoint& shift) { + auto iterator = SkPath::Iter(path, false); + + struct PathData { + union { + SkPoint points[4]; + }; + }; + + PathBuilder builder; + PathData data; + // Reserve a path size with some arbitrarily additional padding. + builder.Reserve(path.countPoints() + 8, path.countVerbs() + 8); + auto verb = SkPath::Verb::kDone_Verb; + do { + verb = iterator.next(data.points); + switch (verb) { + case SkPath::kMove_Verb: + builder.MoveTo(ToDlPoint(data.points[0])); + break; + case SkPath::kLine_Verb: + builder.LineTo(ToDlPoint(data.points[1])); + break; + case SkPath::kQuad_Verb: + builder.QuadraticCurveTo(ToDlPoint(data.points[1]), + ToDlPoint(data.points[2])); + break; + case SkPath::kConic_Verb: { + constexpr auto kPow2 = 1; // Only works for sweeps up to 90 degrees. + constexpr auto kQuadCount = 1 + (2 * (1 << kPow2)); + SkPoint points[kQuadCount]; + const auto curve_count = + SkPath::ConvertConicToQuads(data.points[0], // + data.points[1], // + data.points[2], // + iterator.conicWeight(), // + points, // + kPow2 // + ); + + for (int curve_index = 0, point_index = 0; // + curve_index < curve_count; // + curve_index++, point_index += 2 // + ) { + builder.QuadraticCurveTo(ToDlPoint(points[point_index + 1]), + ToDlPoint(points[point_index + 2])); + } + } break; + case SkPath::kCubic_Verb: + builder.CubicCurveTo(ToDlPoint(data.points[1]), + ToDlPoint(data.points[2]), + ToDlPoint(data.points[3])); + break; + case SkPath::kClose_Verb: + builder.Close(); + break; + case SkPath::kDone_Verb: + break; + } + } while (verb != SkPath::Verb::kDone_Verb); + + FillType fill_type; + switch (path.getFillType()) { + case SkPathFillType::kWinding: + fill_type = FillType::kNonZero; + break; + case SkPathFillType::kEvenOdd: + fill_type = FillType::kOdd; + break; + case SkPathFillType::kInverseWinding: + case SkPathFillType::kInverseEvenOdd: + // Flutter doesn't expose these path fill types. These are only visible + // via the receiver interface. We should never get here. + fill_type = FillType::kNonZero; + break; + } + builder.SetConvexity(path.isConvex() ? Convexity::kConvex + : Convexity::kUnknown); + builder.Shift(shift); + auto sk_bounds = path.getBounds().makeOutset(shift.x, shift.y); + builder.SetBounds(ToDlRect(sk_bounds)); + return builder.TakePath(fill_type); +} + +} // namespace flutter diff --git a/display_list/geometry/dl_path.h b/display_list/geometry/dl_path.h new file mode 100644 index 0000000000000..8232e423ba6d6 --- /dev/null +++ b/display_list/geometry/dl_path.h @@ -0,0 +1,49 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_DISPLAY_LIST_GEOMETRY_DL_PATH_H_ +#define FLUTTER_DISPLAY_LIST_GEOMETRY_DL_PATH_H_ + +#include "flutter/display_list/geometry/dl_geometry_types.h" +#include "flutter/impeller/geometry/path.h" +#include "flutter/third_party/skia/include/core/SkPath.h" + +namespace flutter { + +class DlPath { + public: + DlPath() = default; + explicit DlPath(const SkPath& path) : sk_path_(path) {} + + DlPath(const DlPath& path) = default; + DlPath(DlPath&& path) = default; + + const SkPath& GetSkPath() const; + impeller::Path GetPath() const; + + bool IsInverseFillType() const; + + bool IsRect(DlRect* rect, bool* is_closed = nullptr) const; + bool IsOval(DlRect* bounds) const; + + bool IsSkRect(SkRect* rect, bool* is_closed = nullptr) const; + bool IsSkOval(SkRect* bounds) const; + bool IsSkRRect(SkRRect* rrect) const; + + SkRect GetSkBounds() const; + DlRect GetBounds() const; + + bool operator==(const DlPath& other) const; + + private: + const SkPath sk_path_; + mutable impeller::Path path_; + + static impeller::Path ConvertToImpellerPath(const SkPath& path, + const DlPoint& shift = DlPoint()); +}; + +} // namespace flutter + +#endif // FLUTTER_DISPLAY_LIST_GEOMETRY_DL_PATH_H_ diff --git a/display_list/geometry/dl_path_unittests.cc b/display_list/geometry/dl_path_unittests.cc new file mode 100644 index 0000000000000..f3b671e793f7a --- /dev/null +++ b/display_list/geometry/dl_path_unittests.cc @@ -0,0 +1,171 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/display_list/geometry/dl_path.h" +#include "gtest/gtest.h" + +#include "flutter/third_party/skia/include/core/SkRRect.h" + +namespace flutter { +namespace testing { + +TEST(DisplayListPath, DefaultConstruction) { + DlPath path; + + EXPECT_EQ(path, DlPath()); + EXPECT_EQ(path.GetSkPath(), SkPath()); + + EXPECT_FALSE(path.IsInverseFillType()); + + bool is_closed = false; + EXPECT_FALSE(path.IsRect(nullptr)); + EXPECT_FALSE(path.IsRect(nullptr, &is_closed)); + EXPECT_FALSE(is_closed); + EXPECT_FALSE(path.IsOval(nullptr)); + + is_closed = false; + EXPECT_FALSE(path.IsSkRect(nullptr)); + EXPECT_FALSE(path.IsSkRect(nullptr, &is_closed)); + EXPECT_FALSE(is_closed); + EXPECT_FALSE(path.IsSkOval(nullptr)); + EXPECT_FALSE(path.IsSkRRect(nullptr)); + + EXPECT_EQ(path.GetBounds(), DlRect()); + EXPECT_EQ(path.GetSkBounds(), SkRect()); +} + +TEST(DisplayListPath, ConstructFromEmpty) { + SkPath sk_path; + DlPath path(sk_path); + + EXPECT_EQ(path, DlPath()); + EXPECT_EQ(path.GetSkPath(), SkPath()); + + bool is_closed = false; + EXPECT_FALSE(path.IsRect(nullptr)); + EXPECT_FALSE(path.IsRect(nullptr, &is_closed)); + EXPECT_FALSE(is_closed); + EXPECT_FALSE(path.IsOval(nullptr)); + + is_closed = false; + EXPECT_FALSE(path.IsSkRect(nullptr)); + EXPECT_FALSE(path.IsSkRect(nullptr, &is_closed)); + EXPECT_FALSE(is_closed); + EXPECT_FALSE(path.IsSkOval(nullptr)); + EXPECT_FALSE(path.IsSkRRect(nullptr)); + + EXPECT_EQ(path.GetBounds(), DlRect()); + EXPECT_EQ(path.GetSkBounds(), SkRect()); +} + +TEST(DisplayListPath, ConstructFromRect) { + SkPath sk_path = SkPath::Rect(SkRect::MakeLTRB(10, 10, 20, 20)); + DlPath path(sk_path); + + EXPECT_EQ(path, DlPath(SkPath::Rect(SkRect::MakeLTRB(10, 10, 20, 20)))); + EXPECT_EQ(path.GetSkPath(), SkPath::Rect(SkRect::MakeLTRB(10, 10, 20, 20))); + + EXPECT_FALSE(path.IsInverseFillType()); + + bool is_closed = false; + EXPECT_TRUE(path.IsRect(nullptr)); + DlRect dl_rect; + EXPECT_TRUE(path.IsRect(&dl_rect, &is_closed)); + EXPECT_EQ(dl_rect, DlRect::MakeLTRB(10, 10, 20, 20)); + EXPECT_TRUE(is_closed); + EXPECT_FALSE(path.IsOval(nullptr)); + + is_closed = false; + EXPECT_TRUE(path.IsSkRect(nullptr)); + SkRect sk_rect; + EXPECT_TRUE(path.IsSkRect(&sk_rect, &is_closed)); + EXPECT_EQ(sk_rect, SkRect::MakeLTRB(10, 10, 20, 20)); + EXPECT_TRUE(is_closed); + EXPECT_FALSE(path.IsSkOval(nullptr)); + EXPECT_FALSE(path.IsSkRRect(nullptr)); + + EXPECT_EQ(path.GetBounds(), DlRect::MakeLTRB(10, 10, 20, 20)); + EXPECT_EQ(path.GetSkBounds(), SkRect::MakeLTRB(10, 10, 20, 20)); +} + +TEST(DisplayListPath, ConstructFromOval) { + SkPath sk_path = SkPath::Oval(SkRect::MakeLTRB(10, 10, 20, 20)); + DlPath path(sk_path); + + EXPECT_EQ(path, DlPath(SkPath::Oval(SkRect::MakeLTRB(10, 10, 20, 20)))); + EXPECT_EQ(path.GetSkPath(), SkPath::Oval(SkRect::MakeLTRB(10, 10, 20, 20))); + + EXPECT_FALSE(path.IsInverseFillType()); + EXPECT_FALSE(path.IsRect(nullptr)); + EXPECT_TRUE(path.IsOval(nullptr)); + DlRect dl_bounds; + EXPECT_TRUE(path.IsOval(&dl_bounds)); + EXPECT_EQ(dl_bounds, DlRect::MakeLTRB(10, 10, 20, 20)); + + EXPECT_FALSE(path.IsSkRect(nullptr)); + EXPECT_TRUE(path.IsSkOval(nullptr)); + SkRect sk_bounds; + EXPECT_TRUE(path.IsSkOval(&sk_bounds)); + EXPECT_EQ(sk_bounds, SkRect::MakeLTRB(10, 10, 20, 20)); + EXPECT_FALSE(path.IsSkRRect(nullptr)); + + EXPECT_EQ(path.GetBounds(), DlRect::MakeLTRB(10, 10, 20, 20)); + EXPECT_EQ(path.GetSkBounds(), SkRect::MakeLTRB(10, 10, 20, 20)); +} + +TEST(DisplayListPath, ConstructFromRRect) { + SkPath sk_path = SkPath::RRect(SkRect::MakeLTRB(10, 10, 20, 20), 1, 2); + DlPath path(sk_path); + + EXPECT_EQ(path, + DlPath(SkPath::RRect(SkRect::MakeLTRB(10, 10, 20, 20), 1, 2))); + EXPECT_EQ(path.GetSkPath(), + SkPath::RRect(SkRect::MakeLTRB(10, 10, 20, 20), 1, 2)); + + EXPECT_FALSE(path.IsInverseFillType()); + + EXPECT_FALSE(path.IsRect(nullptr)); + EXPECT_FALSE(path.IsOval(nullptr)); + + EXPECT_FALSE(path.IsSkRect(nullptr)); + EXPECT_FALSE(path.IsSkOval(nullptr)); + EXPECT_TRUE(path.IsSkRRect(nullptr)); + SkRRect rrect2; + EXPECT_TRUE(path.IsSkRRect(&rrect2)); + EXPECT_EQ(rrect2, + SkRRect::MakeRectXY(SkRect::MakeLTRB(10, 10, 20, 20), 1, 2)); + + EXPECT_EQ(path.GetBounds(), DlRect::MakeLTRB(10, 10, 20, 20)); + EXPECT_EQ(path.GetSkBounds(), SkRect::MakeLTRB(10, 10, 20, 20)); +} + +TEST(DisplayListPath, ConstructFromPath) { + SkPath sk_path1; + sk_path1.moveTo(10, 10); + sk_path1.lineTo(20, 20); + sk_path1.lineTo(20, 10); + SkPath sk_path2; + sk_path2.moveTo(10, 10); + sk_path2.lineTo(20, 20); + sk_path2.lineTo(20, 10); + DlPath path(sk_path1); + + ASSERT_EQ(sk_path1, sk_path2); + + EXPECT_EQ(path, DlPath(sk_path2)); + EXPECT_EQ(path.GetSkPath(), sk_path2); + + EXPECT_FALSE(path.IsInverseFillType()); + EXPECT_FALSE(path.IsRect(nullptr)); + EXPECT_FALSE(path.IsOval(nullptr)); + EXPECT_FALSE(path.IsSkRect(nullptr)); + EXPECT_FALSE(path.IsSkOval(nullptr)); + EXPECT_FALSE(path.IsSkRRect(nullptr)); + + EXPECT_EQ(path.GetBounds(), DlRect::MakeLTRB(10, 10, 20, 20)); + EXPECT_EQ(path.GetSkBounds(), SkRect::MakeLTRB(10, 10, 20, 20)); +} + +} // namespace testing +} // namespace flutter diff --git a/display_list/skia/dl_sk_dispatcher.cc b/display_list/skia/dl_sk_dispatcher.cc index c4edaad31ea1a..8a47d8864365e 100644 --- a/display_list/skia/dl_sk_dispatcher.cc +++ b/display_list/skia/dl_sk_dispatcher.cc @@ -133,10 +133,10 @@ void DlSkCanvasDispatcher::clipRRect(const SkRRect& rrect, bool is_aa) { canvas_->clipRRect(rrect, ToSk(clip_op), is_aa); } -void DlSkCanvasDispatcher::clipPath(const SkPath& path, +void DlSkCanvasDispatcher::clipPath(const DlPath& path, ClipOp clip_op, bool is_aa) { - canvas_->clipPath(path, ToSk(clip_op), is_aa); + canvas_->clipPath(path.GetSkPath(), ToSk(clip_op), is_aa); } void DlSkCanvasDispatcher::drawPaint() { @@ -184,8 +184,8 @@ void DlSkCanvasDispatcher::drawDRRect(const SkRRect& outer, const SkRRect& inner) { canvas_->drawDRRect(outer, inner, paint()); } -void DlSkCanvasDispatcher::drawPath(const SkPath& path) { - canvas_->drawPath(path, paint()); +void DlSkCanvasDispatcher::drawPath(const DlPath& path) { + canvas_->drawPath(path.GetSkPath(), paint()); } void DlSkCanvasDispatcher::drawArc(const DlRect& bounds, DlScalar start, @@ -328,12 +328,13 @@ void DlSkCanvasDispatcher::DrawShadow(SkCanvas* canvas, ambient_color, spot_color, flags); } -void DlSkCanvasDispatcher::drawShadow(const SkPath& path, +void DlSkCanvasDispatcher::drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, DlScalar dpr) { - DrawShadow(canvas_, path, color, elevation, transparent_occluder, dpr); + DrawShadow(canvas_, path.GetSkPath(), color, elevation, transparent_occluder, + dpr); } } // namespace flutter diff --git a/display_list/skia/dl_sk_dispatcher.h b/display_list/skia/dl_sk_dispatcher.h index 918bfb4ff2a49..fa5fbdd21259b 100644 --- a/display_list/skia/dl_sk_dispatcher.h +++ b/display_list/skia/dl_sk_dispatcher.h @@ -53,7 +53,7 @@ class DlSkCanvasDispatcher : public virtual DlOpReceiver, void clipRect(const DlRect& rect, ClipOp clip_op, bool is_aa) override; void clipOval(const DlRect& bounds, ClipOp clip_op, bool is_aa) override; void clipRRect(const SkRRect& rrect, ClipOp clip_op, bool is_aa) override; - void clipPath(const SkPath& path, ClipOp clip_op, bool is_aa) override; + void clipPath(const DlPath& path, ClipOp clip_op, bool is_aa) override; void drawPaint() override; void drawColor(DlColor color, DlBlendMode mode) override; @@ -67,7 +67,7 @@ class DlSkCanvasDispatcher : public virtual DlOpReceiver, void drawCircle(const DlPoint& center, DlScalar radius) override; void drawRRect(const SkRRect& rrect) override; void drawDRRect(const SkRRect& outer, const SkRRect& inner) override; - void drawPath(const SkPath& path) override; + void drawPath(const DlPath& path) override; void drawArc(const DlRect& bounds, DlScalar start, DlScalar sweep, @@ -107,7 +107,7 @@ class DlSkCanvasDispatcher : public virtual DlOpReceiver, void drawTextFrame(const std::shared_ptr& text_frame, DlScalar x, DlScalar y) override; - void drawShadow(const SkPath& path, + void drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/display_list/testing/dl_test_snippets.h b/display_list/testing/dl_test_snippets.h index 2647a6bab37b3..1057990baaa33 100644 --- a/display_list/testing/dl_test_snippets.h +++ b/display_list/testing/dl_test_snippets.h @@ -182,15 +182,15 @@ static const SkRRect kTestRRect = SkRRect::MakeRectXY(kTestSkBounds, 5, 5); static const SkRRect kTestRRectRect = SkRRect::MakeRect(kTestSkBounds); static const SkRRect kTestInnerRRect = SkRRect::MakeRectXY(kTestSkBounds.makeInset(5, 5), 2, 2); -static const SkPath kTestPathRect = SkPath::Rect(kTestSkBounds); -static const SkPath kTestPathOval = SkPath::Oval(kTestSkBounds); -static const SkPath kTestPathRRect = SkPath::RRect(kTestRRect); -static const SkPath kTestPath1 = - SkPath::Polygon({{0, 0}, {10, 10}, {10, 0}, {0, 10}}, true); -static const SkPath kTestPath2 = - SkPath::Polygon({{0, 0}, {10, 10}, {0, 10}, {10, 0}}, true); -static const SkPath kTestPath3 = - SkPath::Polygon({{0, 0}, {10, 10}, {10, 0}, {0, 10}}, false); +static const DlPath kTestPathRect = DlPath(SkPath::Rect(kTestSkBounds)); +static const DlPath kTestPathOval = DlPath(SkPath::Oval(kTestSkBounds)); +static const DlPath kTestPathRRect = DlPath(SkPath::RRect(kTestRRect)); +static const DlPath kTestPath1 = + DlPath(SkPath::Polygon({{0, 0}, {10, 10}, {10, 0}, {0, 10}}, true)); +static const DlPath kTestPath2 = + DlPath(SkPath::Polygon({{0, 0}, {10, 10}, {0, 10}, {10, 0}}, true)); +static const DlPath kTestPath3 = + DlPath(SkPath::Polygon({{0, 0}, {10, 10}, {10, 0}, {0, 10}}, false)); static const SkMatrix kTestMatrix1 = SkMatrix::Scale(2, 2); static const SkMatrix kTestMatrix2 = SkMatrix::RotateDeg(45); diff --git a/display_list/utils/dl_receiver_utils.h b/display_list/utils/dl_receiver_utils.h index 397e4015d1857..257d276657b86 100644 --- a/display_list/utils/dl_receiver_utils.h +++ b/display_list/utils/dl_receiver_utils.h @@ -50,7 +50,7 @@ class IgnoreClipDispatchHelper : public virtual DlOpReceiver { void clipRRect(const SkRRect& rrect, DlCanvas::ClipOp clip_op, bool is_aa) override {} - void clipPath(const SkPath& path, + void clipPath(const DlPath& path, DlCanvas::ClipOp clip_op, bool is_aa) override {} }; @@ -96,7 +96,7 @@ class IgnoreDrawDispatchHelper : public virtual DlOpReceiver { void drawCircle(const DlPoint& center, DlScalar radius) override {} void drawRRect(const SkRRect& rrect) override {} void drawDRRect(const SkRRect& outer, const SkRRect& inner) override {} - void drawPath(const SkPath& path) override {} + void drawPath(const DlPath& path) override {} void drawArc(const DlRect& oval_bounds, DlScalar start_degrees, DlScalar sweep_degrees, @@ -138,7 +138,7 @@ class IgnoreDrawDispatchHelper : public virtual DlOpReceiver { void drawTextFrame(const std::shared_ptr& text_frame, DlScalar x, DlScalar y) override {} - void drawShadow(const SkPath& path, + void drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index 37c3daf3e3d67..75a6c11845e03 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/impeller/display_list/dl_dispatcher.cc @@ -880,38 +880,24 @@ void DlDispatcherBase::clipRRect(const SkRRect& rrect, } // |flutter::DlOpReceiver| -void DlDispatcherBase::clipPath(const SkPath& path, ClipOp sk_op, bool is_aa) { - UNIMPLEMENTED; -} - -const Path& DlDispatcherBase::GetOrCachePath(const CacheablePath& cache) { - if (cache.cached_impeller_path.IsEmpty() && !cache.sk_path.isEmpty()) { - cache.cached_impeller_path = skia_conversions::ToPath(cache.sk_path); - } - return cache.cached_impeller_path; -} - -// |flutter::DlOpReceiver| -void DlDispatcherBase::clipPath(const CacheablePath& cache, - ClipOp sk_op, - bool is_aa) { +void DlDispatcherBase::clipPath(const DlPath& path, ClipOp sk_op, bool is_aa) { AUTO_DEPTH_WATCHER(0u); auto clip_op = ToClipOperation(sk_op); - SkRect rect; - if (cache.sk_path.isRect(&rect)) { - GetCanvas().ClipRect(skia_conversions::ToRect(rect), clip_op); - } else if (cache.sk_path.isOval(&rect)) { - GetCanvas().ClipOval(skia_conversions::ToRect(rect), clip_op); + DlRect rect; + if (path.IsRect(&rect)) { + GetCanvas().ClipRect(rect, clip_op); + } else if (path.IsOval(&rect)) { + GetCanvas().ClipOval(rect, clip_op); } else { SkRRect rrect; - if (cache.sk_path.isRRect(&rrect) && rrect.isSimple()) { + if (path.IsSkRRect(&rrect) && rrect.isSimple()) { GetCanvas().ClipRRect(skia_conversions::ToRect(rrect.rect()), skia_conversions::ToSize(rrect.getSimpleRadii()), clip_op); } else { - GetCanvas().ClipPath(GetOrCachePath(cache), clip_op); + GetCanvas().ClipPath(path.GetPath(), clip_op); } } } @@ -1027,43 +1013,37 @@ void DlDispatcherBase::drawDRRect(const SkRRect& outer, const SkRRect& inner) { } // |flutter::DlOpReceiver| -void DlDispatcherBase::drawPath(const SkPath& path) { - UNIMPLEMENTED; -} - -// |flutter::DlOpReceiver| -void DlDispatcherBase::drawPath(const CacheablePath& cache) { +void DlDispatcherBase::drawPath(const DlPath& path) { AUTO_DEPTH_WATCHER(1u); - SimplifyOrDrawPath(GetCanvas(), cache, paint_); + SimplifyOrDrawPath(GetCanvas(), path, paint_); } void DlDispatcherBase::SimplifyOrDrawPath(Canvas& canvas, - const CacheablePath& cache, + const DlPath& path, const Paint& paint) { - SkRect rect; + DlRect rect; // We can't "optimize" a path into a rectangle if it's open. bool closed; - if (cache.sk_path.isRect(&rect, &closed) && closed) { - canvas.DrawRect(skia_conversions::ToRect(rect), paint); + if (path.IsRect(&rect, &closed) && closed) { + canvas.DrawRect(rect, paint); return; } SkRRect rrect; - if (cache.sk_path.isRRect(&rrect) && rrect.isSimple()) { + if (path.IsSkRRect(&rrect) && rrect.isSimple()) { canvas.DrawRRect(skia_conversions::ToRect(rrect.rect()), skia_conversions::ToSize(rrect.getSimpleRadii()), paint); return; } - SkRect oval; - if (cache.sk_path.isOval(&oval)) { - canvas.DrawOval(skia_conversions::ToRect(oval), paint); + if (path.IsOval(&rect)) { + canvas.DrawOval(rect, paint); return; } - canvas.DrawPath(GetOrCachePath(cache), paint); + canvas.DrawPath(path.GetPath(), paint); } // |flutter::DlOpReceiver| @@ -1299,16 +1279,7 @@ void DlDispatcherBase::drawTextFrame( } // |flutter::DlOpReceiver| -void DlDispatcherBase::drawShadow(const SkPath& path, - const flutter::DlColor color, - const DlScalar elevation, - bool transparent_occluder, - DlScalar dpr) { - UNIMPLEMENTED; -} - -// |flutter::DlOpReceiver| -void DlDispatcherBase::drawShadow(const CacheablePath& cache, +void DlDispatcherBase::drawShadow(const DlPath& path, const flutter::DlColor color, const DlScalar elevation, bool transparent_occluder, @@ -1363,7 +1334,7 @@ void DlDispatcherBase::drawShadow(const CacheablePath& cache, GetCanvas().PreConcat( Matrix::MakeTranslation(Vector2(0, -occluder_z * light_position.y))); - SimplifyOrDrawPath(GetCanvas(), cache, paint); + SimplifyOrDrawPath(GetCanvas(), path, paint); AUTO_DEPTH_CHECK(); GetCanvas().Restore(); diff --git a/impeller/display_list/dl_dispatcher.h b/impeller/display_list/dl_dispatcher.h index d8e4bad38a377..4bda27218b9a9 100644 --- a/impeller/display_list/dl_dispatcher.h +++ b/impeller/display_list/dl_dispatcher.h @@ -7,6 +7,7 @@ #include "flutter/display_list/dl_op_receiver.h" #include "flutter/display_list/geometry/dl_geometry_types.h" +#include "flutter/display_list/geometry/dl_path.h" #include "flutter/display_list/utils/dl_receiver_utils.h" #include "fml/logging.h" #include "impeller/aiks/canvas.h" @@ -21,14 +22,12 @@ using DlScalar = flutter::DlScalar; using DlPoint = flutter::DlPoint; using DlRect = flutter::DlRect; using DlIRect = flutter::DlIRect; +using DlPath = flutter::DlPath; class DlDispatcherBase : public flutter::DlOpReceiver { public: Picture EndRecordingAsPicture(); - // |flutter::DlOpReceiver| - bool PrefersImpellerPaths() const override { return true; } - // |flutter::DlOpReceiver| void setAntiAlias(bool aa) override; @@ -132,12 +131,7 @@ class DlDispatcherBase : public flutter::DlOpReceiver { void clipRRect(const SkRRect& rrect, ClipOp clip_op, bool is_aa) override; // |flutter::DlOpReceiver| - void clipPath(const SkPath& path, ClipOp clip_op, bool is_aa) override; - - // |flutter::DlOpReceiver| - void clipPath(const CacheablePath& cache, - ClipOp clip_op, - bool is_aa) override; + void clipPath(const DlPath& path, ClipOp clip_op, bool is_aa) override; // |flutter::DlOpReceiver| void drawColor(flutter::DlColor color, flutter::DlBlendMode mode) override; @@ -170,10 +164,7 @@ class DlDispatcherBase : public flutter::DlOpReceiver { void drawDRRect(const SkRRect& outer, const SkRRect& inner) override; // |flutter::DlOpReceiver| - void drawPath(const SkPath& path) override; - - // |flutter::DlOpReceiver| - void drawPath(const CacheablePath& cache) override; + void drawPath(const DlPath& path) override; // |flutter::DlOpReceiver| void drawArc(const DlRect& oval_bounds, @@ -237,14 +228,7 @@ class DlDispatcherBase : public flutter::DlOpReceiver { DlScalar y) override; // |flutter::DlOpReceiver| - void drawShadow(const SkPath& path, - const flutter::DlColor color, - const DlScalar elevation, - bool transparent_occluder, - DlScalar dpr) override; - - // |flutter::DlOpReceiver| - void drawShadow(const CacheablePath& cache, + void drawShadow(const DlPath& path, const flutter::DlColor color, const DlScalar elevation, bool transparent_occluder, @@ -256,10 +240,8 @@ class DlDispatcherBase : public flutter::DlOpReceiver { Paint paint_; Matrix initial_matrix_; - static const Path& GetOrCachePath(const CacheablePath& cache); - static void SimplifyOrDrawPath(Canvas& canvas, - const CacheablePath& cache, + const DlPath& cache, const Paint& paint); }; diff --git a/impeller/display_list/skia_conversions.cc b/impeller/display_list/skia_conversions.cc index 5457f678da5e3..911814f399795 100644 --- a/impeller/display_list/skia_conversions.cc +++ b/impeller/display_list/skia_conversions.cc @@ -87,89 +87,6 @@ PathBuilder::RoundingRadii ToRoundingRadii(const SkRRect& rrect) { return radii; } -Path ToPath(const SkPath& path, Point shift) { - auto iterator = SkPath::Iter(path, false); - - struct PathData { - union { - SkPoint points[4]; - }; - }; - - PathBuilder builder; - PathData data; - // Reserve a path size with some arbitrarily additional padding. - builder.Reserve(path.countPoints() + 8, path.countVerbs() + 8); - auto verb = SkPath::Verb::kDone_Verb; - do { - verb = iterator.next(data.points); - switch (verb) { - case SkPath::kMove_Verb: - builder.MoveTo(ToPoint(data.points[0])); - break; - case SkPath::kLine_Verb: - builder.LineTo(ToPoint(data.points[1])); - break; - case SkPath::kQuad_Verb: - builder.QuadraticCurveTo(ToPoint(data.points[1]), - ToPoint(data.points[2])); - break; - case SkPath::kConic_Verb: { - constexpr auto kPow2 = 1; // Only works for sweeps up to 90 degrees. - constexpr auto kQuadCount = 1 + (2 * (1 << kPow2)); - SkPoint points[kQuadCount]; - const auto curve_count = - SkPath::ConvertConicToQuads(data.points[0], // - data.points[1], // - data.points[2], // - iterator.conicWeight(), // - points, // - kPow2 // - ); - - for (int curve_index = 0, point_index = 0; // - curve_index < curve_count; // - curve_index++, point_index += 2 // - ) { - builder.QuadraticCurveTo(ToPoint(points[point_index + 1]), - ToPoint(points[point_index + 2])); - } - } break; - case SkPath::kCubic_Verb: - builder.CubicCurveTo(ToPoint(data.points[1]), ToPoint(data.points[2]), - ToPoint(data.points[3])); - break; - case SkPath::kClose_Verb: - builder.Close(); - break; - case SkPath::kDone_Verb: - break; - } - } while (verb != SkPath::Verb::kDone_Verb); - - FillType fill_type; - switch (path.getFillType()) { - case SkPathFillType::kWinding: - fill_type = FillType::kNonZero; - break; - case SkPathFillType::kEvenOdd: - fill_type = FillType::kOdd; - break; - case SkPathFillType::kInverseWinding: - case SkPathFillType::kInverseEvenOdd: - // Flutter doesn't expose these path fill types. These are only visible - // via the receiver interface. We should never get here. - fill_type = FillType::kNonZero; - break; - } - builder.SetConvexity(path.isConvex() ? Convexity::kConvex - : Convexity::kUnknown); - builder.Shift(shift); - auto sk_bounds = path.getBounds().makeOutset(shift.x, shift.y); - builder.SetBounds(ToRect(sk_bounds)); - return builder.TakePath(fill_type); -} - Path ToPath(const SkRRect& rrect) { return PathBuilder{} .AddRoundedRect(ToRect(rrect.getBounds()), ToRoundingRadii(rrect)) @@ -214,14 +131,6 @@ std::vector ToRSXForms(const SkRSXform xform[], int count) { return result; } -Path PathDataFromTextBlob(const sk_sp& blob, Point shift) { - if (!blob) { - return {}; - } - - return ToPath(skia::textlayout::Paragraph::GetPath(blob.get()), shift); -} - std::optional ToPixelFormat(SkColorType type) { switch (type) { case kRGBA_8888_SkColorType: diff --git a/impeller/display_list/skia_conversions.h b/impeller/display_list/skia_conversions.h index edc9aec391309..fa0c9f6917b1a 100644 --- a/impeller/display_list/skia_conversions.h +++ b/impeller/display_list/skia_conversions.h @@ -52,13 +52,8 @@ std::vector ToRSXForms(const SkRSXform xform[], int count); PathBuilder::RoundingRadii ToRoundingRadii(const SkRRect& rrect); -Path ToPath(const SkPath& path, Point shift = Point(0, 0)); - Path ToPath(const SkRRect& rrect); -Path PathDataFromTextBlob(const sk_sp& blob, - Point shift = Point(0, 0)); - std::optional ToPixelFormat(SkColorType type); /// @brief Convert display list colors + stops into impeller colors and stops, diff --git a/shell/common/dl_op_spy.cc b/shell/common/dl_op_spy.cc index b386344952faf..19f54d104768f 100644 --- a/shell/common/dl_op_spy.cc +++ b/shell/common/dl_op_spy.cc @@ -65,7 +65,7 @@ void DlOpSpy::drawRRect(const SkRRect& rrect) { void DlOpSpy::drawDRRect(const SkRRect& outer, const SkRRect& inner) { did_draw_ |= will_draw_; } -void DlOpSpy::drawPath(const SkPath& path) { +void DlOpSpy::drawPath(const DlPath& path) { did_draw_ |= will_draw_; } void DlOpSpy::drawArc(const DlRect& oval_bounds, @@ -142,7 +142,7 @@ void DlOpSpy::drawTextFrame( did_draw_ |= will_draw_; } -void DlOpSpy::drawShadow(const SkPath& path, +void DlOpSpy::drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/shell/common/dl_op_spy.h b/shell/common/dl_op_spy.h index f51297ce7cf38..6d3d9c9587f24 100644 --- a/shell/common/dl_op_spy.h +++ b/shell/common/dl_op_spy.h @@ -55,7 +55,7 @@ class DlOpSpy final : public virtual DlOpReceiver, void drawCircle(const DlPoint& center, DlScalar radius) override; void drawRRect(const SkRRect& rrect) override; void drawDRRect(const SkRRect& outer, const SkRRect& inner) override; - void drawPath(const SkPath& path) override; + void drawPath(const DlPath& path) override; void drawArc(const DlRect& oval_bounds, DlScalar start_degrees, DlScalar sweep_degrees, @@ -98,7 +98,7 @@ class DlOpSpy final : public virtual DlOpReceiver, void drawTextFrame(const std::shared_ptr& text_frame, DlScalar x, DlScalar y) override; - void drawShadow(const SkPath& path, + void drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/testing/display_list_testing.cc b/testing/display_list_testing.cc index dc0205b0d3505..5ef1601433d54 100644 --- a/testing/display_list_testing.cc +++ b/testing/display_list_testing.cc @@ -57,6 +57,7 @@ using DlImageSampling = flutter::DlImageSampling; using SaveLayerOptions = flutter::SaveLayerOptions; using DisplayListOpType = flutter::DisplayListOpType; using DisplayListOpCategory = flutter::DisplayListOpCategory; +using DlPath = flutter::DlPath; using DisplayListStreamDispatcher = flutter::testing::DisplayListStreamDispatcher; @@ -218,9 +219,9 @@ static std::ostream& operator<<(std::ostream& os, const SkRRect& rrect) { << ")"; } -static std::ostream& operator<<(std::ostream& os, const SkPath& path) { - return os << "SkPath(" - << "bounds: " << path.getBounds() +extern std::ostream& operator<<(std::ostream& os, const DlPath& path) { + return os << "DlPath(" + << "bounds: " << path.GetSkBounds() // should iterate over verbs and coordinates... << ")"; } @@ -813,7 +814,7 @@ void DisplayListStreamDispatcher::clipRRect(const SkRRect& rrect, << "isaa: " << is_aa << ");" << std::endl; } -void DisplayListStreamDispatcher::clipPath(const SkPath& path, ClipOp clip_op, +void DisplayListStreamDispatcher::clipPath(const DlPath& path, ClipOp clip_op, bool is_aa) { startl() << "clipPath(" << path << ", " @@ -864,7 +865,7 @@ void DisplayListStreamDispatcher::drawDRRect(const SkRRect& outer, startl() << "drawDRRect(outer: " << outer << ", " << std::endl; startl() << " inner: " << inner << ");" << std::endl; } -void DisplayListStreamDispatcher::drawPath(const SkPath& path) { +void DisplayListStreamDispatcher::drawPath(const DlPath& path) { startl() << "drawPath(" << path << ");" << std::endl; } void DisplayListStreamDispatcher::drawArc(const DlRect& oval_bounds, @@ -974,7 +975,7 @@ void DisplayListStreamDispatcher::drawTextFrame( << x << ", " << y << ");" << std::endl; } -void DisplayListStreamDispatcher::drawShadow(const SkPath& path, +void DisplayListStreamDispatcher::drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/testing/display_list_testing.h b/testing/display_list_testing.h index dbc1c67bf1981..3172203e90656 100644 --- a/testing/display_list_testing.h +++ b/testing/display_list_testing.h @@ -79,6 +79,7 @@ extern std::ostream& operator<<(std::ostream& os, const flutter::DisplayListOpType& type); extern std::ostream& operator<<(std::ostream& os, const flutter::DisplayListOpCategory& category); +extern std::ostream& operator<<(std::ostream& os, const flutter::DlPath& path); } // namespace std @@ -130,7 +131,7 @@ class DisplayListStreamDispatcher final : public DlOpReceiver { void clipRect(const DlRect& rect, ClipOp clip_op, bool is_aa) override; void clipOval(const DlRect& bounds, ClipOp clip_op, bool is_aa) override; void clipRRect(const SkRRect& rrect, ClipOp clip_op, bool is_aa) override; - void clipPath(const SkPath& path, ClipOp clip_op, bool is_aa) override; + void clipPath(const DlPath& path, ClipOp clip_op, bool is_aa) override; void drawColor(DlColor color, DlBlendMode mode) override; void drawPaint() override; @@ -144,7 +145,7 @@ class DisplayListStreamDispatcher final : public DlOpReceiver { void drawCircle(const DlPoint& center, DlScalar radius) override; void drawRRect(const SkRRect& rrect) override; void drawDRRect(const SkRRect& outer, const SkRRect& inner) override; - void drawPath(const SkPath& path) override; + void drawPath(const DlPath& path) override; void drawArc(const DlRect& oval_bounds, DlScalar start_degrees, DlScalar sweep_degrees, @@ -186,7 +187,7 @@ class DisplayListStreamDispatcher final : public DlOpReceiver { void drawTextFrame(const std::shared_ptr& text_frame, DlScalar x, DlScalar y) override; - void drawShadow(const SkPath& path, + void drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, @@ -385,7 +386,7 @@ class DisplayListGeneralReceiver : public DlOpReceiver { break; } } - void clipPath(const SkPath& path, + void clipPath(const DlPath& path, DlCanvas::ClipOp clip_op, bool is_aa) override { switch (clip_op) { @@ -438,7 +439,7 @@ class DisplayListGeneralReceiver : public DlOpReceiver { void drawDRRect(const SkRRect& outer, const SkRRect& inner) override { RecordByType(DisplayListOpType::kDrawDRRect); } - void drawPath(const SkPath& path) override { + void drawPath(const DlPath& path) override { RecordByType(DisplayListOpType::kDrawPath); } void drawArc(const DlRect& oval_bounds, @@ -524,7 +525,7 @@ class DisplayListGeneralReceiver : public DlOpReceiver { DlScalar y) override { RecordByType(DisplayListOpType::kDrawTextFrame); } - void drawShadow(const SkPath& path, + void drawShadow(const DlPath& path, const DlColor color, const DlScalar elevation, bool transparent_occluder, diff --git a/third_party/txt/tests/paragraph_unittests.cc b/third_party/txt/tests/paragraph_unittests.cc index a02899b7d925a..66ce7f298a8ea 100644 --- a/third_party/txt/tests/paragraph_unittests.cc +++ b/third_party/txt/tests/paragraph_unittests.cc @@ -75,14 +75,14 @@ class DlOpRecorder final : public virtual DlOpReceiver, void drawRect(const DlRect& rect) override { rects_.push_back(rect); } - void drawPath(const SkPath& path) override { paths_.push_back(path); } + void drawPath(const DlPath& path) override { paths_.push_back(path); } std::vector> text_frames_; std::vector> blobs_; std::vector> lines_; std::vector> dashed_lines_; std::vector rects_; - std::vector paths_; + std::vector paths_; }; template From e1ecffc4c8a0e77122f6e7eb61e8f0793f1c31b4 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 10 Sep 2024 16:38:00 -0700 Subject: [PATCH 2/2] licenses and unit test for converted path sharing --- ci/licenses_golden/excluded_files | 1 + ci/licenses_golden/licenses_flutter | 4 + display_list/geometry/dl_path.cc | 10 ++ display_list/geometry/dl_path.h | 2 + display_list/geometry/dl_path_unittests.cc | 137 +++++++++++++++++++++ 5 files changed, 154 insertions(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index 9da604db65594..f5e173967285a 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -41,6 +41,7 @@ ../../../flutter/display_list/effects/dl_image_filter_unittests.cc ../../../flutter/display_list/effects/dl_mask_filter_unittests.cc ../../../flutter/display_list/geometry/dl_geometry_types_unittests.cc +../../../flutter/display_list/geometry/dl_path_unittests.cc ../../../flutter/display_list/geometry/dl_region_unittests.cc ../../../flutter/display_list/geometry/dl_rtree_unittests.cc ../../../flutter/display_list/skia/dl_sk_conversions_unittests.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 832c014b39bfb..b2cb8d64c2a0a 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -42345,6 +42345,8 @@ ORIGIN: ../../../flutter/display_list/effects/dl_mask_filter.h + ../../../flutte ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/geometry/dl_geometry_types.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/geometry/dl_path.cc + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/display_list/geometry/dl_path.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/geometry/dl_region.cc + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/geometry/dl_region.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/display_list/geometry/dl_rtree.cc + ../../../flutter/LICENSE @@ -45240,6 +45242,8 @@ FILE: ../../../flutter/display_list/effects/dl_mask_filter.h FILE: ../../../flutter/display_list/effects/dl_runtime_effect.cc FILE: ../../../flutter/display_list/effects/dl_runtime_effect.h FILE: ../../../flutter/display_list/geometry/dl_geometry_types.h +FILE: ../../../flutter/display_list/geometry/dl_path.cc +FILE: ../../../flutter/display_list/geometry/dl_path.h FILE: ../../../flutter/display_list/geometry/dl_region.cc FILE: ../../../flutter/display_list/geometry/dl_region.h FILE: ../../../flutter/display_list/geometry/dl_rtree.cc diff --git a/display_list/geometry/dl_path.cc b/display_list/geometry/dl_path.cc index 768c958e1123f..4ab67aec64740 100644 --- a/display_list/geometry/dl_path.cc +++ b/display_list/geometry/dl_path.cc @@ -57,6 +57,16 @@ bool DlPath::operator==(const DlPath& other) const { return sk_path_ == other.sk_path_; } +bool DlPath::IsConverted() const { + if (!path_.IsEmpty()) { + return true; + } + if (sk_path_.isEmpty()) { + return true; + } + return false; +} + using Path = impeller::Path; using PathBuilder = impeller::PathBuilder; using FillType = impeller::FillType; diff --git a/display_list/geometry/dl_path.h b/display_list/geometry/dl_path.h index 8232e423ba6d6..21f600650dbea 100644 --- a/display_list/geometry/dl_path.h +++ b/display_list/geometry/dl_path.h @@ -36,6 +36,8 @@ class DlPath { bool operator==(const DlPath& other) const; + bool IsConverted() const; + private: const SkPath sk_path_; mutable impeller::Path path_; diff --git a/display_list/geometry/dl_path_unittests.cc b/display_list/geometry/dl_path_unittests.cc index f3b671e793f7a..f0d42be83f4b7 100644 --- a/display_list/geometry/dl_path_unittests.cc +++ b/display_list/geometry/dl_path_unittests.cc @@ -17,6 +17,8 @@ TEST(DisplayListPath, DefaultConstruction) { EXPECT_EQ(path.GetSkPath(), SkPath()); EXPECT_FALSE(path.IsInverseFillType()); + // Empty/default paths are always "pre-converted" by default. + EXPECT_TRUE(path.IsConverted()); bool is_closed = false; EXPECT_FALSE(path.IsRect(nullptr)); @@ -42,6 +44,10 @@ TEST(DisplayListPath, ConstructFromEmpty) { EXPECT_EQ(path, DlPath()); EXPECT_EQ(path.GetSkPath(), SkPath()); + EXPECT_FALSE(path.IsInverseFillType()); + // Empty/default paths are always "pre-converted" by default. + EXPECT_TRUE(path.IsConverted()); + bool is_closed = false; EXPECT_FALSE(path.IsRect(nullptr)); EXPECT_FALSE(path.IsRect(nullptr, &is_closed)); @@ -59,6 +65,90 @@ TEST(DisplayListPath, ConstructFromEmpty) { EXPECT_EQ(path.GetSkBounds(), SkRect()); } +TEST(DisplayListPath, CopyConstruct) { + SkPath sk_path = SkPath::Oval(SkRect::MakeLTRB(10, 10, 20, 20)); + DlPath path1(sk_path); + DlPath path2 = DlPath(path1); + + EXPECT_EQ(path2, path1); + EXPECT_EQ(path2, DlPath(SkPath::Oval(SkRect::MakeLTRB(10, 10, 20, 20)))); + EXPECT_EQ(path2.GetSkPath(), SkPath::Oval(SkRect::MakeLTRB(10, 10, 20, 20))); + + EXPECT_FALSE(path2.IsInverseFillType()); + EXPECT_FALSE(path2.IsConverted()); + + bool is_closed = false; + EXPECT_FALSE(path2.IsRect(nullptr)); + EXPECT_FALSE(path2.IsRect(nullptr, &is_closed)); + EXPECT_FALSE(is_closed); + EXPECT_TRUE(path2.IsOval(nullptr)); + + is_closed = false; + EXPECT_FALSE(path2.IsSkRect(nullptr)); + EXPECT_FALSE(path2.IsSkRect(nullptr, &is_closed)); + EXPECT_FALSE(is_closed); + EXPECT_TRUE(path2.IsSkOval(nullptr)); + EXPECT_FALSE(path2.IsSkRRect(nullptr)); + + EXPECT_EQ(path2.GetBounds(), DlRect::MakeLTRB(10, 10, 20, 20)); + EXPECT_EQ(path2.GetSkBounds(), SkRect::MakeLTRB(10, 10, 20, 20)); +} + +TEST(DisplayListPath, EmbeddingSharedReference) { + SkPath sk_path = SkPath::Oval(SkRect::MakeLTRB(10, 10, 20, 20)); + DlPath path(sk_path); + + class ConversionSharingTester { + public: + explicit ConversionSharingTester(const DlPath& path) : path_(path) {} + + bool ConvertAndTestEmpty() { return path_.GetPath().IsEmpty(); } + + bool Test(const DlPath& reference_path, const std::string& label) { + EXPECT_EQ(path_, reference_path) << label; + EXPECT_EQ(path_, DlPath(SkPath::Oval(SkRect::MakeLTRB(10, 10, 20, 20)))) + << label; + EXPECT_EQ(path_.GetSkPath(), + SkPath::Oval(SkRect::MakeLTRB(10, 10, 20, 20))) + << label; + + EXPECT_FALSE(path_.IsInverseFillType()) << label; + + bool is_closed = false; + EXPECT_FALSE(path_.IsRect(nullptr)) << label; + EXPECT_FALSE(path_.IsRect(nullptr, &is_closed)) << label; + EXPECT_FALSE(is_closed) << label; + EXPECT_TRUE(path_.IsOval(nullptr)) << label; + + is_closed = false; + EXPECT_FALSE(path_.IsSkRect(nullptr)) << label; + EXPECT_FALSE(path_.IsSkRect(nullptr, &is_closed)) << label; + EXPECT_FALSE(is_closed) << label; + EXPECT_TRUE(path_.IsSkOval(nullptr)) << label; + EXPECT_FALSE(path_.IsSkRRect(nullptr)) << label; + + EXPECT_EQ(path_.GetBounds(), DlRect::MakeLTRB(10, 10, 20, 20)) << label; + EXPECT_EQ(path_.GetSkBounds(), SkRect::MakeLTRB(10, 10, 20, 20)) << label; + return path_.IsConverted(); + }; + + private: + const DlPath path_; + }; + + EXPECT_FALSE(path.IsConverted()); + ConversionSharingTester before_tester(path); + EXPECT_FALSE(before_tester.Test(path, "Before triggering conversion")); + EXPECT_FALSE(path.GetPath().IsEmpty()); + EXPECT_FALSE(before_tester.Test(path, "After conversion of source object")); + EXPECT_FALSE(before_tester.ConvertAndTestEmpty()); + EXPECT_TRUE(before_tester.Test(path, "After conversion of captured object")); + + EXPECT_TRUE(path.IsConverted()); + ConversionSharingTester after_tester(path); + EXPECT_TRUE(after_tester.Test(path, "Constructed after conversion")); +} + TEST(DisplayListPath, ConstructFromRect) { SkPath sk_path = SkPath::Rect(SkRect::MakeLTRB(10, 10, 20, 20)); DlPath path(sk_path); @@ -67,6 +157,9 @@ TEST(DisplayListPath, ConstructFromRect) { EXPECT_EQ(path.GetSkPath(), SkPath::Rect(SkRect::MakeLTRB(10, 10, 20, 20))); EXPECT_FALSE(path.IsInverseFillType()); + EXPECT_FALSE(path.IsConverted()); + EXPECT_FALSE(path.GetPath().IsEmpty()); + EXPECT_TRUE(path.IsConverted()); bool is_closed = false; EXPECT_TRUE(path.IsRect(nullptr)); @@ -97,6 +190,10 @@ TEST(DisplayListPath, ConstructFromOval) { EXPECT_EQ(path.GetSkPath(), SkPath::Oval(SkRect::MakeLTRB(10, 10, 20, 20))); EXPECT_FALSE(path.IsInverseFillType()); + EXPECT_FALSE(path.IsConverted()); + EXPECT_FALSE(path.GetPath().IsEmpty()); + EXPECT_TRUE(path.IsConverted()); + EXPECT_FALSE(path.IsRect(nullptr)); EXPECT_TRUE(path.IsOval(nullptr)); DlRect dl_bounds; @@ -124,6 +221,9 @@ TEST(DisplayListPath, ConstructFromRRect) { SkPath::RRect(SkRect::MakeLTRB(10, 10, 20, 20), 1, 2)); EXPECT_FALSE(path.IsInverseFillType()); + EXPECT_FALSE(path.IsConverted()); + EXPECT_FALSE(path.GetPath().IsEmpty()); + EXPECT_TRUE(path.IsConverted()); EXPECT_FALSE(path.IsRect(nullptr)); EXPECT_FALSE(path.IsOval(nullptr)); @@ -157,6 +257,43 @@ TEST(DisplayListPath, ConstructFromPath) { EXPECT_EQ(path.GetSkPath(), sk_path2); EXPECT_FALSE(path.IsInverseFillType()); + EXPECT_FALSE(path.IsConverted()); + EXPECT_FALSE(path.GetPath().IsEmpty()); + EXPECT_TRUE(path.IsConverted()); + + EXPECT_FALSE(path.IsRect(nullptr)); + EXPECT_FALSE(path.IsOval(nullptr)); + EXPECT_FALSE(path.IsSkRect(nullptr)); + EXPECT_FALSE(path.IsSkOval(nullptr)); + EXPECT_FALSE(path.IsSkRRect(nullptr)); + + EXPECT_EQ(path.GetBounds(), DlRect::MakeLTRB(10, 10, 20, 20)); + EXPECT_EQ(path.GetSkBounds(), SkRect::MakeLTRB(10, 10, 20, 20)); +} + +TEST(DisplayListPath, ConstructFromInversePath) { + SkPath sk_path1; + sk_path1.moveTo(10, 10); + sk_path1.lineTo(20, 20); + sk_path1.lineTo(20, 10); + sk_path1.setFillType(SkPathFillType::kInverseWinding); + SkPath sk_path2; + sk_path2.moveTo(10, 10); + sk_path2.lineTo(20, 20); + sk_path2.lineTo(20, 10); + sk_path2.setFillType(SkPathFillType::kInverseWinding); + DlPath path(sk_path1); + + ASSERT_EQ(sk_path1, sk_path2); + + EXPECT_EQ(path, DlPath(sk_path2)); + EXPECT_EQ(path.GetSkPath(), sk_path2); + + EXPECT_TRUE(path.IsInverseFillType()); + EXPECT_FALSE(path.IsConverted()); + EXPECT_FALSE(path.GetPath().IsEmpty()); + EXPECT_TRUE(path.IsConverted()); + EXPECT_FALSE(path.IsRect(nullptr)); EXPECT_FALSE(path.IsOval(nullptr)); EXPECT_FALSE(path.IsSkRect(nullptr));