Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add shortcuts for focused module and sliders/combos/buttons and tabs in it #12829

Merged
merged 4 commits into from Nov 12, 2022

Conversation

dterrahe
Copy link
Member

@dterrahe dterrahe commented Nov 9, 2022

This adds a virtual module (like "blending") called "focused" under processing modules that represents the currently focused module. Any shortcuts assigned to it will impact different modules, depending on which one you are currently working with. So you could assign one shortcut to processing modules/focused with element presets and use that to scroll through presets for whichever module you are working on, rather than having to define separate shortcuts for each one.

Within this virtual module, there are virtual widgets, representing all the widgets of the same type in the focused module. By assigning midi knobs to processing modules/focused/sliders for example, and setting a different element for each (1st, 2nd, etc) you can use the same knobs for (almost) all the sliders across all modules.

If a module contains a notebook / tabs, only the widgets in the currently visible tab are considered/counted. So this can also be used to assign the same three knobs to be used for all the sliders across the color calibration tabs (although you have to make sure the module is the focused one to use this).

The notebook tabs themselves can be switched by assigning shortcuts to processing modules/focused/tabs. The elements has generic (1st, 2nd etc) names rather than the actual tab names because of course they differ between modules.

@jenshannoschwalm maybe you would be able to give feedback?

@dterrahe dterrahe mentioned this pull request Nov 9, 2022
47 tasks
@jenshannoschwalm
Copy link
Collaborator

Spontaneous reaction just from reading diffs, sounds/looks very good! One example from my workflow would be the "contrast" knob used in filmic, sigmoid or color balance rgb. So far only possible with some sort of key combination (which i can never fully remember leading to wrong module accessed).

Otherwise, doesn't this lead to a situation where we would have to expand a module to give control to contrast? Or is there some sort of fallback (to a specific module) possible?

@dterrahe
Copy link
Member Author

dterrahe commented Nov 9, 2022

What you are talking about are group/multi shortcuts, possibly with some optionality/conditionality/ordering features. That is complex from an implementation perspective but also a challenge in how to present this to the user in an understandable manageable way. I have wanted to do this from the start, but given feedback so far think this is too ambitious.

This is not that. This is just "manipulate the first (2nd etc) widget in the module and tab I'm currently in". The way I would see this being used is the knobs of an x-touch being assigned to generic slider 1-8 and a bunch of buttons being assigned to switch between modules and tabs. Because contrast is the 1st slider in some but 3rd in other and sometimes in a specific tab, you still (even more!) need to remember what location they are in, if you have the side panel permanently hidden.

Some might also specifically want to use this for color calibration, as mentioned, but you still, separately, need to switch to the module for the shortcuts to be sent there. Otherwise the 3 or 4 knobs dedicated to this would be useable in whatever other module you are in, but maybe not with as much benefit.

What you want might still be achievable in lua, given that all the widgets can be reached via shortcuts from there (using dt.gui.action; see the widget popup when in shortcut mapping mode) and you can ask the focused state of modules too. You'd have to have two lua callbacks, one for an up and one for a down move, and split the mapping of a turn up and a turn down. (Actually, it seems that splitting functionality might be broken right now :-/ I'll look if I can fix/generalise. My initial implementation only supported splitting up/down to two different effects of the same widget, to support min/max for example. I think the same type of widget that has the same visibility, across views, should work as well.) What you would lose, with this lua route, is fallbacks. So if you want to support ctr+knob for slower, more accurate, movement, you'd have to define two more (up/down) lua routines for that.

Maybe adding the possibility for lua to fully define a new action (with an _action_process callback that takes element/effect/speed) would be worth it. Or even just ones supporting the specific signatures of sliders, combos and togglebuttons, and have the standard fallbacks automatically apply to them. That would be the "most" flexible; you could then in code multiplex a single move to however many widgets using whatever decision complexity. @wpferguson Thoughts?

@dterrahe dterrahe added feature: new new features to add scope: UI user interface and interactions documentation-pending a documentation work is required release notes: pending labels Nov 9, 2022
@dterrahe dterrahe added this to the 4.2 milestone Nov 9, 2022
@dterrahe
Copy link
Member Author

dterrahe commented Nov 10, 2022

@wpferguson what I would like to do, separate from this PR, is allow the "user" to define a simple function in lua that processes shortcuts received by a fake bauhaus slider, combo or button and dispatches them (in the usual case) to real widgets. So far example, I could define

function contrast(element, effect, speed)
  if dt.gui.action("iop/sigmoid", 0, "focus", "toggle", ) then
    dt.gui.action("iop/sigmoid/contrast", 0, element, effect, speed)
  else if  dt.gui.action("iop/colorbalancergb", 0, "focus", "toggle", ) then
    dt.gui.action("iop/colorbalancergb/contrast", 0, element, effect, speed)
  else
    dt.gui.action("iop/filmicrgb/contrast", 0, element, effect, speed)
end

with element and effect being strings like dt.gui.action takes. Then I would like to register this as a callback with a widget type (0=button, 1=slider, 2=combo). I would provide you with a function dt_bauhaus_register_callback(gchar *name, dt_bauhaus_type_t type, gpointer callback) I would then list this in the shortcuts dialog under the lua section as if it were a real slider/combo/button (so it understands the same elements and effects) and then @jenshannoschwalm would assign this to a midi knob and it would intelligently do the right thing and support the same fallbacks as normal bauhaus wigets (ctrl for slow, shift for fast etc).

The function could also multiplex one shortcut to many widgets simultaneously, maybe reversing direction so you can with one key and mouse wheel move two widgets in opposite directions (I had a request a while back for this functionality; you can already hold two separate shortcut keys at the same time and have a move affect both and then set opposite speeds in the shortcuts, but that does require holding two keys.)

I believe providing this advanced functionality via lua would provide more flexibility than the shortcut dispatch mechanism built into dt ever could and without a lot of extra complexity.

EDIT: actually, the function should pass on the return float from the (main) dt.gui.action it called, so it should return a float itself.

@dterrahe
Copy link
Member Author

@TurboGit For the avoidance of doubt, the discussion above is about different functionality that hopefully, with the help of @wpferguson, I will submit in a separate PR probably somewhere post 4.2.

As far as I'm concerned this PR is ready. Not sure if @jenshannoschwalm is available to help testing, even if this is not covering his requirement.

@TurboGit
Copy link
Member

@dterrahe : I have this on my TODO list. I'll check tonight or tomorrow. This is a new feature, so time is short.

@dterrahe
Copy link
Member Author

time is short.

I know!! ;-)

Although the second commit is a bug fix (for something not reported broken by anyone, so probably nobody is using it ;-) again)

@wpferguson
Copy link
Member

@dterrahe I will make some time this weekend so I can wrap my head around this. I've read through it a couple of times but I need a little time to think it through and get it straight in my head.

@dterrahe
Copy link
Member Author

I've read through it a couple of times

Please ask me to explain in a different way if it doesn't make sense. There are no dumb questions! I wrote it in a reasonably compact way, because sometimes more words just make it harder, but I could describe the whole step-by-step flow in more detail.

@dterrahe
Copy link
Member Author

@wpferguson no urgency of course; feature freeze in a few days, so this will wait for 4.4...

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some changes. Found something that I think should be improved.

  • Create 2 shortcuts for 1st & 2nd sliders.
  • Open exposure.
  • Use 1st slider - it is moving the exposure slider - OK
  • Use 2nd slider - it is moving the percentile slider which is in fact hidden - NOK

I think the visible widgets only should be considered.

@@ -118,6 +118,9 @@ void dt_control_init(dt_control_t *s)
s->actions_fallbacks = (dt_action_t){ DT_ACTION_TYPE_CATEGORY, "fallbacks", C_("accel", "fallbacks") };
s->actions = &s->actions_global;

s->actions_focus = (dt_action_t){ DT_ACTION_TYPE_IOP, "focus", C_("accel", "focused") };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm proposing to add virtual for the module to stand out : focused (virtual)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I'll do the same for blending. This would clarify the intent.

@jenshannoschwalm
Copy link
Collaborator

As far as I'm concerned this PR is ready. Not sure if @jenshannoschwalm is available to help testing, even if this is not covering his requirement.

As you guessed i was heavily involved with bugfixing segmentation stuff - that unfortunately kept me away from adding dng lenses to the new embedded stuff ...

I just had a short test and it works for me too though, i am not sure how much use i can make out of it.

@dterrahe
Copy link
Member Author

Thanks for catching that! Some widgets are disabled by hiding them, some by hiding the group they are in and some by switching to another stack page. Now checking for that as well.

The "blending" label is also used in the toast that shows when using the shortcut, so I'd rather not make that too long or too wordy, as that would be distracting. I've now added <> brackets around "blending" and "focused" and grouped them together at the top of the processing modules. Do you think that will be enough to indicate they are "special"?

Separately, I'm considering adding a tooltip field to dt_action_def_t which would allow adding an explanation to these special actions in the configuration dialog (maybe also in a description column). It would just mean adding a lot of text in many places so that's more of a documentation effort (i.e. there should be consistency between the terminology used in the added explanations in the program and in the manual).

i am not sure how much use i can make out of it.

Thanks and that's absolutely fine. This is all about giving people options. Everybody is using the input ng framework in their own way and nobody is required to use any of the functionality at all. But once they do start using it, they always find they just want this one extra thing! ;-)

I do think being able to use module actions on the active module is very useful. Like having a reset button that impacts the most recently changed module. Or scrolling through presets. Or moving the current instance up or down in the stack. These shortcuts were already available, but you would have to dedicate a separate key to each module. Now they can be shared, saving keys. Same with scrolling through notebook tabs; I can now use the same key for filmic/calibration etc. which makes it much more likely that I have enough keys and can remember them...

@TurboGit
Copy link
Member

I've now added <> brackets around "blending" and "focused" and grouped them together at the top of the processing modules. Do you think that will be enough to indicate they are "special"?

It certainly shows that they are special and grouping them at top is much better. I still think that having (virtual) in the naming would be good, but we can always change that later if others have the same feeling.

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go now, thanks!

@TurboGit TurboGit merged commit aedb956 into darktable-org:master Nov 12, 2022
@TurboGit
Copy link
Member

@dterrahe : I'm contemplating buying a specific midi device dedicated to darktable/image processing. What you would you recommend as best? And possibly a second choice less expensive?

@jenshannoschwalm
Copy link
Collaborator

I got a Loupedeck plus, just perfect i think and very convenient if you accept the price. Developing speed on images certainly doubled if you don't have to mess with masks

@dterrahe
Copy link
Member Author

I still think that having (virtual) in the naming would be good, but we can always change that later if others have the same feeling.

For focused it doesn't have the same drawback of taking space in toasts that I mentioned for blending so for that one my only reasoning would be consistency. For blending I really think having the additional text show up every time in the toast makes it more work to get the useful feedback out of it.

Maybe more importantly, for me "virtual" here doesn't add any more information than <> does. "Somehow this is from the other modules". In some way, they are also different from each other, because focused provides duplicates of another module and widgets, whereas blending is the only way to access the blending widgets and the fact that it doesn't always refer to the same widgets from the same module is no different than that "ordinary" module widgets can refer to different instances.

Either way, if the consensus doesn't agree, that would be fine with me of course.

And possibly a second choice less expensive?

I like the x-touch mini because it gives more feedback (lights around the knobs and buttons lighting up) and you can push the knobs to reset but it is quite plasticky and now at least double the price that I got it for. The Arturia BeatStep is much sturdier and works fine, but no feedback and no pushknobs. Now actually cheaper. I assume the Minilab MKii would work the same if you like that form factor or price. Other than that, I'm not allowing myself to buy any more devices, so loupedeck is out of reach (I've never even played with it) as are these guys
https://www.amazon.fr/Console-de-mixage-Behringer-X-Touch/dp/B017BL6EZU
https://www.amazon.com/gp/product/B00WU6FDNG
It would be good to have a diversity of devices amongst developers so we can expand support and testing...

@TurboGit
Copy link
Member

Thanks for the feedback. The 2 last in your list @dterrahe looks intimidating and very large! The Loupdeck plus seems more what I'm looking for... kind of keyboard size.

@jenshannoschwalm : A question if you don't mind, all the scrolls under the P1 - P8 are static or can they be configured to control different things depending on current module? For example controlling the Tone Equalizer sliders or the Color Zones graph...

@dterrahe
Copy link
Member Author

can they be configured to control different things depending on current module?

That's basically what
#12829 (comment)
is about...

@jenshannoschwalm
Copy link
Collaborator

jenshannoschwalm commented Nov 12, 2022

A question if you don't mind, all the scrolls under the P1 - P8 are static or can they be configured to control different things depending on current module? For example controlling the Tone Equalizer sliders or the Color Zones graph...

Thats how i use them. You see Ctl/BW, Hue, Sat and Lum buttons beside the scrollers, i use them to preselect the module (Edit: the scrollers give diffent midi code after pressing the button so you don't have to expand the module at all). If Clr/BW was pressed the scrollers access the tone equalizer, The other buttons select corresponding tabs in Color Zones. :-)

@TurboGit
Copy link
Member

That's basically what
#12829 (comment)
is about...

Yes, that's what I though too. It would certainly help, but there is case where I would probably a config that won't be covered (or not, I've never used such controller) here, hence my question. Anyway, @jenshannoschwalm is reassuring, it looks like the number of controls at hand is really huge!

@jenshannoschwalm
Copy link
Collaborator

I is. BTW i won't shoot fotos myself for the next month or so, have to do some training about hospital hygiene. Wouldn't mind to lend for that period, just do a mail.

@dterrahe
Copy link
Member Author

a config that won't be covered

The idea would be that you could program any logic into lua that you could think of (taking into account the current status of all modules etc and possibly saving additional status within lua so you could define modes yourself and toggle between them), so if there still was anything that would not be covered, I can't really think what it would be...

The drawback of this approach would be that you can't fully tailor it from within the shortcuts dialog anymore. You may have to define a separate routine for each key/knob on your keyboard and make sure the mapping in shortcutsrc links them up correctly and to change the behavior you'd have to edit the lua file and restart.

@TurboGit
Copy link
Member

@jenshannoschwalm : I have finally bought a Loupedeck :) Do you have a shortcut file to share? This will speed-up the setup has there is a huge number of buttons!

@dterrahe
Copy link
Member Author

Would be nice to have that information merged into dtdocs.

@TurboGit
Copy link
Member

Maybe more adding it into the darktable main repository? And a link in the doc of course.

@TurboGit
Copy link
Member

And we should probably also encourage others having different MIDI controllers to contribute the configuration.

@TurboGit
Copy link
Member

We may even want to add loading configuration from the pref dialog. This will certainly makes the use of such device easier for everybody.

@dterrahe
Copy link
Member Author

There's an import button in the shortcuts tab/dialog.

It is especially useful when one has multiple devices and needs to load a layout for a specific one.

@dterrahe
Copy link
Member Author

And we should probably also encourage others having different MIDI controllers to contribute the configuration.

Maybe for loupedeck there could be an authoritative one, but for most devices it will very much depend on individual user preferences. Hence why I would think inclusion with dt itself would not really be appropriate.

@jenshannoschwalm
Copy link
Collaborator

Will send tomorrow. Also did some docs for the setup

@TurboGit
Copy link
Member

Maybe for loupedeck there could be an authoritative one, but for most devices it will very much depend on individual user preferences. Hence why I would think inclusion with dt itself would not really be appropriate.

Understood!

@jenshannoschwalm
Copy link
Collaborator

shortcutsrc.zip

I also had made some docs, some things have changed since then while working on dt code. (C3, C4, C5, C6 for sure) At least a starting point avoiding some initial problems i had.

manual.odp

@TurboGit
Copy link
Member

Thanks a lot, will try this! Looking at the doc I see that you did not use ctrl/shift/alt but instead L1,L2,L3, do you know why it is not possible to use the former? I tried to use ctrl + other key but it doesn't work.

@TurboGit
Copy link
Member

TurboGit commented Nov 18, 2022

If you don't mind some questions...

If the scrollers have been selected to be used in Clr/BW mode, they all work in the advanced tab of tone equalizer

What do you mean selected to be used in Clr/BW mode?

For Color Zones I have tried to reset to 0 when pressing one of the scrollers but this does not work. I tried the set or reset shorcut action.

EDIT: I see, the scrollers are having different code when toggling Clr/BW!

@jenshannoschwalm
Copy link
Collaborator

I don't know. Anyway I preferred to have the color keys

@elstoc elstoc added documentation-complete needed documentation is merged in dtdocs and removed documentation-pending a documentation work is required labels Nov 22, 2022
@dterrahe dterrahe deleted the focused_sc branch February 4, 2023 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation-complete needed documentation is merged in dtdocs feature: new new features to add scope: UI user interface and interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants