Add effect_bounds for DlPathEffect; Delete invalid test cases;

Author: JsouLiang

display_list/display_list.h

@@ -74,6 +74,7 @@ namespace flutter {
                                     \
   V(SetBlender)                     \
   V(ClearBlender)                   \
+                                    \
   V(SetSkPathEffect)                \
   V(SetPodPathEffect)               \
   V(ClearPathEffect)                \

display_list/display_list_canvas_unittests.cc

@@ -387,14 +387,12 @@ class TestParameters {
         NotEquals(ref_attr.getColorSource(), attr.getColorSource())) {
       return false;
     }
-    auto skia_path_effect =
-        attr.getPathEffect() ? attr.getPathEffect()->skia_object() : nullptr;
+
     DisplayListSpecialGeometryFlags geo_flags =
-        flags_.WithPathEffect(skia_path_effect);
+        flags_.WithPathEffect(attr.getPathEffect().get());
     if (flags_.applies_path_effect() &&  //
         ref_attr.getPathEffect() != attr.getPathEffect()) {
-      SkPathEffect::DashInfo info;
-      if (skia_path_effect->asADash(&info) != SkPathEffect::kDash_DashType) {
+      if (attr.getPathEffect()->asDash() == nullptr) {
         return false;
       }
       if (!ignores_dashes()) {
@@ -485,8 +483,10 @@ class TestParameters {
       adjust =
           half_width * paint.getStrokeMiter() + tolerance.discrete_offset();
     }
+    auto paint_effect = paint.refPathEffect();
+
     DisplayListSpecialGeometryFlags geo_flags =
-        flags_.WithPathEffect(paint.refPathEffect());
+        flags_.WithPathEffect(DlPathEffect::From(paint.refPathEffect()).get());
     if (paint.getStrokeCap() == SkPaint::kButt_Cap &&
         !geo_flags.butt_cap_becomes_square()) {
       adjust = std::max(adjust, half_width);
@@ -1465,8 +1465,6 @@ class CanvasCompareTester {
                          b.setPathEffect(effect.get());
                        }));
       }
-      EXPECT_TRUE(testP.is_draw_text_blob() || effect->skia_object()->unique())
-          << "PathEffect == Dash-29-2 Cleanup";
       effect = DlDashPathEffect::Make(TestDashes2, 2, 0.0f);
       {
         RenderWith(testP, stroke_base_env, tolerance,
@@ -1487,8 +1485,6 @@ class CanvasCompareTester {
                          b.setPathEffect(effect.get());
                        }));
       }
-      EXPECT_TRUE(testP.is_draw_text_blob() || effect->skia_object()->unique())
-          << "PathEffect == Dash-17-1.5 Cleanup";
     }
   }
 

display_list/display_list_flags.cc

@@ -3,9 +3,28 @@
 // found in the LICENSE file.
 
 #include "flutter/display_list/display_list_flags.h"
-
+#include "flutter/display_list/display_list_path_effect.h"
 namespace flutter {
 
-// Just exists to ensure that the header can be cleanly imported.
+const DisplayListSpecialGeometryFlags DisplayListAttributeFlags::WithPathEffect(
+    const DlPathEffect* effect) const {
+  if (is_geometric() && effect) {
+    if (effect->asDash()) {
+      // A dash effect has a very simple impact. It cannot introduce any
+      // miter joins that weren't already present in the original path
+      // and it does not grow the bounds of the path, but it can add
+      // end caps to areas that might not have had them before so all
+      // we need to do is to indicate the potential for diagonal
+      // end caps and move on.
+      return special_flags_.with(kMayHaveCaps_ | kMayHaveDiagonalCaps_);
+    } else {
+      // An arbitrary path effect can introduce joins at an arbitrary
+      // angle and may change the geometry of the end caps
+      return special_flags_.with(kMayHaveCaps_ | kMayHaveDiagonalCaps_ |
+                                 kMayHaveJoins_ | kMayHaveAcuteJoins_);
+    }
+  }
+  return special_flags_;
+}
 
 }  // namespace flutter

display_list/display_list_flags.h

@@ -10,6 +10,7 @@
 
 namespace flutter {
 
+class DlPathEffect;
 /// The base class for the classes that maintain a list of
 /// attributes that might be important for a number of operations
 /// including which rendering attributes need to be set before
@@ -158,26 +159,7 @@ class DisplayListSpecialGeometryFlags : DisplayListFlagsBase {
 class DisplayListAttributeFlags : DisplayListFlagsBase {
  public:
   const DisplayListSpecialGeometryFlags WithPathEffect(
-      sk_sp<SkPathEffect> effect) const {
-    if (is_geometric() && effect) {
-      SkPathEffect::DashInfo info;
-      if (effect->asADash(&info) == SkPathEffect::kDash_DashType) {
-        // A dash effect has a very simple impact. It cannot introduce any
-        // miter joins that weren't already present in the original path
-        // and it does not grow the bounds of the path, but it can add
-        // end caps to areas that might not have had them before so all
-        // we need to do is to indicate the potential for diagonal
-        // end caps and move on.
-        return special_flags_.with(kMayHaveCaps_ | kMayHaveDiagonalCaps_);
-      } else {
-        // An arbitrary path effect can introduce joins at an arbitrary
-        // angle and may change the geometry of the end caps
-        return special_flags_.with(kMayHaveCaps_ | kMayHaveDiagonalCaps_ |
-                                   kMayHaveJoins_ | kMayHaveAcuteJoins_);
-      }
-    }
-    return special_flags_;
-  }
+      const DlPathEffect* effect) const;
 
   bool ignores_paint() const { return has_any(kIgnoresPaint_); }
 

display_list/display_list_path_effect.cc

@@ -5,6 +5,7 @@
 #include "flutter/display_list/display_list_path_effect.h"
 
 #include <memory>
+#include <optional>
 #include <utility>
 
 #include "include/core/SkRefCnt.h"
@@ -30,7 +31,7 @@ std::shared_ptr<DlPathEffect> DlPathEffect::From(SkPathEffect* sk_path_effect) {
   if (SkPathEffect::DashType::kDash_DashType ==
       sk_path_effect->asADash(&info)) {
     auto dash_path_effect =
-        DlDashPathEffect::Make(info.fIntervals, info.fCount, info.fPhase);
+        DlDashPathEffect::Make(nullptr, info.fCount, info.fPhase);
     info.fIntervals =
         reinterpret_cast<DlDashPathEffect*>(dash_path_effect.get())
             ->intervals();
@@ -53,4 +54,22 @@ std::shared_ptr<DlPathEffect> DlDashPathEffect::Make(const SkScalar* intervals,
   return std::move(ret);
 }
 
+std::optional<SkRect> DlDashPathEffect::effect_bounds(SkRect& rect) const {
+  // SkDashPathEffect's computeFastBounds is return true which mean it bounds is
+  // the input value;
+  return rect;
+}
+
+std::optional<SkRect> DlUnknownPathEffect::effect_bounds(SkRect& rect) const {
+  if (rect.isSorted()) {
+    return std::nullopt;
+  }
+  SkPaint p;
+  p.setPathEffect(sk_path_effect_);
+  if (!p.canComputeFastBounds()) {
+    return std::nullopt;
+  }
+  return p.computeFastBounds(rect, &rect);
+}
+
 }  // namespace flutter

display_list/display_list_path_effect.h

@@ -6,6 +6,7 @@
 #define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_PATH_EFFECT_H_
 
 #include <cstring>
+#include <optional>
 #include "flutter/display_list/display_list_attributes.h"
 #include "flutter/display_list/types.h"
 #include "flutter/fml/logging.h"
@@ -41,6 +42,8 @@ class DlPathEffect
 
   virtual const DlDashPathEffect* asDash() const { return nullptr; }
 
+  virtual std::optional<SkRect> effect_bounds(SkRect&) const = 0;
+
  protected:
   DlPathEffect() = default;
 
@@ -81,36 +84,36 @@ class DlDashPathEffect final : public DlPathEffect {
   }
 
   std::shared_ptr<DlPathEffect> shared() const override {
-    return Make(pods(), count_, phase_);
+    return Make(intervals(), count_, phase_);
   }
 
   const DlDashPathEffect* asDash() const override { return this; }
 
   sk_sp<SkPathEffect> skia_object() const override {
-    return SkDashPathEffect::Make(pods(), count_, phase_);
+    return SkDashPathEffect::Make(intervals(), count_, phase_);
   }
 
-  SkScalar* intervals() { return reinterpret_cast<SkScalar*>(this + 1); }
+  const SkScalar* intervals() const {
+    return reinterpret_cast<const SkScalar*>(this + 1);
+  }
+
+  std::optional<SkRect> effect_bounds(SkRect& rect) const override;
 
  protected:
   bool equals_(DlPathEffect const& other) const override {
     FML_DCHECK(other.type() == DlPathEffectType::kDash);
     auto that = static_cast<DlDashPathEffect const*>(&other);
-    return memcmp(pods(), that->pods(), count_) == 0 &&
-           count_ == that->count_ && phase_ == that->phase_;
+    return count_ == that->count_ &&
+           memcmp(intervals(), that->intervals(), sizeof(SkScalar) * count_) ==
+               0 &&
+           phase_ == that->phase_;
   }
 
  private:
-  const SkScalar* pods() const {
-    return reinterpret_cast<const SkScalar*>(this + 1);
-  }
-
-  bool base_equals_(DlDashPathEffect const* other) const {
-    // intervals not nullptr, that has value
-
-    return true;
-  }
-
+  SkScalar* intervals() { return reinterpret_cast<SkScalar*>(this + 1); }
+  // DlDashPathEffect constructor assumes the caller has prealloced storage for
+  // the intervals. If the intervals is nullptr the intervals will
+  // uninitialized.
   DlDashPathEffect(const SkScalar intervals[], int count, SkScalar phase)
       : count_(count), phase_(phase) {
     if (intervals != nullptr) {
@@ -120,14 +123,15 @@ class DlDashPathEffect final : public DlPathEffect {
   }
 
   DlDashPathEffect(const DlDashPathEffect* dash_effect)
-      : DlDashPathEffect(dash_effect->pods(),
+      : DlDashPathEffect(dash_effect->intervals(),
                          dash_effect->count_,
                          dash_effect->phase_) {}
 
   int count_;
   SkScalar phase_;
 
   friend class DisplayListBuilder;
+  friend class DlPathEffect;
 
   FML_DISALLOW_COPY_ASSIGN_AND_MOVE(DlDashPathEffect);
 };
@@ -152,6 +156,8 @@ class DlUnknownPathEffect final : public DlPathEffect {
 
   virtual ~DlUnknownPathEffect() = default;
 
+  std::optional<SkRect> effect_bounds(SkRect& rect) const override;
+
  protected:
   bool equals_(const DlPathEffect& other) const override {
     FML_DCHECK(other.type() == DlPathEffectType::kUnknown);

display_list/display_list_path_effect_unittests.cc

@@ -68,12 +68,18 @@ TEST(DisplayListPathEffect, DashEffectEquals) {
 TEST(DisplayListPathEffect, BlurNotEquals) {
   const SkScalar TestDashes1[] = {4.0, 2.0};
   const SkScalar TestDashes2[] = {1.0, 1.5};
+  const SkScalar TestDashes3[] = {4.0, 2.5, 2.0};
   auto effect1 = DlDashPathEffect::Make(TestDashes1, 2, 0.0);
   auto effect2 = DlDashPathEffect::Make(TestDashes2, 2, 0.0);
-  auto effect3 = DlDashPathEffect::Make(TestDashes2, 2, 1.0);
+  auto effect3 = DlDashPathEffect::Make(TestDashes3, 3, 1.0);
+
   ASSERT_NE(effect1, effect2);
-  ASSERT_NE(effect2, effect3);
   ASSERT_NE(effect1->shared(), effect2->shared());
+
+  ASSERT_NE(effect1, effect3);
+  ASSERT_NE(effect1->shared(), effect3->shared());
+
+  ASSERT_NE(effect2, effect3);
   ASSERT_NE(effect2->shared(), effect3->shared());
 }
 
@@ -116,108 +122,5 @@ TEST(DisplayListPathEffect, UnknownNotEquals) {
                 "SkDashPathEffect instance differs");
 }
 
-void testEquals(DlPathEffect* a, DlPathEffect* b) {
-  // a and b have the same nullness or values
-  ASSERT_TRUE(Equals(a, b));
-  ASSERT_FALSE(NotEquals(a, b));
-  ASSERT_TRUE(Equals(b, a));
-  ASSERT_FALSE(NotEquals(b, a));
-}
-
-void testNotEquals(DlPathEffect* a, DlPathEffect* b) {
-  // a and b do not have the same nullness or values
-  ASSERT_FALSE(Equals(a, b));
-  ASSERT_TRUE(NotEquals(a, b));
-  ASSERT_FALSE(Equals(b, a));
-  ASSERT_TRUE(NotEquals(b, a));
-}
-
-void testEquals(std::shared_ptr<const DlPathEffect> a, DlPathEffect* b) {
-  // a and b have the same nullness or values
-  ASSERT_TRUE(Equals(a, b));
-  ASSERT_FALSE(NotEquals(a, b));
-  ASSERT_TRUE(Equals(b, a));
-  ASSERT_FALSE(NotEquals(b, a));
-}
-
-void testNotEquals(std::shared_ptr<const DlPathEffect> a, DlPathEffect* b) {
-  // a and b do not have the same nullness or values
-  ASSERT_FALSE(Equals(a, b));
-  ASSERT_TRUE(NotEquals(a, b));
-  ASSERT_FALSE(Equals(b, a));
-  ASSERT_TRUE(NotEquals(b, a));
-}
-
-void testEquals(std::shared_ptr<const DlPathEffect> a,
-                std::shared_ptr<const DlPathEffect> b) {
-  // a and b have the same nullness or values
-  ASSERT_TRUE(Equals(a, b));
-  ASSERT_FALSE(NotEquals(a, b));
-  ASSERT_TRUE(Equals(b, a));
-  ASSERT_FALSE(NotEquals(b, a));
-}
-
-void testNotEquals(std::shared_ptr<const DlPathEffect> a,
-                   std::shared_ptr<const DlPathEffect> b) {
-  // a and b do not have the same nullness or values
-  ASSERT_FALSE(Equals(a, b));
-  ASSERT_TRUE(NotEquals(a, b));
-  ASSERT_FALSE(Equals(b, a));
-  ASSERT_TRUE(NotEquals(b, a));
-}
-
-TEST(DisplayListPathEffect, ComparableTemplates) {
-  const SkScalar TestDashes1[] = {4.0, 2.0};
-  const SkScalar TestDashes2[] = {1.0, 1.5};
-  auto effect1 = DlDashPathEffect::Make(TestDashes1, 2, 0.0);
-  auto effect2 = DlDashPathEffect::Make(TestDashes1, 2, 0.0);
-  auto effect3 = DlDashPathEffect::Make(TestDashes2, 2, 1.0);
-  std::shared_ptr<DlPathEffect> shared_null;
-
-  // null to null
-  testEquals(nullptr, nullptr);
-  testEquals(shared_null, nullptr);
-  testEquals(shared_null, shared_null);
-
-  // ptr to null
-  testNotEquals(effect1.get(), nullptr);
-  testNotEquals(effect2.get(), nullptr);
-  testNotEquals(effect3.get(), nullptr);
-
-  // shared_ptr to null and shared_null to ptr
-  testNotEquals(effect1->shared(), nullptr);
-  testNotEquals(effect2->shared(), nullptr);
-  testNotEquals(effect3->shared(), nullptr);
-  testNotEquals(shared_null, effect1.get());
-  testNotEquals(shared_null, effect2.get());
-  testNotEquals(shared_null, effect3.get());
-
-  // ptr to ptr
-  testEquals(effect1, effect1);
-  testEquals(effect1, effect2);
-  testEquals(effect3, effect3);
-  testEquals(effect2, effect2);
-
-  // shared_ptr to ptr
-  testEquals(effect1->shared(), effect1);
-  testEquals(effect1->shared(), effect2);
-  testEquals(effect2->shared(), effect2);
-  testEquals(effect3->shared(), effect3);
-  testNotEquals(effect1->shared(), effect3);
-  testNotEquals(effect2->shared(), effect3);
-  testNotEquals(effect3->shared(), effect1);
-  testNotEquals(effect3->shared(), effect2);
-
-  // shared_ptr to shared_ptr
-  testEquals(effect1->shared(), effect1->shared());
-  testEquals(effect1->shared(), effect2->shared());
-  testEquals(effect2->shared(), effect2->shared());
-  testEquals(effect3->shared(), effect3->shared());
-  testNotEquals(effect1->shared(), effect3->shared());
-  testNotEquals(effect2->shared(), effect3->shared());
-  testNotEquals(effect3->shared(), effect1->shared());
-  testNotEquals(effect3->shared(), effect2->shared());
-}
-
 }  // namespace testing
 }  // namespace flutter