Improve cropping experience

[stnKrisna]

Implemented feature request 1 & 2 from #20696

[stnKrisna]

Sometimes I'd like to be able to manually drag a rectangle (constrained to the currently active aspect ratio) instead of resizing the default one which fills the full area. This could be done by dragging left mouse with a modifier key, or maybe dragging middle or right mouse. (Right mouse makes some sense since this action deletes the existing rectangle, which is what right click does.)

I implemented this by using alt+left-click. If we use right-click to also draw rect, I think this will cause unnecessary code complexity.

[jdchristensen]

@stnKrisna Thanks for this! I just tested the first feature, and it works nicely. I wasn't able to test the second feature, because on my system Alt-left-mouse is bound to moving a window around. (In general, I use Alt for meta-operations on windows rather than operations within a program.) Is this binding customizable? Or could two options be provided?

[stnKrisna]

@jdchristensen I can change it to a different key if you want. Im not sure whats the convention is with darktable. I'll look at other apps. See how they deal with this

[pehar1]

Alt-left-mouse is bound to moving a window around

Same for me on Ubuntu 24.04, LXQt. Many ALT related combinations are bound to window / desktop actions. Not a good idea to use them within (other) applications.

[stnKrisna]

@pehar1 @jdchristensen Im thinking of using space+mouse-left+drag. The space key should be configurable

[stnKrisna]

ugh... the space key is used to go to the next photo (views/darkroom/image forward)

[stnKrisna]

@pehar1 @jdchristensen I can only use modifier keys. How about ctrl+shift+left-click+drag?

[pehar1]

I don't have a strong opinion on this. I'm completely satisfied with the way cropping is currently handled without this commit. I just wanted to point out a potential issue for many other users when the ALT modifier is used.

[da-phil]

Alt-left-mouse is bound to moving a window around

Same for me on Ubuntu 24.04, LXQt. Many ALT related combinations are bound to window / desktop actions. Not a good idea to use them within (other) applications.

Same here, and I agree on the shortcut choice using ALT.

[jdchristensen]

Is there a reason that using an unmodified right-click won't work? Having to use a modifier for a simple operation makes it less convenient. If we have to use a modified left-click, what about just Ctrl-left or Shift-left, instead of having to do both?

[stnKrisna]

@jdchristensen Honestly, the answer annoys me. Its because shift+drag locks the crop dragging to vertical axis, ctrl+drag locks the crop dragging to horizontal axis. Im hesitant to repurpose these modifiers as other user might've gotten used to this workflow

[jdchristensen]

@jdchristensen Honestly, the answer annoys me. Its because shift+drag locks the crop dragging to vertical axis, ctrl+drag locks the crop dragging to horizontal axis. Im hesitant to repurpose these modifiers as other user might've gotten used to this workflow

Ok, I see what you mean.

To further complicate things, if you resize the crop by dragging a corner or edge (the existing method), then shift-left-mouse preserves the aspect ratio, while ctrl-left-mouse seems to do the same as left-mouse with no modifier.

This all suggests to me that we should avoid left-mouse, as it is already doing a lot of work here. Can you explain why right-mouse isn't good? A right-mouse-click resets the crop, and a right-mouse-drag could initiate a fresh crop. When done with modifiers, it should do the same as the existing left-mouse approach (i.e. with shift it should preserve aspect ratio). If the aspect ratio is set in the crop module, then it should always respect that, like the existing method does.

[stnKrisna]

right-mouse is not good for the codebase as it can get messy and confusing (I would've reset the crop with double-left-click, but anyway...). As theres no other option, I'll have to continue with right-click+drag

[stnKrisna]

@jdchristensen @pehar1 @da-phil actually, I think this is a good time to overhaul the crop functionality. If left mouse is doing all those things, wouldn't it be better to simplify?

To start, I think shift+drag should just lock to whichever mouse travel direction, similar to axis snapping. Feel free to dump any ideas/wishlist here

[pehar1]

If left mouse is doing all those things, wouldn't it be better to simplify?

As I mentioned earlier, the current functionality is perfectly sufficient for my needs. In my opinion, no additional features are necessary.
If you still want to implement any, you should definitely avoid making any changes to the current behavior. It has existed in this form for years, and people are used to it. Changes would likely result in numerous questions and issue reports.

[stnKrisna]

Right-click reset is harder than I thought. For some reason, crop is reset on RMB down and not RMB released. Still looking to understand how this is implemented

[jdchristensen]

@stnKrisna Thanks for getting right-mouse working! I like that binding.

I've noticed some glitches, though, when you set the aspect ratio to something fixed. First, if I start by placing the top-left corner of a region by right-clicking and dragging down and to the right, the top-left corner will move around as darktable preserves the aspect ratio. It should always stay exactly where I picked it, and the other corner should shift as needed to keep the aspect ratio.

Second, if I again start by placing the upper left corner, drag down and to the right, and then drag along a 45 degree line from upper-right towards lower-right and back, the selected region makes huge jumps at some points. Another program I've used gets around this by always using the mouse's horizontal position to set the width, and only using the mouse's vertical position to change whether the region is above or below the initial point. Maybe there are other approaches too.