Skip to content

Fixes #21594 and #21606 by gating on off_image geometry - #21648

Merged
TurboGit merged 1 commit into
darktable-org:masterfrom
masterpiga:mask_editing
Aug 5, 2026
Merged

Fixes #21594 and #21606 by gating on off_image geometry#21648
TurboGit merged 1 commit into
darktable-org:masterfrom
masterpiga:mask_editing

Conversation

@masterpiga

@masterpiga masterpiga commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Both issues stem from PR #21382 driving the clip/frame/coverage from the live viewport centre instead of the rendered backbuf position. I fixed them by gating the new geometry on off_image — whether the viewport is actually panned past the image, which the pan clamp only allows during off-image mask editing:

  • off_image == FALSE (all normal use): restores darktable's original clip (MIN(viewport,image) centred, never sits on the border → no grey seam) and original coverage test (measured against real zoom, so a drag that brings the border into view still re-renders to fill it).
  • off_image == TRUE (mask handle pushed off-image): keeps the new viewport-centre geometry it was introduced for.
  • The color-assessment frame is now always sized from the visible rect (image ∩ viewport) + border, so it can't grow across the grey background (Color assessment border grows over the gray background while zooming in #21594) — and this matches the original geometry in normal use.

Fixes #21594 and #21606.

@masterpiga

masterpiga commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

@Macchiato17 and @jenshannoschwalm can you please verify that the fix works for your use case?

Note: the color assessment frame behaves differently when mask editing is ON, but this is by design, as you need to be able to access the mask nodes. When set to OFF, the behavior should be exactly as before the #21382.

@masterpiga masterpiga changed the title Fixes #21594 and #21606 by gating off_image geometry Fixes #21594 and #21606 by gating on off_image geometry Jul 27, 2026
@TurboGit

Copy link
Copy Markdown
Member

Note: the color assessment frame behaves differently when mask editing is ON, but this is by design, as you need to be able to access the mask nodes. When set to OFF, the behavior should be exactly as before the #21382.

Just a thought, maybe we should temporarily disable the color assessment mode when editing masks? Having both mode at the same time seems really not good to me and without real value.

@masterpiga

Copy link
Copy Markdown
Collaborator Author

@TurboGit I do not disagree in principle. However, the two functionalities are very disconnected in terms of UI, so users may be surprised to see color assessment switching off when mask editing is enabled. Also, it may be considered a regression wrt to the previous behavior, as the two modalities have been able to coexist before.

@ralfbrown ralfbrown added bugfix pull request fixing a bug scope: UI user interface and interactions labels Jul 27, 2026
@Macchiato17

Copy link
Copy Markdown
Contributor

@masterpiga I will be happy to test it on my side. Do you need a feedback prior to merging this PR into master?

@masterpiga

Copy link
Copy Markdown
Collaborator Author

@Macchiato17 the regressions are minor, so I would say that there is no rush to merge. Your feedback would be appreciated

@Macchiato17

Macchiato17 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

OK, so I checked with [e595eb72]: it's much better now, thanks for looking into it so quickly.
I still see a small remnant of the effect depending on the image ratio (see how the effect changes when I disable the applied crop). Please see the screen-cast:

screencast.mp4

This effect is even stronger, when using darktable not in full screen mode, but reduced to a window size with a more "flat" side ratio, somehow like cinemascope ;-) (I didn't capture that).

Or is - what I call a remnant - intended? When I read your text above
The color-assessment frame is now always sized from the visible rect (image ∩ viewport) + border
I'm not absolutely sure...

I also realized that there is a dark gray overlay on the right side reaching into the gray background, when expanding the crop module, but I don't know if this is also related to this issue here, or if that's a different thing.

@TurboGit TurboGit added this to the 5.8 milestone Jul 29, 2026
@TurboGit TurboGit added the priority: low core features work as expected, only secondary/optional features don't label Jul 29, 2026
@jenshannoschwalm

Copy link
Copy Markdown
Collaborator

@masterpiga you are probably aware of this with current code after that "mask-enlarging" but - when zooming in and dragging the main canvas completely to any border (happens more often at the right or top border)

  1. there is often a grey bar not showing content at that border
  2. the size of preferences "border around image" seems to be influence this ???
  3. I think this is related to the concept of "overcalculation" - "anticipate_move" If setting this to 1.0 in preferences this issue can't be seen here.

@masterpiga

masterpiga commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

@Macchiato17 I fixed the issue with the crop overlay. I cannot reproduce the twitch in the color assessment border that you have. If you still see it, could you please share with me your image and XMP so that I can try to reproduce?

@jenshannoschwalm IIUC, that should be fixed. Can you still reproduce? If not, could you please show me a visual of what you mean?

I also added a couple of UX Improvements:

  1. With color assessment enabled, draw a dashed line on the borders that do not coincide with the image border, to make it clear (that be an overstatement, let's say to give a hint) that the image extends beyond said borders.

  2. In the preview widget, draw a chevron pointing to the center of the canvas region when zooming in to edit a mask element that is outside the image boundaries.

They are both exemplified below:

Screen.Recording.2026-08-04.at.16.23.08.mp4

@jenshannoschwalm

Copy link
Copy Markdown
Collaborator

@masterpiga indeed - can't reproduce with this PR rebased on master

@Macchiato17

Copy link
Copy Markdown
Contributor

Thanks for the update -> I just tested [ecdf3656], the gray border that reached into the gray background when expanding the crop module is gone now 👍 thanks so much.

Regarding the changing border, I'm not sure, if I ever changed the ratio parameters here:
grafik

Perhaps this has an influence? My screen is a 4K, but set to 1920*1200 at 100% scaling (don't know if this information of any relevance). I will send you a link for downloading the RAW+XMP via DM.

@masterpiga

masterpiga commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @Macchiato17 for sharing the file and the XMP, I could reproduce. However, the glitch (1-2 pixel jump of the border near "fit" width) that you observed is not caused by this change. The formula to compute the position and size of the white margin are not changed in this PR.

The white margin is constant, so any change in the gray:white proportion comes entirely from img_w shifting, and img_w = processed_width * backbuf_scale is the weak point across the fit boundary because both factors are recomputed discontinuously there:

  • backbuf_scale switches formula with the zoom mode
  • processed_width snaps when the pipe re-processes at the new scale (it's an integer, and the pipe picks discrete scales), so processed_width · backbuf_scale isn't guaranteed to land on the same on-screen size it had a moment before.

I will (consider) send(ing) a fix as a separate PR.

@masterpiga

masterpiga commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

I believe that the PR fixes what it was meant to fix. PTAL and TIA. Scratch that, @Macchiato17 told me in chat that he found some artifacts related to the chevron that tracks the canvas position when editing off-image that warrant further investigation.

@Macchiato17

Copy link
Copy Markdown
Contributor

OK here we go

I also added a couple of UX Improvements

cool - first I wondered if the dotted line would affect color perception - but I think it's a helpful hint. And when it comes to color calibration or other color adjustments, I'm used to do this while having the image in full view. So it wouldn't annoy me at all. Nice idea.

Then I was interested in the "caret feature" - also nice to give a hint here. When I played around with that feature, I found funny artifacts. OK as I saw them, I stressed it - both leading to an application crash. Please see the screencasts added below. The end of each screencast is the the application crash.

Video_2026-08-04_21-36-25.mp4
Video_2026-08-04_21-41-42.mp4

@masterpiga

Copy link
Copy Markdown
Collaborator Author

You are certainly a better tester than I am, it never occurred to me to drag the whole shape outside the image 🤣

I see that the shape shrinks while it is been dragged, which should not happen. I will investigate, thanks!

@masterpiga

Copy link
Copy Markdown
Collaborator Author

Ah, the shrinking is because of lens correction, cute.

I think it just does not make sense that one can move a shape completely outside the image. I will introduce some constraints to prevent that.

@masterpiga

masterpiga commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

@Macchiato17 I added constraints to how far a mask element can be moved away from the image.

BTW, I was not able to reproduce your failures, even before the latest change. Next time that you observe a crash, please include a stack trace if you have it around.

@Macchiato17

Copy link
Copy Markdown
Contributor

@masterpiga moving the mask and stressing it a little works fine now using [cbc9659] and I also can't reproduce the crash anymore (whatever it was related to). Thanks again for fixing this so quickly 💪

@TurboGit TurboGit left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So good to go for a larger testing. Works also on my side now. Thanks!

@TurboGit
TurboGit merged commit 7a30a26 into darktable-org:master Aug 5, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix pull request fixing a bug priority: low core features work as expected, only secondary/optional features don't scope: UI user interface and interactions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Color assessment border grows over the gray background while zooming in

5 participants