ashift: don't drop the deferred autocrop on an early preview signal - #22332
Merged
TurboGit merged 3 commits intoSep 19, 2026
Merged
Conversation
1 task done
Contributor
|
Nice, I encountered this several times :) |
Collaborator
|
Tested: Fixes the issue. Thanks! |
TurboGit
added
bugfix
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The preview geometry check must be synchronized to avoid races with preview updates.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes a race in ashift where deferred automatic cropping could be lost before preview geometry was available.
Changes:
- Keeps crop requests pending until preview geometry is available.
- Documents the fix in the release notes.
| File | Summary |
|---|---|
src/iop/ashift.c |
Defers cropping when preview dimensions are unavailable. |
RELEASE_NOTES.md |
Documents the automatic-crop fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Collaborator
Author
|
A rotation change made while the module is still disabled defers the autocrop until the preview buffer exists. A preview-finished signal from a run that started before the module was enabled cleared the request, do_crop() bailed out on the empty buffer, and the crop was lost. Keep the request pending until the buffer is filled. Authored by Claude Code. Fixes darktable-org#21513
Authored by Claude Code.
The preview pipe writes buf_width/buf_height under gui_lock on its worker thread; the check in the GTK-side callback read them unlocked. Authored by Claude Code.
kofa73
force-pushed
the
fix/ashift-lost-deferred-autocrop
branch
from
September 19, 2026 14:08
4be5df5 to
5e98e3e
Compare
kofa73
added a commit
to kofa73/darktable
that referenced
this pull request
Sep 29, 2026
…geometry() The buffer-size check that darktable-org#22332 added to the preview callback takes gui_lock itself to read buf_width and buf_height. Use the snapshot helper that the other GTK-side readers of the buffer geometry use, so those fields are read in one place. No change in behavior. Related: darktable-org#21918
kofa73
added a commit
to kofa73/darktable
that referenced
this pull request
Sep 29, 2026
The buffer-size check that darktable-org#22332 added to the preview callback takes gui_lock itself to read buf_width and buf_height. Use the snapshot helper that the other GTK-side readers of the buffer geometry use, so those fields are read in one place. No change in behavior. Related: darktable-org#21918
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #21513
Problem
If rotate and perspective is still disabled when its rotation is changed (for example with an
r+scroll shortcut, before the module is in the history),gui_changed()has no preview buffer yet. It therefore defers the automatic crop until a preview has finished. The preview-finished signal can come from a run that started before the module was enabled. The callback cleared the request,do_crop()returned early on the empty buffer, and the crop was lost. The image stayed rotated with empty corners, in the lighttable and in exports, until the next adjustment.master, burst of 5 r+scroll steps 3.6 s after opening, uncropped wedge at the top:
Fix
The preview callback now leaves the crop request pending while
g->bufis still empty, so the first preview run that fills the buffer does the crop. Otherwise nothing changes. It also no longer adds a history item for a crop that did not happen.same timing with the fix, race hit, crop applied:
This is independent of #22198 (#21918, unlocked buffer geometry reads). Both touch
ashift.c. The new check readsbuf_width/buf_heightthe same way as the surrounding master code, so whichever PR merges second should switch it to_get_buf_geometry().Testing
Linux, Release build, CPU only. darktable running in headless sway, with the input sent by a script.
Not tested: OpenCL, and the integration tests (no
cv2here; this path runs only with the GUI). I only reproduced the case where the module starts out disabled. The original report does not say whether that applies to it.Code, commits, PR by Claude Code.