blending: read the mask back from the device before publishing it as raster - #22041
Merged
Merged
Conversation
Collaborator
Author
|
@TurboGit: I tagged this as EDIT: Added "I" at the beginning of the previous sentence to make it clear that it was I and not Pascal who tagged it. |
TurboGit
requested changes
Aug 29, 2026
…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
force-pushed
the
opencl_upstream_fix
branch
from
August 29, 2026 13:47
3497d38 to
87ab0c2
Compare
Collaborator
Author
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.
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 writesdev_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:
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.