Skip to content

blending: read the mask back from the device before publishing it as raster - #22041

Merged
TurboGit merged 1 commit into
darktable-org:masterfrom
masterpiga:opencl_upstream_fix
Aug 29, 2026
Merged

blending: read the mask back from the device before publishing it as raster#22041
TurboGit merged 1 commit into
darktable-org:masterfrom
masterpiga:opencl_upstream_fix

Conversation

@masterpiga

@masterpiga masterpiga commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

While working on the migration code for Flexi I stumbled upon two issues that cause masks to render differently along the OpenCL and CPU paths.

Both findings are documented extensively in this report.

Addressing both issues would make the CPU and OpenCL paths virtually identical wrt mask rendering, which they should be but currently are not.

This PR fixes the first, which is an actual bug on the OpenCL path, as detailed below. The second issue is a divergence between the OpenCL and CPU paths. The report has a concrete suggestion to resolve it. @TurboGit, let me know if you want me to act on it. Personally, I vote in favor.

What this PR fixes

dt_develop_blend_process_cl() skips the device→host readback of the final mask when the module's own mask came from a raster mask, assuming the host buffer already holds it. That only holds if nothing touches it on the device afterwards — but the mask post-processing right above (feathering, blur, tone curve) reads and writes dev_mask. With any of those active, the published raster mask is the unrefined one: downstream consumers get a different mask than this module blended with, and a different one than the CPU path publishes for the same edit.

The fix

Read back unconditionally.

Found by replaying harvested mask edits on both pipes and comparing. Bucketed by mask mode and active post-processing over 42,078 edits:

mask mode post-processing diverge / edits
raster any 61 / 61
raster none 0 / 590
drawn any 0 / 6,747

The drawn row rules out the post-processing implementations themselves and leaves the publication step. On a corpus of every distinct raster configuration found, divergent edits go 42/76 → 0, worst CPU-vs-OpenCL gap 0.940 → 0.000153.

CPU renders are unchanged; OpenCL renders change to match them.

Co-authored with Claude.

@masterpiga masterpiga added this to the 5.8 milestone Aug 29, 2026
@masterpiga masterpiga added bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes difficulty: trivial some changes in a couple of functions scope: image processing correcting pixels labels Aug 29, 2026
@masterpiga

masterpiga commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

@TurboGit: I tagged this as important because, at the very least, it's something to be aware of.

EDIT: Added "I" at the beginning of the previous sentence to make it clear that it was I and not Pascal who tagged it.

@ralfbrown ralfbrown added the OpenCL Related to darktable OpenCL code label Aug 29, 2026
Comment thread src/develop/blend.c Outdated
…raster

dt_develop_blend_process_cl() skipped the device-to-host readback of the
final mask when this module's own mask came from a raster mask, on the
reasoning that such a mask is built on the host and uploaded, so the host
buffer already holds it.

That only holds while nothing touches it on the device afterwards. The
mask post-processing immediately above -- feathering, blur, tone curve --
reads and writes dev_mask. With any of those active the host buffer is
still the unrefined mask, and that is what gets published: a module that
consumes a raster mask, refines it, and is itself a raster source for a
later module hands every downstream consumer a different mask than it
blended with, and a different one than the CPU path publishes for the
same edit.

Found by replaying harvested mask edits on both pipes and comparing.
Bucketed by mask mode and by active post-processing, over 42,078 edits:
raster with any post-op diverges in 61 of 61 cases; raster with no
post-op in 0 of 590; drawn masks with the same feathering, blur and tone
curve in 0 of 6,747 -- which rules out the post-processing
implementations themselves and leaves the publication step. On a corpus
of every distinct raster configuration found, this takes the divergent
edits from 42 of 76 to 0, and the worst CPU-vs-OpenCL gap from 0.940 to
0.000153.

CPU renders are unchanged; OpenCL renders change to match them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@masterpiga
masterpiga force-pushed the opencl_upstream_fix branch from 3497d38 to 87ab0c2 Compare August 29, 2026 13:47
@masterpiga
masterpiga requested a review from TurboGit August 29, 2026 13:48

@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.

Thanks!

@TurboGit
TurboGit merged commit f54402e into darktable-org:master Aug 29, 2026
6 checks passed
@masterpiga

Copy link
Copy Markdown
Collaborator Author

Thanks, @TurboGit. What about the open question, i.e., this?

Do you want me to act on it?

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 difficulty: trivial some changes in a couple of functions OpenCL Related to darktable OpenCL code priority: high core features are broken and not usable at all, software crashes scope: image processing correcting pixels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants