Tentative fix for issue #19135: Mask points misaligned when zoomed in while cropping to fixed ratio#20556
Conversation
|
Yes, @dterrahe already had hinted at this. After enlarging preview pipe dimensions this is far less irritating but yes ... |
|
Note also that the issue is also present without any crop, so it is more general. |
…hen zoomed in while cropping to fixed ratio
|
What about now? I Tested with a 6022x4024 image and cannot reproduce the bug. Bug 2: mipmap integer-truncation vs Cairo float coordinate mismatch (the general issue)File: (
|
|
Well this is the track, basically a mismatch between dimension/offset calculation. Not absolutely sure from memory if mask coordinates are related to "original dimension" so rawprepare displacers have to be respected. Don't remember my findings when investigating this the last time. |
|
I think the fix is the right one. This is the same image without the fix (master head), you can clearly see that the node snaps when the pointer is away from it: Screen.Recording.2026-03-17.at.15.18.49.mp4To clarify, this is with or without crop, it's the general fix that you were talking about. |
|
What happens if you keep the mask you just created , do some stronger displacement in rawprepare, add a lens or remove it, do a crop, close dt and try again selecting nodes :-) |
|
It works @jenshannoschwalm , I can't break it :) Screen.Recording.2026-03-17.at.15.57.21.mp4 |
|
That's a very good improvement, but I'm sorry for bringing bad news about "not breaking it" in this :) This is very dependent on the size of the image. Take my TIFF (6022 x 4024): https://drive.google.com/file/d/1X1AIuoKPCVidhhEd6D8QEgq_Ut6TJdJN/view?usp=sharing And look at the demo:
Capture.video.du.2026-03-17.17-09-35.mp4 |
🤣 Damn it, you broke it! Let me try again, armed with your tiff :) |
|
Ok, I did further progress and the drag misalignment is resolved. There is only one issue that persists, which may or may not be related. If I enable crop AND high quality processing is disabled, the mask jumps by a few pixels. The whole mask is repositioned. The alignment between nodes and cursor is retained, including during drag: Screen.Recording.2026-03-17.at.20.02.58.mp4This is not a regression, it happens also at master head. Is this a known issue? @jenshannoschwalm @TurboGit |
Not known by me, enabling/disabling crop indeed shift the mask on current master too. Looks like another crop issue then? |
|
I pushed the fix for the dragging issue. Would you like to break this too? 😅 Maybe the crop issue can be resolved independently. |
I'll be more than happy to try... Let me have diner first, I need energy for this :) |
|
Cannot break it... and this is a huge improvement! So what about merging this and handling the crop issue in another PR? |
|
I think it makes sense to decouple the two issues 👍 |
|
Happy to help, this bug was annoying me greatly! Tomorrow I can look into the crop issue, feel free to assign it to me. |
@jenshannoschwalm @TurboGit
With Claude's help I might have a fix #19135. I tested in on a couple of images and it works for me:
Screen.Recording.2026-03-17.at.07.21.21.mp4
Find below Claude's analysis, which seems reasonable to me but certainly @jenshannoschwalm will know better.
Root Cause
crop.c'sdistort_transformanddistort_backtransformcomputed the crop offset using floating-point arithmetic:But
modify_roi_out— which defines the actual pipeline crop offset — uses integer truncation:This discrepancy (
fract(buf_in.width * cx)) means the distort functions report a slightly different crop position than what the pipeline actually processes. The mismatch is always sub-pixel in full-res coordinates, but becomes visible at higher zoom levels because the coordinate system is effectively magnified.A second, larger problem affects fixed-ratio crops on export pipelines:
modify_roi_outadds an alignment correction (dw/2,dh/2) to ensure exact codec-required dimensions — but the distort functions completely ignored this, causing the mask to be applied at the wrong position during export.Fix (
src/iop/crop.c)Applied the same pattern already used by
clipping.c(which has this exact problem documented in comments):Use
factor = 100for preview pipes to reduce the integer rounding error from up to 1 pixel to < 0.01 pixelsCall
modify_roi_outdirectly with the scaledbuf_into compute the crop offset — this ensures the distort functions use the exact same offset as the pipeline, including alignment corrections for fixed-ratio export cropsThe fix makes both
distort_transformanddistort_backtransformconsistent withmodify_roi_outin all pipeline types.