darktable icon indicating copy to clipboard operation
darktable copied to clipboard

Highlight reconstruction incorrectly applies mask with OpenCL

Open tomaszg7 opened this issue 3 years ago • 7 comments

Describe the bug/issue

In my practice highlight reconstruction often needs tweaking depending on the surface where highlights are blown. For example for faces or reflections on some objects reconstruct color or disabled works better and for overexposed sky or windows I usually need to use clip or reconstruct in LCh to avoid color cast. Thus in some cases it would be convenient to have a possibility to apply different settings to various parts of image.

Regretfully the module cannot be duplicated but applying masks should be at least a partial solution. I tried drawing a mask but they barely affected the output. They had some slight effect but it was barely noticeable.

It seems that even switching to uniform mask with 100% opacity shows a problem. The effect should be the same as with masks turned off but it is not. I see the effect with all methods except reconstruct color.

The problem disappears when I disable OpenCL processing. I'm using ROCm with AMD Fury GPU.

To Reproduce

  1. Open image with overexposed highlights
  2. Enable highlight reconstruction module with method other than reconstruct color
  3. Change mask from off to uniform and notice the output changes. It is similar to output with module disabled but not exactly the same. Moreover after preparing images I noticed some green artifacts in top left corner.

Screenshots HR turned off HR turned off

HR turned on without mask HR turned on without mask

HR turned on with uniform 100% mask HR turned on with uniform 100% mask

Platform

  • darktable version : d91ebdb926f218e17bddc28936a8cbd623b069a3
  • OS : Linux
  • Linux - Distro : Gentoo
  • Memory :
  • Graphics card : AMD Fury
  • Graphics driver : amdgpu
  • OpenCL installed : ROCM-4.3.0

tomaszg7 avatar Jun 03 '22 09:06 tomaszg7

Reconstruct color is actually the only mode that doesn't use OpenCL at all. I will have a look.

aurelienpierre avatar Jun 12 '22 07:06 aurelienpierre

Confirmed, it seems there is a bug in the masking API for RAW modules in OpenCL path and the highlights reconstruction module is the only one using that path. The mask preview is affected as well.

aurelienpierre avatar Jun 12 '22 08:06 aurelienpierre

In addition, there is another bug…

RAW images use only 1 channel. RGB images use 4 channels, so they are actually RGB + alpha (mask opacity). This extra alpha channel is used to preview the mask. Since we don't have alpha for RAW images, there is no mask display available.

aurelienpierre avatar Jun 12 '22 09:06 aurelienpierre

After 2h reading the CPU and GPU pathes side to side, I didn't find any discrepancy in the code. The C/CPU path works as expected, except for the drawn mask preview that is unavailable for the reasons mentioned above.

What I found though, is that the guided laplacian mode is unaffected (CPU as well as GPU) and that using the "normal bounded" blending mode for clipping and Lch reconstruction fixes the issue. Only the unbounded normal mode is affected.

So, all in all, we have a discrepancy between CPU and GPU only for "clip highlights" and "reconstruct in Lch" modes blended in "normal unbounded" mode. @tomaszg7 can you confirm ?

aurelienpierre avatar Jun 12 '22 10:06 aurelienpierre

Yes, I confirm that. I noticed the "normal bounded" thing before, but forgot to mention it.

tomaszg7 avatar Jun 12 '22 10:06 tomaszg7

Cool. But then I can't make sense of what's happening and I didn't find a mistake in the code…

aurelienpierre avatar Jun 12 '22 12:06 aurelienpierre

This issue did not get any activity in the past 60 days and will be closed in 365 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

github-actions[bot] avatar Aug 12 '22 00:08 github-actions[bot]

@tomaszg7 #12677, which landed in master yesterday, fixed some small issues in parameter passing to OpenCL routines (and may well have introduced others). Not saying that there is reason to believe it fixes your issue, but would you be able to test if it still exists?

dterrahe avatar Oct 29 '22 13:10 dterrahe

I did a quick test turning on/off uniform 100% blending: clip highlights and reconstruct in LCh seem to still be affected, while new methods inpaint, segmentation and laplacians seem to work fine. I didn't check how these new methods behaved before the fix.

BTW. Is there a reason why highlight reconstruction module can't be duplicated? It stands to reason that if it is allowed to use masks, one could want to use the same module with different settings in different areas of picture.

tomaszg7 avatar Oct 29 '22 13:10 tomaszg7

This issue did not get any activity in the past 60 days and will be closed in 365 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

github-actions[bot] avatar Dec 30 '22 00:12 github-actions[bot]

@tomaszg7 could you recheck please? There have been numerous fixes for mask blending and indeed we had some fixes for opencl masking for pre-demosaic modules. On current master i definitely can't reproduce any longer.

jenshannoschwalm avatar Aug 31 '23 20:08 jenshannoschwalm

It seems it is fixed. I couldn't trigger this behaviour anymore. If I see it again, I'll let you know.

However my second "complaint" is valid: it is not possible to duplicate this module, which is sometimes useful (e.g. for the sample photo above - face needed different settings than window).

tomaszg7 avatar Sep 03 '23 11:09 tomaszg7

So let's close this for now.

I agree with you, it would be useful in some cases and yes, technically there would be no immediate problem, i have been using several highlights modules myself to test.

I hesitate to do a pr though. That would require very careful settings of the masks and at least some algos (laplacian and segmentation for sure, opposed not so much) would run wild as they make assumptions about the surrounding areas or whole image data. So doing that in the normal pipeline ... probably not a good thing.

jenshannoschwalm avatar Sep 03 '23 15:09 jenshannoschwalm