improbe UX of Fillet/Chamfer LPE

6 files changed, 21 insertions, 125 deletions

diff --git a/src/file.cpp b/src/file.cpp
index dd609de50..464454a65 100644
--- a/src/file.cpp
+++ b/src/file.cpp
@@ -217,8 +217,6 @@ bool sp_file_open(const Glib::ustring &uri,
     if (desktop) {
         desktop->setWaitingCursor();
     }
-    Inkscape::Preferences *prefs = Inkscape::Preferences::get();
-    prefs->setString("/options/openmethod/value", "open");
     SPDocument *doc = nullptr;
     bool cancelled = false;
     try {
@@ -231,7 +229,6 @@ bool sp_file_open(const Glib::ustring &uri,
         doc = nullptr;
         cancelled = true;
     }
-    prefs->setString("/options/openmethod/value", "done");
     if (desktop) {
         desktop->clearWaitingCursor();
     }
diff --git a/src/live_effects/effect.cpp b/src/live_effects/effect.cpp
index 1eb08a0e8..46767ac84 100644
--- a/src/live_effects/effect.cpp
+++ b/src/live_effects/effect.cpp
@@ -409,8 +409,7 @@ Effect::Effect(LivePathEffectObject *lpeobject)
     current_zoom = 0.0;
 }
 
-Effect::~Effect()
-= default;
+Effect::~Effect() = default;
 
 Glib::ustring
 Effect::getName() const
diff --git a/src/live_effects/lpe-fillet-chamfer.cpp b/src/live_effects/lpe-fillet-chamfer.cpp
index 6c07f246c..1981c6983 100644
--- a/src/live_effects/lpe-fillet-chamfer.cpp
+++ b/src/live_effects/lpe-fillet-chamfer.cpp
@@ -43,14 +43,12 @@ LPEFilletChamfer::LPEFilletChamfer(LivePathEffectObject *lpeobject)
              "method", FMConverter, &wr, this, FM_AUTO),
       mode(_("Mode:"), _("Mode, fillet or chamfer"),
              "mode", &wr, this, "F", true),
-      radius(_("Radius (unit or %):"), _("Radius, in unit or %"), "radius", &wr,
+      radius(_("Radius:"), _("Radius, in unit or %"), "radius", &wr,
              this, 0.0),
       chamfer_steps(_("Chamfer steps:"), _("Chamfer steps"), "chamfer_steps",
                     &wr, this, 1),
-      flexible(_("Flexible radius size (%)"), _("Flexible radius size (%)"),
+      flexible(_("Radius in %"), _("Flexible radius size (%)"),
                "flexible", &wr, this, false),
-      mirror_knots(_("Mirror Knots"), _("Mirror Knots"), "mirror_knots", &wr,
-                   this, true),
       only_selected(_("Change only selected nodes"),
                     _("Change only selected nodes"), "only_selected", &wr, this,
                     false),
@@ -61,8 +59,6 @@ LPEFilletChamfer::LPEFilletChamfer(LivePathEffectObject *lpeobject)
                  false),
       apply_no_radius(_("Apply changes if radius = 0"), _("Apply changes if radius = 0"), "apply_no_radius", &wr, this, true),
       apply_with_radius(_("Apply changes if radius > 0"), _("Apply changes if radius > 0"), "apply_with_radius", &wr, this, true),
-      helper_size(_("Helper path size with direction to node:"),
-                  _("Helper path size with direction to node"), "helper_size", &wr, this, 0),
       _pathvector_satellites(nullptr),
       _degenerate_hide(false)
 {
@@ -72,10 +68,8 @@ LPEFilletChamfer::LPEFilletChamfer(LivePathEffectObject *lpeobject)
     registerParameter(&mode);
     registerParameter(&radius);
     registerParameter(&chamfer_steps);
-    registerParameter(&helper_size);
     registerParameter(&flexible);
     registerParameter(&use_knot_distance);
-    registerParameter(&mirror_knots);
     registerParameter(&apply_no_radius);
     registerParameter(&apply_with_radius);
     registerParameter(&only_selected);
@@ -88,10 +82,14 @@ LPEFilletChamfer::LPEFilletChamfer(LivePathEffectObject *lpeobject)
     chamfer_steps.param_set_range(1, 999);
     chamfer_steps.param_set_increments(1, 1);
     chamfer_steps.param_set_digits(0);
-    helper_size.param_set_range(0, 999);
-    helper_size.param_set_increments(5, 5);
-    helper_size.param_set_digits(0);
     _provides_knotholder_entities = true;
+    previous_unit = NULL;
+}
+
+LPEFilletChamfer::~LPEFilletChamfer() {
+    if(previous_unit) {
+        g_free(previous_unit);
+    }
 }
 
 void LPEFilletChamfer::doOnApply(SPLPEItem const *lpeItem)
@@ -131,7 +129,7 @@ void LPEFilletChamfer::doOnApply(SPLPEItem const *lpeItem)
                 satellite.setSteps(chamfer_steps);
                 satellite.setAmount(power);
                 satellite.setIsTime(flexible);
-                satellite.setHasMirror(mirror_knots);
+                satellite.setHasMirror(true);
                 satellite.setHidden(hide_knots);
                 subpath_satellites.push_back(satellite);
             }
@@ -144,7 +142,7 @@ void LPEFilletChamfer::doOnApply(SPLPEItem const *lpeItem)
                 satellite.setSteps(chamfer_steps);
                 satellite.setAmount(power);
                 satellite.setIsTime(flexible);
-                satellite.setHasMirror(mirror_knots);
+                satellite.setHasMirror(true);
                 satellite.setHidden(hide_knots);
                 subpath_satellites.push_back(satellite);
             }
@@ -187,13 +185,6 @@ Gtk::Widget *LPEFilletChamfer::newWidget()
                     Gtk::Entry *entry_widget = dynamic_cast<Gtk::Entry *>(childList[1]);
                     entry_widget->set_width_chars(6);
                 }
-//            } else if (param->param_key == "unit") {
-//                Inkscape::UI::Widget::RegisteredUnitMenu* widg_registered = 
-//                    Gtk::manage(dynamic_cast< Inkscape::UI::Widget::RegisteredUnitMenu *>(widg));
-//                widg_registered->setUnit(unit.get_abbreviation());
-//                widg_registered->set_undo_parameters(SP_VERB_DIALOG_LIVE_PATH_EFFECT, _("Change unit parameter"));
-//                widg_registered->getUnitMenu()->signal_changed().connect(sigc::mem_fun(*this, &LPEFilletChamfer::convertUnit));
-//                widg = widg_registered;
             } else if (param->param_key == "chamfer_steps") {
                 Inkscape::UI::Widget::Scalar *widg_registered =
                     Gtk::manage(dynamic_cast<Inkscape::UI::Widget::Scalar *>(widg));
@@ -206,11 +197,6 @@ Gtk::Widget *LPEFilletChamfer::newWidget()
                     Gtk::Entry *entry_widget = dynamic_cast<Gtk::Entry *>(childList[1]);
                     entry_widget->set_width_chars(3);
                 }
-            } else if (param->param_key == "helper_size") {
-                Inkscape::UI::Widget::Scalar *widg_registered =
-                    Gtk::manage(dynamic_cast<Inkscape::UI::Widget::Scalar *>(widg));
-                widg_registered->signal_value_changed().connect(
-                    sigc::mem_fun(*this, &LPEFilletChamfer::refreshKnots));
             } else if (param->param_key == "only_selected") {
                 Gtk::manage(widg);
             }
@@ -280,15 +266,6 @@ void LPEFilletChamfer::updateAmount()
     satellites_param.setPathVectorSatellites(_pathvector_satellites);
 }
 
-//void LPEFilletChamfer::convertUnit()
-//{
-//    SPDocument * document = SP_ACTIVE_DOCUMENT;
-//    SPNamedView *nv = sp_document_namedview(document, NULL);
-//    Glib::ustring display_unit = nv->display_units->abbr;
-//    _pathvector_satellites->convertUnit(unit.get_abbreviation(), display_unit, apply_no_radius, apply_with_radius);
-//    satellites_param.setPathVectorSatellites(_pathvector_satellites);
-//}
-
 void LPEFilletChamfer::updateChamferSteps()
 {
     setSelected(_pathvector_satellites);
@@ -363,7 +340,7 @@ void LPEFilletChamfer::doBeforeEffect(SPLPEItem const *lpeItem)
                 satellite.setSteps(chamfer_steps);
                 satellite.setAmount(power);
                 satellite.setIsTime(flexible);
-                satellite.setHasMirror(mirror_knots);
+                satellite.setHasMirror(true);
                 satellite.setHidden(hide_knots);
                 _pathvector_satellites->recalculateForNewPathVector(pathv, satellite);
                 satellites = _pathvector_satellites->getSatellites();
@@ -375,12 +352,7 @@ void LPEFilletChamfer::doBeforeEffect(SPLPEItem const *lpeItem)
         } else {
             satellites_param.setGlobalKnotHide(false);
         }
-        if (hide_knots) {
-            satellites_param.setHelperSize(0);
-        } else {
-            satellites_param.setHelperSize(helper_size);
-        }
-        for (size_t i = 0; i < satellites.size(); ++i) {
+         for (size_t i = 0; i < satellites.size(); ++i) {
             for (size_t j = 0; j < satellites[i].size(); ++j) {
                 if (j >= pathv[i].size()) {
                     continue;
@@ -397,9 +369,6 @@ void LPEFilletChamfer::doBeforeEffect(SPLPEItem const *lpeItem)
                         satellites[i][j].amount = size;
                     }
                 }
-                if (satellites[i][j].has_mirror != mirror_knots) {
-                    satellites[i][j].has_mirror = mirror_knots;
-                }
                 satellites[i][j].hidden = hide_knots;
                 if (only_selected && isNodePointSelected(curve_in.initialPoint()) ){
                     satellites[i][j].setSelected(true);
@@ -413,6 +382,11 @@ void LPEFilletChamfer::doBeforeEffect(SPLPEItem const *lpeItem)
         _pathvector_satellites->setSatellites(satellites);
         satellites_param.setPathVectorSatellites(_pathvector_satellites, write);
         refreshKnots();
+        gchar const *current_unit = unit.get_abbreviation();
+        if (previous_unit && strcmp(previous_unit, current_unit) ) {
+            updateAmount();
+        }
+        previous_unit = const_cast<gchar *>(current_unit);
     } else {
         g_warning("LPE Fillet can only be applied to shapes (not groups).");
     }
diff --git a/src/live_effects/lpe-fillet-chamfer.h b/src/live_effects/lpe-fillet-chamfer.h
index 46887255e..4ec882799 100644
--- a/src/live_effects/lpe-fillet-chamfer.h
+++ b/src/live_effects/lpe-fillet-chamfer.h
@@ -33,6 +33,7 @@ enum Filletmethod {
 class LPEFilletChamfer : public Effect {
 public:
     LPEFilletChamfer(LivePathEffectObject *lpeobject);
+    ~LPEFilletChamfer();
     void doBeforeEffect(SPLPEItem const *lpeItem) override;
     Geom::PathVector doEffect_path(Geom::PathVector const &path_in) override;
     void doOnApply(SPLPEItem const *lpeItem) override;
@@ -56,17 +57,15 @@ private:
     ScalarParam chamfer_steps;
     BoolParam flexible;
     HiddenParam mode;
-    BoolParam mirror_knots;
     BoolParam only_selected;
     BoolParam use_knot_distance;
     BoolParam hide_knots;
     BoolParam apply_no_radius;
     BoolParam apply_with_radius;
-    ScalarParam helper_size;
     bool _degenerate_hide;
     PathVectorSatellites *_pathvector_satellites;
     Geom::PathVector _hp;
-
+    gchar * previous_unit;
     LPEFilletChamfer(const LPEFilletChamfer &);
     LPEFilletChamfer &operator=(const LPEFilletChamfer &);
 
diff --git a/src/live_effects/parameter/satellitesarray.cpp b/src/live_effects/parameter/satellitesarray.cpp
index 10ad0df45..b1e00b349 100644
--- a/src/live_effects/parameter/satellitesarray.cpp
+++ b/src/live_effects/parameter/satellitesarray.cpp
@@ -31,7 +31,6 @@ SatellitesArrayParam::SatellitesArrayParam(const Glib::ustring &label,
     _knot_shape = SP_KNOT_SHAPE_DIAMOND;
     _knot_mode = SP_KNOT_MODE_XOR;
     _knot_color = 0xAAFF8800;
-    _helper_size = 0;
     _use_distance = false;
     _global_knot_hide = false;
     _current_zoom = 0;
@@ -79,12 +78,6 @@ void SatellitesArrayParam::setEffectType(EffectType et)
     _effectType = et;
 }
 
-void SatellitesArrayParam::setHelperSize(int hs)
-{
-    _helper_size = hs;
-    updateCanvasIndicators();
-}
-
 void SatellitesArrayParam::updateCanvasIndicators(bool mirror)
 {
     if (!_last_pathvector_satellites) {
@@ -141,63 +134,9 @@ void SatellitesArrayParam::updateCanvasIndicators(bool mirror)
                 if (pos <= 0 || pos >= 1) {
                     continue;
                 }
-                Geom::Point point_a = curve_in->pointAt(pos);
-                Geom::Point deriv_a = unit_vector(derivative(curve_in->toSBasis()).pointAt(pos));
-                Geom::Rotate rot(Geom::Rotate::from_degrees(-90));
-                deriv_a = deriv_a * rot;
-                Geom::Point point_c = point_a - deriv_a * _helper_size;
-                Geom::Point point_d = point_a + deriv_a * _helper_size;
-                Geom::Ray ray_1(point_c, point_d);
-                char const *svgd = "M 1,0.25 0.5,0 1,-0.25 M 1,0.5 0,0 1,-0.5";
-                Geom::PathVector pathv = sp_svg_read_pathv(svgd);
-                Geom::Affine aff = Geom::Affine();
-                aff *= Geom::Scale(_helper_size);
-                if (mirror) {
-                    aff *= Geom::Rotate(ray_1.angle() - Geom::rad_from_deg(90));
-                } else {
-                    aff *= Geom::Rotate(ray_1.angle() - Geom::rad_from_deg(270));
-                }
-                aff *= Geom::Translate(curve_in->pointAt(pos));
-                pathv *= aff;
-                _hp.push_back(pathv[0]);
-                _hp.push_back(pathv[1]);
-                if (overflow) {
-                    double diameter = _helper_size;
-                    if (_helper_size == 0) {
-                        diameter = 15;
-                        char const *svgd;
-                        svgd = "M 0.7,0.35 A 0.35,0.35 0 0 1 0.35,0.7 0.35,0.35 0 0 1 0,0.35 "
-                               "0.35,0.35 0 0 1 0.35,0 0.35,0.35 0 0 1 0.7,0.35 Z";
-                        Geom::PathVector pathv = sp_svg_read_pathv(svgd);
-                        aff = Geom::Affine();
-                        aff *= Geom::Scale(diameter);
-                        aff *= Geom::Translate(point_a - Geom::Point(diameter * 0.35, diameter * 0.35));
-                        pathv *= aff;
-                        _hp.push_back(pathv[0]);
-                    } else {
-                        char const *svgd;
-                        svgd = "M 0 -1.32 A 1.32 1.32 0 0 0 -1.32 0 A 1.32 1.32 0 0 0 0 1.32 A "
-                               "1.32 1.32 0 0 0 1.18 0.59 L 0 0 L 1.18 -0.59 A 1.32 1.32 0 0 0 "
-                               "0 -1.32 z";
-                        Geom::PathVector pathv = sp_svg_read_pathv(svgd);
-                        aff = Geom::Affine();
-                        aff *= Geom::Scale(_helper_size / 2.0);
-                        if (mirror) {
-                            aff *= Geom::Rotate(ray_1.angle() - Geom::rad_from_deg(90));
-                        } else {
-                            aff *= Geom::Rotate(ray_1.angle() - Geom::rad_from_deg(270));
-                        }
-                        aff *= Geom::Translate(curve_in->pointAt(pos));
-                        pathv *= aff;
-                        _hp.push_back(pathv[0]);
-                    }
-                }
             }
         }
     }
-    if (!_knot_reset_helper.empty()) {
-        _hp.insert(_hp.end(), _knot_reset_helper.begin(), _knot_reset_helper.end() );
-    }
     if (mirror) {
         updateCanvasIndicators(false);
     }
@@ -353,15 +292,6 @@ void FilletChamferKnotHolderEntity::knot_set(Geom::Point const &p,
     } else {
         satellite.setPosition(s, pathv[path_index][curve_index]);
     }
-    _pparam->_knot_reset_helper.clear();
-    if (satellite.amount == 0) {
-        char const *svgd;
-        svgd = "M -5.39,8.78 -9.13,5.29 -10.38,10.28 Z M -7.22,7.07 -3.43,3.37 m -1.95,-12.16 -3.74,3.5 -1.26,-5 z "
-               "m -1.83,1.71 3.78,3.7 M 5.24,8.78 8.98,5.29 10.24,10.28 Z "
-               "M 7.07,7.07 3.29,3.37 M 5.24,-8.78 l 3.74,3.5 1.26,-5 z M 7.07,-7.07 3.29,-3.37";
-        _pparam->_knot_reset_helper = sp_svg_read_pathv(svgd);
-        _pparam->_knot_reset_helper *= Geom::Affine(_pparam->_helper_size * 0.1,0,0,_pparam->_helper_size * 0.1,0,0) * Geom::Translate(pathv[path_index][curve_index].initialPoint());
-    }
     _pparam->_vector[path_index][curve_index] = satellite;
     sp_lpe_item_update_patheffect(SP_LPE_ITEM(item), false, false);
 }
diff --git a/src/live_effects/parameter/satellitesarray.h b/src/live_effects/parameter/satellitesarray.h
index 005ea63ff..459b499d3 100644
--- a/src/live_effects/parameter/satellitesarray.h
+++ b/src/live_effects/parameter/satellitesarray.h
@@ -40,7 +40,6 @@ public:
     {
         return nullptr;
     }
-    virtual void setHelperSize(int hs);
     void addKnotHolderEntities(KnotHolder *knotholder, SPItem *item) override;
     virtual void addKnotHolderEntities(KnotHolder *knotholder, SPItem *item, bool mirror);
     void addCanvasIndicators(SPLPEItem const *lpeitem, std::vector<Geom::PathVector> &hp_vec) override;
@@ -72,8 +71,6 @@ private:
     SPKnotModeType _knot_mode;
     guint32 _knot_color;
     Geom::PathVector _hp;
-    Geom::PathVector _knot_reset_helper;
-    int _helper_size;
     bool _use_distance;
     bool _global_knot_hide;
     double _current_zoom;