Project

General

Profile

Bug #11497

Center view temporarily shows inconsistent results

Added by Anonymous over 2 years ago. Updated about 2 years ago.

Status:
Fixed
Priority:
Low
Category:
Darkroom
Target version:
Start date:
02/02/2017
Due date:
% Done:

100%

Affected Version:
2.2.3
System:
Ubuntu
bitness:
64-bit
hardware architecture:
amd64/x86

Description

Please see the attachments and open the image in the darkroom.

1. Observe that there is strong vignetting (due to the lens vignetting being amplified by the levels module).
2. Turn on the lens correction module. Observe that the vignetting vanishes.
3. BUG: Turn on the denoise (profiled) module. Observe that the vignetting comes back, even though lens correction is still turned on.
4. BUG: Turn off the lens correction module. Observe that the vignetting increases substantially, far more than it was originally.
5. Turn on the lens correction module. Observe that the vignetting again vanishes completely.
6. BUG: Switch to the lighttable. Observe that the vignetting is back.
7. Export the image. Observe that the vignetting is present in the exported image.
8. Switch to the darkroom. Observe that the vignetting vanishes.
9. BUG: Export from the darkroom (Ctrl-E). Observe that the vignetting is present in the exported image, even though it is not visible in the darkroom.

Let me know if you need anything else.

P1260731.ORF.xmp (4.37 KB) Anonymous, 02/02/2017 11:52 PM

P1260731.ORF (13.4 MB) Anonymous, 02/02/2017 11:52 PM

out-12.ogv (10.7 MB) Anonymous, 02/09/2017 10:38 PM

Associated revisions

Revision 416f2ff7
Added by Ulrich Pegelow over 2 years ago

levels: proof of concept for handling case of bug #11497

Revision b2b21796
Added by Ulrich Pegelow over 2 years ago

Merge pull request #1441 from upegelow/auto_levels

add option to synchronize between preview and full pixelpipe;
affects levels, colorreconstruction and globaltonemap modules;
fixes bug #11497

History

#1 Updated by Ulrich Pegelow over 2 years ago

  • Status changed from New to Confirmed
  • Assignee set to Roman Lebedev
  • % Done changed from 0 to 10

I can confirm that there is an issue but to me it appears to be different from what you describe.

The basis of your issue is having the levels module active in "automatic" mode. Without that module/mode everything is fully predictive.

This changes if the levels module is activated in "automatic" mode:

  1. enable lens correction --> vignetting has gone
  2. zoom into image a bit, then zoom out again --> vignetting is partially back

The same happens if you force a recalculation of the pixelpipe by activating any other module (e.g. profiled denoise).

According to my understanding the problem is that the behavior under #1 is wrong. The image should already get recalculated with the according automatic level corrections applied. Instead this recalculation only happens on further action.

Putting Roman in copy as he has developed the automatic mode and knows best how it's supposed to behave.

#2 Updated by Roman Lebedev over 2 years ago

  • % Done changed from 10 to 20
  • Status changed from Confirmed to Incomplete

Ulrich Pegelow wrote:

I can confirm that there is an issue but to me it appears to be different from what you describe.

The basis of your issue is having the levels module active in "automatic" mode. Without that module/mode everything is fully predictive.

This changes if the levels module is activated in "automatic" mode:

  1. enable lens correction --> vignetting has gone
  2. zoom into image a bit, then zoom out again --> vignetting is partially back

The same happens if you force a recalculation of the pixelpipe by activating any other module (e.g. profiled denoise).

According to my understanding the problem is that the behavior under #1 is wrong. The image should already get recalculated with the according automatic level corrections applied. Instead this recalculation only happens on further action.

Putting Roman in copy as he has developed the automatic mode and knows best how it's supposed to behave.

I'm not seeing any unusual behavior here.
Are you testing with opencl? Please try without opencl.

I do see vignetting when both lens correction and levels are on. But with levels set to such extreme values (i.e. black - 34%), i'd say the vignetting is simply made more visible, and is there because the profile undercorrects it.

#3 Updated by Anonymous over 2 years ago

This is very strange indeed - opencl seems to cause the undesired behaviour, but in addition to that, with opencl enabled, the lens correction will completely remove the vignetting, whereas some vignetting will still be present if opencl is disabled.

Please see the attached video where I demonstrate the problem. First with opencl disabled, and then with opencl enabled. (Disregard the tearing where the blue/red desktop background shows through, that's due to the desktop recording program.)

#4 Updated by Roman Lebedev over 2 years ago

  • Assignee deleted (Roman Lebedev)

No opencl here => nothing i can no.

#5 Updated by Roman Lebedev over 2 years ago

BTW that implies there are at least two bugs, one is lens.c vignetting kernel incorrectness, and two, some generic cache invalidation issue in opencl codepath.

#6 Updated by Ulrich Pegelow over 2 years ago

  • Assignee set to Ulrich Pegelow
  • Status changed from Incomplete to Triaged

Confirmed. It's opencl related. Will further investigate.

#7 Updated by Ulrich Pegelow over 2 years ago

OK, some further results. I'm finally sure that this is an issue in the levels module which silently assumes that the preview pixelpipe is running faster than the full pixelpipe. This is typically the case for an non-OpenCL system, but not generally true, especially not if darktable is OpenCL driven.

Some details: in automatic mode the module requires histogram data to calculate the needed adjustments, function commit_params_late() and dt_iop_levels_compute_levels_automatic(). Histogram data are produced in the preview pixelpipe only but needed in the full pixelpipe as well. If the history stack changes (e.g. activating the lens correction module) both pixelpipes need to be reprocessed. If the preview runs slower than the full pixelpipe, current histogram data are not available in the full pixelpipe when they are needed and the levels module fails to process the image correctly.

I typically run the full pixelpipe on my fast GPU and the preview pipe either on CPU or on my slower secondary GPU and can preproduce the issue. It's already visible with the naked eye that the center view (full pixelpipe) appears always slightly earlier than the preview. If I change the allocation of devices to give the preview pixelpipe my faster GPU, preview is quicker than the full view and the problem disappears, but that's certainly not a solution.

BTW I don't think that there are relevant issues with lens correction differences between OpenCL and CPU. All what is described here is only visible with the levels module in automatic mode. Obviously the lens profile does not correct 100% for vignetting. If boosted strongly by the levels module the remaining vignetting gets amplified and is visible. You can reach the same look with levels in manual mode by shifting the black level accordingly. As to be expected the problems do not appear at all in this case, OpenCL and CPU give the same result.

#8 Updated by Roman Lebedev over 2 years ago

Does colorreconstruction, ashift handle such case properly?
I guess the fix will be the same

#9 Updated by Ulrich Pegelow over 2 years ago

ashift should be safe as it uses DT_SIGNAL_DEVELOP_PREVIEW_PIPE_FINISHED to synchronize between the two pixelpipes. colorreconstruction might be affected by the same issue (though I never noticed a problem) - this is t.b.c.

#10 Updated by Ulrich Pegelow over 2 years ago

Just checked: colorreconstruction is affected by the same problem. It's a bit less obvious because the module only relies on preview pixelpipe if the full pixelpipe is zoomed in. To reproduce the issue one needs to zoom in a bit and then e.g. switch on/off the whitebalance module with extreme settings on a OpenCL driven system.

#11 Updated by Ulrich Pegelow over 2 years ago

  • Subject changed from Turning on denoise messes with lens correction to Center view temporarily shows inconsistent results

Title changed to better reflect the underlying issue.

#12 Updated by Ulrich Pegelow over 2 years ago

A few further thoughts.

I don't think that DT_SIGNAL_DEVELOP_PREVIEW_PIPE_FINISHED is a general solution to this issue as that signal is only emitted when the preview pixelpipe is finished. Firstly, we can't wait in the full pixelpipe for the signal as a general option. It's possible in ashift when waiting is related to user interaction (pressing a button), but it would be a bad idea in modules like levels where it would effectively slow down the whole process. Secondly, that signal doesn't tell us if the preview pipe has been processed with the most recent parameter set after a history stack change rather than the previous one.

A more general solution would involve the following:

  1. we would need a means to query the preview pixelpipe for its status. This involves telling us if a certain module has already been processed and if this had happened with the right history stack (i.e. the same that the full pixelpipe is seeing).
  2. we would then need an option for a module to take appropriate action if the preview pixelpipe data is not (yet) up to date. In it's simplest form a module like levels could just emit an indication to the user that the image is "inconsistent" and the user could then decide if its worth to reprocess the center view like with pressing some shortkey. Other synchronization options like waiting for the missing result are possible but more complicated (risk of dead-locks etc.).

For (1) I could imagine to extend a feature that I once implemented to support ashift (but finally did not make use of). We have a set of functions in develop.c to calculate hash values for a pixelpipe. The basis are individual hash values of each module (taking params, blending and masks into account). Those individual hash values are combined consecutively for all active modules in the pixelpipe.

We could store the hash value calculated this way as we process the pixelpipe into pipe->hash. For a given parameter set this hash value is fully predictive. If we know the parameter set pipe->hash tells us what module has just been processed, i.e. where we are in the pixelpipe. If pipe->hash does not fit to our prediction (is not found among the possible values that is should have) this means that the parameter set has changed.

Finally we should also not completely rule out the option to leave everything as it is. In the end wrong results are only visible temporarily. It's annoying but normally already the next user action will bring back the right result.

RFC

#13 Updated by Ulrich Pegelow over 2 years ago

  • Status changed from Triaged to In Progress
  • % Done changed from 20 to 50

A first working solution can be found in PR1441.

#14 Updated by Ulrich Pegelow over 2 years ago

  • Status changed from In Progress to Fixed
  • % Done changed from 50 to 100

Fixed in master with merge of PR1441

#15 Updated by Roman Lebedev about 2 years ago

  • Target version set to 2.4.0

Also available in: Atom PDF