From 4b785ac22fc617aef70000be1d7385ef354d28e9 Mon Sep 17 00:00:00 2001 From: Antonio Ospite Date: Tue, 30 Oct 2018 17:17:09 +0100 Subject: SPKnot::SPKnot(): improve color style of selected knots Starting from commit ff04a08000 (working on knots selection, 2017-07-01) it is possible to select knots for shapes, but the selection style can be confusing. The current default colors for knots states are: NORMAL -> white MOUSEOVER -> red DRAGGING -> blue SELECTED -> red Using the same color for the MOUSEOVER and SELECTED states looks like a contradiction considering that the former is an instantaneous indicator while the latter represents a more persistent state. MOUSEOVER and DRAGGING are more similar (in either state the mouse button is pressed), so switch to the following style: NORMAL -> white MOUSEOVER -> red DRAGGING -> red SELECTED -> blue The new style also matches what path nodes look like. --- src/knot.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/knot.cpp b/src/knot.cpp index d861004f4..ab83eff42 100644 --- a/src/knot.cpp +++ b/src/knot.cpp @@ -81,8 +81,8 @@ SPKnot::SPKnot(SPDesktop *desktop, gchar const *tip) this->fill[SP_KNOT_STATE_NORMAL] = 0xffffff00; this->fill[SP_KNOT_STATE_MOUSEOVER] = 0xff0000ff; - this->fill[SP_KNOT_STATE_DRAGGING] = 0x0000ffff; - this->fill[SP_KNOT_STATE_SELECTED] = 0xff0000ff; + this->fill[SP_KNOT_STATE_DRAGGING] = 0xff0000ff; + this->fill[SP_KNOT_STATE_SELECTED] = 0x0000ffff; this->stroke[SP_KNOT_STATE_NORMAL] = 0x01000000; this->stroke[SP_KNOT_STATE_MOUSEOVER] = 0x01000000; -- cgit v1.2.3 From 201800d1be542ac95ac9651e7aaa6f74c6c472bc Mon Sep 17 00:00:00 2001 From: Antonio Ospite Date: Wed, 31 Oct 2018 11:07:04 +0100 Subject: KnotHolder: misc cosmetic cleanups Remove some trailing spaces and reindent with spaces instad of tabs. --- src/knotholder.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/knotholder.cpp b/src/knotholder.cpp index 1c5e59b2d..3f462c541 100644 --- a/src/knotholder.cpp +++ b/src/knotholder.cpp @@ -76,7 +76,7 @@ KnotHolder::KnotHolder(SPDesktop *desktop, SPItem *item, SPKnotHolderReleasedFun } KnotHolder::~KnotHolder() { - sp_object_unref(item); + sp_object_unref(item); for (std::list::iterator i = entity.begin(); i != entity.end(); ++i) { @@ -185,7 +185,7 @@ KnotHolder::knot_clicked_handler(SPKnot *knot, guint state) } // for drag, this is done by ungrabbed_handler, but for click we must do it here - + if (saved_item) { //increasingly aggressive sanity checks if (saved_item->document) { // enum is unsigned so can't be less than SP_VERB_INVALID @@ -208,7 +208,7 @@ KnotHolder::transform_selected(Geom::Affine transform){ } } -void +void KnotHolder::unselect_knots(){ if (tools_isactive(desktop, TOOLS_NODES)) { Inkscape::UI::Tools::NodeTool *nt = static_cast(desktop->event_context); @@ -238,7 +238,7 @@ KnotHolder::knot_moved_handler(SPKnot *knot, Geom::Point const &p, guint state) this->dragging = true; } - // this was a local change and the knotholder does not need to be recreated: + // this was a local change and the knotholder does not need to be recreated: this->local_change = TRUE; for(std::list::iterator i = this->entity.begin(); i != this->entity.end(); ++i) { -- cgit v1.2.3 From 875f9ea97195b77e92625c6f41257758b0e8a880 Mon Sep 17 00:00:00 2001 From: Antonio Ospite Date: Wed, 31 Oct 2018 11:16:58 +0100 Subject: KnotHolder: remove stale comment The code in knot_ungrabbed_handler does not use the click signal, so the comment does not apply there. --- src/knotholder.cpp | 1 - 1 file changed, 1 deletion(-) (limited to 'src') diff --git a/src/knotholder.cpp b/src/knotholder.cpp index 3f462c541..eba9996eb 100644 --- a/src/knotholder.cpp +++ b/src/knotholder.cpp @@ -275,7 +275,6 @@ KnotHolder::knot_ungrabbed_handler(SPKnot *knot, guint state) e->knot->selectKnot(false); } if (e->knot == knot) { - // no need to test whether knot_click exists since it's virtual now if (!(e->knot->flags & SP_KNOT_SELECTED) || !(state & GDK_SHIFT_MASK)){ e->knot->selectKnot(true); } else { -- cgit v1.2.3 From b8ed9d236dc560adc83245ce50e0630a0c482496 Mon Sep 17 00:00:00 2001 From: Antonio Ospite Date: Tue, 30 Oct 2018 23:22:11 +0100 Subject: 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. --- src/knot-holder-entity.cpp | 2 ++ src/knot-holder-entity.h | 1 + src/knotholder.cpp | 45 +++++++++++++++++++++------------------------ src/knotholder.h | 1 + 4 files changed, 25 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/knot-holder-entity.cpp b/src/knot-holder-entity.cpp index 3664bbc81..9408e7942 100644 --- a/src/knot-holder-entity.cpp +++ b/src/knot-holder-entity.cpp @@ -58,6 +58,7 @@ void KnotHolderEntity::create(SPDesktop *desktop, SPItem *item, KnotHolder *pare update_knot(); knot->show(); + _mousedown_connection = knot->mousedown_signal.connect(sigc::mem_fun(*parent_holder, &KnotHolder::knot_mousedown_handler)); _moved_connection = knot->moved_signal.connect(sigc::mem_fun(*parent_holder, &KnotHolder::knot_moved_handler)); _click_connection = knot->click_signal.connect(sigc::mem_fun(*parent_holder, &KnotHolder::knot_clicked_handler)); _ungrabbed_connection = knot->ungrabbed_signal.connect(sigc::mem_fun(*parent_holder, &KnotHolder::knot_ungrabbed_handler)); @@ -66,6 +67,7 @@ void KnotHolderEntity::create(SPDesktop *desktop, SPItem *item, KnotHolder *pare KnotHolderEntity::~KnotHolderEntity() { + _mousedown_connection.disconnect(); _moved_connection.disconnect(); _click_connection.disconnect(); _ungrabbed_connection.disconnect(); diff --git a/src/knot-holder-entity.h b/src/knot-holder-entity.h index e6efaa7e8..5e7868b70 100644 --- a/src/knot-holder-entity.h +++ b/src/knot-holder-entity.h @@ -87,6 +87,7 @@ public: unsigned int _ungrab_handler_id; private: + sigc::connection _mousedown_connection; sigc::connection _moved_connection; sigc::connection _click_connection; sigc::connection _ungrabbed_connection; 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::iterator i = knot_holder->entity.begin(); i != knot_holder->entity.end(); ++i) { + for(std::list::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::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(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::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 diff --git a/src/knotholder.h b/src/knotholder.h index b82ba61f9..05adb2241 100644 --- a/src/knotholder.h +++ b/src/knotholder.h @@ -51,6 +51,7 @@ public: void update_knots(); void unselect_knots(); + void knot_mousedown_handler(SPKnot *knot, unsigned int state); void knot_moved_handler(SPKnot *knot, Geom::Point const &p, unsigned int state); void knot_clicked_handler(SPKnot *knot, unsigned int state); void knot_ungrabbed_handler(SPKnot *knot, unsigned int state); -- cgit v1.2.3 From e521fdc704b7640f6cfee5f9a519a3d780445f2b Mon Sep 17 00:00:00 2001 From: Antonio Ospite Date: Wed, 31 Oct 2018 17:37:16 +0100 Subject: ToolBase::root_handler: fix keyboard movement for KEY_Up and KEY_Down events Moving shape knots up and down with the keyboard moves them in the opposite direction when the Y axis goes downwards. This happens because commit 1fa0c72b66 (New option to invert y-axis, 2018-09-12) forgot to handle Y direction in src/ui/tools/tool-base.cpp. Fix the issue by applying the Y axis direction factor just like it's done in src/ui/tool/control-point-selection.cpp. --- src/ui/tools/tool-base.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/ui/tools/tool-base.cpp b/src/ui/tools/tool-base.cpp index 8637b640d..20aad1424 100644 --- a/src/ui/tools/tool-base.cpp +++ b/src/ui/tools/tool-base.cpp @@ -651,10 +651,10 @@ bool ToolBase::root_handler(GdkEvent* event) { acceleration, desktop->getCanvas())); gobble_key_events(get_latin_keyval(&event->key), GDK_CONTROL_MASK); - this->desktop->scroll_relative(Geom::Point(0, i)); + this->desktop->scroll_relative(Geom::Point(0, -i * desktop->yaxisdir())); ret = TRUE; } else { - ret = _keyboardMove(event->key, Geom::Point(0, 1)); + ret = _keyboardMove(event->key, Geom::Point(0, -desktop->yaxisdir())); } break; @@ -681,10 +681,10 @@ bool ToolBase::root_handler(GdkEvent* event) { acceleration, desktop->getCanvas())); gobble_key_events(get_latin_keyval(&event->key), GDK_CONTROL_MASK); - this->desktop->scroll_relative(Geom::Point(0, -i)); + this->desktop->scroll_relative(Geom::Point(0, i * desktop->yaxisdir())); ret = TRUE; } else { - ret = _keyboardMove(event->key, Geom::Point(0, -1)); + ret = _keyboardMove(event->key, Geom::Point(0, desktop->yaxisdir())); } break; -- cgit v1.2.3