Create Intervals by other mode

display_list/display_list_canvas_unittests.cc

@@ -1504,7 +1504,7 @@ class CanvasCompareTester {
     {
       const SkScalar TestDashes1[] = {29.0, 2.0};
       const SkScalar TestDashes2[] = {17.0, 1.5};
-      auto effect = SkDashPathEffect::Make(TestDashes1, 2, 0.0f);
+      auto effect = DlDashPathEffect::Make(TestDashes1, 2, 0.0f);
       {
         RenderWith(testP, stroke_base_env, tolerance,
                    CaseParameters(
@@ -1514,19 +1514,19 @@ class CanvasCompareTester {
                          p.setStyle(SkPaint::kStroke_Style);
                          // Provide some non-trivial stroke size to get dashed
                          p.setStrokeWidth(5.0);
-                         p.setPathEffect(effect);
+                         p.setPathEffect(effect->skia_object());
                        },
                        [=](DisplayListBuilder& b) {
                          // Need stroke style to see dashing properly
                          b.setStyle(DlDrawStyle::kStroke);
                          // Provide some non-trivial stroke size to get dashed
                          b.setStrokeWidth(5.0);
-                         b.setPathEffect(DlPathEffect::From(effect).get());
+                         b.setPathEffect(effect.get());
                        }));
       }
-      EXPECT_TRUE(testP.is_draw_text_blob() || effect->unique())
+      EXPECT_TRUE(testP.is_draw_text_blob() || effect->skia_object()->unique())
           << "PathEffect == Dash-29-2 Cleanup";
-      effect = SkDashPathEffect::Make(TestDashes2, 2, 0.0f);
+      effect = DlDashPathEffect::Make(TestDashes2, 2, 0.0f);
       {
         RenderWith(testP, stroke_base_env, tolerance,
                    CaseParameters(
@@ -1536,17 +1536,17 @@ class CanvasCompareTester {
                          p.setStyle(SkPaint::kStroke_Style);
                          // Provide some non-trivial stroke size to get dashed
                          p.setStrokeWidth(5.0);
-                         p.setPathEffect(effect);
+                         p.setPathEffect(effect->skia_object());
                        },
                        [=](DisplayListBuilder& b) {
                          // Need stroke style to see dashing properly
                          b.setStyle(DlDrawStyle::kStroke);
                          // Provide some non-trivial stroke size to get dashed
                          b.setStrokeWidth(5.0);
-                         b.setPathEffect(DlPathEffect::From(effect).get());
+                         b.setPathEffect(effect.get());
                        }));
       }
-      EXPECT_TRUE(testP.is_draw_text_blob() || effect->unique())
+      EXPECT_TRUE(testP.is_draw_text_blob() || effect->skia_object()->unique())
           << "PathEffect == Dash-17-1.5 Cleanup";
     }
   }

display_list/display_list_path_effect.cc

@@ -3,11 +3,24 @@
 // found in the LICENSE file.
 
 #include "flutter/display_list/display_list_path_effect.h"
+
 #include <memory>
+#include <utility>
+
 #include "include/core/SkRefCnt.h"
+#include "include/core/SkScalar.h"
 
 namespace flutter {
 
+static void DlPathEffectDeleter(void* p) {
+  // Some of our target environments would prefer a sized delete,
+  // but other target environments do not have that operator.
+  // Use an unsized delete until we get better agreement in the
+  // environments.
+  // See https://github.com/flutter/flutter/issues/100327
+  ::operator delete(p);
+}
+
 std::shared_ptr<DlPathEffect> DlPathEffect::From(SkPathEffect* sk_path_effect) {
   if (sk_path_effect == nullptr) {
     return nullptr;
@@ -16,14 +29,28 @@ std::shared_ptr<DlPathEffect> DlPathEffect::From(SkPathEffect* sk_path_effect) {
   SkPathEffect::DashInfo info;
   if (SkPathEffect::DashType::kDash_DashType ==
       sk_path_effect->asADash(&info)) {
-    auto dash_path_effect = std::make_shared<DlDashPathEffect>(
-        info.fIntervals, info.fCount, info.fPhase);
-    info.fIntervals = dash_path_effect->interval();
+    auto dash_path_effect =
+        DlDashPathEffect::Make(info.fIntervals, info.fCount, info.fPhase);
+    info.fIntervals =
+        reinterpret_cast<DlDashPathEffect*>(dash_path_effect.get())
+            ->intervals();
     sk_path_effect->asADash(&info);
     return dash_path_effect;
   }
   // If not dash path effect, we will use UnknownPathEffect to wrap it.
   return std::make_shared<DlUnknownPathEffect>(sk_ref_sp(sk_path_effect));
 }
 
+std::shared_ptr<DlPathEffect> DlDashPathEffect::Make(const SkScalar* intervals,
+                                                     int count,
+                                                     SkScalar phase) {
+  size_t needed = sizeof(DlDashPathEffect) + sizeof(SkScalar) * count;
+  void* storage = ::operator new(needed);
+
+  std::shared_ptr<DlDashPathEffect> ret;
+  ret.reset(new (storage) DlDashPathEffect(intervals, count, phase),
+            DlPathEffectDeleter);
+  return std::move(ret);
+}
+
 }  // namespace flutter

display_list/display_list_path_effect.h

@@ -39,6 +39,12 @@ class DlPathEffect
   }
 
   virtual const DlDashPathEffect* asDash() const { return nullptr; }
+
+ protected:
+  DlPathEffect() = default;
+
+ private:
+  FML_DISALLOW_COPY_ASSIGN_AND_MOVE(DlPathEffect);
 };
 
 /// The DashPathEffect which specifies modifying the path effect, and it
@@ -62,37 +68,19 @@ class DlPathEffect
 /// A phase of -5, 25, 55, 85, etc. would all result in the same path,
 /// because the sum of all the intervals is 30.
 ///
-class DlDashPathEffect : public DlPathEffect {
+class DlDashPathEffect final : public DlPathEffect {
  public:
-  DlDashPathEffect(const SkScalar intervals[], int count, SkScalar phase)
-      : count_(count), phase_(phase) {
-    intervals_ = new SkScalar[count];
-    if (intervals != nullptr) {
-      memcpy(intervals_, intervals, sizeof(SkScalar) * count);
-    }
-  }
-
-  DlDashPathEffect(const DlDashPathEffect& dash_effect)
-      : DlDashPathEffect(dash_effect.intervals_,
-                         dash_effect.count_,
-                         dash_effect.phase_) {}
-
-  DlDashPathEffect(const DlDashPathEffect* dash_effect)
-      : DlDashPathEffect(dash_effect->intervals_,
-                         dash_effect->count_,
-                         dash_effect->phase_) {}
-
-  static std::shared_ptr<DlDashPathEffect> Make(const SkScalar intervals[],
-                                                int count,
-                                                SkScalar phase) {
-    return std::make_shared<DlDashPathEffect>(intervals, count, phase);
-  }
+  static std::shared_ptr<DlPathEffect> Make(const SkScalar intervals[],
+                                            int count,
+                                            SkScalar phase);
 
   DlPathEffectType type() const override { return DlPathEffectType::kDash; }
-  size_t size() const override { return sizeof(*this); }
+  size_t size() const override {
+    return sizeof(*this) + sizeof(SkScalar) * count_;
+  }
 
   std::shared_ptr<DlPathEffect> shared() const override {
-    return std::make_shared<DlDashPathEffect>(this);
+    return Make(intervals_, count_, phase_);
   }
 
   const DlDashPathEffect* asDash() const override { return this; }
@@ -101,22 +89,49 @@ class DlDashPathEffect : public DlPathEffect {
     return SkDashPathEffect::Make(intervals_, count_, phase_);
   }
 
-  ~DlDashPathEffect() { delete[] intervals_; }
-
-  SkScalar* interval() const { return intervals_; }
+  SkScalar* intervals() const { return intervals_; }
 
  protected:
   bool equals_(DlPathEffect const& other) const override {
     FML_DCHECK(other.type() == DlPathEffectType::kDash);
     auto that = static_cast<DlDashPathEffect const*>(&other);
-    return memcmp(intervals_, that->intervals_, sizeof(SkScalar) * count_) ==
-               0 &&
-           count_ == that->count_ && phase_ == that->phase_;
+    return count_ == that->count_ && base_equals_(that) &&
+           phase_ == that->phase_;
+  }
+
+ private:
+  bool base_equals_(DlDashPathEffect const* other) const {
+    // intervals not nullptr, that has value
+    if (intervals_ != nullptr && other != nullptr) {
+      for (int i = 0; i < count_; i++) {
+        if (intervals_[i] != other->intervals_[i]) {
+          return false;
+        }
+      }
+    }
+    return true;
   }
 
+  DlDashPathEffect(const SkScalar intervals[], int count, SkScalar phase)
+      : count_(count), phase_(phase) {
+    intervals_ = reinterpret_cast<SkScalar*>(this + 1);
+    if (intervals != nullptr) {
+      memcpy(intervals_, intervals, sizeof(SkScalar) * count);
+    }
+  }
+
+  DlDashPathEffect(const DlDashPathEffect* dash_effect)
+      : DlDashPathEffect(dash_effect->intervals_,
+                         dash_effect->count_,
+                         dash_effect->phase_) {}
+
   SkScalar* intervals_;
   int count_;
   SkScalar phase_;
+
+  friend class DisplayListBuilder;
+
+  FML_DISALLOW_COPY_ASSIGN_AND_MOVE(DlDashPathEffect);
 };
 
 class DlUnknownPathEffect final : public DlPathEffect {

display_list/display_list_path_effect_unittests.cc

@@ -16,13 +16,13 @@ const SkScalar TestDashes1[] = {4.0, 2.0};
 const SkScalar TestDashes2[] = {1.0, 1.5};
 
 TEST(DisplayListPathEffect, BuilderSetGet) {
-  DlDashPathEffect dash_path_effect(TestDashes1, 2, 0.0);
+  auto dash_path_effect = DlDashPathEffect::Make(TestDashes1, 2, 0.0);
   DisplayListBuilder builder;
   ASSERT_EQ(builder.getPathEffect(), nullptr);
-  builder.setPathEffect(&dash_path_effect);
+  builder.setPathEffect(dash_path_effect.get());
   ASSERT_NE(builder.getPathEffect(), nullptr);
   ASSERT_TRUE(Equals(builder.getPathEffect(),
-                     static_cast<DlPathEffect*>(&dash_path_effect)));
+                     static_cast<DlPathEffect*>(dash_path_effect.get())));
   builder.setPathEffect(nullptr);
   ASSERT_EQ(builder.getPathEffect(), nullptr);
 }
@@ -39,36 +39,39 @@ TEST(DisplayListPathEffect, FromSkiaPathEffect) {
   std::shared_ptr<DlPathEffect> dl_path_effect =
       DlPathEffect::From(sk_path_effect);
 
-  ASSERT_EQ(dl_path_effect->type(), DlPathEffectType::kUnknown);
+  ASSERT_EQ(dl_path_effect->type(), DlPathEffectType::kDash);
   // We cannot recapture the dash parameters from an SkDashPathEffect
-  ASSERT_EQ(dl_path_effect->asDash(), nullptr);
-  ASSERT_EQ(dl_path_effect->skia_object(), sk_path_effect);
+  ASSERT_EQ(dl_path_effect->asDash(), dl_path_effect.get());
+  ASSERT_TRUE(
+      Equals(dl_path_effect, DlDashPathEffect::Make(TestDashes2, 2, 0.0)));
 }
 
 TEST(DisplayListPathEffect, EffectShared) {
-  DlDashPathEffect effect(TestDashes2, 2, 0.0);
-  ASSERT_NE(effect.shared().get(), &effect);
-  ASSERT_EQ(*effect.shared(), effect);
+  auto effect = DlDashPathEffect::Make(TestDashes2, 2, 0.0);
+  ASSERT_TRUE(Equals(effect->shared(), effect));
 }
 
 TEST(DisplayListPathEffect, DashEffectAsDash) {
-  DlDashPathEffect effect(TestDashes2, 2, 0.0);
-  ASSERT_NE(effect.asDash(), nullptr);
-  ASSERT_EQ(effect.asDash(), &effect);
+  auto effect = DlDashPathEffect::Make(TestDashes2, 2, 0.0);
+  ASSERT_NE(effect->asDash(), nullptr);
+  ASSERT_EQ(effect->asDash(), effect.get());
 }
 
 TEST(DisplayListPathEffect, DashEffectEquals) {
-  DlDashPathEffect effect1(TestDashes2, 2, 0.0);
-  DlDashPathEffect effect2(TestDashes2, 2, 0.0);
-  TestEquals(effect1, effect2);
+  auto effect1 = DlDashPathEffect::Make(TestDashes2, 2, 0.0);
+  auto effect2 = DlDashPathEffect::Make(TestDashes2, 2, 0.0);
+  ASSERT_TRUE(Equals(effect1, effect2));
+  ASSERT_TRUE(Equals(effect1->shared(), effect2->shared()));
 }
 
 TEST(DisplayListPathEffect, BlurNotEquals) {
-  DlDashPathEffect effect1(TestDashes1, 2, 0.0);
-  DlDashPathEffect effect2(TestDashes2, 2, 0.0);
-  DlDashPathEffect effect3(TestDashes2, 3, 0.0);
-  TestNotEquals(effect1, effect2, "intervals differs");
-  TestNotEquals(effect2, effect3, "phase differs");
+  auto effect1 = DlDashPathEffect::Make(TestDashes1, 2, 0.0);
+  auto effect2 = DlDashPathEffect::Make(TestDashes2, 2, 0.0);
+  auto effect3 = DlDashPathEffect::Make(TestDashes2, 3, 0.0);
+  ASSERT_NE(effect1, effect2);
+  ASSERT_NE(effect2, effect3);
+  ASSERT_NE(effect1->shared(), effect2->shared());
+  ASSERT_NE(effect2->shared(), effect3->shared());
 }
 
 TEST(DisplayListPathEffect, UnknownConstructor) {
@@ -156,9 +159,9 @@ void testNotEquals(std::shared_ptr<const DlPathEffect> a,
 }
 
 TEST(DisplayListPathEffect, ComparableTemplates) {
-  DlDashPathEffect effect1(TestDashes1, 2, 0.0);
-  DlDashPathEffect effect2(TestDashes1, 2, 0.0);
-  DlDashPathEffect effect3(TestDashes2, 3, 0.0);
+  auto effect1 = DlDashPathEffect::Make(TestDashes1, 2, 0.0);
+  auto effect2 = DlDashPathEffect::Make(TestDashes1, 2, 0.0);
+  auto effect3 = DlDashPathEffect::Make(TestDashes2, 3, 0.0);
   std::shared_ptr<DlPathEffect> shared_null;
 
   // null to null
@@ -167,43 +170,43 @@ TEST(DisplayListPathEffect, ComparableTemplates) {
   testEquals(shared_null, shared_null);
 
   // ptr to null
-  testNotEquals(&effect1, nullptr);
-  testNotEquals(&effect2, nullptr);
-  testNotEquals(&effect3, nullptr);
+  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);
-  testNotEquals(shared_null, &effect2);
-  testNotEquals(shared_null, &effect3);
+  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);
+  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);
+  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());
+  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