Graphical glitch in the histogram while color picking (rgb module)

[c-h-r-i-s-t-i-a-n]

I'm getting graphical glitches in the histogram while color picking in the rgb curve module.

Reproduce

Open the RAW file from this thread: https://discuss.pixls.us/t/darktables-filmic-rgb-defaults-render-this-desaturated/29068

with this xmp file (sorry for zip, xmp is not allowed for upload): P1200988.RW2.zip

Go to the rgb curve module, select the color picker and drag the cross around in the picture. Now the graphical glitch is appearing occasionally within the histogram (see screenshot).

Screenshots

Platform

• darktable version : 3.8.0
• OS : Win10 64bit
• Memory : 16 GB
• Graphics card : Intel/Nvidia
• Graphics driver :
• OpenCL installed : yes
• OpenCL activated : no

[c-h-r-i-s-t-i-a-n]

In the vectorscope display the glitch is appearing as a little speck.

[c-h-r-i-s-t-i-a-n]

Maybe it's related to demosaic RCD? When I switch to another method the glitch is gone. Also turning on demosaic color smoothing seems to help.

[Nilvus]

@jenshannoschwalm: if this could be related to RCD demosaicer, maybe your view could be helpful here.

[jenshannoschwalm]

I will have a look tonight

[jenshannoschwalm]

@c-h-r-i-s-t-i-a-n before me looking into this deeper (i tried this morning for a short time and couldn't trigger the issue yet), a few answered questions will help

1. what setting are you using in preferences->pixel interpolator (scaling)? What you see could be the result of using lanczos3.
2. It seems you are taking histogram data in point mode. Does the issue persist in area mode?
3. Do you see a difference between OpenCL on/off?
4. Is the issue persisting if you switch off modules that might correct chromatic aberration (lens ...)? I ask this because such low-level artefacts could also produced here by lensfun algo as can be regularly seen while working on monochrome images.

[c-h-r-i-s-t-i-a-n]

1. Also occurs with other interpolator methods like bilinear, bicubic.
2. issue persists in area mode.
3. issue persists with opencl on.
4. issue persists when leaving on only: blackpoint/white bal/demosaic/rgb curve/input output profile
5. the glitch in the histogram can also be triggered in the tone curve module.

[jenshannoschwalm]

Leaves me clueless atm, just to make sure: it's only happening with RCD?

[jenshannoschwalm]

Just tried for some time to trigger the issue and couldn't reproduce it a single time. I confess i don't fully understand the way the histogram data are fed but any demosaicer might do some overshoots but like this? I have never observed something like that on PPG, RCD or LMMSE (those are the ones i know the code quite well).

I also tried with "performance mode" or downscaled previews btw.

Maybe it would help to see a video to get a clue? Could you produce one?

[TurboGit]

Maybe @dtorop can shed some light on the histogram stuff?

[jenshannoschwalm]

@TurboGit @Nilvus could you reproduce the issue?

[c-h-r-i-s-t-i-a-n]

Video:

https://user-images.githubusercontent.com/57685590/151607371-559fb3ec-f5b4-4401-9214-2290b212087d.mp4

[dtorop]

Hmmm. Interesting video. And seriously confusing, as the appearance is so glitchy.

Out of curiosity, what is the "display profile" and "histogram profile" settings? Right click the toggle gamut or softproofing buttons at the bottom right of the center view to see these in a pop-up menu. If the display profile is non-standard (e.g. it's a calibrated screen), it may be helpful to upload the ICC profile.

[jenshannoschwalm]

The "range numbers" look a bit strange

[jenshannoschwalm]

I still can't reproduce as reported but there is something not working correctly with the shadowed histogram data presented in the rgb-curve module.

You can see low-signal data at the right side of both histograms. If you scroll though the demosaicers you will see that changing in the main hist correctly. But in the rgb-curve histo this is not always correct like here

For me it seems to be a gtk widget update problem ???

[TurboGit]

@jenshannoschwalm : I'll try to reproduce today.

[TurboGit]

Tried, tried hard and cannot reproduce on my GNU/Linux box. Either a Gtk+ issue or a general issue only seen on Windows. Would be hard to say.

@wpferguson : as you build on Windows can you try to reproduce this with the RAW + xmp linked on first comment?

@windows-users: If you read this, can you test and report is you reproduce or not? TIA.

[c-h-r-i-s-t-i-a-n]

The "range numbers" look a bit strange

Which numbers do you mean?

[c-h-r-i-s-t-i-a-n]

Out of curiosity, what is the "display profile" and "histogram profile" settings? Right click the toggle gamut or softproofing buttons at the bottom right of the center view to see these in a pop-up menu.

display profile = system softproof profile = sRGB histogram profile= sRGB

I'm not using a calibrated screen / icc stuff.

On my linux laptop (manjaro) I'm also not able to reproduce the glitch.

[wpferguson]

Tried to reproduce and couldn't in either windows or linux with current master

[c-h-r-i-s-t-i-a-n]

Could reproduce this also with a Canon CR2 raw file.

[dtorop]

display profile = system softproof profile = sRGB histogram profile= sRGB

I'm not using a calibrated screen / icc stuff.

A long shot, but if you change "display profile" to something known, such as sRGB (or really anything on the list except for system), does the problem still occur?

[c-h-r-i-s-t-i-a-n]

Yes, problem still occurs.

[jenshannoschwalm]

@dtorop i looked into the histogram code somewhat deeper yesterday, also watching the video again and tried to understand what's going on.

Form my current understanding it seems we have possibly two different issues.

1. The helper histograms inside the modules are not updated if data are read from pipeline cache. (You can do whatever changes visible output like switching on/off the highlights reconstruction module offering a nice purple cast - the main histo is changed always, the modules histo is not) This is easy to understand, as in dt_dev_pixelpipe_process_rec we immediately go to post_process_collect_info, where we only update the main histo but do not check/update inside the modules. (This can and should probably be fixed there)
2. This leaves out: where are the wrong data for the main histogram are coming from? I looked at the histo Worker code and there we use c/realloc for the histogram buffer. (From the docs this should be aligned for the largest native datatype, which would be 8 on windows ...) But the code seems to assume we are simd aligned - this would take/write bogus data.

What do you think?

[dtorop]

@jenshannoschwalm These are both really good points. Regarding 2), all that code seems heavily optimized for decade-old CPUs/compilers, and could use some attention.

I spent a bit of time looking at the pixelpipe code:

• In the normal case for a "modern" pixelpipe, the final histogram is calculated from gamma input. But as gamma nowadays is usually just after colorout, it makes sense to instead calculate this from colorout input, which saves a colorspace hop.
• But this cause caching problems. If colorout is cached, we don't have access to its input.
• Probably the right thing to do then is just not update the histogram. But if I turn off caching for colorout, I see some interesting noise in the upper part of the preview image.

I haven't traced this noise, but wonder if this is connected to 2), or yet another bug. Here is a diff which can trigger the bug for me:

modified   src/develop/pixelpipe_hb.c
@@ -1110,7 +1110,7 @@ static int dt_dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe, dt_develop_t *
   // do not get gamma from cache on preview pipe so we can compute the final histogram
   if((pipe->type & DT_DEV_PIXELPIPE_PREVIEW) != DT_DEV_PIXELPIPE_PREVIEW
      || module == NULL
-     || strcmp(module->op, "gamma") != 0)
+     || !(strcmp(module->op, "colorout") == 0 || strcmp(module->op, "gamma") == 0))
   {
     dt_dev_pixelpipe_cache_fullhash(pipe->image.id, roi_out, pipe, pos, &basichash, &hash);
     cache_available = dt_dev_pixelpipe_cache_available(&(pipe->cache), hash);

(Also, because the gamma is never fetched from cache, some later code that handles no gamma input data for the histogram is irrelevant.)

Yet another thing is that colorout always asks for Lab input data, which doesn't seem to make sense anymore when the pipeline up until then could be in RGB -- an unnecessary hop?

Maybe the question is just who will work on fixes. I have some time, if it helps... Of course none of this may fix this unsual Windows bug.

[jenshannoschwalm]

all that code seems heavily optimized for decade-old CPUs/compilers, and could use some attention.

Absolutely, it just make the code less clear.

I see some interesting noise in the upper part of the preview image

confirmed :-)

none of this may fix this unsual Windows bug

Yes, can be. But i think it's worth to use the dt_aligned versions of alloc and friend to make sure. I remember other issues also related to different aligns between windows/linux.

Maybe the question is just who will work on fixes. I have some time, if it helps

Good. I think we both have grossly the same ideas here so go ahead :-)

[c-h-r-i-s-t-i-a-n]

BTW CPU : Intel(R) Core(TM) i7-3720QM CPU @ 2.60GHz

[dtorop]

I see some interesting noise in the upper part of the preview image

confirmed :-)

Good, though worrying! I really wish I had a clue of why... But it could be enough to cause an unreliable histogram.

i think it's worth to use the dt_aligned versions of alloc and friend to make sure

Yes. My guess is that this histogram code has negligible execution time, hence is over-optimized, and never was built for Windows compatibility.

here so go ahead :-)

I'll do my best! Time is still patchy, but some an initial pass of fixups couldn't hurt.

[github-actions[bot]]

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]

@dtorop @c-h-r-i-s-t-i-a-n coming back to this - i can't reproduce any longer now!

From my current understanding the issue is likely related to the (now updated) pixelpipe cache code. As we have a module expanded we do (and did) reweigh the cacheline of the module with the inside gtk widget. NOW very likely we have correct data from the cache, with the old cache code - likely not as there were only 5 cachelines and overwritten data was often taking place.

Could you check?

[c-h-r-i-s-t-i-a-n]

Which version? Bug is still there in 4.0.0.

[jenshannoschwalm]

In master from today ...

[c-h-r-i-s-t-i-a-n]

It's still occuring in 4.1.0+338.

[jenshannoschwalm]

Thanks for testing...

[github-actions[bot]]

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]

@c-h-r-i-s-t-i-a-n i checked this after some time - and after finding some very old pixelpipe-cache bug possibly resulting in wrongly read histogram data from the preview pixelpipe.

I can't reproduce any longer, could you please check & confirm the fix and possibly close the issue?

[c-h-r-i-s-t-i-a-n]

Is this in master? If not how can I integrate the patch?

[jenshannoschwalm]

In master

[c-h-r-i-s-t-i-a-n]

Glitch in histogram is still there.

Version tested: 4.1.0+997~g49764adbe

[jenshannoschwalm]

I can't reproduce in top histogram any more...

[Mark-64]

Can't reproduce, Windows 11, DT 4.2, nVidia 1060

[dtorop]

Just looking at this again, after long hiatus. I should have a bit more time and would finally like to modernize/rework the common/histogram.c code, for all the reasons above (it's optimized for obsolete compilers/CPUs and may have alloc bugs on Windows). This could be a follow-up to cleanup of the common colorpicker code (#13216).

Is part of the weirdness because of pixelpipe's handling of histograms and colorpickers? Unless recent work by @jenshannoschwalm changed this, the pixelpipe will update per-module histogram/pickers after processing that module, but if the module output is pulled from the cache, these aren't updated. I dabbled a while back with a fix for this via forcing re-processing of iops which needed histogram output. This added a bunch of special-case code and processing. Wouldn't it be better if the pixelpipe cache included not just the iop's output data but also the associated histogram? (And picker data as well?)

[jenshannoschwalm]

@dtorop there is a pending PR still but that is not related to histogram code.

I am somewhat surprised about this "but if the module output is pulled from the cache". From how i understand the pixelpipe code i would say that a module might possibly get a cached buffer (from a module earlier in the pipeline) as input. That's btw how the important hint for expanded modules works. An expanded module is very likely to change a parameter so it wants it's input to be available. And a heavy-processing module (A) passes a hint to the next module (B) in the pipeline to take it's (B) input as "important" to avoid reprocessing (A). Or do we misunderstand here?

[github-actions[bot]]

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.

[dtorop]

I'm curious if this bug is reproducible now that #13330 is merged, as that PR altered a bunch of the histogram-calculating code?

[c-h-r-i-s-t-i-a-n]

Hi, I can still reproduce it (V 4.3.0+558, build on Feb-10).

[dtorop]

Hi, I can still reproduce it (V 4.3.0+558, build on Feb-10).

Well, being able to reproduce a bug is at least a start to the process of fixing it!

I'm off the table for dt work for a bit (weeks, at least), but it is good that github auto-reminds once the report goes fallow -- that will be about the right interval for me.