darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.64k stars 1.13k forks source link

allow soft slider bounds to be exceeded in scroll operations #7989

Closed elstoc closed 2 years ago

elstoc commented 3 years ago

When interacting with sliders by scrolling over them or via a keyboard shortcut, I would like to be able to exceed soft limits without having to find and open the module (when interacting using a shortcut), right-click and type, especially if I then have to repeat the activity when I find I haven't pushed the slider hard enough.

If we're worried about new users pushing modules too hard, perhaps we can add a new preference that defaults to the current behaviour.

dterrahe commented 3 years ago

This would be part of a somewhat significant overhaul of bauhaus that I'm mulling over as my next project, building on top of the work I'm currently doing for #5063. So I would suggest not spending too much time on it now, if you are not desperate for a short term solution...

One of the preconditions I feel, for a clean solution, is that basically all sliders should be able to deal with soft boundaries. So the user can narrow (so the opposite of what you are asking for here) and expand the range they want to see for any slider they want. This would be a way to allow for individual preferences in step size too (if we also, at the same time, define the "default" step size as 1/100th, or 1/200th in case of min = -max, of the visible range). Currently, gui_update treats sliders with and without soft bounds differently, so that needs to be cleaned up/refactored. Which all fits neatly into my long term plan. So I'm asking for a little patience, but I'm hoping to show a taster in a week or so.

elstoc commented 3 years ago

Thanks for the feedback @dterrahe. I was just raising the issue for discussion for now - it's just a minor irritation, mostly on old badly-exposed photos where I need to push exposure beyond +3EV to bring the mid-tones back. It's the only reason I ever need to expand the exposure module as I do it all via keyboard shortcuts.

dterrahe commented 3 years ago

One of the long term benefits, I hope, of the framework improvements I'm working on, is that all sliders could be exposed to lua so that you could define custom soft bounds there if you wish. This would definitely need @wpferguson involvement at that point, since my lua skills are low, but is a little while down the road yet.

github-actions[bot] commented 3 years ago

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.

elstoc commented 3 years ago

One of the long term benefits, I hope, of the framework improvements I'm working on, is that all sliders could be exposed to lua so that you could define custom soft bounds there if you wish.

Returning to this because someone mentioned it on pixls, I don't wish to set custom soft bounds, I just want to be able to exceed them with scroll and shortcuts, in the same way as if I typed a number. Soft bounds are useful to have a pleasant UI and guide the user to normal values, but it shouldn't need so much effort to exceed them.

wpferguson commented 3 years ago

@elstoc How would you like to see sliders behave when you exceed the soft limits?

I'm thinking that you "push" through the soft limit and the slider auto-magically makes the new displayed limit the hard limit, redraws the widget, and maybe adjusts the stepping to account for the changed size.

elstoc commented 3 years ago

I would expect it to work the same as if you'd typed a larger value into the popup. I've been experimenting but I can't work out how it works right now.

dterrahe commented 3 years ago

This is exactly the functionality I'm right now implementing in #8078.

I'm considering two approaches :

  1. The slider widget exposes the following elements:

    • value: scrolling adjust value; stops at soft limit
    • button: color picker or whatever else is in the quad area. Toggleable (with or without ctrl held).
    • min
    • max: change soft boundaries (or more exactly; move current min/max between soft and hard boundaries).
    • zoom: expand or reduce current min/max around current value. After further bauhaus changes (with variable step+digits) this will allow more fine tuning.
    • value: as before
    • button:
    • force: same as value except when at (current) min or max; in that case the min or max will be dragged (i.e. range expanded or reduced depending on what way move is.)
    • zoom: as before.
  2. is probably the better/easier approach? It could come with a (default) fallback where scrolling while holding ctrl & shift automatically engages "force" and increases stepsize. shift on its own only increases stepsize. ctrl reduces it.

elstoc commented 3 years ago

@dterrahe I'm not sure I understand what you've just said. It all seems very complicated. But that's probably just lack of familiarity.

Anyway I wants it now and I'm frustrated because it feels like it should be easy peasy.

wpferguson commented 3 years ago

I added print statements to the bauhaus slider code to figure out how it works. Here's what I learned.

  1. The slider always scales min to 0 and max to 1 for plotting the position of the indicator.
  2. There are 2 ways to set the value a. dt_bauhaus_slider_set_normalized() which takes a value between 0 and 1 to update the position then figures out what number that is and displays it. This function respects the soft limits and wont exceed them. b.dt_bauhaus_slider_set(), set_val(), set_soft() which take an actual value, converts it to a scaled value, then plots it with set_normalized(). If the the value exceeds the soft limit, then the new max or min value is set to the supplied value. The other end of the slider keeps the same value. The new range is scaled from 0 to 1 and then the normalized value is plotted. If the supplied value exceeds the hard limit, then the hard limit is used.
  3. Double clicking on the slider to reset, resets the ranges back to the soft min.

Example I used exposure to test so I'll describe how it worked

Soft min is -3 and soft max is +3. I could scroll back and forth but not exceed those values. If I right clicked and set the exposure to 4, the the range changed from -3 to +4 and the value got set to 4. If I scrolled back under the soft max the max still stayed at 4.

If I tried to set the exposure to 30EV, then the max limit went to 12 and that's what was set. Double clicking on the slider reset it to -3 to +3.

@dterrahe it looks like the slider already does option 2 from above except for scrolling.

wpferguson commented 3 years ago

dt_bauhaus_slider_motion_notify() handles the scrolling and dragging. It gets the position from the widget itself, so it can never exceed the bounds of 0 to 1. A check could probably be added to see if the position is at soft min or soft max and you try to scroll or drag past it. I noticed when I was testing that if I kept scrolling when I hit the max the setting kept trying to change but it was at the max.

dterrahe commented 3 years ago

@wpferguson there are basically 6 ways the slider value can be changed (in the ui; it obviously also gets changed in code after image loading/changing or when changing one slider/setting adjusts the value of other sliders)

  1. By clicking (and dragging) on the slider itself
  2. By using the scroll wheel on the mouse while hovering over the slider
  3. By calling up a pop-up and adjusting the position there with the pointer
  4. By directly typing a value (or formula) in the popup
  5. By pressing a dynamic shortcut key and scrolling the mouse
  6. By entering a value (or formula) via vim keys Most of these get CLAMPed [0,1] to the slider range and then translated back to a value, with at least 4 being an exception. The step size depends on the actual value scale though, rather than being just, say, 0.01 always in slider range terms.

IMHO bauhaus is fundamental and needs significant restructuring. The hardcoded soft ranges and step sizes are always contentious. My multi-step plan has been for a while to first introduce the new input system with much more flexibility/configurability (this is planned to be merged immediately after 3.6 release) and then restructure bauhaus to fit into and take advantage of that; simplify, but with more control over configuration, on a per-slider basis with default fallbacks.

As such, I would consider fine-tuning the current implementation somewhat a waste of time, although of course experimenting with features and creating POCs is always useful to generate ideas.

On the other hand I can not guarantee that after finishing #8078 (and linking in all widgets/shortcuts to optimally and consistently use it and refactoring/simplifying code) I'll have motivation to pick up the next big sensitive project.

Specifically with regard to this FR; if we decide at some point that the default stepsize (adjustable via modifiers) would be, say 1/100s of the slider range (https://github.com/darktable-org/darktable/pull/5506#issuecomment-645190579), then it would be awkward if the default soft-range of, say, -2.0 to +2.0 could easily accidentally be bumped to -2.04 to +2.0. What I'm saying is, we probably want to have some mechanism to end up with nice round numbers (for whatever definition of "round") rather than small broken values all over. That's also why I suggested "dragging" the min or max, if the current value was on either one (i.e. both increasing and decreasing the range), rather than just bumping it out of the way. That would mean an accidental change to -2.04 could be easily fixed to -2 again (without having to fully reset the whole slider).

elstoc commented 3 years ago

Are we talking about the same thing here? I just want to change the current value of a slider past the current min/max and have the limits change to accommodate, not change the soft limit globally, so small broken values all over is not really an issue is it, unless people are pushing sliders to the max all over the place? I'm not sure what you mean by "dragging". Presumably not click-dragging because I want to be able to do this while the module is collapsed (shortcuts on exposure are my main use case - so perhaps changing the soft limits might help with that for now). Also we still have hard limits so presumably accidentally pushing past the soft limits slightly shouldn't be a big issue?

Obviously harder to implement but some sort of buffer at the soft limit might work to prevent accidents, so if you pushed the value of a slider to where it exceeded the soft limit perhaps it could require a couple of extra scrolls / clicks to push past with a toast to advise you to keep pushing it you're sure.

parafin commented 3 years ago

I don't think slider should be mouse-scrollable over soft limit by default. Maybe using some keyboard modificator (not sure if any are still free to make use of) or at least it should require some big scroll delta over the edge (so say some dead zone near the edge scrolling over which doesn't change the set value).

elstoc commented 3 years ago

TBH maybe what I really want is to just change the soft limits of exposure. Now I'm adjusting for middle-grey instead of clipping, I'm doing much larger adjustments to exposure and often find myself exceeding +3, but very very rarely (never?) go below -1.

dterrahe commented 3 years ago

Are we talking about the same thing here?

Yes

I'm not sure what you mean by "dragging"

What I meant, for lack of a better word, is that I'd prefer the min/max range to not just be "bumpable" (i.e. it is 3, you go higher, it goes to 3.1, 3.2 but if you now go down it stays at 3.2) but "draggable" (by scrolling or any of the other "moves" that can be used in Input NG) i.e. if you go up past the current limit, the limit moves to accomodate, but if you then retrace (back to 3.1 and 3) the limit moves back as well (so you end up with the limit at 3 again if your final value is 3).

Obviously harder to implement but some sort of buffer at the soft limit might work to prevent accidents

Interesting idea! Many different choices how to finetune this which might make it unpredictable but definitely something I would look at in the context of "Bauhaus NG" if it ever comes to that.

Maybe using some keyboard modificator (not sure if any are still free to make use of)

Input NG provides many new key/modifier/button/pattern combinations so this can easily be accommodated. With default fallbacks (so this feature will be available for any slider that has a shortcut assigned to it; you don't need to separately configure the "force push through soft limits" shortcut.

wpferguson commented 3 years ago

Thinking about the purpose of soft limits I come up with:

  1. They make the GUI look good
  2. They are the most common values (though I'm not sure about -3 on exposure).

If the above assumptions are correct, then I don't think you should have to explicitly override the soft limit. If you're using a dynamic shortcut you would need to add another modifier to it to do the override, but you would have to know that you hit the limit. My dream (fantasy?) is to collapse the side panels and just edit the image with dynamic shortcuts, maybe once @dterrahe gets the new input system finished and I have lots and lots of shortcuts :smile:

_ What I'm saying is, we probably want to have some mechanism to end up with nice round numbers (for whatever definition of "round") rather than small broken values all over.

Currently if you type an odd value, say 3.31 for exposure, that becomes the new upper limit and that's what's displayed until you double click to reset the slider. I think my preference would be "elastic" boundaries with nice round numbers.

Another problem with the "hard" soft limits when scrolling is that users unfamiliar with how sliders behave will just add a new instance of a module instead of overriding the soft limit.

parafin commented 3 years ago

It's about consistency and not surprising the user. You have a slider that can be dragged with left mouse button from, say, -1 to 1. I would expect that scrolling that slider with a mouse wheel will have the same limits. So I can go to exactly -1 either by dragging a slider a lot to the left of by scrolling down (or up, forget which one it is) a lot, with no need to be precise about it. That's why I think for mouse operations soft limits should stay "hard". This reasoning doesn't really apply to keyboard shortcuts, so I would be fine with them ignoring the soft limits - it would make total sense.

wpferguson commented 3 years ago

I could see the "hard" soft limits when using the GUI. Maybe drag or scroll to the limit, then do a second drag or scroll to go past the limit or override with a modifier.

elstoc commented 3 years ago

users unfamiliar with how sliders behave will just add a new instance of a module instead of overriding the soft limit.

I used to be guilty of this :)

Honestly my pain is much reduced by the merging of #8771, and perhaps just altering some of the other soft limits is the best solution. The other ones I find myself frequently altering are the color calibration grey sliders (to negative values) and the wavelet denoise strength (sometimes up to double the current maximum)

dterrahe commented 3 years ago

Thinking about the purpose of soft limits I come up with:

  1. You have more precision when clicking/dragging on the slider to pick a value. And it is easier to "read" the value (maybe that's what you mean by 1.)

Currently if you type an odd value, say 3.31 for exposure, that becomes the new upper limit

One of the goals for Bauhaus NG would be to avoid that. Another to have automatic number of digits adjustments. Currently, if the hardcoded number of digits is, say 3, then it will never be possible to enter something like 0.1234. Not even when you edit darktablerc to have a very small value for darkroom/ui/scale_precise_step_multiplier (or in Input NG when you set up a shortcut with a very small speed multiplier). So people request a large number of digits for each slider (even if for most uses 2 or so digits should be enough). And then when they have many digits, they want small step sizes. So now using the scroll wheel becomes useless because you have to scroll forever to make a change. If the number of digits automatically adjusts when I make a very small change (by holding ctrl or some other means; maybe typing 0.1234 directly), then the default number of digits can be more sensible. And the step size too.

This reasoning doesn't really apply to keyboard shortcuts

Why not? A dynamic shortcut is just like a normal "scroll" except you don't have to hold the mouse above the slider. Or even be able to see the slider. I would still "expect" to be stopped when I'm scrolling into "ridiculous" territory. Just making the counterargument to yours. But in general, I like consistency.

elstoc commented 3 years ago

I would still "expect" to be stopped when I'm scrolling into "ridiculous" territory

Some of the bounds aren't "ridiculous" territory though they're just what the developer thought was reasonable for the UI. With the denoise example I mentioned the original developer even suggests going beyond the limit when using the curves. So again, perhaps some of the soft bounds just need to be revisited.

dterrahe commented 3 years ago

@elstoc have you played with the ctrl+shift fallback for slider shortcuts? This switches to force mode which "ignores" soft limits. And increases speed 10x. Not fully addressing this of course, since it (for now) only applies to shortcuts.

elstoc commented 3 years ago

Ah yes that's very nice - I will definitely start using that for exposure. So all that's needed is to have something similar when scrolling over the actual sliders.

dterrahe commented 3 years ago

A longer term idea, beyond #9487, is to refactor/redesign bauhaus to integrate more closely with ING. That would mean modifiers configurable (as fallbacks) and per-widget configuration of speed. So in the mean time I myself won't be working to enhance bauhaus's current implementation. Feel free to implement it yourself though (it shouldn't be too hard, hopefully); just please don't be offended if it gets ripped out/replaced again at some point in the future, ok?

elstoc commented 3 years ago

I wasn't offended when you ripped out/replaced all my shortcut work. Better is better.

github-actions[bot] commented 2 years ago

This issue did not get any activity in the past 60 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.