Fix for bug:#1659476; Horizontal slider in "Power Stroke" LPE not movable with mouse correctly

12 files changed, 47 insertions, 34 deletions

diff --git a/src/live_effects/effect.cpp b/src/live_effects/effect.cpp
index d1dd8d963..8aa59e4fe 100644
--- a/src/live_effects/effect.cpp
+++ b/src/live_effects/effect.cpp
@@ -398,7 +398,7 @@ Effect::Effect(LivePathEffectObject *lpeobject)
       concatenate_before_pwd2(false),
       sp_lpe_item(nullptr),
       current_zoom(1),
-      upd_params(true),
+      upd_params(false),
       current_shape(nullptr),
       provides_own_flash_paths(true), // is automatically set to false if providesOwnFlashPaths() is not overridden
       defaultsopen(false),
diff --git a/src/live_effects/parameter/enum.h b/src/live_effects/parameter/enum.h
index 0360a5f0a..cc100ad98 100644
--- a/src/live_effects/parameter/enum.h
+++ b/src/live_effects/parameter/enum.h
@@ -46,10 +46,14 @@ public:
 
         regenum->set_active_by_id(value);
         regenum->combobox()->setProgrammatically = false;
+        regenum->combobox()->signal_changed().connect(sigc::mem_fun (*this, &EnumParam::_on_change_combo));
         regenum->set_undo_parameters(SP_VERB_DIALOG_LIVE_PATH_EFFECT, _("Change enumeration parameter"));
+        
         return dynamic_cast<Gtk::Widget *> (regenum);
     };
-
+    void _on_change_combo() {
+        param_effect->upd_params = true;
+    }
     bool param_readSVGValue(const gchar * strvalue) override {
         if (!strvalue) {
             param_set_default();
@@ -89,9 +93,6 @@ public:
     }
     
     void param_set_value(E val) {
-        if (value != val) {
-            param_effect->upd_params = true;
-        }
         value = val;
     }
 
diff --git a/src/live_effects/parameter/parameter.cpp b/src/live_effects/parameter/parameter.cpp
index 1bdd31a0f..158db91a4 100644
--- a/src/live_effects/parameter/parameter.cpp
+++ b/src/live_effects/parameter/parameter.cpp
@@ -41,7 +41,6 @@ Parameter::Parameter( Glib::ustring  label, Glib::ustring  tip,
 void
 Parameter::param_write_to_repr(const char * svgd)
 {
-    param_effect->upd_params = true;
     param_effect->getRepr()->setAttribute(param_key.c_str(), svgd);
 }
 
@@ -128,9 +127,6 @@ ScalarParam::param_update_default(const gchar * default_value)
 void
 ScalarParam::param_set_value(gdouble val)
 {
-    if (value != val) {
-        param_effect->upd_params = true;
-    }
     value = val;
     if (integer)
         value = round(value);
@@ -182,7 +178,7 @@ ScalarParam::param_newWidget()
     if (widget_is_visible) {
         Inkscape::UI::Widget::RegisteredScalar *rsu = Gtk::manage( new Inkscape::UI::Widget::RegisteredScalar(
             param_label, param_tooltip, param_key, *param_wr, param_effect->getRepr(), param_effect->getSPDoc() ) );
-
+        
         rsu->setValue(value);
         rsu->setDigits(digits);
         rsu->setIncrements(inc_step, inc_page);
@@ -191,7 +187,6 @@ ScalarParam::param_newWidget()
         if (add_slider) {
             rsu->addSlider();
         }
-        rsu->signal_button_release_event().connect(sigc::mem_fun (*this, &ScalarParam::on_button_release));
         if(_set_undo){
             rsu->set_undo_parameters(SP_VERB_DIALOG_LIVE_PATH_EFFECT, _("Change scalar parameter"));
         }
@@ -201,11 +196,6 @@ ScalarParam::param_newWidget()
     }
 }
 
-bool ScalarParam::on_button_release(GdkEventButton* button_event) {
-    param_effect->upd_params = true;
-    return false;
-}
-
 void
 ScalarParam::param_set_digits(unsigned digits)
 {
diff --git a/src/live_effects/parameter/parameter.h b/src/live_effects/parameter/parameter.h
index 040955e17..db2b7e13f 100644
--- a/src/live_effects/parameter/parameter.h
+++ b/src/live_effects/parameter/parameter.h
@@ -141,8 +141,6 @@ protected:
     bool _set_undo;
 
 private:
-    bool on_button_release(GdkEventButton* button_event);
-
     ScalarParam(const ScalarParam&) = delete;
     ScalarParam& operator=(const ScalarParam&) = delete;
 };
diff --git a/src/live_effects/parameter/point.cpp b/src/live_effects/parameter/point.cpp
index 292b8fdac..3cdfd72c0 100644
--- a/src/live_effects/parameter/point.cpp
+++ b/src/live_effects/parameter/point.cpp
@@ -209,6 +209,7 @@ PointParamKnotHolderEntity::knot_set(Geom::Point const &p, Geom::Point const &or
             s = A;
         }
     }
+    pparam->param_effect->upd_params = true;
     if(this->pparam->liveupdate){
         pparam->param_setValue(s, true);
     } else {
diff --git a/src/live_effects/parameter/powerstrokepointarray.cpp b/src/live_effects/parameter/powerstrokepointarray.cpp
index 4afe35e47..772088e81 100644
--- a/src/live_effects/parameter/powerstrokepointarray.cpp
+++ b/src/live_effects/parameter/powerstrokepointarray.cpp
@@ -179,7 +179,7 @@ PowerStrokePointArrayParamKnotHolderEntity::knot_set(Geom::Point const &p, Geom:
     if (!valid_index(_index)) {
         return;
     }
-
+    _pparam->param_effect->upd_params = true;
     /// @todo how about item transforms???
     Piecewise<D2<SBasis> > const & pwd2 = _pparam->get_pwd2();
     Piecewise<D2<SBasis> > const & n = _pparam->get_pwd2_normal();
diff --git a/src/live_effects/parameter/satellitesarray.cpp b/src/live_effects/parameter/satellitesarray.cpp
index b1e00b349..808d76fcd 100644
--- a/src/live_effects/parameter/satellitesarray.cpp
+++ b/src/live_effects/parameter/satellitesarray.cpp
@@ -455,6 +455,7 @@ void FilletChamferKnotHolderEntity::knot_set_offset(Satellite satellite)
     if (!_pparam->_last_pathvector_satellites) {
         return;
     }
+    _pparam->param_effect->upd_params = true;
     size_t total_satellites = _pparam->_last_pathvector_satellites->getTotalSatellites();
     bool is_mirror = false;
     size_t index = _index;
diff --git a/src/live_effects/parameter/transformedpoint.cpp b/src/live_effects/parameter/transformedpoint.cpp
index e35cbc04d..d7e42e28b 100644
--- a/src/live_effects/parameter/transformedpoint.cpp
+++ b/src/live_effects/parameter/transformedpoint.cpp
@@ -120,7 +120,7 @@ TransformedPointParam::param_newWidget()
     pointwdg->setValue( vector, origin );
     pointwdg->clearProgrammatically();
     pointwdg->set_undo_parameters(SP_VERB_DIALOG_LIVE_PATH_EFFECT, _("Change vector parameter"));
-
+    
     Gtk::HBox * hbox = Gtk::manage( new Gtk::HBox() );
     static_cast<Gtk::HBox*>(hbox)->pack_start(*pointwdg, true, true);
     static_cast<Gtk::HBox*>(hbox)->show_all_children();
@@ -166,9 +166,11 @@ public:
     ~TransformedPointParamKnotHolderEntity_Vector() override = default;
 
     void knot_set(Geom::Point const &p, Geom::Point const &/*origin*/, guint /*state*/) override {
+        param->param_effect->upd_params = true;
         Geom::Point const s = p - param->origin;
         /// @todo implement angle snapping when holding CTRL
         param->setVector(s);
+        param->set_and_write_new_values(param->origin, param->vector);
         sp_lpe_item_update_patheffect(SP_LPE_ITEM(item), false, false);
     };
     Geom::Point knot_get() const override{
diff --git a/src/live_effects/parameter/unit.cpp b/src/live_effects/parameter/unit.cpp
index 9dbbaffac..d6c545f3b 100644
--- a/src/live_effects/parameter/unit.cpp
+++ b/src/live_effects/parameter/unit.cpp
@@ -90,7 +90,7 @@ UnitParam::param_newWidget()
 
     unit_menu->setUnit(unit->abbr);
     unit_menu->set_undo_parameters(SP_VERB_DIALOG_LIVE_PATH_EFFECT, _("Change unit parameter"));
-
+    
     return dynamic_cast<Gtk::Widget *> (unit_menu);
 }
 
diff --git a/src/live_effects/parameter/vector.cpp b/src/live_effects/parameter/vector.cpp
index 0e8948268..365608a36 100644
--- a/src/live_effects/parameter/vector.cpp
+++ b/src/live_effects/parameter/vector.cpp
@@ -173,8 +173,10 @@ public:
     ~VectorParamKnotHolderEntity_Origin() override = default;
 
     void knot_set(Geom::Point const &p, Geom::Point const &/*origin*/, guint state) override {
+        param->param_effect->upd_params = true;
         Geom::Point const s = snap_knot_position(p, state);
         param->setOrigin(s);
+        param->set_and_write_new_values(param->origin, param->vector);
         sp_lpe_item_update_patheffect(SP_LPE_ITEM(item), false, false);
     };
     Geom::Point knot_get() const override {
diff --git a/src/ui/dialog/livepatheffect-editor.cpp b/src/ui/dialog/livepatheffect-editor.cpp
index 512831f00..d6fe43b67 100644
--- a/src/ui/dialog/livepatheffect-editor.cpp
+++ b/src/ui/dialog/livepatheffect-editor.cpp
@@ -56,8 +56,8 @@ void lpeeditor_selection_changed (Inkscape::Selection * selection, gpointer data
 {
     LivePathEffectEditor *lpeeditor = static_cast<LivePathEffectEditor *>(data);
     lpeeditor->lpe_list_locked = false;
-    lpeeditor->lpe_changed = true;
     lpeeditor->onSelectionChanged(selection);
+    lpeeditor->_on_button_release(nullptr); //to force update widgets
 }
 
 void lpeeditor_selection_modified (Inkscape::Selection * selection, guint /*flags*/, gpointer data)
@@ -88,7 +88,6 @@ LivePathEffectEditor::LivePathEffectEditor()
     : UI::Widget::Panel("/dialogs/livepatheffect", SP_VERB_DIALOG_LIVE_PATH_EFFECT),
       deskTrack(),
       lpe_list_locked(false),
-      lpe_changed(true),
       effectwidget(nullptr),
       status_label("", Gtk::ALIGN_CENTER),
       effectcontrol_frame(""),
@@ -114,7 +113,10 @@ LivePathEffectEditor::LivePathEffectEditor()
 
     effectlist_vbox.pack_start(scrolled_window, Gtk::PACK_EXPAND_WIDGET);
     effectlist_vbox.pack_end(toolbar_hbox, Gtk::PACK_SHRINK);
-    effectcontrol_frame.add(effectcontrol_vbox);
+    effectcontrol_eventbox.add_events(Gdk::BUTTON_RELEASE_MASK);
+    effectcontrol_eventbox.signal_button_release_event().connect(sigc::mem_fun(*this, &LivePathEffectEditor::_on_button_release) );
+    effectcontrol_eventbox.add(effectcontrol_vbox);
+    effectcontrol_frame.add(effectcontrol_eventbox);
 
     button_add.set_tooltip_text(_("Add path effect"));
     lpe_style_button(button_add, INKSCAPE_ICON("list-add"));
@@ -196,11 +198,29 @@ LivePathEffectEditor::~LivePathEffectEditor()
     }
 }
 
+bool LivePathEffectEditor::_on_button_release(GdkEventButton* button_event) {
+    Glib::RefPtr<Gtk::TreeSelection> sel = effectlist_view.get_selection();
+    if (sel->count_selected_rows () == 0) {
+        return true;
+    }
+    Gtk::TreeModel::iterator it = sel->get_selected();
+    LivePathEffect::LPEObjectReference * lperef = (*it)[columns.lperef];
+    if (lperef && current_lpeitem && current_lperef != lperef) {
+        if (lperef->getObject()) {
+            LivePathEffect::Effect * effect = lperef->lpeobject->get_lpe();
+            if (effect) {
+                effect->upd_params = true;
+                showParams(*effect);
+            }
+        }
+    }
+    return true;
+}
+
 void
 LivePathEffectEditor::showParams(LivePathEffect::Effect& effect)
 {
-    if (!effect.upd_params && !lpe_changed) {
-        lpe_changed = false;
+    if (effectwidget && !effect.upd_params) {
         return;
     }
     if (effectwidget) {
@@ -211,14 +231,13 @@ LivePathEffectEditor::showParams(LivePathEffect::Effect& effect)
     effectwidget = effect.newWidget();
     effectcontrol_frame.set_label(effect.getName());
     effectcontrol_vbox.pack_start(*effectwidget, true, true);
-
+ 
     button_remove.show();
     status_label.hide();
     effectcontrol_frame.show();
     effectcontrol_vbox.show_all_children();
     // fixme: add resizing of dialog
     effect.upd_params = false;
-    lpe_changed = false;
 }
 
 void
@@ -277,9 +296,7 @@ LivePathEffectEditor::onSelectionChanged(Inkscape::Selection *sel)
             SPLPEItem *lpeitem = dynamic_cast<SPLPEItem *>(item);
             if ( lpeitem ) {
                 effect_list_reload(lpeitem);
-
                 current_lpeitem = lpeitem;
-
                 set_sensitize_all(true);
                 if ( lpeitem->hasPathEffect() ) {
                     Inkscape::LivePathEffect::Effect *lpe = lpeitem->getCurrentLPE();
@@ -550,7 +567,7 @@ void LivePathEffectEditor::on_effect_selection_changed()
             current_lperef = lperef;
             LivePathEffect::Effect * effect = lperef->lpeobject->get_lpe();
             if (effect) {
-                lpe_changed = true;
+                effect->upd_params = true;
                 showParams(*effect);
                 //To reload knots and helper paths
                 Inkscape::Selection *sel = _getSelection();
diff --git a/src/ui/dialog/livepatheffect-editor.h b/src/ui/dialog/livepatheffect-editor.h
index 26e278071..31513d2f9 100644
--- a/src/ui/dialog/livepatheffect-editor.h
+++ b/src/ui/dialog/livepatheffect-editor.h
@@ -19,6 +19,7 @@
 #include "ui/widget/frame.h"
 #include "live_effects/effect-enum.h"
 #include <gtkmm/liststore.h>
+#include <gtkmm/eventbox.h>
 #include <gtkmm/treeview.h>
 #include <gtkmm/scrolledwindow.h>
 #include <gtkmm/toolbar.h>
@@ -96,7 +97,6 @@ private:
     };
 
     bool lpe_list_locked;
-    bool lpe_changed;
     //Inkscape::UI::Widget::ComboBoxEnum<LivePathEffect::EffectType> combo_effecttype;
     
     Gtk::Widget * effectwidget;
@@ -104,6 +104,7 @@ private:
     UI::Widget::Frame effectcontrol_frame;
     Gtk::HBox effectapplication_hbox;
     Gtk::VBox effectcontrol_vbox;
+    Gtk::EventBox effectcontrol_eventbox;
     Gtk::VBox effectlist_vbox;
     ModelColumns columns;
     Gtk::ScrolledWindow scrolled_window;
@@ -111,8 +112,8 @@ private:
     Glib::RefPtr<Gtk::ListStore> effectlist_store;
     Glib::RefPtr<Gtk::TreeSelection> effectlist_selection;
 
-    void on_visibility_toggled( Glib::ustring const& str );
-
+    void on_visibility_toggled( Glib::ustring const& str);
+    bool _on_button_release(GdkEventButton* button_event);
     Gtk::ButtonBox toolbar_hbox;
     Gtk::Button button_add;
     Gtk::Button button_remove;