Rename isLPEParam() to isDeletable(), create new class LPEKnotHolderEntity and make inheritance from it a bit less clumsy

(bzr r6448)

14 files changed, 43 insertions, 56 deletions

diff --git a/src/knot-holder-entity.h b/src/knot-holder-entity.h
index 9cdc908de..e0d0ca8e8 100644
--- a/src/knot-holder-entity.h
+++ b/src/knot-holder-entity.h
@@ -41,10 +41,11 @@ public:
                         SPKnotModeType mode = SP_KNOT_MODE_XOR,
                         guint32 color = 0xffffff00);
 
-    /* derived classes like PointParam for LPEs use this, e.g., to indicate that we must not call
-       delete on their pointer when a knotholder is destroyed */
-    // TODO: purge this now that PointParams are not KnotHolderEntities any more!
-    virtual bool isLPEParam() { return false; }
+    /* derived classes used for LPE knotholder handles use this to indicate that they
+       must not be deleted when a knotholder is destroyed */
+    // TODO: it would be nice to ditch this but then we need to dynamically create instances of different
+    //       KnotHolderEntity classes in Effect::addKnotHolderEntities. How to do this???
+    virtual bool isDeletable() { return true; }
 
     /* the get/set/click handlers are virtual functions; each handler class for a knot
        should be derived from KnotHolderEntity and override these functions */
@@ -75,6 +76,11 @@ public:
     sigc::connection _ungrabbed_connection;
 };
 
+// derived KnotHolderEntity class for LPEs
+class LPEKnotHolderEntity : public KnotHolderEntity {
+    virtual bool isDeletable() { return false; }
+};
+
 /* pattern manipulation */
 
 class PatternKnotHolderEntityXY : public KnotHolderEntity {
diff --git a/src/knotholder.cpp b/src/knotholder.cpp
index f2bbbfbc0..ae9737b32 100644
--- a/src/knotholder.cpp
+++ b/src/knotholder.cpp
@@ -60,17 +60,16 @@ KnotHolder::~KnotHolder() {
     g_object_unref(G_OBJECT(item));
     for(std::list<KnotHolderEntity *>::iterator i = entity.begin(); i != entity.end(); ++i) {
         KnotHolderEntity* e = (*i);
-        if (!e->isLPEParam()) {
-            // knotholder entity may be deleted
+        if (e->isDeletable()) {
             delete (*i);
         } else {
-            // we must not delete the entity since it's an LPE parameter,
+            // we must not delete the entity (since it's attached to an LPE parameter),
             // but the handle should be destroyed
             g_object_unref(e->knot);
         }
         (*i) = NULL;
     }
-    entity.clear(); // this shouldn't be necessary, though
+    entity.clear(); // is this necessary?
 }
 
 /**
diff --git a/src/live_effects/effect.cpp b/src/live_effects/effect.cpp
index c4bbc31e1..467b1417d 100644
--- a/src/live_effects/effect.cpp
+++ b/src/live_effects/effect.cpp
@@ -425,6 +425,21 @@ Effect::addHandles(KnotHolder *knotholder, SPDesktop *desktop, SPItem *item) {
     using namespace Inkscape::LivePathEffect;
 
     // add handles provided by the effect itself
+    addKnotHolderEntities(knotholder, desktop, item);
+
+    // add handles provided by the effect's parameters (if any)
+    for (std::vector<Parameter *>::iterator p = param_vector.begin(); p != param_vector.end(); ++p) {
+        (*p)->addKnotHolderEntities(knotholder, desktop, item);
+    }
+}
+
+void
+Effect::addKnotHolderEntities(KnotHolder *knotholder, SPDesktop *desktop, SPItem *item) {
+    // TODO: The entities in kh_entity_vector are already instantiated during the call
+    //       to registerKnotHolderHandle(), but they are recreated here. Also, we must not
+    //       delete them when the knotholder is destroyed, whence the clumsy function
+    //       isDeletable(). If we could create entities of different classes dynamically,
+    //       this would be much nicer. How to do this?
     std::vector<std::pair<KnotHolderEntity*, const char*> >::iterator i;
     for (i = kh_entity_vector.begin(); i != kh_entity_vector.end(); ++i) {
         KnotHolderEntity *entity = i->first;
@@ -433,11 +448,6 @@ Effect::addHandles(KnotHolder *knotholder, SPDesktop *desktop, SPItem *item) {
         entity->create(desktop, item, knotholder, descr);
         knotholder->add(entity);
     }
-
-    // add handles provided by the effect's parameters (if any)
-    for (std::vector<Parameter *>::iterator p = param_vector.begin(); p != param_vector.end(); ++p) {
-        (*p)->addKnotHolderEntities(knotholder, desktop, item);
-    }
 }
 
 void
diff --git a/src/live_effects/effect.h b/src/live_effects/effect.h
index b6386169c..2798f88ce 100644
--- a/src/live_effects/effect.h
+++ b/src/live_effects/effect.h
@@ -129,7 +129,6 @@ public:
     //       (but spiro lpe still needs it!)
     virtual LPEPathFlashType pathFlashType() { return DEFAULT; }
     void addHandles(KnotHolder *knotholder, SPDesktop *desktop, SPItem *item);
-    void addPointParamHandles(KnotHolder *knotholder, SPDesktop *desktop, SPItem *item);
 
     void addHelperPaths(SPLPEItem *lpeitem, SPDesktop *desktop);
     inline bool providesOwnFlashPaths() {
@@ -167,6 +166,7 @@ protected:
     void registerKnotHolderHandle(KnotHolderEntity* entity, const char* descr);
     Parameter * getNextOncanvasEditableParam();
 
+    void addKnotHolderEntities(KnotHolder *knotholder, SPDesktop *desktop, SPItem *item);
     virtual void addHelperPathsImpl(SPLPEItem *lpeitem, SPDesktop *desktop);
 
     std::vector<Parameter *> param_vector;
@@ -193,7 +193,6 @@ private:
     Effect& operator=(const Effect&);
 };
 
-
 } //namespace LivePathEffect
 } //namespace Inkscape
 
diff --git a/src/live_effects/lpe-angle_bisector.cpp b/src/live_effects/lpe-angle_bisector.cpp
index c7c7b5262..41c11fc5e 100644
--- a/src/live_effects/lpe-angle_bisector.cpp
+++ b/src/live_effects/lpe-angle_bisector.cpp
@@ -30,20 +30,16 @@ namespace LivePathEffect {
 
 namespace AB {
 
-class KnotHolderEntityLeftEnd : public KnotHolderEntity
+class KnotHolderEntityLeftEnd : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
 
-class KnotHolderEntityRightEnd : public KnotHolderEntity
+class KnotHolderEntityRightEnd : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
diff --git a/src/live_effects/lpe-copy_rotate.cpp b/src/live_effects/lpe-copy_rotate.cpp
index 948da335a..142b6eb64 100644
--- a/src/live_effects/lpe-copy_rotate.cpp
+++ b/src/live_effects/lpe-copy_rotate.cpp
@@ -26,11 +26,9 @@ namespace LivePathEffect {
 
 namespace CR {
 
-class KnotHolderEntityAngle : public KnotHolderEntity
+class KnotHolderEntityAngle : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
diff --git a/src/live_effects/lpe-parallel.cpp b/src/live_effects/lpe-parallel.cpp
index dba0f3a8d..e0e156263 100644
--- a/src/live_effects/lpe-parallel.cpp
+++ b/src/live_effects/lpe-parallel.cpp
@@ -24,20 +24,16 @@ namespace LivePathEffect {
 
 namespace Pl {
 
-class KnotHolderEntityLeftEnd : public KnotHolderEntity
+class KnotHolderEntityLeftEnd : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
 
-class KnotHolderEntityRightEnd : public KnotHolderEntity
+class KnotHolderEntityRightEnd : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
diff --git a/src/live_effects/lpe-perp_bisector.cpp b/src/live_effects/lpe-perp_bisector.cpp
index bcfe57614..c4146a1d9 100644
--- a/src/live_effects/lpe-perp_bisector.cpp
+++ b/src/live_effects/lpe-perp_bisector.cpp
@@ -25,20 +25,16 @@ namespace Inkscape {
 namespace LivePathEffect {
 namespace PB {
 
-class KnotHolderEntityLeftEnd : public KnotHolderEntity
+class KnotHolderEntityLeftEnd : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
 
-class KnotHolderEntityRightEnd : public KnotHolderEntity
+class KnotHolderEntityRightEnd : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
diff --git a/src/live_effects/lpe-perspective_path.cpp b/src/live_effects/lpe-perspective_path.cpp
index 9aa26e05d..e3397a20e 100644
--- a/src/live_effects/lpe-perspective_path.cpp
+++ b/src/live_effects/lpe-perspective_path.cpp
@@ -37,11 +37,9 @@ namespace LivePathEffect {
 
 namespace PP {
 
-class KnotHolderEntityOffset : public KnotHolderEntity
+class KnotHolderEntityOffset : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
diff --git a/src/live_effects/lpe-skeleton.cpp b/src/live_effects/lpe-skeleton.cpp
index 59a034196..5fc4ba6f2 100644
--- a/src/live_effects/lpe-skeleton.cpp
+++ b/src/live_effects/lpe-skeleton.cpp
@@ -87,11 +87,9 @@ LPESkeleton::doEffect_pwd2 (Geom::Piecewise<Geom::D2<Geom::SBasis> > const & pwd
 /**
 namespace Skeleton {
 
-class KnotHolderEntityMyHandle : public KnotHolderEntity
+class KnotHolderEntityMyHandle : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; } // this is always needed
-
     // the set() and get() methods must be implemented, click() is optional
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
diff --git a/src/live_effects/lpe-tangent_to_curve.cpp b/src/live_effects/lpe-tangent_to_curve.cpp
index 83f73cf8b..33188382c 100644
--- a/src/live_effects/lpe-tangent_to_curve.cpp
+++ b/src/live_effects/lpe-tangent_to_curve.cpp
@@ -29,29 +29,23 @@ namespace LivePathEffect {
 
 namespace TtC {
 
-class KnotHolderEntityAttachPt : public KnotHolderEntity
+class KnotHolderEntityAttachPt : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
 
-class KnotHolderEntityLeftEnd : public KnotHolderEntity
+class KnotHolderEntityLeftEnd : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
 
-class KnotHolderEntityRightEnd : public KnotHolderEntity
+class KnotHolderEntityRightEnd : public LPEKnotHolderEntity
 {
 public:
-    virtual bool isLPEParam() { return true; }
-
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);
     virtual NR::Point knot_get();
 };
diff --git a/src/live_effects/parameter/point.cpp b/src/live_effects/parameter/point.cpp
index eed16725d..0a782d4ec 100644
--- a/src/live_effects/parameter/point.cpp
+++ b/src/live_effects/parameter/point.cpp
@@ -152,7 +152,7 @@ PointParam::set_oncanvas_looks(SPKnotShapeType shape, SPKnotModeType mode, guint
     knot_color = color;
 }
 
-class PointParamKnotHolderEntity : public KnotHolderEntity {
+class PointParamKnotHolderEntity : public LPEKnotHolderEntity {
 public:
     PointParamKnotHolderEntity(PointParam *p) { this->pparam = p; }
     virtual ~PointParamKnotHolderEntity() {}
diff --git a/src/live_effects/parameter/point.h b/src/live_effects/parameter/point.h
index bad138c9c..9718b232b 100644
--- a/src/live_effects/parameter/point.h
+++ b/src/live_effects/parameter/point.h
@@ -54,9 +54,6 @@ public:
 
     virtual void addKnotHolderEntities(KnotHolder *knotholder, SPDesktop *desktop, SPItem *item);
 
-    // TODO: ditch this!
-    virtual bool isLPEParam() { return true; }
-
 private:
     PointParam(const PointParam&);
     PointParam& operator=(const PointParam&);
diff --git a/src/live_effects/parameter/pointparam-knotholder.cpp b/src/live_effects/parameter/pointparam-knotholder.cpp
index 3ab07a951..d92532e32 100644
--- a/src/live_effects/parameter/pointparam-knotholder.cpp
+++ b/src/live_effects/parameter/pointparam-knotholder.cpp
@@ -60,7 +60,7 @@ PointParamKnotHolder::~PointParamKnotHolder()
     g_object_unref(G_OBJECT(this->lpeobject));
 }
 
-class KnotHolderEntityPointParam : public KnotHolderEntity {
+class KnotHolderEntityPointParam : public LPEKnotHolderEntity {
 public:
     virtual NR::Point knot_get();
     virtual void knot_set(NR::Point const &p, NR::Point const &origin, guint state);