Skip to content

Commit 3c8a77a

Browse files
dtoropTurboGit
authored andcommitted
scopes: cleaner exposure change code and bauhaus interactions
Decouple (as much as possible) the exposure change drag/scroll behavior from bauhaus internals. Within the scopes drawable, use a GtkGestureDrag to handle drag events. Pass minimal event information (delta, # of clicks, modifiers) to the handler code in exposure iop. The exposure handler now uses the regular bauhaus API (e.g. dt_bauhaus_slider_set()) to adjust slider values. Dragging within the scopes widget is now much closer to how it is handled in the colorequal iop. The exception is that this commit doesn't use gtk_widget_event() to handle scroll events, so as not to need to rewrite the event to account for changes in scroll direction. Previously things were much more entangled: On a user interaction, the scope would pass either an GtkGestureSingle, GtkEventControllerMotion, or GtkEventControllerScroll to the exposure event handler. That event handler would then inject the gesture/event directly into the bauhaus press/release/motion/scroll code. Separating things out has these benefits: - Ease update to GTK4 by no longer require that scope, exposure, and bauhaus use the same event/gesture handling. - Remove hack to handle exposure changes via scope when the exposure iop widgets hasn't yet been allocated. - Acknowledges that the scopes drag regions are a widget with their own behavior which is different from the behavior of a bauhaus slider. - Allow for removing the darktable.bauhaus->scroll(), ->press(), ->motion() and ->release() interface which was only used by scopes. Dragging using speed multiplier modifiers (control/shift) continues to work. Unlike bauhaus sliders, though, dragging in the scope with speed multipliers doesn't adjust values in discrete steps, for simplicity of code. This commit adds a couple helpers in gtk.h & gtk.c: - dt_gui_connect_drag(), analog to dt_gui_connect_click() - dt_gui_deny(gesture), analog to dt_gui_claim() This commit also: - fixes a bug which caused all drags to effect the exposure slider, even when they started in the black point slider region - removes verbose warning output in split mode when drag from waveform to vectorscope - fixes double negation of direction when scrolling black point - removes the broken behavior by which right clicking in the scopes widget would open a bauhaus exposure/blackpoint change popup which was often inaccessible as it was not under the pointer
1 parent d8adda6 commit 3c8a77a

13 files changed

Lines changed: 174 additions & 156 deletions

File tree

src/bauhaus/bauhaus.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3706,12 +3706,6 @@ static void dt_bh_class_init(DtBauhausWidgetClass *class)
37063706
widget_class->get_preferred_height = _widget_get_preferred_height;
37073707
// widget_class->measure = _widget_measure;
37083708
G_OBJECT_CLASS(class)->finalize = _widget_finalize;
3709-
3710-
// for histogram -> exposure proxy
3711-
bh->press = _widget_button_press;
3712-
bh->release = _widget_button_release;
3713-
bh->motion = _widget_motion;
3714-
bh->scroll = _widget_scroll;
37153709
}
37163710

37173711
static void dt_bh_init(DtBauhausWidget *w)

src/bauhaus/bauhaus.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,6 @@ typedef struct dt_bauhaus_t
154154
GdkRGBA graph_bg, graph_exterior, graph_border, graph_fg, graph_grid, graph_fg_active, graph_overlay, inset_histogram;
155155
GdkRGBA graph_colors[3]; // primaries
156156
GdkRGBA colorlabels[DT_COLORLABELS_LAST];
157-
158-
// for use by histogram -> exposure proxy
159-
void (*press)(GtkGestureSingle*, int, double, double, GtkWidget*);
160-
void (*release)(GtkGestureSingle*, int, double, double, GtkWidget*);
161-
void (*motion)(GtkEventControllerMotion*, double, double, GtkWidget*);
162-
void (*scroll)(GtkEventControllerScroll*, double, double, GtkWidget*);
163157
} dt_bauhaus_t;
164158

165159
#define DT_BAUHAUS_SPACE 0

src/develop/develop.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3334,13 +3334,12 @@ float dt_dev_exposure_get_black(dt_develop_t *dev)
33343334
return instance && instance->get_black && instance->module->enabled ? instance->get_black(instance->module) : 0.0f;
33353335
}
33363336

3337-
void dt_dev_exposure_handle_event(GtkEventController *controller,
3338-
int n_press, gdouble x, gdouble delta,
3339-
const gboolean blackwhite)
3337+
void dt_dev_exposure_handle_event(int n_press, gdouble delta,
3338+
GdkModifierType state,
3339+
const gboolean is_blackpoint)
33403340
{
33413341
if(darktable.develop->proxy.exposure.handle_event)
3342-
darktable.develop->proxy.exposure.handle_event(controller, n_press, x, delta,
3343-
blackwhite);
3342+
darktable.develop->proxy.exposure.handle_event(n_press, delta, state, is_blackpoint);
33443343
}
33453344

33463345
void dt_dev_modulegroups_set(dt_develop_t *dev,

src/develop/develop.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ typedef struct dt_dev_proxy_exposure_t
103103
float (*get_exposure)(struct dt_iop_module_t *exp);
104104
float (*get_effective_exposure)(struct dt_iop_module_t *exp);
105105
float (*get_black)(struct dt_iop_module_t *exp);
106-
void (*handle_event)(GtkEventController*, int, gdouble, gdouble, const gboolean);
106+
void (*handle_event)(int, gdouble, GdkModifierType, const gboolean);
107107
} dt_dev_proxy_exposure_t;
108108

109109
struct dt_dev_pixelpipe_t;
@@ -495,9 +495,9 @@ float dt_dev_exposure_get_effective_exposure(dt_develop_t *dev);
495495
/** get exposure black level */
496496
float dt_dev_exposure_get_black(dt_develop_t *dev);
497497

498-
void dt_dev_exposure_handle_event(GtkEventController *controller,
499-
int n_press, gdouble x, gdouble delta,
500-
const gboolean blackwhite);
498+
void dt_dev_exposure_handle_event(int n_press, gdouble delta,
499+
GdkModifierType state,
500+
const gboolean is_blackpoint);
501501

502502
/*
503503
* modulegroups plugin hooks

src/gui/gtk.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4617,6 +4617,24 @@ GtkGestureSingle *(dt_gui_connect_click)(GtkWidget *widget,
46174617
return (GtkGestureSingle *)gesture;
46184618
}
46194619

4620+
GtkGesture *(dt_gui_connect_drag)(GtkWidget *widget,
4621+
GCallback drag_begin,
4622+
GCallback drag_end,
4623+
GCallback drag_update,
4624+
gpointer data)
4625+
{
4626+
GtkGesture *gesture = gtk_gesture_drag_new(widget);
4627+
g_object_weak_ref(G_OBJECT (widget), (GWeakNotify) g_object_unref, gesture);
4628+
// GTK4 GtkGesture *gesture = gtk_gesture_drag_new();
4629+
// gtk_widget_add_controller(widget, GTK_EVENT_CONTROLLER(gesture));
4630+
4631+
if(drag_begin) g_signal_connect(gesture, "drag-begin", G_CALLBACK(drag_begin), data);
4632+
if(drag_end) g_signal_connect(gesture, "drag-end", G_CALLBACK(drag_end), data);
4633+
if(drag_update) g_signal_connect(gesture, "drag-update", G_CALLBACK(drag_update), data);
4634+
4635+
return gesture;
4636+
}
4637+
46204638
GtkEventController *(dt_gui_connect_motion)(GtkWidget *widget,
46214639
GCallback motion,
46224640
GCallback enter,

src/gui/gtk.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,8 +553,21 @@ GtkGestureSingle *(dt_gui_connect_click)(GtkWidget *widget,
553553
#define dt_gui_connect_click_all(widget, pressed, released, data) \
554554
gtk_gesture_single_set_button(dt_gui_connect_click(widget, pressed, released, data), 0)
555555

556+
GtkGesture *(dt_gui_connect_drag)(GtkWidget *widget,
557+
GCallback drag_begin,
558+
GCallback drag_end,
559+
GCallback drag_update,
560+
gpointer data);
561+
#define dt_gui_connect_drag(widget, drag_begin, drag_end, drag_update, data) ( \
562+
ASSERT_FUNC_TYPE(drag_begin, void(*)(GtkGestureDrag *, double, double, __typeof__(data))), \
563+
ASSERT_FUNC_TYPE(drag_end, void(*)(GtkGestureDrag *, double, double, __typeof__(data))), \
564+
ASSERT_FUNC_TYPE(drag_update, void(*)(GtkGestureDrag *, double, double, __typeof__(data))), \
565+
dt_gui_connect_drag(GTK_WIDGET(widget), G_CALLBACK(drag_begin), G_CALLBACK(drag_end), G_CALLBACK(drag_update), (data)))
566+
556567
#define dt_gui_claim(gesture) \
557568
gtk_gesture_set_state(GTK_GESTURE(gesture), GTK_EVENT_SEQUENCE_CLAIMED)
569+
#define dt_gui_deny(gesture) \
570+
gtk_gesture_set_state(GTK_GESTURE(gesture), GTK_EVENT_SEQUENCE_DENIED)
558571

559572
GtkEventController *(dt_gui_connect_motion)(GtkWidget *widget,
560573
GCallback motion,

src/iop/exposure.c

Lines changed: 26 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -825,48 +825,36 @@ static float _exposure_proxy_get_effective_exposure(dt_iop_module_t *self)
825825
return g->effective_exposure;
826826
}
827827

828-
static void _exposure_proxy_handle_event(GtkEventController *controller,
829-
int n_press,
830-
gdouble x, gdouble delta,
831-
const gboolean blackwhite)
828+
static void _exposure_proxy_handle_event(int n_press,
829+
gdouble delta,
830+
GdkModifierType state,
831+
const gboolean is_blackpoint)
832832
{
833-
dt_iop_module_t *self = darktable.develop->proxy.exposure.module;
833+
const dt_iop_module_t *const self = darktable.develop->proxy.exposure.module;
834834
if(self && self->gui_data)
835835
{
836-
static gboolean black = FALSE;
837-
if((n_press > 0 && GTK_IS_GESTURE_SINGLE(controller)) // button press
838-
|| !n_press) // scroll event
839-
black = blackwhite;
840-
841-
if(black)
842-
x *= -1;
843-
844-
const dt_iop_exposure_params_t *p = self->params;
845-
dt_iop_exposure_gui_data_t *g = self->gui_data;
846-
GtkWidget *widget = black ? g->black :
847-
p->mode == EXPOSURE_MODE_DEFLICKER
848-
? g->deflicker_target_level : g->exposure;
849-
if(!n_press)
850-
darktable.bauhaus->scroll(GTK_EVENT_CONTROLLER_SCROLL(controller), 0.0, delta, widget);
836+
const dt_iop_exposure_params_t *const p = self->params;
837+
const dt_iop_exposure_gui_data_t *const g = self->gui_data;
838+
GtkWidget *const widget =
839+
is_blackpoint ? g->black : (p->mode == EXPOSURE_MODE_DEFLICKER
840+
? g->deflicker_target_level : g->exposure);
841+
const float val = dt_bauhaus_slider_get(widget);
842+
const float accel = dt_accel_get_speed_multiplier(widget, state);
843+
if(is_blackpoint)
844+
delta = -delta;
845+
846+
if(n_press == 2)
847+
dt_bauhaus_widget_reset(widget);
848+
else if(!n_press)
849+
{ // scroll
850+
const float step = dt_bauhaus_slider_get_step(widget);
851+
dt_bauhaus_slider_set(widget, val + delta * step * accel);
852+
}
851853
else
852-
{
853-
// ignores the quad width, but is accurate enough
854-
const int slider_width = gtk_widget_get_allocated_width(widget);
855-
// FIXME: it would be nice to fetch scope_height when using waveform
856-
const int scope_width =
857-
gtk_widget_get_allocated_width(darktable.lib->proxy.histogram.module->widget);
858-
// bauhaus scales motion to slider widget, but we want to scale
859-
// proportional to the scope widget, particularly when the
860-
// slider has not been allocated and has a width of 1 (which can
861-
// happen if its module group has not yet been displayed)
862-
x = x * slider_width / scope_width;
863-
if(GTK_IS_GESTURE_SINGLE(controller))
864-
if(n_press > 0)
865-
darktable.bauhaus->press(GTK_GESTURE_SINGLE(controller), n_press, x, 0, widget);
866-
else
867-
darktable.bauhaus->release(GTK_GESTURE_SINGLE(controller), -n_press, x, 0, widget);
868-
else
869-
darktable.bauhaus->motion(GTK_EVENT_CONTROLLER_MOTION(controller), x, 0, widget);
854+
{ // drag
855+
const float s_min = dt_bauhaus_slider_get_soft_min(widget);
856+
const float s_max = dt_bauhaus_slider_get_soft_max(widget);
857+
dt_bauhaus_slider_set(widget, val + delta * (s_max - s_min) * accel);
870858
}
871859

872860
gchar *text = dt_bauhaus_slider_get_text(widget, dt_bauhaus_slider_get(widget));

src/libs/histogram.c

Lines changed: 71 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -303,72 +303,91 @@ void lib_histogram_update_tooltip(const dt_scopes_t *const scopes)
303303
g_free(tip);
304304
}
305305

306-
static void _drawable_button_press(GtkGestureSingle *gesture,
307-
int n_press,
308-
double x,
309-
double y,
310-
dt_scopes_t *s)
306+
static void _drawable_drag_begin(GtkGestureDrag* gesture,
307+
gdouble start_x,
308+
gdouble start_y,
309+
dt_scopes_t *s)
311310
{
312311
if(s->highlight != DT_SCOPES_HIGHLIGHT_NONE
313-
&& dt_scopes_func_exists(s->cur_mode, get_exposure_pos))
312+
&& dt_scopes_func_exists(s->cur_mode, get_exposure_delta)
313+
&& dt_gui_claim(gesture))
314314
{
315315
dt_control_change_cursor("grabbing");
316-
s->dragging = (n_press == 1);
317-
const double pos = dt_scopes_call(s->cur_mode, get_exposure_pos, x, y);
318-
dt_dev_exposure_handle_event(GTK_EVENT_CONTROLLER(gesture), n_press, pos, 0.0,
319-
s->highlight == DT_SCOPES_HIGHLIGHT_BLACK_POINT);
316+
s->last_offset_x = s->last_offset_y = 0.0;
317+
s->dragging = TRUE;
320318
}
319+
else
320+
dt_gui_deny(gesture);
321321
}
322322

323-
static void _drawable_button_release(GtkGestureSingle *gesture,
324-
int n_press,
325-
double x,
326-
double y,
327-
dt_scopes_t *s)
323+
static void _drawable_drag_end(GtkGestureDrag* g, gdouble ox, gdouble oy, dt_scopes_t *s)
328324
{
329-
if(s->highlight != DT_SCOPES_HIGHLIGHT_NONE)
325+
dt_control_change_cursor("grab");
326+
s->dragging = FALSE;
327+
}
328+
329+
static void _drawable_drag_update(GtkGestureDrag* gesture,
330+
gdouble offset_x,
331+
gdouble offset_y,
332+
dt_scopes_t *s)
333+
{
334+
dt_scopes_mode_t *const cur_mode = s->cur_mode;
335+
// GTK4: use gtk_event_controller_get_current_event_state()
336+
GdkEvent *event = gtk_get_current_event();
337+
if(!event) return;
338+
if(gdk_event_get_event_type(event) == GDK_MOTION_NOTIFY)
330339
{
331-
dt_control_change_cursor("grab");
332-
s->dragging = FALSE;
333-
dt_dev_exposure_handle_event(GTK_EVENT_CONTROLLER(gesture), -n_press, x, 0.0, FALSE);
340+
// at any time user may user a modifier key to adjust speed
341+
// multiplier, so calculate delta since last move, rather than
342+
// offset since drag start
343+
const gdouble ox = offset_x - s->last_offset_x;
344+
const gdouble oy = offset_y - s->last_offset_y;
345+
const double delta = dt_scopes_call(cur_mode, get_exposure_delta, ox, oy);
346+
dt_dev_exposure_handle_event(1, delta, event->motion.state,
347+
s->highlight == DT_SCOPES_HIGHLIGHT_BLACK_POINT);
348+
s->last_offset_x = offset_x;
349+
s->last_offset_y = offset_y;
334350
}
351+
gdk_event_free(event);
352+
}
353+
354+
static void _drawable_button_press(GtkGestureSingle *gesture,
355+
int n_press,
356+
double x,
357+
double y,
358+
dt_scopes_t *s)
359+
{
360+
if(s->highlight != DT_SCOPES_HIGHLIGHT_NONE && n_press == 2 && dt_gui_claim(gesture))
361+
dt_dev_exposure_handle_event(2, 0.0, 0,
362+
s->highlight == DT_SCOPES_HIGHLIGHT_BLACK_POINT);
335363
}
336364

337365
static void _drawable_motion(GtkEventControllerMotion *controller,
338366
double x,
339367
double y,
340368
dt_scopes_t *s)
341369
{
342-
dt_scopes_mode_t *const cur_mode = s->cur_mode;
343-
if(s->dragging
344-
&& s->highlight != DT_SCOPES_HIGHLIGHT_NONE
345-
&& dt_scopes_func_exists(cur_mode, get_exposure_pos))
346-
{
347-
const double pos = dt_scopes_call(cur_mode, get_exposure_pos, x, y);
348-
dt_dev_exposure_handle_event(GTK_EVENT_CONTROLLER(controller), 1, pos, 0.0, FALSE);
349-
}
370+
if(s->dragging) return;
371+
372+
GtkAllocation allocation;
373+
gtk_widget_get_allocation(s->scope_draw, &allocation);
374+
const double posx = x / (double)(allocation.width);
375+
const double posy = y / (double)(allocation.height);
376+
const dt_scopes_highlight_t prior_highlight = s->highlight;
377+
378+
if(dt_scopes_func_exists(s->cur_mode, get_highlight))
379+
s->highlight = dt_scopes_call(s->cur_mode, get_highlight, posx, posy);
350380
else
381+
s->highlight = DT_SCOPES_HIGHLIGHT_NONE;
382+
383+
if(prior_highlight != s->highlight)
351384
{
352-
GtkAllocation allocation;
353-
gtk_widget_get_allocation(s->scope_draw, &allocation);
354-
const double posx = x / (double)(allocation.width);
355-
const double posy = y / (double)(allocation.height);
356-
const dt_scopes_highlight_t prior_highlight = s->highlight;
357-
358-
if(dt_scopes_func_exists(cur_mode, get_highlight))
359-
s->highlight = dt_scopes_call(cur_mode, get_highlight, posx, posy);
385+
lib_histogram_update_tooltip(s);
386+
dt_scopes_refresh(s);
387+
if(s->highlight == DT_SCOPES_HIGHLIGHT_NONE)
388+
dt_control_change_cursor("default");
360389
else
361-
s->highlight = DT_SCOPES_HIGHLIGHT_NONE;
362-
363-
if(prior_highlight != s->highlight)
364-
{
365-
lib_histogram_update_tooltip(s);
366-
dt_scopes_refresh(s);
367-
if(s->highlight == DT_SCOPES_HIGHLIGHT_NONE)
368-
dt_control_change_cursor("default");
369-
else
370-
dt_control_change_cursor("grab");
371-
}
390+
dt_control_change_cursor("grab");
372391
}
373392
}
374393

@@ -474,10 +493,11 @@ static void _eventbox_scroll_callback(GtkEventControllerScroll* self,
474493
{
475494
// FIXME: should scroll for exposure change be handled by each scope, rather than here?
476495
// FIXME: should handle horizontal scrolling as well?
496+
// FIXME: should handle smooth scrolling rather than discrete?
477497
// FIXME: should scrolling of scope be handled in the drawable rather than
478498
// the eventbox.
479-
const gboolean black = s->highlight == DT_SCOPES_HIGHLIGHT_BLACK_POINT;
480-
dt_dev_exposure_handle_event(GTK_EVENT_CONTROLLER(self), 0, 0, black ? -dy : dy, black);
499+
dt_dev_exposure_handle_event(0, dy, event->scroll.state,
500+
s->highlight == DT_SCOPES_HIGHLIGHT_BLACK_POINT);
481501
}
482502
else
483503
{
@@ -831,8 +851,9 @@ void gui_init(dt_lib_module_t *self)
831851
// FIXME: why does cursor motion over buttons trigger multiple draw callbacks?
832852
g_signal_connect(G_OBJECT(s->scope_draw),
833853
"draw", G_CALLBACK(_drawable_draw_callback), s);
834-
dt_gui_connect_click_all(s->scope_draw, _drawable_button_press,
835-
_drawable_button_release, s);
854+
dt_gui_connect_drag(s->scope_draw, _drawable_drag_begin, _drawable_drag_end,
855+
_drawable_drag_update, s);
856+
dt_gui_connect_click(s->scope_draw, _drawable_button_press, NULL, s);
836857
dt_gui_connect_motion(s->scope_draw, _drawable_motion, NULL,
837858
_drawable_leave, s);
838859
// constrain height of button_box_right to s->scope_draw, necessary

src/libs/scopes.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,11 @@ typedef struct dt_scopes_functions_t
9595
dt_scopes_highlight_t (*get_highlight)(const struct dt_scopes_mode_t *const self,
9696
const double posx,
9797
const double posy);
98-
double (*get_exposure_pos)(const struct dt_scopes_mode_t *const self,
99-
const double x,
100-
const double y);
98+
// returns 0 <= pos <= 1 where a pos of 1 is a drag across the full
99+
// width/height of the scope (depending on mode exposure drag axis)
100+
double (*get_exposure_delta)(const struct dt_scopes_mode_t *const self,
101+
const double offset_x,
102+
const double offset_y);
101103
void (*append_to_tooltip)(const struct dt_scopes_mode_t *const self,
102104
gchar **tip);
103105
void (*eventbox_scroll)(struct dt_scopes_mode_t *const self,
@@ -139,7 +141,8 @@ typedef struct dt_scopes_t
139141
int update_counter; // most recent pixelpipe vs mode data
140142
dt_scopes_highlight_t highlight; // depends on mouse position
141143
scopes_channels_t channels; // display state chosen by RGB buttons
142-
gboolean dragging; // pre-GtkGestureDrag hack
144+
gboolean dragging; // to block motion handling during drag
145+
gdouble last_offset_x, last_offset_y; // for drag handling
143146
// UI elements
144147
GtkWidget *overlay; // GtkOverlay -- scope and buttons
145148
GtkWidget *button_box_left; // GtkBox -- scope mode buttons

0 commit comments

Comments
 (0)