Fixing LPE recalculations warnings on update

15 files changed, 75 insertions, 42 deletions

diff --git a/src/live_effects/lpe-bendpath.cpp b/src/live_effects/lpe-bendpath.cpp
index b9821cc9c..11694249a 100644
--- a/src/live_effects/lpe-bendpath.cpp
+++ b/src/live_effects/lpe-bendpath.cpp
@@ -166,7 +166,11 @@ LPEBendPath::resetDefaults(SPItem const* item)
     bend_path.set_new_value( path.toPwSb(), true );
 }
 
-
+void
+LPEBendPath::transform_multiply(Geom::Affine const& postmul, bool set)
+{
+    //block parameters be transformed because shapes with bend store transform in the element
+}
 
 void
 LPEBendPath::addCanvasIndicators(SPLPEItem const */*lpeitem*/, std::vector<Geom::PathVector> &hp_vec)
diff --git a/src/live_effects/lpe-bendpath.h b/src/live_effects/lpe-bendpath.h
index c028f0b98..18389391f 100644
--- a/src/live_effects/lpe-bendpath.h
+++ b/src/live_effects/lpe-bendpath.h
@@ -44,7 +44,9 @@ public:
     virtual void resetDefaults(SPItem const* item);
 
     void addCanvasIndicators(SPLPEItem const */*lpeitem*/, std::vector<Geom::PathVector> &hp_vec);
-
+    
+    virtual void transform_multiply(Geom::Affine const& postmul, bool set);
+    
     virtual void addKnotHolderEntities(KnotHolder * knotholder, SPItem * item);
 
     PathParam bend_path;
diff --git a/src/object/box3d-side.cpp b/src/object/box3d-side.cpp
index c88c7a7b9..b6b9bbbf7 100644
--- a/src/object/box3d-side.cpp
+++ b/src/object/box3d-side.cpp
@@ -196,7 +196,12 @@ void Box3DSide::set_shape() {
      * This is very important for LPEs to work properly! (the bbox might be recalculated depending on the curve in shape)*/
     SPCurve * before = this->getCurveBeforeLPE();
     if (before || this->hasPathEffectRecursive()) {
-        this->setCurveBeforeLPE(c);
+        if (c && before && before->get_pathvector() != c->get_pathvector()){
+            this->setCurveBeforeLPE(c);
+            sp_lpe_item_update_patheffect(this, true, false);
+        } else {
+            this->setCurveBeforeLPE(c);
+        }
     } else {
         this->setCurveInsync(c);
     }
diff --git a/src/object/sp-ellipse.cpp b/src/object/sp-ellipse.cpp
index 76145c603..74ade912c 100644
--- a/src/object/sp-ellipse.cpp
+++ b/src/object/sp-ellipse.cpp
@@ -481,10 +481,16 @@ void SPGenericEllipse::set_shape()
      * This is very important for LPEs to work properly! (the bbox might be recalculated depending on the curve in shape)*/
     SPCurve * before = this->getCurveBeforeLPE();
     if (before || this->hasPathEffectRecursive()) {
-        this->setCurveBeforeLPE(c);
+        if (c && before && before->get_pathvector() != c->get_pathvector()){
+            this->setCurveBeforeLPE(c);
+            sp_lpe_item_update_patheffect(this, true, false);
+        } else {
+            this->setCurveBeforeLPE(c);
+        }
     } else {
         this->setCurveInsync(c);
     }
+
     if (before) {
         before->unref();
     }
@@ -555,7 +561,7 @@ Geom::Affine SPGenericEllipse::set_transform(Geom::Affine const &xform)
 
     // Adjust livepatheffect
     this->adjust_livepatheffect(xform);
-
+    
     return ret;
 }
 
@@ -650,7 +656,7 @@ bool SPGenericEllipse::set_elliptical_path_attribute(Inkscape::XML::Node *repr)
     // Make sure our pathvector is up to date.
     this->set_shape();
 
-    if (_curve != NULL) {
+    if (_curve) {
         gchar* d = sp_svg_write_path(_curve->get_pathvector());
 
         repr->setAttribute("d", d);
diff --git a/src/object/sp-item.cpp b/src/object/sp-item.cpp
index accf110ad..5ca22cabd 100644
--- a/src/object/sp-item.cpp
+++ b/src/object/sp-item.cpp
@@ -1542,7 +1542,7 @@ void SPItem::doWriteTransform(Geom::Affine const &transform, Geom::Affine const
     updateRepr();
 
     if (lpeitem && lpeitem->hasPathEffectRecursive()) {
-        sp_lpe_item_update_patheffect(lpeitem, false, false);
+        sp_lpe_item_update_patheffect(lpeitem, true, false);
     }
 
     // send the relative transform with a _transformed_signal
diff --git a/src/object/sp-lpe-item.cpp b/src/object/sp-lpe-item.cpp
index 7b90319ab..27ea45ca6 100644
--- a/src/object/sp-lpe-item.cpp
+++ b/src/object/sp-lpe-item.cpp
@@ -171,9 +171,9 @@ void SPLPEItem::update(SPCtx* ctx, unsigned int flags) {
 
 void SPLPEItem::modified(unsigned int flags) {
     //stop update when modified and make the effect update on the LPE transform method if the effect require it
-    if (SP_IS_GROUP(this) && (flags & SP_OBJECT_MODIFIED_FLAG) && (flags & SP_OBJECT_USER_MODIFIED_FLAG_B)) {
-        this->update_patheffect(false);
-    }
+    //if (SP_IS_GROUP(this) && (flags & SP_OBJECT_MODIFIED_FLAG) && (flags & SP_OBJECT_USER_MODIFIED_FLAG_B)) {
+    //  sp_lpe_item_update_patheffect(this, true, false);
+    //}
 }
 
 Inkscape::XML::Node* SPLPEItem::write(Inkscape::XML::Document *xml_doc, Inkscape::XML::Node *repr, guint flags) {
diff --git a/src/object/sp-path.cpp b/src/object/sp-path.cpp
index 4d1f1311f..de31b45e2 100644
--- a/src/object/sp-path.cpp
+++ b/src/object/sp-path.cpp
@@ -280,7 +280,7 @@ Inkscape::XML::Node* SPPath::write(Inkscape::XML::Document *xml_doc, Inkscape::X
 g_message("sp_path_write writes 'd' attribute");
 #endif
 
-    if ( this->_curve != NULL ) {
+    if (this->_curve) {
         gchar *str = sp_svg_write_path(this->_curve->get_pathvector());
         repr->setAttribute("d", str);
         g_free(str);
diff --git a/src/object/sp-shape.cpp b/src/object/sp-shape.cpp
index df8a200d0..1708ecfb3 100644
--- a/src/object/sp-shape.cpp
+++ b/src/object/sp-shape.cpp
@@ -412,6 +412,9 @@ void SPShape::modified(unsigned int flags) {
             }
         }
     }
+    if (!this->getCurve()) {
+        sp_lpe_item_update_patheffect(this, true, false);
+    }
 }
 
 Geom::OptRect SPShape::bbox(Geom::Affine const &transform, SPItem::BBoxType bboxtype) const {
diff --git a/src/object/sp-spiral.cpp b/src/object/sp-spiral.cpp
index 40abff7d7..e17ddb10d 100644
--- a/src/object/sp-spiral.cpp
+++ b/src/object/sp-spiral.cpp
@@ -349,7 +349,12 @@ void SPSpiral::set_shape() {
      * This is very important for LPEs to work properly! (the bbox might be recalculated depending on the curve in shape)*/
     SPCurve * before = this->getCurveBeforeLPE();
     if (before || this->hasPathEffectRecursive()) {
-        this->setCurveBeforeLPE(c);
+        if (c && before && before->get_pathvector() != c->get_pathvector()){
+            this->setCurveBeforeLPE(c);
+            sp_lpe_item_update_patheffect(this, true, false);
+        } else {
+            this->setCurveBeforeLPE(c);
+        }
     } else {
         this->setCurveInsync(c);
     }
@@ -400,6 +405,17 @@ void SPSpiral::snappoints(std::vector<Inkscape::SnapCandidatePoint> &p, Inkscape
  */
 Geom::Affine SPSpiral::set_transform(Geom::Affine const &xform)
 {
+    if (hasPathEffect() && pathEffectsEnabled() && 
+        (this->hasPathEffectOfType(Inkscape::LivePathEffect::CLONE_ORIGINAL) || 
+         this->hasPathEffectOfType(Inkscape::LivePathEffect::BEND_PATH) || 
+         this->hasPathEffectOfType(Inkscape::LivePathEffect::FILL_BETWEEN_MANY) ||
+         this->hasPathEffectOfType(Inkscape::LivePathEffect::FILL_BETWEEN_STROKES) ) )
+    {
+        // if path has this LPE applied, don't write the transform to the pathdata, but write it 'unoptimized'
+        // also if the effect is type BEND PATH to fix bug #179842
+        this->adjust_livepatheffect(xform);
+        return xform;
+    }
     // Only set transform with proportional scaling
     if (!xform.withoutTranslation().isUniformScale()) {
         // Adjust livepatheffect
diff --git a/src/object/sp-star.cpp b/src/object/sp-star.cpp
index 1be7651f8..da00080a3 100644
--- a/src/object/sp-star.cpp
+++ b/src/object/sp-star.cpp
@@ -82,11 +82,13 @@ Inkscape::XML::Node* SPStar::write(Inkscape::XML::Document *xml_doc, Inkscape::X
     }
 
     this->set_shape();
-
-    char *d = sp_svg_write_path (this->_curve->get_pathvector());
-    repr->setAttribute("d", d);
-    g_free(d);
-
+    if (this->_curve) {
+        char *d = sp_svg_write_path (this->_curve->get_pathvector());
+        repr->setAttribute("d", d);
+        g_free(d);
+    } else {
+        repr->setAttribute("d", NULL);
+    }
     // CPPIFY: see header file
     SPShape::write(xml_doc, repr, flags);
 
@@ -432,7 +434,12 @@ void SPStar::set_shape() {
      * This is very important for LPEs to work properly! (the bbox might be recalculated depending on the curve in shape)*/
     SPCurve * before = this->getCurveBeforeLPE();
     if (before || this->hasPathEffectRecursive()) {
-        this->setCurveBeforeLPE(c);
+        if (c && before && before->get_pathvector() != c->get_pathvector()){
+            this->setCurveBeforeLPE(c);
+            sp_lpe_item_update_patheffect(this, true, false);
+        } else {
+            this->setCurveBeforeLPE(c);
+        }
     } else {
         this->setCurveInsync(c);
     }
@@ -484,6 +491,17 @@ void SPStar::snappoints(std::vector<Inkscape::SnapCandidatePoint> &p, Inkscape::
 Geom::Affine SPStar::set_transform(Geom::Affine const &xform)
 {
     bool opt_trans = (randomized == 0);
+    if (hasPathEffect() && pathEffectsEnabled() && 
+        (this->hasPathEffectOfType(Inkscape::LivePathEffect::CLONE_ORIGINAL) || 
+         this->hasPathEffectOfType(Inkscape::LivePathEffect::BEND_PATH) || 
+         this->hasPathEffectOfType(Inkscape::LivePathEffect::FILL_BETWEEN_MANY) ||
+         this->hasPathEffectOfType(Inkscape::LivePathEffect::FILL_BETWEEN_STROKES) ) )
+    {
+        // if path has this LPE applied, don't write the transform to the pathdata, but write it 'unoptimized'
+        // also if the effect is type BEND PATH to fix bug #179842
+        this->adjust_livepatheffect(xform);
+        return xform;
+    }
     // Only set transform with proportional scaling
     if (!xform.withoutTranslation().isUniformScale()) {
         // Adjust livepatheffect
diff --git a/src/ui/tools/arc-tool.cpp b/src/ui/tools/arc-tool.cpp
index c7243302b..34b29f3bb 100644
--- a/src/ui/tools/arc-tool.cpp
+++ b/src/ui/tools/arc-tool.cpp
@@ -424,10 +424,7 @@ void ArcTool::finishItem() {
         desktop->canvas->endForcedFullRedraws();
 
         desktop->getSelection()->set(this->arc);
-        if (this->arc->hasPathEffectRecursive()) {
-            this->arc->set_shape();
-            sp_lpe_item_update_patheffect (this->arc, true, false);
-        }
+
         DocumentUndo::done(desktop->getDocument(), SP_VERB_CONTEXT_ARC, _("Create ellipse"));
 
         this->arc = NULL;
diff --git a/src/ui/tools/eraser-tool.cpp b/src/ui/tools/eraser-tool.cpp
index c95cc8b47..6d1c24f07 100644
--- a/src/ui/tools/eraser-tool.cpp
+++ b/src/ui/tools/eraser-tool.cpp
@@ -835,13 +835,8 @@ void EraserTool::set_to_accumulated() {
                 selection->clear();
 
                 if ( wasSelection ) {
-                    for (std::vector<SPItem*>::const_iterator j = remainingItems.begin(); j != remainingItems.end(); ++j){
-                        SPItem * item = *j;
-                        SPLPEItem * lpeitem = dynamic_cast<SPLPEItem *>(item);
-                        if (lpeitem->hasPathEffectRecursive()) {
-                            sp_lpe_item_update_patheffect (lpeitem, true, false);
-                        }
-                        selection->add(item);
+                    if ( !remainingItems.empty() ) {
+                        selection->add(remainingItems.begin(), remainingItems.end());
                     }
                 }
             }
diff --git a/src/ui/tools/freehand-base.cpp b/src/ui/tools/freehand-base.cpp
index 974a97995..a9b3ff8f5 100644
--- a/src/ui/tools/freehand-base.cpp
+++ b/src/ui/tools/freehand-base.cpp
@@ -588,10 +588,6 @@ static void spdc_check_for_and_apply_waiting_LPE(FreehandBase *dc, SPItem *item,
             sp_repr_css_attr_unref(css);
             return;
         }
-        SPLPEItem * lpeitem = dynamic_cast<SPLPEItem *>(item);
-        if (!lpeitem->hasPathEffect() && lpeitem->hasPathEffectRecursive()) {
-            sp_lpe_item_update_patheffect (lpeitem, true, false);
-        }
         if (dc->waiting_LPE_type != INVALID_LPE) {
             Effect::createAndApply(dc->waiting_LPE_type, dc->desktop->doc(), item);
             dc->waiting_LPE_type = INVALID_LPE;
diff --git a/src/ui/tools/spiral-tool.cpp b/src/ui/tools/spiral-tool.cpp
index ffca6ce4a..61ee1026b 100644
--- a/src/ui/tools/spiral-tool.cpp
+++ b/src/ui/tools/spiral-tool.cpp
@@ -399,11 +399,6 @@ void SpiralTool::finishItem() {
 
         this->desktop->getSelection()->set(this->spiral);
         
-        if (spiral->hasPathEffectRecursive()) {
-            this->spiral->set_shape();
-            sp_lpe_item_update_patheffect (this->spiral, true, false);
-        }
-        
         DocumentUndo::done(this->desktop->getDocument(), SP_VERB_CONTEXT_SPIRAL, _("Create spiral"));
 
         this->spiral = NULL;
diff --git a/src/ui/tools/star-tool.cpp b/src/ui/tools/star-tool.cpp
index 586b7df38..3a5615c3b 100644
--- a/src/ui/tools/star-tool.cpp
+++ b/src/ui/tools/star-tool.cpp
@@ -420,10 +420,6 @@ void StarTool::finishItem() {
         desktop->canvas->endForcedFullRedraws();
 
         desktop->getSelection()->set(this->star);
-        if (this->star->hasPathEffectRecursive()) {
-            this->star->set_shape();
-            sp_lpe_item_update_patheffect (this->star, true, false);
-        }
         DocumentUndo::done(desktop->getDocument(), SP_VERB_CONTEXT_STAR,
                            _("Create star"));