color picker: restrict histogram to selection

[caiux]

Describe the bug

When selecting "area mode" and "restrict histogram to selection" in color picker, the restricted histogram is substituted by the full image histogram as soon as any adjustment to the image is performed.

To Reproduce

1. Go to 'color picker'
2. Select 'area' mode
3. Select image area
4. Activate 'restrict histogram to selection' to show the selected area histogram on the histogram panel
5. As soon as any adjustment to the image is made, the histogram panel refreshes to the full image histogram, although 'restrict histogram to selection' is still checked
6. One has to uncheck and check again 'restrict histogram to selection' to get it back

Expected behavior

The restricted histogram should be maintained visible. It would be useful to see how the adjustments change the selected histogram.

Platform (please complete the following information):

• Darktable Version: 3.1.0+2405~g08c1dcd75 (latest on git)
• OS: macOS Catalina 10.15.5
• No OpenCL activated

Additional context

Same behaviour in 3.0.2, official macOS release

[github-actions]

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.

[dterrahe]

What happens here (at least in 3.4) is that when in your step 5. an adjustment is made directly in one of the modules, that module gets the focus and the color picker module loses it. That then disables the color picker and therefore the selection is no longer active and the histogram cannot be restricted to it.

You can make changes without giving another module focus, for example when you use the mouse scroll button over the histogram to change the exposure; the histogram will continue to be limited to the color picker selection, which in this case doesn't get disabled. If you set up dynamic shortcuts to sliders in other modules, you can use those as well without the module taking focus.

The behavior that the color picker gets deactivated when another module gains focus cannot easily be changed, because it is intentional. After all, the "new" focus window could also have an active color picker. Or a drawing mask. Or resize handles. Having both active at the same time would create problems; basically there's always just maximum one interactive overlay enabled at the same time.

[johnny-bit]

I was wondering about it and i'm split between closing this with "works as intended" and wondering wether we could somehow make restrict histogram to selection work as long as no other picker nor mask drawing nor anything that's not strictly adjustment would get enabled.

what do you think @dterrahe and @dtorop ?

[dterrahe]

In theory this could work well. In practice implementing this would probably need to touch focus and color picker code and lead to a fun game of whack-a-bug. If someone (not me) wants to have another go at restructuring that logic to make it all clean and reliable and behave logically, I'd be very happy. But I would discourage them from starting to mess around with it to see if they can make this one thing work without a reasonable understanding of all the interactions and the possibilities for breakage everywhere...

Just as an indication, dt_iop_color_picker_reset gets called 117 times just to make sure it doesn't interfere with anything. This may be easy to reliably restructure and treat the lib color picker separately from any iop color picker. Or it may not.

[johnny-bit]

@dtorop - let's continue here from #8680 if you don't mind.

[dtorop]

What @dterrahe writes is pretty compelling.

I would discourage them from starting to mess around with it to see if they can make this one thing work without a reasonable understanding of all the interactions and the possibilities for breakage everywhere...

Maybe a way forward is to figure out what we would want to happen, from a UI POV. Should the color picker lib (left panel) be totally orthogonal to the color pickers in iops?

The behavior that the color picker gets deactivated when another module gains focus cannot easily be changed, because it is intentional. After all, the "new" focus window could also have an active color picker. Or a drawing mask. Or resize handles.

This is key. If "restrict histogram" is selected and the lib iop loses focus, then the user should lose the ability to change the color picker area, so they can use other overlays on the center view. (Unless we want to implement some sort of full-fledged multi-widget overlay on top of center view, compared to the current modal UI.)

The whole interaction between focus and the color picker lib seems a bit hacky to me. Another approach would be to have "restrict histogram to selection" be tied to the "live samples" mechanism. If a "live sample" line is created then clicked, the histogram would be restricted to that sample. Clicking it again would unrestrict the histogram. Clicking another live sample would restrict histogram to that sample.

The live samples don't have "focus" the way that the color picker does, so this could be an end run around focus conflicts.

If this change happened, we wouldn't want to lose the ability to use the color picker to sample different parts of the image and see the histogram reflect this. It could be possible to always have "restrict histogram to selection" enabled when creating a color picker sample. This would allow for being able to run the color picker over an image and see the histogram update live. But it would be understood that the moment the color picker loses focus, the user would need to rely on a "live sample" to maintain the histogram restriction.

I'm traveling (vacation?) until 7/12, but if it's possible to come up with a good plan (which doesn't cause bug whack-a-bug), I'd be happy to look at this after that.

[johnny-bit]

Hmm... Tying locking histogram to samples mechanism seems like a way to remove "hackish" focus funzies and step out of play about what gets to control overlay. I think I like it.

@dterrahe - what do you think?

I'm traveling (vacation?) until 7/12

Ah, american. I always get confused with month/day notation. ;) My initial thought was "vacation till 7th dec? nice!"