search

darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.86k stars 1.15k forks source link

Highlight reconstruction incorrectly applies mask with OpenCL #11968

Closed tomaszg7 closed 1 year ago

tomaszg7 commented 2 years ago

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

aurelienpierre commented 2 years ago

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

aurelienpierre commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 ?

tomaszg7 commented 2 years ago

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

aurelienpierre commented 2 years ago

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

github-actions[bot] commented 2 years ago

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.

dterrahe commented 2 years ago

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

tomaszg7 commented 2 years ago

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.

github-actions[bot] commented 1 year ago

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.

jenshannoschwalm commented 1 year ago

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

tomaszg7 commented 1 year ago

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

jenshannoschwalm commented 1 year ago

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.