Code cleanup for the ColorPicker

* Remove awful, broken drag-start forcing code from the base class.
  - we didn't need it anyway
  - there are subtler and better ways of starting the drag's grab
* Better get_usage string.
  - Not every invocation needs clicking
* Improve ColorPicker docs.
* General code cleanup, explanatory variable names.

Along with the previous commit, this fixes mypaint#182.

gui/colorpicker.py

@@ -27,15 +27,35 @@
 ## Color picking mode, with a preview rectangle overlay
 
 class ColorPickMode (gui.mode.OneshotDragMode):
-    """Mode for picking colors from the screen, with a preview"""
+    """Mode for picking colors from the screen, with a preview
+
+    This can be invoked in quite a number of ways:
+
+    * The keyboard hotkey ("R" by default)
+    * Modifier and pointer button: (Ctrl+Button1 by default)
+    * From the toolbar or menu
+
+    The first two methods pick immediately. Moving the mouse with the
+    initial keys or buttons held down keeps picking with a little
+    preview square appearing.
+
+    The third method doesn't pick immediately: you have to click on the
+    canvas to start picking.
+
+    While the preview square is visible, it's possible to pick outside
+    the window. This "hidden" functionality may not work at all with
+    more modern window managers and DEs, and may be removed if it proves
+    slow or faulty.
+
+    """
 
     # Class configuration
     ACTION_NAME = 'ColorPickMode'
     PICK_SIZE = 6
 
     # Keyboard activation behaviour (instance defaults)
     # See keyboard.py and doc.mode_flip_action_activated_cb()
-    keyup_timeout = 0
+    keyup_timeout = 0   # don't change behaviour by timeout
 
     pointer_behavior = gui.mode.Behavior.EDIT_OBJECTS
     scroll_behavior = gui.mode.Behavior.NONE  # XXX grabs ptr, so no CHANGE_VIEW
@@ -50,57 +70,67 @@ def get_name(cls):
         return _(u"Pick Color")
 
     def get_usage(self):
-        return _(u"Click to set the color used for painting")
+        return _(u"Set the color used for painting")
 
     def __init__(self, **kwds):
         super(ColorPickMode, self).__init__(**kwds)
         self._overlay = None
-        self._preview_needs_button_press = 'ignore_modifiers' not in kwds
-        self._button_press_seen = False
+        self._started_from_key_press = self.ignore_modifiers
+        self._start_drag_on_next_motion_event = False
 
     def enter(self, **kwds):
-        """Enters the mode, starting the grab immediately.
-        """
+        """Enters the mode, arranging for necessary grabs ASAP"""
         super(ColorPickMode, self).enter(**kwds)
-        if self._picking():
+        if self._started_from_key_press:
+            # Pick now, and start the drag when possible
             self.doc.app.pick_color_at_pointer(self.doc.tdw, self.PICK_SIZE)
-        self._force_drag_start()
+            self._start_drag_on_next_motion_event = True
+            self._needs_drag_start = True
 
     def leave(self, **kwds):
-        if self._overlay is not None:
-            self._overlay.cleanup()
-            self._overlay = None
+        self._remove_overlay()
         super(ColorPickMode, self).leave(**kwds)
 
     def button_press_cb(self, tdw, event):
-        self._button_press_seen = True
         self.doc.app.pick_color_at_pointer(self.doc.tdw, self.PICK_SIZE)
+        # Supercall will start the drag normally
+        self._start_drag_on_next_motion_event = False
         return super(ColorPickMode, self).button_press_cb(tdw, event)
 
+    def motion_notify_cb(self, tdw, event):
+        if self._start_drag_on_next_motion_event:
+            self._start_drag(tdw, event)
+            self._start_drag_on_next_motion_event = False
+        return super(ColorPickMode, self).motion_notify_cb(tdw, event)
+
     def drag_stop_cb(self, tdw):
-        if self._overlay is not None:
-            self._overlay.cleanup()
-            self._overlay = None
+        self._remove_overlay()
         super(ColorPickMode, self).drag_stop_cb(tdw)
 
-    def _picking(self):
-        return not (self._preview_needs_button_press
-                    and not self._button_press_seen)
-
     def drag_update_cb(self, tdw, event, dx, dy):
-        picking = self._picking()
-        if picking:
-            self.doc.app.pick_color_at_pointer(tdw, self.PICK_SIZE)
-            if self._overlay is None:
-                self._overlay = ColorPickPreviewOverlay(self.doc, tdw,
-                                                        event.x, event.y)
-        if self._overlay is not None:
-            self._overlay.move(event.x, event.y)
+        self.doc.app.pick_color_at_pointer(tdw, self.PICK_SIZE)
+        self._place_overlay(tdw, event.x, event.y)
         return super(ColorPickMode, self).drag_update_cb(tdw, event, dx, dy)
 
+    def _place_overlay(self, tdw, x, y):
+        if self._overlay is None:
+            self._overlay = ColorPickPreviewOverlay(self.doc, tdw, x, y)
+        else:
+            self._overlay.move(x, y)
+
+    def _remove_overlay(self):
+        if self._overlay is None:
+            return
+        self._overlay.cleanup()
+        self._overlay = None
+
 
 class ColorPickPreviewOverlay (Overlay):
     """Preview overlay during color picker mode.
+
+    This is only shown when dragging the pointer with a button or the
+    hotkey held down, to avoid flashing and distraction.
+
     """
 
     PREVIEW_SIZE = 70

gui/mode.py

@@ -981,44 +981,6 @@ def key_release_cb(self, win, tdw, event):
         # Fall through to other behavioral mixins
         return super(DragMode, self).key_release_cb(win, tdw, event)
 
-    def _force_drag_start(self):
-        # Attempt to force a drag to start, using the current event.
-
-        # XXX: This is only used by the picker mode, which needs to begin
-        # picking straight away in enter even if this is in response to an
-        # action activation.
-        event = gtk.get_current_event()
-        if event is None:
-            logger.warning("no event")
-            return
-        if self.in_drag:
-            return
-        tdw = self.doc.tdw
-        # Duck-profile the starting event. If it's a keypress event or a
-        # button-press event, or anything that quacks like those, we can
-        # attempt the grab and start the drag if it succeeded.
-        if hasattr(event, "keyval"):
-            if event.keyval != self._start_keyval:
-                self._start_keyval = event.keyval
-                self._start_drag(tdw, event)
-        elif (hasattr(event, "x") and hasattr(event, "y")
-              and hasattr(event, "button")):
-            self._start_drag(tdw, event)
-            if self.in_drag:
-                # Grab succeeded
-                self.last_x = event.x
-                self.last_y = event.y
-                # For the toolbar button, and for menus, it's a button-release
-                # event.
-                # Record which button is being pressed at start
-                if isinstance(event.button, int):
-                    # PyGTK supplies the actual button number ...
-                    self._start_button = event.button
-                else:
-                    # ... but GI+GTK3 supplies a <void at 0xNNNNNNNN> object
-                    # when the event comes from gtk.get_current_event() :(
-                    self._start_button = None
-
 
 class OneshotDragMode (DragMode):
     """Drag modes that can exit immediately when the drag stops