[New actions] [Builds available] Cycle groups, modules, instances and widgets

First of all, many thanks for getting involved! It is really great to finally see someone taking enough of an interest in the workings of the shortcut system to want to help improve it.

Since this is touching a fundamental part of the application, there are many aspects/thoughts to take into account.

“focused widgets” hasn’t been that much of a thing in dt, because everybody was assumed to be using the mouse anyway, so one can’t just assume things will “just work”. The only things (that I can think of) that take focus are the bauhaus widgets and Entry fields. Anything else quite often forcefully returns focus to the center (so that shortcuts work again; this was more an issue with the previous shortcut system, but to be able to pan with arrow keys, you need the center focused). you don’t want a toggle to retain focus after you clicked it if that means you can’t pan (also reason for colorpicker behavior). But that also (?) means you can’t assign it focus as part of cycling through all widgets. So if people suddenly want to be able to navigate between nodes of a curve with the keyboard, that needs to all be built from scratch. And the shortcut system doesn’t deal with configurability of actions-on-focused-objects, so you can’t change what arrow keys do to bauhaus widgets, but equally you can’t change what shift+scroll does when you’re manipulating mask shapes. Could all be built on top of what we have, but it is work that at the very least would need people to show enough interest to do the (regression) testing. Which first means they need to understand the intent and what is and isn’t supposed to work. Shortcuts in dt sometimes feels a little like quantum physics in that only maybe five people in the world really understand it. And that may be an overestimate.

Anyway, back to the topic at hand :wink: And continuing with the random order nature of this response:

The shortcuts dialog has an import button…

Maybe most importantly, bauhaus widgets are not darkroom specific. So though it is tempting to implement something that will work just there, people will then be disappointed if they can’t work with the, say, export module widgets in the same way. So the action for “focused slider or dropdown” needs to live in global. As I suggested earlier, to make fallbacks work correctly we need to intercept this special proxy action early on in the shortcut matching system. I’ve implemented this here shortcut action for focused bh widget · dterrahe/darktable@4a5f42d. With this you won’t need a separate action for manipulating the button anymore (even the +left click fallback just works). I’m having the proxy action masquerade as a combo because that has fewer elements than the slider type, so you don’t end up with a combo unexpectedly being confronted with a fourth element (“zoom”) that only a slider would know what to do with. To really productise this commit, we’d introduce a separate _action_def_slider_or_combo that has more generic names for the first element and its effects (“selection or value”, “next or up”, “previous or down”) but that is just polish. You’d also want to show a different error rather than “shortcut not assigned” when there is no focused widget.

In theory _action_def_slider_or_combo could have a 3rd element, “change focus”, that would cause it not to be proxied to the focused widget but be handled by the action itself.

Cycle module groups should really live inside utility modules/module groups. In fact, when implementing the “active modules” and “quick access panel” actions there I was a bit lazy; this should really have looked more like the notebook type actions (that you often see as “page”). So the first two elements should be “active modules” and “qap”, but then if you assign a shortcut to either one of these, a fallback would let you “scroll” to other pages (or you could, of course, assign PgUp/Down whatever).

At this point I’d be happy if you submit this as a draft PR so we can have further discussions on github. That way there’ll be a record of the deliberations so future interested persons can find out why certain choices were made. There’s no urgency so lets aim to get something merged early next year.

3 Likes

Thanks for the all the insight, @dterrahe.

Very nice, thanks!

Yes. Again, this is just a tech preview, if you like. I implemented everything in darkroom because it was faster and to avoid touching too many files.

Agreed. Also in this case, I took shortcuts for a proof of concept, but I understand that it would need to be done in a more principled way if it were to be merged.

I noticed that :sweat_smile: I tried for a couple of hours to focus on a toggle button before giving up :stuck_out_tongue_winking_eye:

With pleasure, after 5.4 is released. In the meantime, I would appreciate if folks could try it out and let me know what they think.

2 Likes

Hi Daniele,

I follow this thread. Am in the middle of a dt reorganization towards a ‘test’, ‘prod’ and ‘play-raw’ separation. I believe that to be finished next Monday.

Now for GitHub, I do have an account but have no idea how to work with cloning/building software. Is there a doc I can read to enlighten myself in this?

Would love to try even if it involves completely new routines.

Kind regards, Jetze

2 Likes

Hi @Jetze,

If you are a Mac on Apple silicon user I am happy to share a precompiled binary.

Otherwise, building instructions are on the github page (link points to the right section, even though it’s not reflected in the preview):

Send me a private message if you would like me to assist you in the process. On Linux I should be able to help, but on Windows I have no clue :slight_smile:

2 Likes

Thanks @Masterpiga
I’m on Linux Ubuntu, I will start learning building myself. When others can do I will manage - finally? - too. I’ll plunge in the deep. Thanks for the link!
Kind regards, Jetze

1 Like

On linux it should be a breeze: clone the repo, install the deps, run the build script and you are good to go. Reach out if you need assistance :+1:

Note that the instructions on the page point to the official DT repository.

If you want to try this feature, the correct repository to clone is:

$ git clone -b cycle-through-modules-action \
    https://github.com/masterpiga/darktable.git
2 Likes

I just looked at the code, it’s pretty cool and very useful.
Shouldn’t that be included in 5.6? What happened to it?

1 Like

Oh boy, I haven’t looked at it in 4 months.

There didn’t seem to be a lot of excitement about this idea, but maybe I should prepare a binary so that people can actually play with it and decide if it’s something that we should add.

Thanks for the ping :slight_smile:

1 Like

Even though I would only need 1 1/2 functions, I find them all very useful. The concept is extremely helpful. This is one of those changes where you wonder afterwards why it wasn’t always there :slight_smile:
At least for people who love shortcuts.

In your current code, there is a function to switch up and down between modules.
That’s the part I wouldn’t need, but others would certainly find useful, as I want to switch to the modules using shortcuts anyway.

What I would like, however, is to be able to switch between instances:

Something along the lines of:

// — CYCLE MODULE INSTANCES BEGIN —
void _cycle_module_instances(const gboolean down)
{
dt_iop_module_t *current_module = dt_dev_gui_module();
if(!current_module) return;

const char *op_name = current_module->op;
GList *instances = NULL;

// Collect only instances of the currently focused module
for(GList *l = darktable.develop->iop; l; l = l->next)
{
dt_iop_module_t *m = l->data;
if(g_strcmp0(m->op, op_name) == 0)
{
instances = g_list_append(instances, m);
}
}

if (g_list_length(instances) <= 1)
{
g_list_free(instances);
return; // Nothing to do, there is only one instance
}

GList *current_item = g_list_find(instances, current_module);
GList *next_item = NULL;

if(down)
{
next_item = g_list_next(current_item);
if(!next_item) next_item = g_list_first(instances);
}
else
{
next_item = g_list_previous(current_item);
if(!next_item) next_item = g_list_last(instances);
}

dt_iop_module_t module_to_focus = (dt_iop_module_t)next_item->data;

// Set focus and expand the next instance
dt_iop_request_focus(module_to_focus);
dt_iop_gui_set_expanded(module_to_focus, TRUE, dt_conf_get_bool(“darkroom/ui/single_module”));

// Show toast message
const gchar* instance_name = dt_iop_get_instance_name(module_to_focus);
if(strlen(instance_name) > 0)
dt_toast_log((“focused instance [%s] of module [%s]”),
instance_name, dt_iop_get_localized_name(module_to_focus->op));
else
dt_toast_log(
(“focused module [%s]”),
dt_iop_get_localized_name(module_to_focus->op));

g_list_free(instances);
}

static float _action_callback_cycle_instances(gpointer widget, dt_action_element_t element, const dt_action_effect_t effect, const float move_size)
{
if(DT_PERFORM_ACTION(move_size) && effect != DT_ACTION_EFFECT_DEFAULT_KEY)
{
_cycle_module_instances(effect == DT_ACTION_EFFECT_DOWN);
return 0;
}
return DT_ACTION_NOT_VALID;
}

const dt_action_element_def_t _action_elements_cycle_instances = { { NULL, dt_action_effect_cycle } };
static const dt_action_def_t action_def_cycle_instances = { N(“cycle module instances”), _action_callback_cycle_instances, _action_elements_cycle_instances, NULL, TRUE };
// — CYCLE MODULE INSTANCES END —

// — CYCLE MODULE GROUPS (TABS) BEGIN —
void _cycle_module_groups(const gboolean down)
{
dt_develop_t dev = darktable.develop;
const uint32_t current_group = dt_dev_modulegroups_get(dev);
int num_groups = dt_dev_modulegroups_count(dev) + 1 /
all groups */;
int next_group = ((int)current_group + (down ? +1 : -1)) % num_groups;
if (next_group < 0) next_group = num_groups + next_group;
dt_dev_modulegroups_set(dev, next_group);
}

static float _action_callback_cycle_module_groups(gpointer widget, dt_action_element_t element, const dt_action_effect_t effect, const float move_size)
{
if(DT_PERFORM_ACTION(move_size) && effect != DT_ACTION_EFFECT_DEFAULT_KEY)
{
_cycle_module_groups(effect == DT_ACTION_EFFECT_DOWN);
return 0;
}
return DT_ACTION_NOT_VALID;
}

const dt_action_element_def_t _action_elements_cycle_module_groups = { { NULL, dt_action_effect_cycle } };
static const dt_action_def_t action_def_cycle_module_groups = { N(“cycle module groups”), _action_callback_cycle_module_groups, _action_elements_cycle_module_groups, NULL, TRUE };
// — CYCLE MODULE GROUPS END —

Switching between instances was actually the v0 (if you read the OP, that was the original motivation). Then I made it more general. But I agree that there should be a shortcut specifically for that. It’s pretty trivial to do.

1 Like

Sounds good :slight_smile: Together with the ability to switch between tabs, this would make my workflow with shortcuts completely “round”. Then I can adjust all the controls with the mouse and don’t have to click anything in the menu anymore.

This is even more powerful, but it was almost ignored:

You can find the widget that you need directly from the search bar. Very useful for the controls that do not have a dedicated shortcut.

1 Like

Shortcuts still seem to play a very minor role, even though they are THE feature in DT.

https://www.reddit.com/r/DarkTable/comments/1rlugrf/learning_darktable_shortcuts_the_easy_way/

I’ll take a look!

[quote=“Masterpiga, post:30, topic:54074”].
It’s pretty trivial to do.
[/quote]

For you, yes :slight_smile: I would be very grateful if you could add that.

1 Like

During the weekend I refreshed this code a bit.

Here are the builds (being cooked as I write):

If you want to try, scroll down to the Artifacts section and download the appropriate image for your platform.

These new actions are added:

  • cycle module groups, with effects toggle (switches between group and all modules), up and down (for cycling)
  • cycle modules, with effects toggle (enables/disable module), up and down (for cycling)
  • cycle instances (of currently module), which is like cycle modules but in only cycles through instances of the currently focused module
  • cycle widgets (of currently focused module), with effects edit (open popup), up/down (change value) and toggle picker.
  • edit focused widget, which is what cycle widgets uses to set the value of the widget.

With this setup I can basically navigate the whole darkroom UI with three or four keys.

These are the shortcuts that I used for the videos below:

q=lib/modulegroups/cycle module groups
q;pan=lib/modulegroups/cycle module groups
s=views/darkroom/cycle modules
s;scroll=views/darkroom/cycle modules
x=views/darkroom/cycle instances;up
x;shift=views/darkroom/cycle instances;down
z;scroll=views/darkroom/cycle widgets
z=global/edit focused widget
z;pan=global/edit focused widget;*-1
z;double=global/edit focused widget;button

(the keys are pretty random, I just used whatever I found that was not associated that something that I use already. I am sure you can do better)

However, using the same key (z, in this case) for all widget related stuff is very powerful. After a module is selected I can basically control everything with just one key (z,scroll to select the widget, z,pan to change the value of a slider/combo, z to open the edit box, and zz to toggle on/off the picker for the widget). Admittedly, this is cumbersome for multi-tab widgets or widgets with very long pages, but it works very well in all other cases.

Check out the videos below:

@Qor looking forward to your impressions :slight_smile:

1 Like

Thank you very much!

That’s really brilliant :slight_smile: I’ve set the instance toggle to tab, as I use my left hand for all shortcuts. It fills the gap in my workflow perfectly, even better than I expected!

1 Like

After using it extensively, it would be great to include it in DT.

@Pascal_Obry Do you have any concerns about this?

I’m really not sure about this. We have recently removed an action to cycle through all histogram (kind and mode) because there was too many at this point and cycling through was just not usable. Moreover the code for this was around 1000 lines.

Here I’m a bit worried that cycling through module’s group, widgets is also a corner case that would add code for very few people.

2 Likes

Most of the complexity (and potential for controversy) in the CL is related to the widget-focusing and activation actions.

If I factor that out, the code footprint for cycling modules/instances/groups is very little.

@Pascal_Obry would you consider a stripped down version with no widget cycling?

1 Like

@Pascal_Obry Thank you very much for your reply!

That would fill a (very) important gap.

The shortcut system in darktable is almost perfect. However, there are two points where the mouse is always required for activation.

  • Switching to different module groups

  • Switching between instances of a module

While the first is more of a clarity issue (you can still access the module with a shortcut), the latter is a real dead end.

A typical example is exposure with a second instance. One instance is for basic exposure and another is for dodging and burning or highlighting specific details. I suspect I’m not the only one who often switches between instances here.

Here’s an example of how I use the shortcuts for modules. “F” is for exposure, and with the suggested extension, I use ‘Tab’ to switch between instances and “Q” to switch between module groups.

A few days ago, we added a “starter kit” to dt.info, which includes shortcuts, module presets and a theme that displays the above shortcuts directly in the modules.
We have an average of about 40 downloads per day, which is (perhaps) not much, but I am sure that the change would help some people.