KnotHolder: fix knots selection status when a knot is clicked without Shift

When there are already selected knots, and  _another_ knot is clicked
without the Shift key pressed, the user would expects the previously
selected knots to be unselected immediately, but this does not happen
until the mouse button is released.

Steps to replicate:
  1. Change size of a rect using one corner.
  2. Change the size using the opposite corner (without pressing Shift).

Observed behavior: The first corner looks still selected during grab.

Expected behavior: The first corner gets unelected as soon as the mouse
                   button is pressed..

This happens because the knots selection status is not updated until the
mouse button is released, i.e. in the click/ungrabbed signal handler.

In order to have a more immediate feedback of the knot selection status,
add a "mousedown" signal handler and update the selection status in
there.

While at it also remove the knot_holder local variable in
knot_clicked_handler() which seems to be unnecessary.

1 files changed, 21 insertions, 24 deletions

diff --git a/src/knotholder.cpp b/src/knotholder.cpp
index eba9996eb..7d62ff6a5 100644
--- a/src/knotholder.cpp
+++ b/src/knotholder.cpp
@@ -127,22 +127,17 @@ bool KnotHolder::knot_mouseover() const {
 }
 
 void
-KnotHolder::knot_clicked_handler(SPKnot *knot, guint state)
+KnotHolder::knot_mousedown_handler(SPKnot *knot, guint state)
 {
-    KnotHolder *knot_holder = this;
-    SPItem *saved_item = this->item;
-
     if (!(state & GDK_SHIFT_MASK)) {
         unselect_knots();
     }
-    for(std::list<KnotHolderEntity *>::iterator i = knot_holder->entity.begin(); i != knot_holder->entity.end(); ++i) {
+    for(std::list<KnotHolderEntity *>::iterator i = this->entity.begin(); i != this->entity.end(); ++i) {
         KnotHolderEntity *e = *i;
         if (!(state & GDK_SHIFT_MASK)) {
             e->knot->selectKnot(false);
         }
         if (e->knot == knot) {
-            // no need to test whether knot_click exists since it's virtual now
-            e->knot_click(state);
             if (!(e->knot->flags & SP_KNOT_SELECTED) || !(state & GDK_SHIFT_MASK)){
                 e->knot->selectKnot(true);
             } else {
@@ -150,6 +145,19 @@ KnotHolder::knot_clicked_handler(SPKnot *knot, guint state)
             }
         }
     }
+}
+
+void
+KnotHolder::knot_clicked_handler(SPKnot *knot, guint state)
+{
+    SPItem *saved_item = this->item;
+
+    for(std::list<KnotHolderEntity *>::iterator i = this->entity.begin(); i != this->entity.end(); ++i) {
+        KnotHolderEntity *e = *i;
+        if (e->knot == knot)
+            // no need to test whether knot_click exists since it's virtual now
+            e->knot_click(state);
+    }
 
     {
         SPShape *savedShape = dynamic_cast<SPShape *>(saved_item);
@@ -158,7 +166,7 @@ KnotHolder::knot_clicked_handler(SPKnot *knot, guint state)
         }
     }
 
-    knot_holder->update_knots();
+    this->update_knots();
 
     unsigned int object_verb = SP_VERB_NONE;
 
@@ -266,22 +274,11 @@ KnotHolder::knot_ungrabbed_handler(SPKnot *knot, guint state)
     if (this->released) {
         this->released(this->item);
     } else {
-        if (!(state & GDK_SHIFT_MASK)) {
-            unselect_knots();
-        }
-        for(std::list<KnotHolderEntity *>::iterator i = this->entity.begin(); i != this->entity.end(); ++i) {
-            KnotHolderEntity *e = *i;
-            if (!(state & GDK_SHIFT_MASK)) {
-                e->knot->selectKnot(false);
-            }
-            if (e->knot == knot) {
-                if (!(e->knot->flags & SP_KNOT_SELECTED) || !(state & GDK_SHIFT_MASK)){
-                    e->knot->selectKnot(true);
-                } else {
-                    e->knot->selectKnot(false);
-                }
-            }
-        }
+        // if a point is dragged while not selected, it should select itself,
+        // even if it was just unselected in the mousedown event handler.
+        if (!(knot->flags & SP_KNOT_SELECTED))
+            knot->selectKnot(true);
+
         SPObject *object = (SPObject *) this->item;
 
         // Caution: this call involves a screen update, which may process events, and as a