search

darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.82k stars 1.14k forks source link

crop tool: edge drag doesn't work if mouse moves too fast #11124

Closed hqhoang closed 2 years ago

hqhoang commented 2 years ago

Describe the bug/issue Not sure if it's designed intentionally, but in the "crop" module, dragging from an edge to resize the photo only works if I leave a small delay after pressing the mouse, then I can drag the mouse as fast as I want. But if I started dragging the edge too quickly, the cursor changes to the drag arrow but doesn't do anything, or moves the cropbox instead.

To Reproduce

  1. select the "crop" tool
  2. hold down the mouse at an edge area, move the mouse cursor immediately and quickly
  3. the edge of the photo doesn't move with the mouse cursor

Expected behavior The edge of the photo moves with the mouse, regardless of how fast the mouse moves initially

Platform

Additional context I think I've encountered this weird behavior for a while, but didn't try to reproduce it until now. Sorry I don't know since when it's introduced, but at least it didn't happen with the old "crop and rotate" module. It seems to me that the mouse-down event triggers some calculations, and thus, I need to delay a bit for it to initiate the resize mode before I can start dragging.

TurboGit commented 2 years ago

@hqhoang : Can you test with latest master to see if my last change fix this on your side? TIA.

hqhoang commented 2 years ago

@hqhoang : Can you test with latest master to see if my last change fix this on your side? TIA.

Tested 3.9.0+410~g47ee206a7-dirty, still have the same problem. If I mousedown on the edge without moving, wait for half a second, then I can jerk/move the mouse how fast I want and it will still work correctly. But if I don't wait, or if the mouse is still moving on mousedown, then it either moves the cropbox instead or doesn't do anything (although the cursor changes).

TurboGit commented 2 years ago

I cannot reproduce, so I'm a bit in the dark, and I have almost the same config GNU/Linux, Nvidia 470.103.01-2, OpenCL. I'm on kernel 5.16 and GNU/Debian sid. But I don't think this can change anything. Maybe you Window Manager? I'm using GNOME Shell.

hqhoang commented 2 years ago

I have gnome-shell 40.5 on Ubuntu 21.10. I pulled out my old Dell 7567 laptop with Ubuntu 20.04 (gnome-shell 3.36.9) and still have the same problem. My mouse is a Logitech G300s. Tested with another cheap non-gaming mouse and still have problem. Turning OpenCL on/off doesn't make any difference. I'll keep testing to see if I can isolate the cause, but let me know if you can think of anything else. Thanks much!

TurboGit commented 2 years ago

Maybe I do not try to reproduce exactly as you. Can you describe precisely and step-by-step how to reproduce systematically? TIA.

hqhoang commented 2 years ago

I'm sorry I still haven't found a way to consistently reproduce it. I found that if I make darktable busy by adding intense modules to the stack, then it seems to happen more often. I have a Ryzen 7 5800H + RTX 3060, not sure how much it was slowed down.

Here's a screen-recording just so you have an idea of what it is. About 40-70% of the times? I tested with an almost empty history, then enabled "diffuse or sharpen" with medium deblur and cranked iterations to 68.

https://youtu.be/fFqLrbJO-XU

TurboGit commented 2 years ago

@hqhoang : I see, but I don't see the time between the click and the move. Anyway... looks like some Gtk+ events not being properly handled.

hqhoang commented 2 years ago

It's a gaming mouse but I don't do any flick-shotting, just normal usage, although a little bit faster than a typical user. Let me know how I can help with further troubleshooting, I'm not experienced with debugging desktop software :-(

geni1105 commented 2 years ago

I have the same issue like described by @hqhoang. I need to wait for several seconds with the mouse button clicked, until I can start to move it reliably. Often I just get the drag cursor, but moving it around does nothing.

May be related or not

I'm on Mac Retina 5k, Quad-Core i5, 16 GB, AMD R9 M290X / 2GB and MacOS 11.6.4, and usual image adjusting operations are aceptably fast, about a second even with a lot of modules like diffuse/sharpen (demosaic sharpen non-AA), local contrast, tone equalizer, lens correction, denoise, ... activated. OpenCL is enabled and active.

geni1105 commented 2 years ago

More observations:

jec993 commented 2 years ago

This issue ("crop tool: edge drag doesn't work if mouse moves too fast") also shows itself on my win11 fresh install of 3.8.1 for me, the issue is not affected by having openCL on or off - happens regardless of openCL state.

TurboGit commented 2 years ago

Suddenly I was able to reproduce on a set of pictures. Issue is understood, fix coming...

TurboGit commented 2 years ago

@hqhoang @jec993 @geni1105 : See #11196. If one of you can test it would be good. TIA.

hqhoang commented 2 years ago

tested TurboGit:po/fix-crop-handles, problem fixed. Even tried flick-shotting with the mouse cursor moving while being clicked, still works. Thanks much!

geni1105 commented 2 years ago

Strange, on my side I can only crop via the sides, not via the corners anymore (see comment in the PR). Grabbing a corner always ends up with just one of the sides changed - maybe something went wrong with me pulling the PR.

hqhoang commented 2 years ago

Strange, on my side I can only crop via the sides, not via the corners anymore (see comment in the PR). Grabbing a corner always ends up with just one of the sides changed - maybe something went wrong with me pulling the PR.

Crap, I've never grabbed the corner, thus my test was incomplete. Can confirm that grabbing each corner only moves one edge. Sorry!

TurboGit commented 2 years ago

Can confirm that grabbing each corner only moves one edge. Sorry!

A step by step reproducer please. I cannot reproduce this. Since you are two having this bad issue I certainly do something different. I have tested with a portrait and landscape picture.

hqhoang commented 2 years ago

Not sure what could be the difference, I use the same previous image/edit. I did delete /opt/darktable completely before recompiling/reinstalling your branch. Here's a video of the corner issue:

https://youtu.be/shLjNCudIeQ

TurboGit commented 2 years ago

There is huge mystery there... How is that possible that I do not see that??? Will try on another computer.

geni1105 commented 2 years ago

There's else-if conditions in mouse_moved(), where the new boundaries are calculated - removing the else's corrects the issue on my side. Clearly, with the else conditions it can only move the edges, not the corners.

Edit: just looked, the code on master does not have the else's ...

if(grab & GRAB_LEFT) ... else if(grab & GRAB_TOP) ... else if(grab & GRAB_RIGHT) ... else if(grab & GRAB_BOTTOM) ...

TurboGit commented 2 years ago

@geni1105 : Thanks, indeed the code looks wrong. I have now fixed that. The mystery is still there, why I didn't reproduce the issue!!!! Anyway, can you test latest version? TIA.

geni1105 commented 2 years ago

Tried to test, but something weird is going on here - I can‘t get rid of the else‘s, they always show up when I switch to your branch or the PR (vs. the master is clean). However on the web interface, the code on your branch is also good now - why can‘t I get it to replicate locally … somewhat confusing!??

TurboGit commented 2 years ago

I suppose you have a local branch named after my remote branch po/fix-crop-handles. So when on this branch, just do:

$ git fetch <myremote>
$ git reset --hard <myremote>/po/fix-crop-handles

But it actually depends on how you have integrated my changes locally.

geni1105 commented 2 years ago

Had some local changes (for debugging) that I needed to stash first, ...

Anyway, tested and works, thanks! :-)

hqhoang commented 2 years ago

github had a timeout for half an hour on me, also had to delete the local branch before being able to pull again without conflict.

Anyway, tested the corners and edges with flick-shottings, all work. Thanks!

geni1105 commented 2 years ago

@TurboGit BTW, I noticed that whenever the mouse button is released (after either changing or moving the crop window), the pixelpipe is rerun, although the change is not yet committed by closing the crop module and therefore the preview is unchanged. Is this intentional?

TurboGit commented 2 years ago

@geni1105 : The preview on top-left of the UI? Yes, it is expected as it corresponds to the whole image not yet cropped.

TurboGit commented 2 years ago

But in another scenario we are still missing a refresh see #11197.

jec993 commented 2 years ago

Thank you. When I learn how to compile for win, I can be of more help.

geni1105 commented 2 years ago

@geni1105 : The preview on top-left of the UI? Yes, it is expected as it corresponds to the whole image not yet cropped.

If I interpret the debug output correctly, it is not just the navigation preview, but the full pipe. See https://github.com/darktable-org/darktable/issues/11124#issuecomment-1046232959 above. I am puzzled about the jerky behavior of crop (and other iops with overlaid structures, like gradient), as I cannot imagine that simply pulling the rectangle around can cause a lot of computation.

geni1105 commented 2 years ago

Found out through various hints, that the jerkiness on MacOS is due to issues of gtk3 and/or Cairo with custom monitor profiles. Switching the iMac monitor to sRGB drastically improves the responsiveness for actions like moving/changing the crop rectangle, moving the EV picker in tone equalizer etc.

Luckily, my monitor seems to be pretty close to sRGB, as I see only small changes between sRGB and my custom profile - barely perceptible, and only in highly saturated magenta. Still wondering why a different monitor profile makes such a performance difference. There must be numerous translations back and forth already for simple operations like moving the crop rectangle ...?

parafin commented 2 years ago

As far as I remember, color correction for display is handled in macOS by OS and operates in terms of surfaces. Displayed image is one such surface, so I guess whenever darktable updates its content (e.g. caused by pipeline re-run when enabling crop) some processing is taking place, slowing things down. BTW, do you use OpenCL? Color correction being done by OS might be running on GPU, and the way GPUs work it may introduce more latency than expected when multiple processing pipelines are running on it.

geni1105 commented 2 years ago

Yes, OpenCL is active. However, the slowing down simply occurs when when I resize the crop rectangle or move it around (or similar overlaid structures, like tone equalizer‘s EV picker). Pixelpipe is not re-run in this case, at least I do not see re-runs in the output of -d opencl.

I understand from cairo_quartz_surface.c (if that‘s what is being used) that the surface is created under DeviceRGB color space, meaning essentially no color management. Then color space translation must happen somewhere else - maybe DeviceRGB is assumed to be sRGB there, and it is skipped if the calibrated color space is set to sRGB? This would explain the speed-up in this case.

parafin commented 2 years ago

it is sRGB, not monitor colorspace

geni1105 commented 2 years ago

I found that replacing the four instances of
... = CGColorSpaceCreateDeviceRGB (); by ... = CGDisplayCopyColorSpace(CGMainDisplayID()); in cairo-quartz-surface.c resolves the issue. This should create the surface in the monitor's set color space, not in a generic "device RGB" that is fixed to sRGB apparently. (I did not bother how to correctly set the color space for dual-monitor setup, as I use just one monitor).

I rebuilt cairo locally by (after cloning from github and patching cairo-quartz-surface.c as indicated above) ./autogen.sh --prefix=/opt/local make sudo make install which is a bit dirty, as it copies the locally built libraries over the macports ones. But it serves the purpose ...

parafin commented 2 years ago

Note that this way you broke color management. You need to export display's profile, put it where dt can find it and select it explicitly as monitor profile in dt.

geni1105 commented 2 years ago

Thanks for the hint - but I am not sure why I would break color management: first, I assumed (maybe wrong?) that cairo would only be used for drawing overlays like the crop rectangle, but not for the image itself. Second, in today‘s implementation cairo is always using sRBG, independent of the monitor calibration - wouldn’t that break color management even more?

parafin commented 2 years ago

Thanks for the hint - but I am not sure why I would break color management: first, I assumed (maybe wrong?) that cairo would only be used for drawing overlays like the crop rectangle, but not for the image itself.

As far as I know everything is rendered using cairo. Maybe I'm wrong, but I guess you could just check if image changes visually when you switch between patched and un-patched cairo.

Second, in today‘s implementation cairo is always using sRBG, independent of the monitor calibration - wouldn’t that break color management even more?

No. macOS is using what's called full-screen color management. GTK/cairo renders everything in sRGB, which is then converted by OS to monitor's profile. Since you've changed sRGB to monitor profile, you now have to tell DT to do the conversion to this profile instead of sRGB. Of course your patched approach would give better results, since you're no longer limited to sRGB gamut.

geni1105 commented 2 years ago

I compared the patched and un-patched cairo, and I guess you are right - the patched version does not work correctly. In particular and strangely, the rendering of the patched version does not change if I switch the monitor in the system settings to a different profile - the rendered image always looks according to the output profile selected in darktable, even if I switch the monitor to completely weird color space like ACES Linear. Or is this to be expected?

The unpatched version's rendering changes with the selected system monitor profile, and always looks identical to MacOS quicklook or preview (tried out with a profile/printer evaluation image in TIF format).

I simply do not understand the logic behind the color management: the pixelpipe is mostly running in its working profile (linear Rec2020 RGB by default), then GTK/Cairo renders in sRGB (or in monitor profile, with my patch), and finally the OS transforms into the monitor profile selected in the system settings (or skips it, if it's already there). Where does the output profile come into play, that can be selected in the soft proof list?

I'm confused and think I will simply revert the patch, switch everything to sRGB and live happily ever after ... ;-)

parafin commented 2 years ago

Soft-proof and output profile doesn't matter for this discussion - only display profile setting in darktable and monitor profile in macOS settings.

For unpatched cairo it goes like this: dt outputs image in display profile (sRGB by default and shouldn't be changed) -> cairo renders it to Quartz surface with sRGB colorspace -> macOS transforms sRGB into monitor profile selected in OS settings

For your patched cairo: dt outputs image in display profile (sRGB by default, should be changed by you to real monitor profile instead) -> cairo renders it to Quartz surface with monitor colorspace -> macOS does nothing and displays image as is on display

For unpatched cairo changing dt display profile or OS monitor profile affects how image looks (not that you should change any of those). For patched cairo changing dt display profile affects the image look the same (and should be changed to monitor profile), changing OS monitor profile doesn't affect the described conversions, but it still affects gamma correction LUTs, so still can have visible difference.

geni1105 commented 2 years ago

Thanks for the explanation, that clarifies it for me:-) What confused me was the behavior of the "system display profile" option that I used in the soft proof popup: it seems to always lead to sRGB, not to the display profile selected in the MacOS system settings. So I was relying on darktable automatically switching to the correct profile, if I select that option - instead I need to do it manually, like you described.

Now regarding the jerky / laggy behavior, when a profile other than sRGB is selected in the system settings: the last step "macOS transforms sRGB into monitor profile selected in OS settings" seems to be exceptionally CPU consuming for some reason, and apparently is skipped if the monitor profile == sRGB (or permanently in my patched version), hence the speedup.

BTW a big thanks to you for bringing darktable to MacOS!

parafin commented 2 years ago

Thanks for the explanation, that clarifies it for me:-) What confused me was the behavior of the "system display profile" option that I used in the soft proof popup: it seems to always lead to sRGB, not to the display profile selected in the MacOS system settings. So I was relying on darktable automatically switching to the correct profile, if I select that option - instead I need to do it manually, like you described.

Because it’s default value for that setting and effectively it is display profile, since OS takes care of that. Obviously dt can’t know you patched cairo.