Make sure that guides always snap correctly when dropping them, and improve perseverance of snap-indicator

(bzr r9103)

6 files changed, 30 insertions, 23 deletions

diff --git a/src/desktop-events.cpp b/src/desktop-events.cpp
index f229c642a..4c0ebc5a8 100644
--- a/src/desktop-events.cpp
+++ b/src/desktop-events.cpp
@@ -5,6 +5,7 @@
  *   Lauris Kaplinski <lauris@kaplinski.com>
  *
  * Copyright (C) 1999-2002 Lauris Kaplinski
+ * Copyright (C) 1999-2010 Others
  *
  * Released under GNU GPL, read the file 'COPYING' for more information
  */
@@ -156,6 +157,8 @@ static gint sp_dt_ruler_event(GtkWidget *widget, GdkEvent *event, SPDesktopWidge
             break;
     case GDK_BUTTON_RELEASE:
             if (dragging && event->button.button == 1) {
+                sp_event_context_discard_delayed_snap_event(desktop->event_context);
+
                 gdk_pointer_ungrab(event->button.time);
                 Geom::Point const event_w(sp_canvas_window_to_world(dtw->canvas, event_win));
                 Geom::Point event_dt(desktop->w2d(event_w));
@@ -169,8 +172,6 @@ static gint sp_dt_ruler_event(GtkWidget *widget, GdkEvent *event, SPDesktopWidge
 
                 dragging = false;
 
-                sp_event_context_discard_delayed_snap_event(desktop->event_context);
-
                 gtk_object_destroy(GTK_OBJECT(guide));
                 guide = NULL;
                 if ((horiz ? wy : wx) >= 0) {
@@ -184,14 +185,6 @@ static gint sp_dt_ruler_event(GtkWidget *widget, GdkEvent *event, SPDesktopWidge
                                      _("Create guide"));
                 }
                 desktop->set_coordinate_status(from_2geom(event_dt));
-
-                // A dt_ruler_event might be emitted when dragging a guide of the rulers
-                // while drawing a Bezier curve. In such a situation, we're already in that
-                // specific context and the snap delay is already active. We should interfere
-                // with that context and we should therefore leave the snap delay status
-                // as it is. So although it might have been set to active above on
-                // GDK_BUTTON_PRESS, we should not set it back to inactive here. That must be
-                // done by the context.
             }
     default:
             break;
@@ -341,6 +334,8 @@ gint sp_dt_guide_event(SPCanvasItem *item, GdkEvent *event, gpointer data)
             break;
     case GDK_BUTTON_RELEASE:
             if (drag_type != SP_DRAG_NONE && event->button.button == 1) {
+                sp_event_context_discard_delayed_snap_event(desktop->event_context);
+
                 if (moved) {
                     Geom::Point const event_w(event->button.x,
                                               event->button.y);
@@ -402,7 +397,6 @@ gint sp_dt_guide_event(SPCanvasItem *item, GdkEvent *event, gpointer data)
                         sp_document_done(sp_desktop_document(desktop), SP_VERB_NONE,
                                      _("Delete guide"));
                     }
-                    sp_event_context_discard_delayed_snap_event(desktop->event_context);
                     moved = false;
                     desktop->set_coordinate_status(from_2geom(event_dt));
                     desktop->setPosition (from_2geom(event_dt));
diff --git a/src/display/snap-indicator.cpp b/src/display/snap-indicator.cpp
index b135dd9fe..c0ed322e4 100644
--- a/src/display/snap-indicator.cpp
+++ b/src/display/snap-indicator.cpp
@@ -6,7 +6,7 @@
  *   Diederik van Lierop
  *
  * Copyright (C) Johan Engelen 2009 <j.b.c.engelen@utwente.nl>
- * Copyright (C) Diederik van Lierop 2009 <mail@diedenrezi.nl>
+ * Copyright (C) Diederik van Lierop 2010 <mail@diedenrezi.nl>
  *
  * Released under GNU GPL, read the file 'COPYING' for more information
  */
@@ -31,6 +31,7 @@ SnapIndicator::SnapIndicator(SPDesktop * desktop)
         _snaptarget_tooltip(NULL),
         _snaptarget_bbox(NULL),
         _snapsource(NULL),
+        _snaptarget_is_presnap(false),
         _desktop(desktop)
 {
 }
@@ -240,6 +241,7 @@ SnapIndicator::set_new_snaptarget(Inkscape::SnappedPoint const &p, bool pre_snap
 
         SP_CTRL(canvasitem)->moveto(p.getPoint());
         _snaptarget = _desktop->add_temporary_canvasitem(canvasitem, timeout_val);
+        _snaptarget_is_presnap = pre_snap;
 
         // Display the tooltip, which reveals the type of snap source and the type of snap target
         gchar *tooltip_str = g_strconcat(source_name, _(" to "), target_name, NULL);
@@ -271,11 +273,16 @@ SnapIndicator::set_new_snaptarget(Inkscape::SnappedPoint const &p, bool pre_snap
 }
 
 void
-SnapIndicator::remove_snaptarget()
+SnapIndicator::remove_snaptarget(bool only_if_presnap)
 {
+    if (only_if_presnap && !_snaptarget_is_presnap) {
+        return;
+    }
+
     if (_snaptarget) {
         _desktop->remove_temporary_canvasitem(_snaptarget);
         _snaptarget = NULL;
+        _snaptarget_is_presnap = false;
     }
 
     if (_snaptarget_tooltip) {
diff --git a/src/display/snap-indicator.h b/src/display/snap-indicator.h
index 259af8ae6..5475f9f60 100644
--- a/src/display/snap-indicator.h
+++ b/src/display/snap-indicator.h
@@ -9,7 +9,7 @@
  *   Diederik van Lierop
  *
  * Copyright (C) Johan Engelen 2008 <j.b.c.engelen@utwente.nl>
- * Copyright (C) Diederik van Lierop 2008 <mail@diedenrezi.nl>
+ * Copyright (C) Diederik van Lierop 2010 <mail@diedenrezi.nl>
  *
  * Released under GNU GPL, read the file 'COPYING' for more information
  */
@@ -27,7 +27,7 @@ public:
     virtual ~SnapIndicator();
 
     void set_new_snaptarget(Inkscape::SnappedPoint const &p, bool pre_snap = false);
-    void remove_snaptarget();
+    void remove_snaptarget(bool only_if_presnap = false);
 
     void set_new_snapsource(Inkscape::SnapCandidatePoint const &p);
     void remove_snapsource();
@@ -37,6 +37,7 @@ protected:
     TemporaryItem *_snaptarget_tooltip;
     TemporaryItem *_snaptarget_bbox;
     TemporaryItem *_snapsource;
+    bool _snaptarget_is_presnap;
     SPDesktop *_desktop;
 
 private:
diff --git a/src/event-context.cpp b/src/event-context.cpp
index 397ec6b80..6184fb4c7 100644
--- a/src/event-context.cpp
+++ b/src/event-context.cpp
@@ -8,7 +8,7 @@
  *   Frank Felfe <innerspace@iname.com>
  *   bulia byak <buliabyak@users.sf.net>
  *
- * Copyright (C) 1999-2005 authors
+ * Copyright (C) 1999-2010 authors
  * Copyright (C) 2001-2002 Ximian, Inc.
  *
  * Released under GNU GPL, read the file 'COPYING' for more information
@@ -1194,8 +1194,7 @@ void sp_event_context_snap_delay_handler(SPEventContext *ec,
         // Snap when speed drops below e.g. 0.02 px/msec, or when no motion events have occurred for some period.
         // i.e. snap when we're at stand still. A speed threshold enforces snapping for tablets, which might never
         // be fully at stand still and might keep spitting out motion events.
-        ec->desktop->namedview->snap_manager.snapprefs.setSnapPostponedGlobally(
-                true); // put snapping on hold
+        ec->desktop->namedview->snap_manager.snapprefs.setSnapPostponedGlobally(true); // put snapping on hold
 
         Geom::Point event_pos(event->x, event->y);
         guint32 event_t = gdk_event_get_time((GdkEvent *) event);
@@ -1331,6 +1330,7 @@ gboolean sp_event_context_snap_watchdog_callback(gpointer data) {
 void sp_event_context_discard_delayed_snap_event(SPEventContext *ec) {
     delete ec->_delayed_snap_event;
     ec->_delayed_snap_event = NULL;
+    ec->desktop->namedview->snap_manager.snapprefs.setSnapPostponedGlobally(false);
 }
 
 /*
diff --git a/src/snap.cpp b/src/snap.cpp
index 996ec2e2c..1033b0a2c 100644
--- a/src/snap.cpp
+++ b/src/snap.cpp
@@ -224,7 +224,7 @@ void SnapManager::preSnap(Inkscape::SnapCandidatePoint const &p)
         if (s.getSnapped()) {
             _desktop->snapindicator->set_new_snaptarget(s, true);
         } else {
-            _desktop->snapindicator->remove_snaptarget();
+            _desktop->snapindicator->remove_snaptarget(true);
         }
         _snapindicator = true; // restore the original value
     }
diff --git a/src/ui/tool/control-point.cpp b/src/ui/tool/control-point.cpp
index a03a8b639..b9a793ca4 100644
--- a/src/ui/tool/control-point.cpp
+++ b/src/ui/tool/control-point.cpp
@@ -378,10 +378,15 @@ bool ControlPoint::_eventHandler(GdkEvent *event)
         
     case GDK_BUTTON_RELEASE:
         if (_event_grab && event->button.button == 1) {
-            // TODO I think snapping on release is wrong, or at least counter-intuitive.
-            sp_event_context_snap_watchdog_callback(_desktop->event_context->_delayed_snap_event);
-            sp_event_context_discard_delayed_snap_event(_desktop->event_context);
-            _desktop->snapindicator->remove_snaptarget();
+            // If we have any pending snap event, then invoke it now!
+            // (This is needed because we might not have snapped on the latest GDK_MOTION_NOTIFY event
+            // if the mouse speed was too high. This is inherent to the snap-delay mechanism.
+            // We must snap at some point in time though, and this is our last chance)
+            // PS: For other contexts this is handled already in sp_event_context_item_handler or
+            // sp_event_context_root_handler
+            if (_desktop->event_context->_delayed_snap_event) {
+                sp_event_context_snap_watchdog_callback(_desktop->event_context->_delayed_snap_event);
+            }
 
             sp_canvas_item_ungrab(_canvas_item, event->button.time);
             _setMouseover(this, event->button.state);