Skip to content

Two pixelpipe bugfixes plus maintenance #21106

Merged
TurboGit merged 4 commits into
darktable-org:masterfrom
jenshannoschwalm:pixelpipe_colorspace_fix
May 23, 2026
Merged

Two pixelpipe bugfixes plus maintenance #21106
TurboGit merged 4 commits into
darktable-org:masterfrom
jenshannoschwalm:pixelpipe_colorspace_fix

Conversation

@jenshannoschwalm
Copy link
Copy Markdown
Collaborator

(1) Bugfix CPU pixelpipe cache integrity after module colorspace conversion

Before a piece is processed the pixelpipe checks if the current cst is different from the module's requested input cst.
In this case, the input data are converted in-place (for less pixelpipe mem pressure) but then the cacheline - possibly kept for reuse - has an incorrect cst!

Note: in opencl code path this is different as the required colorspace conversion is always done in cl_mem and thus does not affect the pixelpipe cache.

For now we just invalidate that single cacheline.

Comment: Quite an old bug! Very hard to trigger with current code - might have been the underlying cause for some "fixes" done invalidating the pipecache :-) Also not sure if this could be a reason for false color issues while being in darkroom.


(2) Performance regression: Fix preview pixelpipe caching issue

If we process the preview pixelpipe on CPU - either because there is no OpenCL device at all or the OpenCL scheduler delivered the pipe to the CPU - the importance hint for the cacheline was not tested correctly so with a performance penalty.


(3) maintenance:The colorspace transform functions use dt_iop_colorspace_type_t()

Use correct dt_iop_colorspace_type_t instead of int for dt_ioppr_transform_image_colorspace() and dt_ioppr_transform_image_colorspace_cl()


(4) Some maintenance work for pixelpipe processing

  1. refactored _is_debug_pipe() also testing for a no-shutdown situation.
  2. refactored the cpu bound benchmark loop for readability. Both benchmarks check for shutdown in (1)
  3. some _pipe_has_shutdown() tests have been removed as clearly not helping
  4. Some constify work, use of #define and log improvements

@TurboGit no integration tests regressions, nothing "new"

Before a piece is processed the pixelpipe checks if the current cst is different from
the module's requested input cst.
In this case, the input data are converted in-place (for less pixelpipe mem pressure)
but then the cacheline - possibly kept for reuse - has an incorrect cst!

Note: in opencl code path this is different as the required colorspace conversion
is always done in cl_mem and thus does not affect the pixelpipe cache.

For now we just invalidate that single cacheline.

Fixme: we should implement and use a pipecache helper that modifies the cacheline cst.
If we process the preview pixelpipe on CPU - either because there is no OpenCL device at all
or the OpenCL scheduler delivered the pipe to the CPU - the importance hint for the cacheline
was not tested correctly so with a performance penalty.
Use correct `dt_iop_colorspace_type_t` instead of `int` for
`dt_ioppr_transform_image_colorspace()` and `dt_ioppr_transform_image_colorspace_cl()`
1. refactored `_is_debug_pipe()` also testing for a no-shutdown situation.
2. refactored the cpu bound benchmark loop. Both benchmarks check for shutdown in (1)
3. some `_pipe_has_shutdown()` tests have been removed as clearly not helping
4. Some constify work, use of #defines and log improvements
@jenshannoschwalm jenshannoschwalm added this to the 5.6 milestone May 23, 2026
@jenshannoschwalm jenshannoschwalm added bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes scope: image processing correcting pixels scope: codebase making darktable source code easier to manage labels May 23, 2026
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.

Looks good, thanks!

@TurboGit TurboGit merged commit 9b472f4 into darktable-org:master May 23, 2026
5 checks passed
@jenshannoschwalm jenshannoschwalm deleted the pixelpipe_colorspace_fix branch May 23, 2026 19:07
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: high core features are broken and not usable at all, software crashes scope: codebase making darktable source code easier to manage scope: image processing correcting pixels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants