Properly allow effect stacking with knotholders (and add extra LPE functionality)

(bzr r13090.1.48)

11 files changed, 77 insertions, 57 deletions

diff --git a/src/live_effects/effect.cpp b/src/live_effects/effect.cpp
index 8bf210270..99282a312 100644
--- a/src/live_effects/effect.cpp
+++ b/src/live_effects/effect.cpp
@@ -373,6 +373,24 @@ void Effect::doOnRemove (SPLPEItem const* lpeitem)
 {
 }
 
+//secret impl methods (shhhh!)
+void Effect::doOnApply_impl(SPLPEItem const* lpeitem)
+{
+    sp_lpe_item = const_cast<SPLPEItem *>(lpeitem);
+    sp_curve = SP_SHAPE(sp_lpe_item)->getCurve();
+    pathvector_before_effect = sp_curve->get_pathvector();
+    doOnApply(lpeitem);
+}
+
+void Effect::doBeforeEffect_impl(SPLPEItem const* lpeitem)
+{
+    sp_lpe_item = const_cast<SPLPEItem *>(lpeitem);
+    sp_curve = SP_SHAPE(sp_lpe_item)->getCurve();
+    pathvector_before_effect = sp_curve->get_pathvector();
+    
+    doBeforeEffect(lpeitem);
+}
+
 /**
  * Effects can have a parameter path set before they are applied by accepting a nonzero number of
  * mouse clicks. This method activates the pen context, which waits for the specified number of
diff --git a/src/live_effects/effect.h b/src/live_effects/effect.h
index 85c930def..d2d9f3b63 100644
--- a/src/live_effects/effect.h
+++ b/src/live_effects/effect.h
@@ -53,6 +53,11 @@ public:
 
     EffectType effectType() const;
 
+    //basically, to get this method called before the derived classes, a bit
+    //of indirection is needed. We first call these methods, then the below.
+    void doOnApply_impl(SPLPEItem const* lpeitem);
+    void doBeforeEffect_impl(SPLPEItem const* lpeitem);
+
     virtual void doOnApply (SPLPEItem const* lpeitem);
     virtual void doBeforeEffect (SPLPEItem const* lpeitem);
     
@@ -149,6 +154,9 @@ protected:
     // instead of normally 'splitting' the path into continuous pwd2 paths and calling doEffect_pwd2 for each.
     bool concatenate_before_pwd2;
 
+    SPLPEItem * sp_lpe_item; // these get stored in doBeforeEffect_impl, and derived classes may do as they please with them.
+    SPCurve * sp_curve;
+    std::vector<Geom::Path> pathvector_before_effect;
 private:
     bool provides_own_flash_paths; // if true, the standard flash path is suppressed
 
diff --git a/src/live_effects/lpe-attach-path.cpp b/src/live_effects/lpe-attach-path.cpp
index 0cceb1cb7..96372892e 100644
--- a/src/live_effects/lpe-attach-path.cpp
+++ b/src/live_effects/lpe-attach-path.cpp
@@ -62,15 +62,10 @@ void LPEAttachPath::resetDefaults(SPItem const * item)
     curve_end_previous_origin = end_path_curve_end.getOrigin();
 }
 
-void LPEAttachPath::doBeforeEffect(SPLPEItem const *lpeitem)
-{
-    lpe_effect = const_cast<SPLPEItem*> (lpeitem);
-}
-
 void LPEAttachPath::doEffect (SPCurve * curve)
 {
     std::vector<Geom::Path> this_pathv = curve->get_pathvector();
-    if (lpe_effect && !this_pathv.empty()) {
+    if (sp_lpe_item && !this_pathv.empty()) {
         Geom::Path p = Geom::Path(this_pathv.front().initialPoint());
         
         bool set_start_end = start_path_curve_end.getOrigin() != curve_start_previous_origin;
@@ -79,7 +74,7 @@ void LPEAttachPath::doEffect (SPCurve * curve)
         if (start_path.linksToPath()) {
 
             std::vector<Geom::Path> linked_pathv = start_path.get_pathvector();
-            Geom::Affine linkedtransform = start_path.getObject()->getRelativeTransform(lpe_effect);
+            Geom::Affine linkedtransform = start_path.getObject()->getRelativeTransform(sp_lpe_item);
 
             if ( !linked_pathv.empty() )
             {
@@ -132,7 +127,7 @@ void LPEAttachPath::doEffect (SPCurve * curve)
         if (end_path.linksToPath()) {
 
             std::vector<Geom::Path> linked_pathv = end_path.get_pathvector();
-            Geom::Affine linkedtransform = end_path.getObject()->getRelativeTransform(lpe_effect);
+            Geom::Affine linkedtransform = end_path.getObject()->getRelativeTransform(sp_lpe_item);
 
             if ( !linked_pathv.empty() )
             {
diff --git a/src/live_effects/lpe-attach-path.h b/src/live_effects/lpe-attach-path.h
index 390282f90..1d6c590d1 100644
--- a/src/live_effects/lpe-attach-path.h
+++ b/src/live_effects/lpe-attach-path.h
@@ -25,7 +25,6 @@ public:
     LPEAttachPath(LivePathEffectObject *lpeobject);
     virtual ~LPEAttachPath();
 
-    virtual void doBeforeEffect(const SPLPEItem *lpeitem);
     virtual void doEffect (SPCurve * curve);
     virtual void resetDefaults(SPItem const * item);
 
@@ -45,7 +44,6 @@ private:
     ScalarParam end_path_position;
     TransformedPointParam end_path_curve_start;
     VectorParam end_path_curve_end;
-    SPLPEItem * lpe_effect;
 };
 
 }; //namespace LivePathEffect
diff --git a/src/live_effects/lpe-knot.cpp b/src/live_effects/lpe-knot.cpp
index 581c632f5..05b6ed795 100644
--- a/src/live_effects/lpe-knot.cpp
+++ b/src/live_effects/lpe-knot.cpp
@@ -538,6 +538,10 @@ LPEKnot::doBeforeEffect (SPLPEItem const* lpeitem)
 {
     using namespace Geom;
     original_bbox(lpeitem);
+    
+    if (SP_IS_PATH(lpeitem)) {
+        supplied_path = SP_PATH(lpeitem)->getCurve()->get_pathvector();
+    }
 
     gpaths = std::vector<Geom::Path>();
     gstroke_widths = std::vector<double>();
diff --git a/src/live_effects/lpe-knot.h b/src/live_effects/lpe-knot.h
index b937f9021..12ab32e5b 100644
--- a/src/live_effects/lpe-knot.h
+++ b/src/live_effects/lpe-knot.h
@@ -64,7 +64,8 @@ public:
   void addKnotHolderEntities(KnotHolder *knotholder, SPDesktop *desktop, SPItem *item);
 
 protected:
-    virtual void addCanvasIndicators(SPLPEItem const *lpeitem, std::vector<Geom::PathVector> &hp_vec);
+  virtual void addCanvasIndicators(SPLPEItem const *lpeitem, std::vector<Geom::PathVector> &hp_vec);
+  std::vector<Geom::Path> supplied_path; //for knotholder business
   
 private:
   void updateSwitcher();
diff --git a/src/live_effects/lpe-tangent_to_curve.cpp b/src/live_effects/lpe-tangent_to_curve.cpp
index dbebdf7fb..bce4876af 100644
--- a/src/live_effects/lpe-tangent_to_curve.cpp
+++ b/src/live_effects/lpe-tangent_to_curve.cpp
@@ -16,8 +16,6 @@
 #include <glibmm/i18n.h>
 
 #include "live_effects/lpe-tangent_to_curve.h"
-// FIXME: The following are only needed to convert the path's SPCurve* to pwd2.
-//        There must be a more convenient way to achieve this.
 #include "sp-path.h"
 #include "display/curve.h"
 
@@ -108,13 +106,13 @@ LPETangentToCurve::addKnotHolderEntities(KnotHolder *knotholder, SPDesktop *desk
     {
         KnotHolderEntity *e = new TtC::KnotHolderEntityLeftEnd(this);
         e->create( desktop, item, knotholder, Inkscape::CTRL_TYPE_UNKNOWN,
-                    _("Adjust the \"left\" end of the tangent") );
+                    _("Adjust the <b>left</b> end of the tangent") );
         knotholder->add(e);
     }
     {
         KnotHolderEntity *e = new TtC::KnotHolderEntityRightEnd(this);
         e->create( desktop, item, knotholder, Inkscape::CTRL_TYPE_UNKNOWN,
-                   _("Adjust the \"right\" end of the tangent") );
+                   _("Adjust the <b>right</b> end of the tangent") );
         knotholder->add(e);
     }
 };
@@ -130,14 +128,13 @@ KnotHolderEntityAttachPt::knot_set(Geom::Point const &p, Geom::Point const &/*or
 
     Geom::Point const s = snap_knot_position(p, state);
 
-    // FIXME: There must be a better way of converting the path's SPCurve* to pwd2.
-    SPCurve *curve = SP_PATH(item)->get_curve_for_edit();
-    Geom::PathVector pathv = curve->get_pathvector();
-    Piecewise<D2<SBasis> > pwd2;
-    for (unsigned int i=0; i < pathv.size(); i++) {
-        pwd2.concat(pathv[i].toPwSb());
+    if ( !SP_IS_SHAPE(lpe->sp_lpe_item) ) {
+        //lpe->t_attach.param_set_value(0);
+        g_warning("LPEItem is not a path! %s:%d\n", __FILE__, __LINE__);
+        return;
     }
-
+    Piecewise<D2<SBasis> > pwd2 = paths_to_pw( lpe->pathvector_before_effect );
+    
     double t0 = nearest_point(s, pwd2);
     lpe->t_attach.param_set_value(t0);
 
diff --git a/src/live_effects/lpe-tangent_to_curve.h b/src/live_effects/lpe-tangent_to_curve.h
index 309afc14b..8e44c01d1 100644
--- a/src/live_effects/lpe-tangent_to_curve.h
+++ b/src/live_effects/lpe-tangent_to_curve.h
@@ -34,7 +34,6 @@ class LPETangentToCurve : public Effect {
 public:
     LPETangentToCurve(LivePathEffectObject *lpeobject);
     virtual ~LPETangentToCurve();
-
     virtual Geom::Piecewise<Geom::D2<Geom::SBasis> >
       doEffect_pwd2 (Geom::Piecewise<Geom::D2<Geom::SBasis> > const & pwd2_in);
 
diff --git a/src/live_effects/lpe-taperstroke.cpp b/src/live_effects/lpe-taperstroke.cpp
index dc70782c4..901abff70 100644
--- a/src/live_effects/lpe-taperstroke.cpp
+++ b/src/live_effects/lpe-taperstroke.cpp
@@ -235,9 +235,11 @@ Geom::Path return_at_first_cusp (Geom::Path const & path_in, double smooth_toler
         }
 
         //clearly it's collinear if two occupy the same point
-        g_assert(!are_near(start_point, cross_point, 0.0000001));
-        g_assert(!are_near(cross_point, end_point,   0.0000001));
-        g_assert(!are_near(start_point, end_point,   0.0000001));
+        if (are_near(start_point, cross_point, 0.0000001) ||
+            are_near(cross_point, end_point,   0.0000001) ||
+            are_near(start_point, end_point,   0.0000001)  ) {
+                g_critical("Holy crap, something went wrong! %s:%d\n", __FILE__, __LINE__);
+            }
 
         if (!are_collinear(start_point, cross_point, end_point, smooth_tolerance))
             break;
@@ -533,12 +535,6 @@ Geom::Piecewise<Geom::D2<Geom::SBasis> > stretch_along(Geom::Piecewise<Geom::D2<
         double xspace  = 0;
         double noffset = 0;
         double toffset = 0;
-        /*if (prop_units.get_value() && pattBndsY){
-            xspace  *= pattBndsX->extent();
-            noffset *= pattBndsY->extent();
-            toffset *= pattBndsX->extent();
-        }*/
-
         //Prevent more than 90% overlap...
         if (xspace < -pattBndsX->extent()*.9) {
             xspace = -pattBndsX->extent()*.9;
@@ -587,12 +583,6 @@ Geom::Piecewise<Geom::D2<Geom::SBasis> > stretch_along(Geom::Piecewise<Geom::D2<
                 offs+=pattWidth;
             }
         }
-        /*if (false){
-            pre_output = fuse_nearby_ends(pre_output, fuse_tolerance);
-            for (unsigned i=0; i<pre_output.size(); i++){
-                output.concat(pre_output[i]);
-            }
-        }*/
         return output;
     } else {
         return pwd2_in;
@@ -656,19 +646,27 @@ void KnotHolderEntityAttachBegin::knot_set(Geom::Point const &p, Geom::Point con
 
     Geom::Point const s = snap_knot_position(p, state);
 
-    SPCurve *curve = SP_PATH(item)->get_curve_for_edit();
-    if (!curve) {
+    if (!SP_IS_SHAPE(lpe->sp_lpe_item) ) {
+        g_warning("LPEItem is not a path! %s:%d\n", __FILE__, __LINE__);
+        return;
+    }
+    
+    SPCurve* curve;
+    if ( !(curve = SP_SHAPE(lpe->sp_lpe_item)->getCurve()) ) {
         //oops
-        lpe->attach_start.param_set_value(0);
+        //lpe->attach_start.param_set_value(0);
         return;
     }
-    Geom::PathVector pathv = curve->get_pathvector();
+    //in case you are wondering, the above are simply sanity checks. we never want to actually
+    //use that object.
+    
+    Geom::PathVector pathv = lpe->pathvector_before_effect;
+    
     Piecewise<D2<SBasis> > pwd2;
     Geom::Path p_in = return_at_first_cusp(pathv[0]);
     pwd2.concat(p_in.toPwSb());
-    std::vector<Geom::Piecewise<Geom::D2<Geom::SBasis> > > pwd_vec = split_at_discontinuities(pwd2);
 
-    double t0 = nearest_point(s, pwd_vec[0]);
+    double t0 = nearest_point(s, pwd2);
     lpe->attach_start.param_set_value(t0);
 
     // FIXME: this should not directly ask for updating the item. It should write to SVG, which triggers updating.
@@ -682,22 +680,24 @@ void KnotHolderEntityAttachEnd::knot_set(Geom::Point const &p, Geom::Point const
 
     Geom::Point const s = snap_knot_position(p, state);
 
-    SPCurve *curve = SP_PATH(item)->get_curve_for_edit();
-    if (!curve) {
+    if (!SP_IS_SHAPE(lpe->sp_lpe_item) ) {
+        g_warning("LPEItem is not a path! %s:%d\n", __FILE__, __LINE__);
+        return;
+    }
+    
+    SPCurve* curve;
+    if ( !(curve = SP_SHAPE(lpe->sp_lpe_item)->getCurve()) ) {
         //oops
-        lpe->attach_end.param_set_value(0);
+        //lpe->attach_end.param_set_value(0);
         return;
     }
-    Geom::PathVector pathv = curve->get_pathvector();
-    Piecewise<D2<SBasis> > pwd2;
+    Geom::PathVector pathv = lpe->pathvector_before_effect;
     Geom::Path p_in = return_at_first_cusp(pathv[0].reverse());
-    pwd2.concat(p_in.toPwSb());
-    std::vector<Geom::Piecewise<Geom::D2<Geom::SBasis> > > pwd_vec = split_at_discontinuities(pwd2);
-
-    double t0 = nearest_point(s, pwd_vec[0]);
+    Piecewise<D2<SBasis> > pwd2 = p_in.toPwSb();
+    
+    double t0 = nearest_point(s, pwd2);
     lpe->attach_end.param_set_value(t0);
 
-    // FIXME: this should not directly ask for updating the item. It should write to SVG, which triggers updating.
     sp_lpe_item_update_patheffect (SP_LPE_ITEM(item), false, true);
 }
 Geom::Point KnotHolderEntityAttachBegin::knot_get() const
diff --git a/src/snap.cpp b/src/snap.cpp
index ecf799cdc..c3891baea 100644
--- a/src/snap.cpp
+++ b/src/snap.cpp
@@ -116,7 +116,7 @@ void SnapManager::freeSnapReturnByRef(Geom::Point &p,
                                       Inkscape::SnapSourceType const source_type,
                                       Geom::OptRect const &bbox_to_snap) const
 {
-    Inkscape::SnappedPoint const s = freeSnap(Inkscape::SnapCandidatePoint(p, source_type), bbox_to_snap);
+    Inkscape::SnappedPoint const s = freeSnap(Inkscape::SnapCandidatePoint(p, source_type, Inkscape::SNAPTARGET_PATH), bbox_to_snap);
     s.getPointIfSnapped(p);
 }
 
diff --git a/src/sp-lpe-item.cpp b/src/sp-lpe-item.cpp
index eda34a0b5..712a0d579 100644
--- a/src/sp-lpe-item.cpp
+++ b/src/sp-lpe-item.cpp
@@ -242,7 +242,7 @@ bool SPLPEItem::performPathEffect(SPCurve *curve) {
 
                 // Groups have their doBeforeEffect called elsewhere
                 if (!SP_IS_GROUP(this)) {
-                    lpe->doBeforeEffect(this);
+                    lpe->doBeforeEffect_impl(this);
                 }
 
                 try {
@@ -414,7 +414,7 @@ void SPLPEItem::addPathEffect(gchar *value, bool reset)
             }
 
             // perform this once when the effect is applied
-            lpe->doOnApply(this);
+            lpe->doOnApply_impl(this);
 
             // indicate that all necessary preparations are done and the effect can be performed
             lpe->setReady();