This version does its own cursor handling inside the edit tools.
I first tried it by ignoring/accepting the mouse and key events and let AbstractImageView show the magnifying glass (see preview Diff 38774), but for crop tool it still doesn't work in all situations. Sometimes an event has to be accepted for one action but the same should be ignored to do something in AbstractImageView.

What do you think, is this way here ok?

Still missing: After clicking on a tool option widget (e.g. red eye size slider) the focus is removed from the image and no more keyPressEvents are received.

Thanks for the patch.

The focus and keyboard problems you are describing sound much like what I was faced with when trying to improve the cursor handling for zooming in Compare mode. Perhaps it might make sense to wait for that in case I'm able to finish that patch (IIRC there were still some issues, and getting 18.08 out is currently more important). Then we'll see what is a generic issue, and what has to be fixed for Crop specifically.

Thus, can we work on this later, given that it's not quite ready yet and it's for master anyway?

In D14485#300895, @rkflx wrote:

Thus, can we work on this later, given that it's not quite ready yet and it's for master anyway?

Sure! I did not work on the focus problem yet. I thought about forwarding the events from the widgets but did not try anything.
Let's see what your patch brings.

In D14485#311314, @rkflx wrote:

Sorry for the wait, but I did not get to the focus/hover issue yet (and thus did not start with the actual review).

No hurry, this isn't the easiest one and has a couple of pitfalls.

Meanwhile, I discovered a problem: Ctrl-double-click now wrongly accepts the Crop instead of zooming, same thing for Reduce Red Eye where invalid rect is printed (after D14755 it still worked fine, though). In particular, calling accept() from AbstractImageView::mousePressEvent triggers the issue.

However, maybe the root cause is elsewhere: I noticed that without your patch double-clicking will only call RasterImageView::mouseDoubleClickEvent when no tool is active. With an active tool, there are two calls to RasterImageView::mousePressEvent, which is a bit strange.

I think this diff was branched before your invalid-rect and double-click-accept patches. After rebase and adding checks in mouseDoubleClickEvent these issues should be gone.

In Crop there is a difference for left/right click while hovering over a handle. Left click is passed to the handle dragging code while right click zooms out. Should we filter this out to be consistent?

In D14485#312747, @muhlenpfordt wrote:

I think this diff was branched before your invalid-rect and double-click-accept patches. After rebase and adding checks in mouseDoubleClickEvent these issues should be gone.

No, I rebased already, because otherwise I would not have been able to trigger the incorrect accept by double-clicking in the first place ;)

In Crop there is a difference for left/right click while hovering over a handle. Left click is passed to the handle dragging code while right click zooms out. Should we filter this out to be consistent?

Ah, good catch. I think this should be consistent with what the cursor is showing. Ideally holding Ctrl will never change to a resizing cursor, and zooming in and out will work everywhere.

This is already working great for both the OpenHandCursor and ArrowCursor areas, only the handles are still off.

In D14485#313022, @rkflx wrote:

Ideally holding Ctrl will never change to a resizing cursor, and zooming in and out will work everywhere.
This is already working great for both the OpenHandCursor and ArrowCursor areas, only the handles are still off.

What about the lock ratio by holding Ctrl / ⇧? That's why I did not change the cursor in the handle areas. Ctrl+click over a handle does not zoom but resize with locked ratio.
Maybe we could do it this way?

• pressing Ctrl first and then clicking on a handle (or anywhere else) does zoom, including zoom cursor anywhere
• first clicking on handle and then pressing Ctrl / ⇧ will lock the ratio

In D14485#313153, @muhlenpfordt wrote:

What about the lock ratio by holding Ctrl / ⇧? That's why I did not change the cursor in the handle areas. Ctrl+click over a handle does not zoom but resize with locked ratio.

Oh, I missed that, sorry.

Maybe we could do it this way?

• pressing Ctrl first and then clicking on a handle (or anywhere else) does zoom, including zoom cursor anywhere
• first clicking on handle and then pressing Ctrl / ⇧ will lock the ratio

I think that's a bit too complicated, both from a UI standpoint and in the implementation, in particular when actions depend on the order you click and press a key. Some users might not expect zooming in the second case, because they are used to press the modifier first. (Not zooming when clicking on a handle might also be surprising, but given that zooming works everywhere else and resizing only works over the handles, we should probably assume resizing to happen more often when the cursor is over the handle.)

I'd say the best compromise is to not zoom over handles, and adapt the cursor accordingly. This way there is a clear separation: The standard method of zooming is overridden by a more specialized interaction, in this case locked resizing.