Darktable: Keyboard shortcuts with multiple instances

[elstoc]

darktable has a number of keyboard shortcuts that allow you to interact with darkroom image processing modules. However, where multiple instances of a module are present, it is not easy to predict which instance of the module the shortcuts will affect (especially where instances have been renamed). I would like to propose a set of consistent rules that can be used to decide which module instance the keyboard shortcuts interact with.

I’m using the exposure module as an example, because it has a multiple ways to interact with it and it highlights inconsistencies between those methods. Also it’s probably the module most likely to be used with multiple instances.

I have set up shortcuts to show the module (Shift+e), increase the exposure (=), decrease the exposure (-), and a dynamic shortcut that allows me to adjust the exposure with the scroll wheel (e).

The following observations are all using an image that has two instances of the exposure module (“exposure” and “exposure 1”).

If I use the histogram to change the exposure (by mouse-dragging), those adjustments always impact the latest module in the pixelpipe (in this case “exposure 1” - if I swap the modules in the pixelpipe, the change impacts “exposure”).

If I use “Shift+e”, this also activates “exposure 1”. However, if I change the pixelpipe order, that shortcut still activates “exposure 1”. The same is true with the dynamic shortcut (e), increase (=) and decrease (-).

I assume from these tests that darktable is applying the keyboard shortcut to the last created instance of the module. Renaming and reordering the instances doesn’t change which one is updated, and this is fairly important. Many users utilise named instances to manage their workflow and once an instance has been renamed it’s hard to know which was the last one created, potentially making shortcuts less useful or leading them to have unexpected/undesired effects.

I would like to propose the following set of rules to decide which module instance a shorcut interacts with – this would be the most logical for my workflow but I know it may not suit everyone so I leave the exact details for discussion.

I suggest that darktable starts with a list of all instances of a given module and applies the shortcut whenever the following filter operations reduce the number of available instances to 1:

1. If any instances are displayed (module is open on the RHS of the screen), select only those instances, else select all instances
2. If any instances from the previous list are active, select only those instances, else leave the list unchanged
3. If any instances from the previous list are global in operation (do not have masks associated) select only those instances, else leave the list unchanged
4. If there are still multiple instances in the list, select the instance that is latest in the pixelpipe

Assuming that we are keeping the interaction between the histogram and exposure module (I know there have been discussions about removing it), the same order should be used for this interaction.

[aurelienpierre]

Ping @AlicVB

[elstoc]

Thanks. Am happy to try my hand at sorting this myself but I might have bitten off more than I can chew for something to start off with!

[AlicVB]

Implementing that shouldn't be too difficult, I think (well... when I have the time...)
What is more difficult is to define the rules, so it's great that you start working on that.

Let's see what others think about that...
For my part, I fully agree for 1 and 2. For 3 and 4, I'm not so sure. imho, it'll not really be obvious for the user pov... and for step 4, I would maybe add an option in pref to either "apply to all instance" or "apply to last in pipe" or "don"t apply" (and maybe "apply to first in pipe")

[elstoc]

Thanks @AlicVB. From the point-of-view of my workflow, if I'm using a keyboard shortcut and I don't have a module open I think it's safe to assume I'm trying to do a global adjustment - so perhaps point 3 should only apply to those shortcuts that change parameters within a module. Point 4 was more about consistency with the way the exposure-histogram link works (from what I can tell, though, that's implemented in a different way to the keyboard shortcuts so the two aren't necessarily equivalent). However, the main point is that the shortcuts are not implemented in an obvious way right now and there should be some rules as to how they are applied. I'm all for a configuration setting which, if nothing else, will explicitly spell out to the user how it all works.

I'm keen to get a handle on how the code is put together, even if I'm not quite up to making the change myself at the moment. Any pointers you could give me to getting my head around how it works right now would be greatly appreciated. I've been delving around in the code and I can't keep it all in my head long enough for it to become clear!

[johnny-bit]

This issue happened to me just today! In my opinion @elstoc is right about point 3 - I wholeheartly did expect that keyboard shortcuts will be grabbed by global module or by "active" module. I also agree with @AlicVB case for point 4 - the application in case of multiple non-filtered-out modules present should be confugureable and not a surprise(to those who don't read manual ;))

Additionally (especially for dynamic keys) currently there's tip in topbar: "Scroll to change exposure of exposure module". It would be great if that took in filtering and name of module which it would effect so for exampe if instance face is the one that'll take input, the topbar tip should say: "Scroll to change exposure of module exposure face".

I thought of any good visual indication of what's going on, but I think that topbar tip would be enough... Unless of course user hid the topbar, in which case (when module in question isn't open) the message drawn during altering of setting should say instead of

exposure: 1.50 EV

should say:

face (or exposure face)
exposure: 1.50 EV

Immediatelly alerting user about module being in use.

[elstoc]

There are ongoing discussions about moving stuff out of the top bar (https://github.com/darktable-org/darktable/issues/3740) because many (myself included) keep it hidden to maximize the size of the image. Whatever solution is found to that issue, I agree that visual indication of which module instance you're adjusting would be ideal. Even better might be if the module instance in question was temporarily expanded (in the right panel, if not already expanded) while the adjustment was being made, then the panel returned to its previous state after. Though that might be over-engineering it a bit.

[johnny-bit]

Designing best fits-all way of indicating such things (and many others) is problematic. I remember in ye olde times of Win98 developing windows app, we were told to put hints in status bar. Then allow users to hide status bar, which of course meant that user had no indicators nor hints present... This is hard UI/UX problem which needs to be somehow addressed and honestly doing ALL of indicators isn't out of questions (what if user has EVERYTING hidden? the only indication left now is message drawn on image - dunno how it's called :/)

[elstoc]

Probably a discussion best left for the other thread but, as with this issue, the best thing is probably some config setting with a few options in it, like has been done for the "image infos" line.

[elstoc]

Still some way from understanding how it all works but looking at the code prompted some new tests.

It appears that dynamic shorcuts and static shortcuts work differently:

• static shortcuts are always allocated to the first created instance of a module
• dynamic shortcuts are always allocated to the last created instance of a module

However both exhibit the same issue (and this one is definitely a fault). Create two instances of a a module. If you delete the [first/second] instance and then try to modify it with a [static/dynamic] (respectively) shortcut you get a popup indicating that the value has changed but because the callback is attached to the removed instance the shortcut no longer actually does anything to the image. In fact if you use the 'show module' shortcut it causes darktable to crash (because it's trying to show a non-existent widget).

[johnny-bit]

If you delete the [first/second] instance and then try to modify it with a [static/dynamic] (respectively) shortcut you get a popup indicating that the value has changed but because the callback is attached to the removed instance the shortcut no longer actually does anything to the image. In fact if you use the 'show module' shortcut it causes darktable to crash (because it's trying to show a non-existent widget).

Aaand you've found a bug and provided reproduction steps!

[elstoc]

I think this is just another symptom of a more general problem that accelerators don't interact well with duplicated instances and probably require a general redesign. Unfortunately most of the code is uncommented and I'm finding it pretty hard to pull apart what it does.

[elstoc]

I think that's most of my code in place in the PR now (all accelerators now apply to the latest module in the pipe). It's time to think about what rules I should implement for choosing the module to attach the accelerators to.

Does anyone have any further suggestions/comments on my original list?

[johnny-bit]

Your list is IMO perfectly fine. If you could also grab and do visual indication as I mentioned earlier then it'd be perfect for me.

[elstoc]

@johnny-bit your wish is my command. I have changed dynamic accels so that, if an instance is named, the hint message now displays as "scroll to change [slider] of [module] [instance] module". If the module has no instance name the message displays as before.

[elstoc]

Thinking more about it, some of the rules I've suggested above could have unintended consequences, especially with 'toggling' shortcuts.

For example if you have multiple module instances and you use the 'enable module' shortcut to enable one of them followed by the same shortcut to disable it again, you would reasonably expect the same module to be impacted both times but there's a good chance it wouldn't work that way in some scenarios.

Similarly with the 'show module' shortcut.

[elstoc]

My current plan is to stick to points 3 and 4 only, both controllable via new preferences options:

• Apply keyboard shortcuts to [first instance / last instance] (default last - which matches current behaviour of exposure changes via histogram)
• Prefer unmasked modules in keyboard shortcuts [yes / no] (default no - current behaviour)

[johnny-bit]

I still think open modules are good canidates for shortcuts and the toggles... Maybe "remember last toggled module for toggle switch"? Generally toggle shortcuts are weird in logic when you can have multiple instances, there's no easy way to logically address toggle issue that won't be illogical for some ;)

[elstoc]

Now that there's a PR in progress and to keep the thread in one place, let's continue this discussion there...

[github-actions[bot]]

This issue did not get any activity in the past 30 days and will be closed in 365 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

[johnny-bit]

There IS activity :) Once pr #4424 lands this will be closed :)