Skip to content

Fix crash in scalepixels when used with rotate-and-perspective module#20706

Merged
TurboGit merged 1 commit intodarktable-org:masterfrom
da-phil:fix_crash_in_scalepixels_when_rotation_and_cropping_is_active
Apr 2, 2026
Merged

Fix crash in scalepixels when used with rotate-and-perspective module#20706
TurboGit merged 1 commit intodarktable-org:masterfrom
da-phil:fix_crash_in_scalepixels_when_rotation_and_cropping_is_active

Conversation

@da-phil
Copy link
Copy Markdown
Contributor

@da-phil da-phil commented Mar 30, 2026

  • Compute x_scale/y_scale locally in process() from actual roi_in/roi_out instead of using d->x_scale/d->y_scale which can be overwritten by precalculate_scale() called from distort_transform/distort_backtransform with different ROI dimensions, causing out-of-bounds interpolation access.

  • Implement distort_mask() enabling masks to work correctly when module is at non-standard pipeline position.

  • Fix buffer overflow in (ashift.c) g->buf allocation: the buffer was allocated using piece->buf_in.width * piece->buf_in.height but then filled with roi_in->width * roi_in->height * 4 floats. When roi_in->scale > 1.0 (zoomed in), roi_in dimensions exceed buf_in, causing a write past the end of the buffer. Fixed in both the CPU path (process()) and OpenCL path (process_cl()) by changing requested_size, g->buf_width, and g->buf_height to use roi_in dimensions instead of piece->buf_in.

Fixes #17286

Disclaimer: co-created with Claude.

* Compute x_scale/y_scale locally in process() from actual roi_in/roi_out
  instead of using d->x_scale/d->y_scale which can be overwritten by
  precalculate_scale() called from distort_transform/distort_backtransform
  with different ROI dimensions, causing out-of-bounds interpolation access.

* Implement distort_mask() enabling masks to work correctly when module
  is at non-standard pipeline position.

* Fix buffer overflow in (ashift.c) g->buf allocation:
  the buffer was allocated using piece->buf_in.width * piece->buf_in.height
  but then filled with roi_in->width * roi_in->height * 4 floats.
  When roi_in->scale > 1.0 (zoomed in), roi_in dimensions exceed buf_in,
  causing a ~6.6 MB write past the end of the buffer.
  Fixed in both the CPU path (process()) and OpenCL path (process_cl())
  by changing requested_size, g->buf_width, and g->buf_height to use
  roi_in dimensions instead of piece->buf_in.
@da-phil da-phil marked this pull request as ready for review March 30, 2026 21:51
@da-phil
Copy link
Copy Markdown
Contributor Author

da-phil commented Mar 30, 2026

I did some intensive testing and could not reproduce the crash mentioned in #17286.

@ralfbrown ralfbrown added bugfix pull request fixing a bug scope: image processing correcting pixels labels Mar 31, 2026
@TurboGit TurboGit added this to the 5.6 milestone Apr 1, 2026
@jenshannoschwalm
Copy link
Copy Markdown
Collaborator

Indeed the ashift code was simply not prepared to do some "upscaling".

  1. Code looks ok to me
  2. Tested here without issues - including combinations with crop, orientation ...
  3. No issues in integration tests

Good to me.

Copy link
Copy Markdown
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

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

Thanks!

@TurboGit TurboGit merged commit 5ad3d44 into darktable-org:master Apr 2, 2026
7 of 10 checks passed
@da-phil da-phil deleted the fix_crash_in_scalepixels_when_rotation_and_cropping_is_active branch April 2, 2026 21:23
@da-phil
Copy link
Copy Markdown
Contributor Author

da-phil commented Apr 2, 2026

@TurboGit I added the release notes in a follow-up PR: #20737

@Leon-Plickat
Copy link
Copy Markdown

Very sad to see AI generated code merged.

I have been a happy darktable user for a few years, but I will now be looking for alternatives.

@victoryforce
Copy link
Copy Markdown
Collaborator

Very sad to see AI generated code merged.

I also got really, really upset when I saw people using code from Stack Overflow. It made me so sad to see that, that I'm now relieved that LLM is helping people. LLMs explain their advice much better, so people get more knowledge than mindlessly copying someone else's code from SO.

I have been a happy darktable user for a few years, but I will now be looking for alternatives.

@Leon-Plickat The only thing I can say to that is Please do what makes you happy. That's the main thing!
Good luck in your search for a chaste code!

@da-phil
Copy link
Copy Markdown
Contributor Author

da-phil commented Apr 14, 2026

Please, let's not start this discussion all over again, there is already a very lenghty post on pixls.us about this topic:
https://discuss.pixls.us/t/worried-about-llm-written-modules/56430/258
and the remotely connected discussion here:
https://discuss.pixls.us/t/should-darktable-remain-ai-free/52778/736

This is not the right place to discuss this topic.

@jenshannoschwalm
Copy link
Copy Markdown
Collaborator

The relevant point, whatever the origin of a pr is, it gets reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash when "scale pixels" module is moved on top of pixelpipe and "rotate and perspective" is used

6 participants