darktable icon indicating copy to clipboard operation
darktable copied to clipboard

po/colorpicker edit - edit area & point

Open TurboGit opened this issue 1 year ago • 6 comments

Allow for editing area & point as hinted by the tooltip.

@jenshannoschwalm : This is based on top of your PR but it independent, can you test?

How it works:

  1. click on picker icon
  2. right click on a sample
  3. move it / change size of area
  4. deselect the picker

I'd like to avoid 1 and as soon as we right click we enter in edit mode and maybe right-click again to update the sample. Seems a better user interaction to me, what do you think?

TurboGit avatar Jun 24 '24 16:06 TurboGit

Yes, indeed a step into a better workflow.

One usability downside, you have to prior-select the picker with the correct "mode" (either click for point or right-click for area) and thus have to know the live samples mode. Could we instead activate the main picker in the correct way via right-click on the live sample?

jenshannoschwalm avatar Jun 24 '24 17:06 jenshannoschwalm

Could we instead activate the main picker in the correct way via right-click on the live sample?

Sure, that's what I commented in my first post :)

TurboGit avatar Jun 24 '24 18:06 TurboGit

Ahh, didn't get that. So show me the way to the next ... :-)

jenshannoschwalm avatar Jun 24 '24 18:06 jenshannoschwalm

No time for 4.8.1 and actually not a fix so moving this for 5.0.

TurboGit avatar Jul 16 '24 16:07 TurboGit

@TurboGit let me know if you want a review / test!

jenshannoschwalm avatar Oct 13 '24 06:10 jenshannoschwalm

@TurboGit let me know if you want a review / test!

Fact is that this is still buggy IIRC. I need to resume work on it when I'll have time.

TurboGit avatar Oct 13 '24 10:10 TurboGit

@jenshannoschwalm : New version pushed and ready for review.

How it works:

  • Hover over a live sample
  • Right-click on the sample
  • The live sample is now copied to the primary sample
  • Change the size/positon of the sample
  • Hover over a live sample (not necessary the same used above)
  • Right-click on the sample
  • The live sample is receiving the primary sample

TurboGit avatar Nov 13 '24 18:11 TurboGit

I just checked and for me The live sample is now copied to the primary sample was not working. It was like "primary picker is copied to hover-over live sample" ???

jenshannoschwalm avatar Nov 14 '24 05:11 jenshannoschwalm

It was like "primary picker is copied to hover-over live sample" ???

Hum, the second right-click is to copy back the primary to the live sample. What the steps you did? Maybe there is a state reset to do somewhere.

TurboGit avatar Nov 14 '24 07:11 TurboGit

@jenshannoschwalm : I found 2 cases where a reset of the copy-state should be done. Added and force pushed. Can you test again?

One thing I'd like to add is a status (GUI indicator) somehow that a copy has been initialized so a second right-click would reset the live-sample. But I don't know what to do at the moment GUI wise...

TurboGit avatar Nov 15 '24 07:11 TurboGit

Will check again tomorrow .

jenshannoschwalm avatar Nov 15 '24 10:11 jenshannoschwalm

@jenshannoschwalm : I've just added an indicator inside the copied-patch. Should be better this way.

TurboGit avatar Nov 15 '24 12:11 TurboGit

To help the review, when right-click on a live sample to edit it, a "save" icon is displayed over it:

image

From there, either:

  • click on the picker icon, the edited sample is kept on the primary sample.
  • click to add sample, a new sample is created and the edited sample is kept also in the primary sample.
  • right-click on any live sample, the edited sample is copied back into the clicked live sample (not necessary the one we copied from). the sample is also kept into the primary sample.

Note also that it is not necessary to select the picker before. If one right-click on a live-sample the picker is automatically enabled.

The only behavior that bothers me (but not a regression, it is already the case with current master) is that you need to deselect the picker by using the same action as the one used to enter the picker mode. That is, click for a point and right-click for an area. If one click when editing a box then the picker is switched into the point mode. This is quite annoying to me, if anyone has a proposal to make this module better UX wise let us know.

TurboGit avatar Nov 15 '24 16:11 TurboGit

Just tested and

  1. the symbol helps and the tooltips reflect what happens.
  2. No unexpected things happened :-)

Codewise: nothing noteworthy. About the debug code, we have -d picker so maybe use that instead of fprint and keep it active?

jenshannoschwalm avatar Nov 16 '24 06:11 jenshannoschwalm

@jenshannoschwalm : I'll add back the debug code at some point. Thanks for the review.

TurboGit avatar Nov 16 '24 15:11 TurboGit

That is, click for a point and right-click for an area. If one click when editing a box then the picker is switched into the point mode. This is quite annoying to me, if anyone has a proposal to make this module better UX wise let us know.

Could we not maybe look at a dedicated button down with color assessment etc for a picker . Selecting that would open the CP module panel. Moving the mouse pointer to the desired spot and left clicking would add a point picker and a modifier or right click would add an area selection instead. Each new selection would populate the panel with samples. Clicking that icon down in the bottom bar would toggle the picker on and off... so set it up a bit like ART and RT.... I am not sure of the implications but working with the samples in the panel etc could be the same and the pipette could also just be a toggle for on off. In addition if the point mode could be added much like the variable nodes of a mask people could add a point but then scroll wheel to make the selection a bit larger. THe same with a selection area ...maybe hovering and the scroll wheel could be used to contract or expand the size of the drawn box in addition to the handles. Finally in the way that we can move those nodes maybe the pickers could be allowed to be dragged on the screen to adjust the location.... just some random thoughts and maybe there are issues I haven't thought of .....

todd-prior avatar Nov 17 '24 00:11 todd-prior

Moving the mouse pointer to the desired spot and left clicking would add a point picker and a modifier or right click would add an area selection instead.

This could become very messy as one still need a left-click to edit an area colorpicker (changing position, size...).

TurboGit avatar Nov 25 '24 08:11 TurboGit