Preset names lost when applying style

[kofa73]

Describe the bug

I have a preset that contains 2 instances of diffuse or sharpen, both built-in ones:

• sharpen demosaicing: AA filter
• local contrast

Previously, when I applied the style, the two instances were shown accordingly. Now, they are shown as:

The style contains:

        <plugin>
            <num>17</num>
            <module>2</module>
            <operation>diffuse</operation>
            <op_params>
                0100000000000000080000000000803f000000000000803f0000803f0000803f0000803f00000000000080be000080be000080be000080be00000000
            </op_params>
            <enabled>1</enabled>
            <blendop_params>gz09eJxjYGBgYAFiCQYYOOHEgAZY0QVwggZ7CB6pfOygYtaVAyCMi48L/AcCEA0AmawnoA==</blendop_params>
            <blendop_version>12</blendop_version>
            <multi_priority>1</multi_priority>
            <multi_name>sharpen demosaicing: AA filter</multi_name>
            <multi_name_hand_edited>0</multi_name_hand_edited>
        </plugin>
        <plugin>
            <num>16</num>
            <module>2</module>
            <operation>diffuse</operation>
            <op_params>
                0a00000000000000800100000000803f0000803f000020c00000000000000000000020c000000000000000bf0000000000000000000000bf00020000
            </op_params>
            <enabled>1</enabled>
            <blendop_params>gz09eJxjYGBgYAFiCQYYOOHEgAZY0QVwggZ7CB6pfOygYtaVAyCMi48L/AcCEA0AmawnoA==</blendop_params>
            <blendop_version>12</blendop_version>
            <multi_priority>0</multi_priority>
            <multi_name>local contrast</multi_name>
            <multi_name_hand_edited>0</multi_name_hand_edited>
        </plugin>

Steps to reproduce

Apply the attached style to a photo.

Expected behavior

Display the module instance names

Logfile | Screenshot | Screencast

No response

Commit

No response

Where did you obtain darktable from?

self compiled

darktable version

4.5.0+1391~g0a39901aa5-dirty (because of the tone-eq experiment from #15743 (comment))

What OS are you using?

Linux

What is the version of your OS?

Ubuntu 23.10

Describe your system?

No response

Are you using OpenCL GPU in darktable?

Yes

If yes, what is the GPU card and driver?

not relevant

Please provide additional context if applicable. You can attach files too, but might need to rename to .txt or .zip

000-sigmoid standard.dtstyle.txt

[TurboGit]

Most probably because the name is not hand-edited (0 in <multi_name_hand_edited>). Then one applied the name is computed based on the current params & blend-params. Since the later has been changed recently there is no match and then the name get created based on the standard naming.

[kofa73]

Indeed, blendop_version changed from 12 to 13, and blendop_params also changed, but there is actually no mask of any kind applied (I guess blendop_params describes exactly that, using a new param set), so this is rather annoying/confusing. op_params did not change.

[TurboGit]

Right, confusing but that's the way presets are matched (i.e. checking iop_params and blend_params). The only way to get stable naming is to hand-edit the name, so in your case create your own presets an create the style based on it. This was discussed and we don't want to be using the preset name if hand-edited is not set, it was too disturbing for some people.

[kofa73]

The style includes the preset name. So if there is no match by iop_params and blend_params, maybe fall back to the stored name, even if multi_name_hand_edited is not set?

Otherwise, there is no point in storing multi_name in the style, unless multi_name_hand_edited = 1 (since it will never be used). If multi_name were only stored when hand-edited, multi_name_hand_edited could be removed, as multi_name != null would imply hand_edited.

[TurboGit]

The style includes the preset name. So if there is no match by iop_params and blend_params, maybe fall back to the stored name, even if multi_name_hand_edited is not set?

No that was the scenario that was discussed and discarded as disturbing.

Otherwise, there is no point in storing multi_name in the style

There is a point, if the params & blend_params matches then the multi-name will be used. At the time the preset was created it was true. In the case at hand the blend_params has changed, so the actual style entry does not match the stored preset anymore. You can refresh your style if you want to see the stored multi-name again.

[kofa73]

I'll try to summarise the current situation, which is very complicated and, I think, contradictory. I'll get back later, probably not today.

[kofa73]

OK, here is what I have found. I think it is no wonder people are confused.

that's the way presets are matched (i.e. checking iop_params and blend_params)

One can create a preset from the current state of the module.
If the name of the module instance has been set manually, the name is stored inside the preset.

When applying a preset, the module instance it is applied to may or may not have a manually edited name.

• If neither the module, nor the module instance from which the preset was made have a manually edited name, the name of the preset will be used as the (default) name of the module instance.
• If the module has no manually set name, but the module instance on which the preset was based had one, the manually set name of the original module instance, stored inside the preset, is used as the name of the module instance; the name will be marked as manually set on the module instance.
• If the module instance already has a manually set name (which may have been entered manually, or it may have been applied e.g. via a previous application of of a preset (see above)), the name is kept, ignoring both the name of the preset, and any manually edited instance name stored in the preset.

In short, once a manually set name is applied, it is cast in stone, not even resetting the module can remove it. One can delete the manually set name, though, to remove it.

It is also possible to simply edit the settings of a module so that they happen to match a preset. If and only if the module instance has no manually set name, and the current module params match those stored in a preset:

• if that preset contains a manually edited module instance name, that manually edited name is displayed as the name of the module instance, but said instance name is not set as a manually set name on the instance;
• if the preset does not contain a manually saved instance name, the name of the preset is displayed
  Modifying the params will remove the instance name that was determined based on the module instance.

Now, one can also create a style. In the style, one can save a module, whose settings may or may not match one (or more) of the presets; the preset itself may contain a manually set module instance name, and the module instance may have a manually set name.

• If the module instance stored in the style had no automatically assigned (based on applying a preset) or manually edited name when the style was created, the stlye will not contain a name for the module, and when applying the style, the module instance will not be assigned a name, unless it matches one of the presets. (<multi_name></multi_name><multi_name_hand_edited>0</multi_name_hand_edited> in the syle.)
• If the module instance stored in the style had an automatically assigned name (based on applying a preset, the latter containing no manually edited name), the name of the module (= name of the preset) is recorded in the style: <multi_name>name of preset</multi_name><multi_name_hand_edited>0</multi_name_hand_edited>). When the style is applied, the module's name will be assigned based not on the name, but on matching the parameters of an existing preset. If the preset's parameters have changed, or the preset has been removed, no name will be assigned to the module, even though one is stored in the style. If the preset has been renamed (or a new preset with matching params exists), the name of that preset will be used (if multiple presets match, there is additional logic to select the 'first' of them).
• If the module instance stored in the style had a manually edited name (<multi_name>manually edited instance name</multi_name><multi_name_hand_edited>1</multi_name_hand_edited> in the style), that name will be assigned to the module instance. However, if a preset with matching parameters AND a manually assigned instance name stored in the preset exists, the <multi_name> stored in the style is ignored, and the manually edited name in the preset is used instead.

[elstoc]

I think the word you're looking for is incomprehensible.

[TurboGit]

Well @elstoc let's be honest, part of the complexity has been added because the initial behavior was not ok to you.

Anyway, @kofa73 thanks for this summary. I'll read it. If we agree on the current situation we'll need to put a proper solution on the table, for all of us.

[elstoc]

part of the complexity has been added because the initial behavior was not ok to you

If you say so. I did offer a few suggestions to make it less confusing, but IMO it was just getting worse as the discussion continued, so I gave up in the knowledge that I could switch it off. I still haven't documented it in the manual yet though because it's hard to describe beyond a simple sentence "the module label might get automatically updated to something unless you disable the preference".

It's not even possible to examine the properties of the preset to work out what will happen, since some of the behaviour is dependent on hidden db fields.

[TurboGit]

Let's start step-by-step and number the paragraph for easier reference.

If neither the module, nor the module instance from which the preset was made have a manually edited name, the name of the preset will be used as the (default) name of the module instance.

This is good, right?

If the module has no manually set name, but the module instance on which the preset was based had one, the manually set name of the original module instance, stored inside the preset, is used as the name of the module instance; the name will be marked as manually set on the module instance.

This is also good, right?

[TurboGit]

If the module instance already has a manually set name (which may have been entered manually, or it may have been applied e.g. via a previous application of of a preset (see above)), the name is kept, ignoring both the name of the preset, and any manually edited instance name stored in the preset.

That could be seen as good or not. A module can be named not according to what it does but the part of the image it act on. For example sky, or eye. If you now take a more generic preset name like +2EV or Strong blue, and overwrite the name of the module this won't be good either.

That's the explanation, if a module has been named explicitly we don't overwrite it.

What's your take on this? Ok? Not ok?

[elstoc]

My opinion remains that the autonaming should be entirely controllable from within the preset dialog and then any behaviour can be understood just by looking at the preset.

1. Have a tickbox "auto-set module label" - this is automatically ticked for new and built-in presets
2. Have a new field (it already exists in the db - expose it) "module label" - similarly this is always populated with the preset name for built-in presets
3. Have a final tickbox "unset module label if preset no longer matches" for when you've used a preset but the parameters no longer match - probably should be ticked for built-ins

Any manually added label always takes precedence.

[TurboGit]

I'd prefer to concentrate first on the problem than proposing solutions. The problem may be clear to you but not to me. That's why I'm trying to get OK or NOT OK on the statement listed by @kofa73. When we'll have a clear idea of what is wrong or annoying then we will try to find a solution. Having new ticks and field is maybe not the best solution GUI wise.

[elstoc]

The problem for me is that there's nowhere on the UI that I can find out what will happen when I add a preset, and nowhere that I can set/change it on a per-preset basis. That makes it hard if not impossible to document what will happen as it may well depend on something I did inadvertently (changed the label) when creating the preset. The only way to fix that is to drop the preset and create a new one (or manually edit the db).

I guess this issue impacts all three points

1. I'm happy that applying a preset to an instance that has already been manually renamed, will never alter the name. I don't like "[if the] module instance on which the preset was based had [a manually created name], the manually set name of the original module instance [is set to any instance on which the preset is subsequently applied]" because this is dependent on a hidden field that I can neither see nor update, and I might not have intended that behaviour when I created the preset. Having to drop and recreate a preset to fix this is a pain.

2. Same issues as point 1. Again I don't have an issue with auto-setting the module name based on the instance against which it was created but I'd rather see this in the UI and be able to set/update it manually.

3. If I'm understanding this point I think I'm fine with this -- any manually edited instance never has its label updated if it's been manually set

The reason I jump to the solution is that it's easier to get my head round than trying to describe the issues with the current solution. I hope this has clarified things.

From an implementation point-of-view, I'd say that the cleanest thing is to only store manually edited names against the module instance, otherwise (if the manual name is empty) look up the name that's stored against the preset (and in my suggested implementation that will only ever come from a single user-exposed field, that may or may not be the same as the preset name).

Having new ticks and field is maybe not the best solution GUI wise.

Please don't close off this option without discussion. You may not like making the dialog bigger but the alternative is confusing functionality, and IMO this is much worse.

Edit: Perhaps my second suggested tick box is unnecessary

[elstoc]

If we're really looking for a clean UI I can't really see much point to the "only show this preset" tickbox or the "description" field so we could remove them?

[kofa73]

I think hand-edited names should not be saved to presets:

• it leads to confusion ('why did applying this preset assign a name different from the name of the preset?' -- the end-user has no way to 'look inside the preset' to figure out the instance name stored inside);
• sometimes I assign module instance names in the context of the current image, but find that the settings could be useful for many images, so I create a preset, and assign it a generally meaningful name. However, currently the hand edited name, based on the image context is stored and then restored when the preset is applied to other images.

[kofa73]

Sorry about the slow response.

I consider 1 as 'good', 2 as 'bad' (see my post above regarding contexts), 3 as 'bad' (but I see your point, so I'm no longer so convinced).

I think there's no universal solution, and thorough documentation with an explanation (like yours for 3).

[EC1000]

For me, point 2 is good: it is particularly useful for auto-apply presets (that is the only way to define a manually set name that will stay consistent between images, to later copy-paste parameters easily), see initial discussion in #13982.

I agree that the way to edit the label that will be save is:
a) not obvious to find (logical, but not obvious),
b) somehow cumbersome if you want to save a preset without it (remove label, save preset, rewrite label) or edit a preset to remove it (apply preset, modify label, save preset).
I am ok with the way it works, since I know how to proceed and I understand the motivation for not adding a new field, but I also understand it can generate misunderstandings.