Color picker UX - allow swicth mode on canvas

src/common/darktable.h

@@ -280,6 +280,7 @@ struct dt_l10n_t;
 
 typedef float dt_boundingbox_t[4];  //(x,y) of upperleft, then (x,y) of lowerright
 typedef float dt_pickerbox_t[8];
+typedef float dt_pickerpoint_t[2];
 typedef float dt_dev_zoom_pos_t[6];
 
 typedef enum dt_debug_thread_t

src/gui/color_picker_proxy.c

@@ -150,11 +150,11 @@ static void _init_picker(dt_iop_color_picker_t *picker,
   picker->changed    = FALSE;
   picker->fixed_cst  = FALSE;
 
-  // default values
-  picker->pick_box[0] = picker->pick_box[1] = 0.0f;
-  picker->pick_box[2] = picker->pick_box[3] = 1.0f;
-  picker->pick_pos[0] = picker->pick_pos[1] = 0.0f;
   _color_picker_reset(picker);
+
+  // set default positions
+  dt_lib_colorpicker_reset_box_area(picker->pick_box);
+  dt_lib_colorpicker_reset_point(picker->pick_pos);
 }
 
 static gboolean _color_picker_callback_button_press(GtkWidget *button,
@@ -182,12 +182,8 @@ static gboolean _color_picker_callback_button_press(GtkWidget *button,
     dt_modifier_is(state, GDK_CONTROL_MASK) || (e != NULL && e->button == GDK_BUTTON_SECONDARY);
   dt_iop_color_picker_flags_t flags = self->flags;
 
-  // setup if a new picker or switching between point/area mode
-  if(prior_picker != self
-     || (((flags & DT_COLOR_PICKER_POINT_AREA) == DT_COLOR_PICKER_POINT_AREA)
-         && (to_area_mode !=
-             (darktable.lib->proxy.colorpicker.primary_sample->size ==
-              DT_LIB_COLORPICKER_SIZE_BOX))))
+  // setup if a new picker
+  if(prior_picker != self)
   {
     darktable.lib->proxy.colorpicker.picker_proxy = self;
 
@@ -198,24 +194,14 @@ static gboolean _color_picker_callback_button_press(GtkWidget *button,
     dt_iop_color_picker_flags_t kind = self->flags & DT_COLOR_PICKER_POINT_AREA;
     if(kind == DT_COLOR_PICKER_POINT_AREA)
       kind = to_area_mode ? DT_COLOR_PICKER_AREA : DT_COLOR_PICKER_POINT;
+
     // pull picker's last recorded positions
     if(kind & DT_COLOR_PICKER_AREA)
     {
-      if(   self->pick_box[0] == 0.0f && self->pick_box[1] == 0.0f
-         && self->pick_box[2] == 1.0f && self->pick_box[3] == 1.0f)
-      {
-        dt_boundingbox_t reset = { 0.02f, 0.02f, 0.98f, 0.98f };
-        dt_color_picker_backtransform_box(darktable.develop, 2, reset, self->pick_box);
-      }
       dt_lib_colorpicker_set_box_area(darktable.lib, self->pick_box);
     }
     else if(kind & DT_COLOR_PICKER_POINT)
     {
-      if(self->pick_pos[0] == 0.0f && self->pick_pos[1] == 0.0f)
-      {
-        dt_boundingbox_t middle = { 0.5f, 0.5f };
-        dt_color_picker_backtransform_box(darktable.develop, 1, middle, self->pick_pos);
-      }
       dt_lib_colorpicker_set_point(darktable.lib, self->pick_pos);
     }
     else

src/gui/color_picker_proxy.h

@@ -63,7 +63,7 @@ typedef struct dt_iop_color_picker_t
   // positions are associated with the current picker widget: will set
   // the picker request for the primary picker when this picker is
   // activated, and will remember the most recent picker position
-  float pick_pos[2];
+  dt_pickerpoint_t pick_pos;
   dt_pickerbox_t pick_box;
   gboolean changed;
 } dt_iop_color_picker_t;

src/libs/colorpicker.c

@@ -333,7 +333,7 @@ static void _set_sample_box_area(dt_lib_module_t *self,
 }
 
 static void _set_sample_point(dt_lib_module_t *self,
-                              const float pos[2])
+                              const dt_pickerpoint_t pos)
 {
   dt_lib_colorpicker_t *data = self->data;
 
@@ -701,6 +701,9 @@ void gui_init(dt_lib_module_t *self)
   // _update_samples_output() will update the RGB values
   data->primary_sample.swatch.alpha = 1.0;
 
+  dt_lib_colorpicker_reset_box_area(data->primary_sample.box);
+  dt_lib_colorpicker_reset_point(data->primary_sample.point);
+
   // Initializing proxy functions and data
   darktable.lib->proxy.colorpicker.module = self;
   darktable.lib->proxy.colorpicker.display_samples =
@@ -772,7 +775,8 @@ void gui_init(dt_lib_module_t *self)
   data->picker_button = dt_color_picker_new(NULL, DT_COLOR_PICKER_POINT_AREA, picker_row);
   gtk_widget_set_tooltip_text
     (data->picker_button,
-     _("turn on color picker\nctrl+click or right-click to select an area"));
+     _("turn on color picker\nctrl+click or right-click to select an area\n"
+       "ctrl+click on canvas to switch between point/area"));
   gtk_widget_set_name(GTK_WIDGET(data->picker_button), "color-picker-button");
   g_signal_connect(G_OBJECT(data->picker_button), "toggled",
                    G_CALLBACK(_picker_button_toggled), data);

src/libs/colorpicker.h

@@ -44,8 +44,9 @@ typedef struct dt_colorpicker_sample_t
   // For the primary sample, these are the current sample area,
   // whether from colorpicker lib or an iop. They are used for showing
   // the sample in the center view, and sampling in the pixelpipe.
-  float point[2];
+  dt_pickerpoint_t point;
   dt_pickerbox_t box;
+  dt_pickerpoint_t pdrag; // for dragging box corner
   dt_lib_colorpicker_size_t size;
   gboolean denoise;
   gboolean pick_output;

src/libs/lib.c

@@ -20,6 +20,7 @@
 #include "common/debug.h"
 #include "common/module.h"
 #include "common/presets.h"
+#include "common/color_picker.h"
 #include "control/conf.h"
 #include "control/control.h"
 #include "dtgtk/button.h"
@@ -1520,6 +1521,19 @@ void dt_lib_colorpicker_set_point(dt_lib_t *lib,
   gtk_widget_grab_focus(dt_ui_center(darktable.gui->ui));
 }
 
+/* clear color picker pos */
+void dt_lib_colorpicker_reset_box_area(dt_pickerbox_t box)
+{
+  dt_boundingbox_t reset = { 0.02f, 0.02f, 0.98f, 0.98f };
+  dt_color_picker_backtransform_box(darktable.develop, 2, reset, box);
+}
+
+void dt_lib_colorpicker_reset_point(dt_pickerpoint_t pos)
+{
+  dt_boundingbox_t reset = { 0.5f, 0.5f };
+  dt_color_picker_backtransform_box(darktable.develop, 1, reset, pos);
+}
+
 void dt_lib_colorpicker_setup(dt_lib_t *lib,
                               const gboolean denoise,
                               const gboolean pick_output)

src/libs/lib.h

@@ -217,6 +217,10 @@ void dt_lib_colorpicker_set_box_area(dt_lib_t *lib,
 void dt_lib_colorpicker_set_point(dt_lib_t *lib,
                                   const float pos[2]);
 
+/* reset color picker pos to default */
+void dt_lib_colorpicker_reset_box_area(dt_pickerbox_t box);
+void dt_lib_colorpicker_reset_point(dt_pickerpoint_t pos);
+
 /** setup colorpicker options */
 void dt_lib_colorpicker_setup(dt_lib_t *lib,
                               const gboolean denoise,

src/views/darkroom.c

@@ -54,6 +54,7 @@
 #include "imageio/imageio_common.h"
 #include "imageio/imageio_module.h"
 #include "libs/colorpicker.h"
+#include "libs/lib.h"
 #include "views/view.h"
 #include "views/view_api.h"
 
@@ -3356,8 +3357,8 @@ void mouse_moved(dt_view_t *self,
 
     if(sample->size == DT_LIB_COLORPICKER_SIZE_BOX)
     {
-      float corner[2];
-      dt_color_picker_transform_box(dev, 1, sample->point, corner, TRUE);
+      dt_pickerpoint_t corner;
+      dt_color_picker_transform_box(dev, 1, sample->pdrag, corner, TRUE);
 
       pbox[0] = MAX(0.0, MIN(corner[0], zoom_x) - delta_x);
       pbox[1] = MAX(0.0, MIN(corner[1], zoom_y) - delta_y);
@@ -3519,13 +3520,36 @@ int button_pressed(dt_view_t *self,
     const int procw = dev->preview_pipe->backbuf_width;
     const int proch = dev->preview_pipe->backbuf_height;
 
+    // For a Ctrl+Click we do change the color picker from/to area <-> point
+    if(which == GDK_BUTTON_PRIMARY
+       && dt_modifier_is(state, GDK_CONTROL_MASK))
+    {
+      if(sample->size == DT_LIB_COLORPICKER_SIZE_POINT)
+      {
+        // dt_lib_colorpicker_reset_box_area(sample->box);
+        dt_lib_colorpicker_set_box_area(darktable.lib, sample->box);
+      }
+      else if(sample->size == DT_LIB_COLORPICKER_SIZE_BOX)
+      {
+        dt_lib_colorpicker_set_point(darktable.lib, sample->point);
+      }
+
+      dev->preview_pipe->status = DT_DEV_PIXELPIPE_DIRTY;
+      dt_control_queue_redraw_center();
+
+      return 1;
+    }
+
     if(which == GDK_BUTTON_PRIMARY)
     {
       _get_zoom_pos(&dev->full, x, y, &zoom_x, &zoom_y, &zoom_scale);
-      sample->point[0] = zoom_x;
-      sample->point[1] = zoom_y;
 
-      if(sample->size == DT_LIB_COLORPICKER_SIZE_BOX)
+      if(sample->size == DT_LIB_COLORPICKER_SIZE_POINT)
+      {
+        sample->point[0] = zoom_x;
+        sample->point[1] = zoom_y;
+      }
+      else if(sample->size == DT_LIB_COLORPICKER_SIZE_BOX)
       {
         dt_boundingbox_t sbox;
         dt_color_picker_transform_box(dev, 2, sample->box, sbox, TRUE);
@@ -3541,8 +3565,8 @@ int button_pressed(dt_view_t *self,
 
         if(MIN(dx0, dx1) < hx && MIN(dy0, dy1) < hy)
         {
-          sample->point[0] = sbox[dx0 < dx1 ? 2 : 0];
-          sample->point[1] = sbox[dy0 < dy1 ? 3 : 1];
+          sample->pdrag[0] = sbox[dx0 < dx1 ? 2 : 0];
+          sample->pdrag[1] = sbox[dy0 < dy1 ? 3 : 1];
         }
         else
         {
@@ -3607,11 +3631,8 @@ int button_pressed(dt_view_t *self,
       }
       if(sample->size == DT_LIB_COLORPICKER_SIZE_BOX)
       {
-        // default is hardcoded this way
-        // FIXME: color_pixer_proxy should have an dt_iop_color_picker_clear_area() function for this
-        dt_boundingbox_t reset = { 0.02f, 0.02f, 0.98f, 0.98f };
         dt_pickerbox_t box;
-        dt_color_picker_backtransform_box(dev, 2, reset, box);
+        dt_lib_colorpicker_reset_box_area(box);
         dt_lib_colorpicker_set_box_area(darktable.lib, box);
         dev->preview_pipe->status = DT_DEV_PIXELPIPE_DIRTY;
         dt_control_queue_redraw_center();