Darktable: White balance presets copy/paste keeps refinement

[alcomposer]

Darktable: Latest GIT Master

Issue:

When copy/pasting modules from one image to another, white balance keeps any customisation made to it.

If the white balance module is set to camera it will keep the camera settings of the copied camera white balance.

When copying white balance settings across the white balance module should read the image that it is copied into- when set to camera.

Possibly a way to clarify this behaviour, and allow easy copy/paste - would be to have a lock checkbox in the white balance module, which would stop the camera preset reading the image that it is added into.

A user would then be able to set camera white balance, then press lock to keep that setting persistent between copies. If it was kept unlocked then the camera preset would read the image white balance that it was applied to.

The lock would only apply to camera white balance preset. Possibly either the lock checkbox could be greyed out for other presets, or alternatively an additional preset could be added: camera-locked

[parafin]

Easy workaround is to modify settings - e.g. change tint to some value and back to 1. Real fix would be to always copy-paste exact values - I don't see a use-case for copy-pasting camera auto preset, because it is a default setting anyway.

[alcomposer]

The thing is that it wont be at the default setting when it is copy-pasted into a photograph. It will be whatever the original is set to.

[parafin]

What is the observed behavior and what is the desired behavior? I may have misunderstood you.

[aurelienpierre]

This is due to the fact that white balance is not added in history if default params have not been changed. @TurboGit shouldn't that be fixed by now, as it was for colorin module ?

[aurelienpierre]

What is the observed behavior and what is the desired behavior? I may have misunderstood you.

If the origin white balance has default params (aka use the camera measured WB), copy/pasting history keeps the camera white balance of the destination for the destination. @alcomposer wants to import the camera measured WB from origin and apply the same on destination (useful when you do a series of shots in same lighting conditions, but your camera sets the WB differently between shots).

[parafin]

Then I understood correctly and my first comment makes sense;)

[TurboGit]

@TurboGit shouldn't that be fixed by now, as it was for colorin module ?

No because white-balance is a module that can be deactivated.

[aurelienpierre]

So would it be possible to still add it to history ?

[alcomposer]

I'm starting to think camera should not be in the preset list, but a check box. Read white ballance from camera?

The reason is that it's not a preset. It will always change - but only when selected (in which case it reads the exif data).

So if read white balance is checked, then preset list would be greyed out. If it were unchecked, then module would read whatever is the currently loaded settings.

If read white balance is toggled (switched on and off) if would read the white balance from camera- set the module to that - then not do any further checks for white balance when copied into another image.

Possibly a tool tip could let the user know to keep read white balance checked if they would like the module to read the white balance of images that it is copied into.

Obviously this is only an idea. I'm just flagging this as an area of improvement.

[aurelienpierre]

No because white-balance is a module that can be deactivated.

Yes it can (for a couple of versions back).

[TurboGit]

Yes it can (for a couple of versions back).

Sorry Aurélien, not sure to get it :) whitebalance is a module that can be deactivated by the user (it is not always on - even if it should I suppose). That's has been this way all the way back since I've been using dt. So, what was your comment about ? Seems like I miss something....

[TurboGit]

Anyway to have the whitebalance in the history even if it has the default params we will need to special case it. I'll look at this.

[parafin]

Hmm, what else do we skip when creating history stack?

[parafin]

So I guess instead of adding just always enabled modules, we need to add all enabled by default iops. So this check here:
https://github.com/darktable-org/darktable/blob/f70a03ab371ca0c2069901cf3208c6cc9d009088/src/develop/develop.c#L1332
should be changed from module->enabled == 1 && module->hide_enable_button == 1 to module->default_enabled == 1

[TurboGit]

Thanks @parafin! In fact it was a bit more complex as the temperature (and also module highlights reconstruction) was only enabled on some file kind (non monochrome RAW).

Anyway, I think with my last commit we should have now all active modules in the history and this issue should be fixed.

[parafin]

Hard-coding like that looks bad... Are you saying that at that point default_enabled isn't yet set according to file type? Meaning reload_defaults() hasn't been called yet? That sounds wrong, if settings weren't yet updated for this specific image, what are we saving to the history then?

[TurboGit]

Hard-coding like that looks bad...

Agreed!

Are you saying that at that point default_enabled isn't yet set according to file type?

Exactly.

if settings weren't yet updated for this specific image, what are we saving to the history then?

Don't know.

[TurboGit]

I suppose that this is because this is happening at the loading time. And we fail to load default if a given module is not yet found on the stack. It is done very early.... I'll try something...

[TurboGit]

In fact we don't want to check for module->enabled but module->default_enabled. Doing that all seems fine.

[TurboGit]

@parafin : thanks for the comments, this put me on the right track and I suppose my last change is clean.

[parafin]

If you notice, this is exactly what I proposed originally;) Glad that it works, I haven't tested it myself.

[TurboGit]

Stupid me :( I didn't read the comment correctly!!! Quite a lost of time for nothing!