From cdfe88fdf72647144a10d3230c663977364f3d00 Mon Sep 17 00:00:00 2001
From: Nathan Lee <2431820-nathanal@users.noreply.gitlab.com>
Date: Sat, 25 May 2019 20:14:13 +1000
Subject: [PATCH 1/2] Check knot's pattern still exists before update

Stop crash when pattern changed via swatch palette,
and knot is still present.

https://gitlab.com/inkscape/inkscape/issues/70
---
 src/knot-holder-entity.cpp | 22 +++++++++++++++++++++-
 src/knot-holder-entity.h   |  4 ++++
 src/knotholder.cpp         | 11 +++++++++--
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/src/knot-holder-entity.cpp b/src/knot-holder-entity.cpp
index 7abd16cbaf..e8f97e453d 100644
--- a/src/knot-holder-entity.cpp
+++ b/src/knot-holder-entity.cpp
@@ -172,6 +172,27 @@ static Geom::Point sp_pattern_knot_get(SPPattern const *pat, gdouble x, gdouble
     return Geom::Point(x, y) * pat->getTransform();
 }
 
+bool
+PatternKnotHolderEntityXY::knot_missing() const
+{
+    SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
+    return (pat == nullptr);
+}
+
+bool
+PatternKnotHolderEntityAngle::knot_missing() const
+{
+    SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
+    return (pat == nullptr);
+}
+
+bool
+PatternKnotHolderEntityScale::knot_missing() const
+{
+    SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
+    return (pat == nullptr);
+}
+
 Geom::Point
 PatternKnotHolderEntityXY::knot_get() const
 {
@@ -183,7 +204,6 @@ Geom::Point
 PatternKnotHolderEntityAngle::knot_get() const
 {
     SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
-
     return sp_pattern_knot_get(pat, pat->width(), 0);
 }
 
diff --git a/src/knot-holder-entity.h b/src/knot-holder-entity.h
index e6d2ba2317..1aa686f658 100644
--- a/src/knot-holder-entity.h
+++ b/src/knot-holder-entity.h
@@ -63,6 +63,7 @@ public:
        should be derived from KnotHolderEntity and override these functions */
     virtual void knot_set(Geom::Point const &p, Geom::Point const &origin, unsigned int state) = 0;
     virtual void knot_ungrabbed(Geom::Point const &p, Geom::Point const &origin, unsigned int state) = 0;
+    virtual bool knot_missing() const { return false; }
     virtual Geom::Point knot_get() const = 0;
     virtual void knot_click(unsigned int /*state*/) {}
 
@@ -108,6 +109,7 @@ protected:
 class PatternKnotHolderEntityXY : public KnotHolderEntity {
 public:
     PatternKnotHolderEntityXY(bool fill) : KnotHolderEntity(), _fill(fill) {}
+    bool knot_missing() const override;
     Geom::Point knot_get() const override;
     void knot_ungrabbed(Geom::Point const &p, Geom::Point const &origin, guint state) override {};
     void knot_set(Geom::Point const &p, Geom::Point const &origin, unsigned int state) override;
@@ -119,6 +121,7 @@ private:
 class PatternKnotHolderEntityAngle : public KnotHolderEntity {
 public:
     PatternKnotHolderEntityAngle(bool fill) : KnotHolderEntity(), _fill(fill) {}
+    bool knot_missing() const override;
     Geom::Point knot_get() const override;
     void knot_ungrabbed(Geom::Point const &p, Geom::Point const &origin, guint state) override {};
     void knot_set(Geom::Point const &p, Geom::Point const &origin, unsigned int state) override;
@@ -129,6 +132,7 @@ private:
 class PatternKnotHolderEntityScale : public KnotHolderEntity {
 public:
     PatternKnotHolderEntityScale(bool fill) : KnotHolderEntity(), _fill(fill) {}
+    bool knot_missing() const override;
     Geom::Point knot_get() const override;
     void knot_ungrabbed(Geom::Point const &p, Geom::Point const &origin, guint state) override {};
     void knot_set(Geom::Point const &p, Geom::Point const &origin, unsigned int state) override;
diff --git a/src/knotholder.cpp b/src/knotholder.cpp
index 9594a233f6..d12f61a98f 100644
--- a/src/knotholder.cpp
+++ b/src/knotholder.cpp
@@ -105,8 +105,15 @@ void KnotHolder::updateControlSizes()
 
 void KnotHolder::update_knots()
 {
-    for (auto e : entity) {
-        e->update_knot();
+    for (auto e = entity.begin(); e != entity.end(); ) {
+        // check if pattern was removed without deleting the knot
+        if ((*e)->knot_missing()) {
+            delete (*e);
+            e = entity.erase(e);
+        } else {
+            (*e)->update_knot();
+            ++e;
+        }
     }
 }
 
-- 
GitLab


From abe7037c965055f7a10f8e2c15578d167a7f027e Mon Sep 17 00:00:00 2001
From: Nathan Lee <2431820-nathanal@users.noreply.gitlab.com>
Date: Tue, 4 Jun 2019 20:17:00 +1000
Subject: [PATCH 2/2] Derive pattern knots off the same subclass

---
 src/knot-holder-entity.cpp | 38 +++++++++++++++-----------------------
 src/knot-holder-entity.h   | 33 ++++++++++++++++-----------------
 2 files changed, 31 insertions(+), 40 deletions(-)

diff --git a/src/knot-holder-entity.cpp b/src/knot-holder-entity.cpp
index e8f97e453d..75f36089c9 100644
--- a/src/knot-holder-entity.cpp
+++ b/src/knot-holder-entity.cpp
@@ -173,40 +173,39 @@ static Geom::Point sp_pattern_knot_get(SPPattern const *pat, gdouble x, gdouble
 }
 
 bool
-PatternKnotHolderEntityXY::knot_missing() const
+PatternKnotHolderEntity::knot_missing() const
 {
-    SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
+    SPPattern *pat = _pattern();
     return (pat == nullptr);
 }
 
-bool
-PatternKnotHolderEntityAngle::knot_missing() const
+SPPattern*
+PatternKnotHolderEntity::_pattern() const
 {
-    SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
-    return (pat == nullptr);
-}
-
-bool
-PatternKnotHolderEntityScale::knot_missing() const
-{
-    SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
-    return (pat == nullptr);
+    return _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
 }
 
 Geom::Point
 PatternKnotHolderEntityXY::knot_get() const
 {
-    SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
+    SPPattern *pat = _pattern();
     return sp_pattern_knot_get(pat, 0, 0);
 }
 
 Geom::Point
 PatternKnotHolderEntityAngle::knot_get() const
 {
-    SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
+    SPPattern *pat = _pattern();
     return sp_pattern_knot_get(pat, pat->width(), 0);
 }
 
+Geom::Point
+PatternKnotHolderEntityScale::knot_get() const
+{
+    SPPattern *pat = _pattern();
+    return sp_pattern_knot_get(pat, pat->width(), pat->height());
+}
+
 void
 PatternKnotHolderEntityAngle::knot_set(Geom::Point const &p, Geom::Point const &/*origin*/, guint state)
 {
@@ -234,7 +233,7 @@ PatternKnotHolderEntityAngle::knot_set(Geom::Point const &p, Geom::Point const &
 void
 PatternKnotHolderEntityScale::knot_set(Geom::Point const &p, Geom::Point const &origin, guint state)
 {
-    SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
+    SPPattern *pat = _pattern();
 
     // FIXME: this snapping should be done together with knowing whether control was pressed. If GDK_CONTROL_MASK, then constrained snapping should be used.
     Geom::Point p_snapped = snap_knot_position(p, state);
@@ -262,13 +261,6 @@ PatternKnotHolderEntityScale::knot_set(Geom::Point const &p, Geom::Point const &
 }
 
 
-Geom::Point
-PatternKnotHolderEntityScale::knot_get() const
-{
-    SPPattern *pat = _fill ? SP_PATTERN(item->style->getFillPaintServer()) : SP_PATTERN(item->style->getStrokePaintServer());
-    return sp_pattern_knot_get(pat, pat->width(), pat->height());
-}
-
 /* Filter manipulation */
 void FilterKnotHolderEntity::knot_set(Geom::Point const &p, Geom::Point const &origin, unsigned int state)
 {
diff --git a/src/knot-holder-entity.h b/src/knot-holder-entity.h
index 1aa686f658..92c4d55366 100644
--- a/src/knot-holder-entity.h
+++ b/src/knot-holder-entity.h
@@ -24,6 +24,7 @@
 class SPItem;
 class SPKnot;
 class SPDesktop;
+class SPPattern;
 class KnotHolder;
 
 namespace Inkscape {
@@ -106,38 +107,36 @@ protected:
 
 /* pattern manipulation */
 
-class PatternKnotHolderEntityXY : public KnotHolderEntity {
+class PatternKnotHolderEntity : public KnotHolderEntity {
 public:
-    PatternKnotHolderEntityXY(bool fill) : KnotHolderEntity(), _fill(fill) {}
+    PatternKnotHolderEntity(bool fill) : KnotHolderEntity(), _fill(fill) {}
     bool knot_missing() const override;
-    Geom::Point knot_get() const override;
     void knot_ungrabbed(Geom::Point const &p, Geom::Point const &origin, guint state) override {};
-    void knot_set(Geom::Point const &p, Geom::Point const &origin, unsigned int state) override;
-private:
+protected:
     // true if the entity tracks fill, false for stroke 
     bool _fill;
+    SPPattern *_pattern() const;
 };
 
-class PatternKnotHolderEntityAngle : public KnotHolderEntity {
+class PatternKnotHolderEntityXY : public PatternKnotHolderEntity {
 public:
-    PatternKnotHolderEntityAngle(bool fill) : KnotHolderEntity(), _fill(fill) {}
-    bool knot_missing() const override;
+    PatternKnotHolderEntityXY(bool fill) : PatternKnotHolderEntity(fill) {}
     Geom::Point knot_get() const override;
-    void knot_ungrabbed(Geom::Point const &p, Geom::Point const &origin, guint state) override {};
     void knot_set(Geom::Point const &p, Geom::Point const &origin, unsigned int state) override;
-private:
-    bool _fill;
 };
 
-class PatternKnotHolderEntityScale : public KnotHolderEntity {
+class PatternKnotHolderEntityAngle : public PatternKnotHolderEntity {
 public:
-    PatternKnotHolderEntityScale(bool fill) : KnotHolderEntity(), _fill(fill) {}
-    bool knot_missing() const override;
+    PatternKnotHolderEntityAngle(bool fill) : PatternKnotHolderEntity(fill) {}
+    Geom::Point knot_get() const override;
+    void knot_set(Geom::Point const &p, Geom::Point const &origin, unsigned int state) override;
+};
+
+class PatternKnotHolderEntityScale : public PatternKnotHolderEntity {
+public:
+    PatternKnotHolderEntityScale(bool fill) : PatternKnotHolderEntity(fill) {}
     Geom::Point knot_get() const override;
-    void knot_ungrabbed(Geom::Point const &p, Geom::Point const &origin, guint state) override {};
     void knot_set(Geom::Point const &p, Geom::Point const &origin, unsigned int state) override;
-private:
-    bool _fill;
 };
 
 /* Filter manipulation */
-- 
GitLab