toneEQ: move exposure/contrast compensation sliders to advanced tab
since there're endless discussions on tweaking the tone equalizer a lowest common denominator might be simply moving the controls for mask exposure and contrast compensation to the advanced tab. since the real estate of the gui is driven by the simple tab, the additional required vertical space is taken from the graph and in masking tab there's unused space. the bar indication the spread of the histogram on masking tab is no longer needed, so removed
I haven't build this PR, so I can't see the layout. But I had an idea for a while. What if we get rid of the top 3 tabs? Make the advance be the default (without the word advance, just show the histogram) and make the simple tab and the masking options tab be an expand pull down (like you did in Agx POC). This way we save the vertical space most of the time and it keeps the histogram always showing.
tested just now:
- There is a UI gap in the masking tab -
as this is missing :-)
- Not sure - shouldn't mask quantization be in the advanced tab too?
the gap is there because the minimum height of the module is determined by the sliders in simple tab. Mask quantisation results can't be seen in the histogram graph - so it seems to be better to keep it on masking tab. also the graph will be compressed further, if there's a further slider put on advanced graph.
the histogram representing mask bar is no longer used, since the effect of the moved sliders can be seen directly in the histogram.
Obviously I don't have anything working right now on my PR. Even though I haven't given up just yet, it looks like my stuff will take a lot longer.
Just for inspiration, I wanted to show what my current UI looks like, after all the discussions in that forum thread.
I went for a version, where the histogram is always visible and the tabs are placed below it. This is useful, because smoothing changes the histogram too, which is immediately visible here. I also put almost everything into collapsible sections, so people cen decide for themselves, how much vertical space they want to use.
Regarding mask quantization: I don't think that this is an option that should be placed in a prominent location. I don't think that this is widely used, I regard it as an unsuccessful attempt to have a little bit of zone system in the module.
This pr keeps the vertical space needed for the module, since there’re users running darktable on capable notebooks where screen size is limited. this pr doesn’t want to change the UX fundamentally but is improving the users workflow in a often demanded way.
Changes are looking fine for me.
Because folks have been talking about how precious vertical space was to them I wanted to ask how you feel about removing the x-axis labels of the node UI, see framed in red:
IHMO there is no real usage for it, only the y-axis labels are. However they are probably still helpful to cross-reference the sliders in the simple tab.
This pr isn’t intended to be a major change of the toneequalizer UI - so no further changes beside what’s needed to optimize the handling of the mask histogram. There are plenty of non converging discussions on the UX of tone equalizer - so let’s keep this pr small and without scope creepers …
@MStraeten : You may want to try out https://github.com/darktable-org/darktable/pull/18656 (be ware that this will break all your ToneEQ presets). I think #18656 is the way to go, the auto-fit is especially fixing a itch I had for this module.
This pull request has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please verify it has no conflicts with the master branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work.
Thanks for keeping the PR up-to-date, I've been using those changes for 5 months now and I'm really happy with the tone equalizer workflow improvements, which probably saved me already many hours of my precious lifetime :sweat_smile:
@TurboGit since there’s no progress on the big changes for toneEQ maybe this makes sense to be a working solution for 5.4
I have thought about this already.
My version won't be in time for the Christmas release. I am pretty busy right now, there is still a lot to do and afterwards we need testing, community feedback, write new documentation etc. My current worst-case plan is to code a release candidate version during the holidays. Then we would have full 6 months to make sure that everything is nice and polished.
Temporary UI improvements are something that could be merged before. However there is a caveat: Your proposal would be moving sliders to the advanced page and I would be moving them back to their original place in the next release (and add similar but different sliders to the equivalent of the advanced page).
I am afraid that this would confuse users. If the tone equalizer UI is changed now, we need to make sure that the changes are consistent.
One such option would be to leave all slides where they are, but also show the graph on the masking page (for example in a collapsible section). In my future version the graph will always be visible and the tabs will be below it, but this requires considerations regarding vertical space.
you‘re changing the UX so the sliders won’t be used as they are used in the moment. So keeping a weak solution just to be consistent with a future solution (no one knows when it’s ready to be released yet) is quite strange…
I can just say that I intend to deliver a polished version for next summer. Everything else has to be decided by @TurboGit .
@TurboGit since there’s no progress on the big changes for toneEQ maybe this makes sense to be a working solution for 5.4
I've been using this UI improvement for quite a while and would be very happy if we could merge it until Marc's PR is ready as it has been improving usability for me a lot. Given that Marc is going to make fundamental UI changes anyway, I don't see moving the sliders forth and back as super critical, users will re-discover them as their name will not change (I assume).
Like most people I use the stable DT versions for my productive editing, so I have to wait for my new toneeq as long as everyone else. I have not tried it, but I am pretty sure that I like @MStraeten's proposal more than the current state of things.
So if changing something now and possibly changing it back in 6 months is not a concern, I am fine with this change.
Or you could just assign keyboard shortcuts to the sliders and adjust them using the shortcuts while you're in the advanced tab.
Ok, I have no string opinion. Really. @marc-fouquet arguments about the fact that the sliders will be moved again is a valid one. On the other hand the next version of the TE module will have lot more changes and so could be considered as a new UI requiring some adjustment from our users. The modifications proposed here are sound and limited to some sliders (at the same time removing a redundant display of the graph in masking tab).
So, let's move on. Add this small usability fix now.
@MStraeten : Sorry for the long delay. I was hoping to have some more pro/cons discussion here before merging.