When switching processing profiles, missing values should be taken from Neutral

[Beep6581]

Originally reported on Google Code with ID 1751

When switching processing profiles, RT replaces values of the current profile with those
from the new one. I think RT should never do that unless the user explicitly chooses
to do a partial load/paste. If he tries to open a profile with missing values without
explicitly doing a partial-*, then RT should load values from Neutral.

If the user wants to keep existing values, there is a "partial load" feature.

Reported by entertheyoni on 2013-03-04 01:45:28

[Beep6581]

If profile is partial, why auto-apply Neutral before it? This should be upto the user:)
We can have many partial profiles and thier effcts could be acculumalted.
For example, I use partial profiles with specific CA adjustments, then add tonal adjustment
profiles, etc. It is much faster to load them via existing dropdown rather than partial
load dialog.

Using partial load for patial profiles will be a pain, as one has to exactly recall
which particlar tools does the profile engage.
I thimk what you are missing is a clarification in tge workflow step-reset to neutral,
when necessary.
Actually, we probably could add a shortcut for that:)

Reported by michaelezra000 on 2013-03-04 02:36:13

[Beep6581]

You are very right, it should be up to the user! That's why the user can select "load
partial profile". If he doesn't select that, the assumption is that the profile he's
loading is *not* partial. If I open a profile, I don't know whether it's partial or
not. I'm certainly not going to open it in a text editor just to check, and I can't
tell by file size either. Last thing I want is to have to go through all the settings
looking for some slider it failed to reset.

I agree with your apprehension, so the correct solution would be for the Partial Load
window to scan the profile and disable those checkboxes not in the file. Then it would
be immediately clear.

Reported by entertheyoni on 2013-03-04 03:13:23

[Beep6581]

Or.. we could add a shift-load profile to auto-apply neutral for the missing values

Reported by michaelezra000 on 2013-03-04 03:26:56

[Beep6581]

I really think that auto-applying neutral by default will negatively impact fluidity
of applying any profile that is a partial one... With your proposal partial application
will be possible only via load (partial) profile path - quite a few more steps. It
is so much smoother that profile applies only what is in it - and nothing else. 
E.g. I have my own version of the neutral profile, with Prophoto color space. It would
become a real burden to always have to reset it back!:)

But we seem to have different preferences to editing, and they can be resolved:
1. I think we both agree on load partial profile dialog to be reflective of the profile
content.
2. It seems that adding a shortcut to auto-apply Neutral values to the omitted parameters
would satisfy both of our preferences.

Reported by michaelezra000 on 2013-03-04 04:32:11

[Beep6581]

Both points sound good!

Reported by entertheyoni on 2013-03-04 12:55:13

[Beep6581]

Re #4:

About 1.: this is complicated to code because some tools are entirely applied with
one checkbox, but the tool's parameter list itself can be partial because hand edited.
I suggest not to bother with that.

About 2: this could be done more easily i guess. But what should be the behavior: we
can't be sure that the Neutral profile has been read if the user has chosen not to
use the bundled profiles. The only remaining options is (1) use the internal default
values, or (2) use the selected raw or std "default" profile.

Reported by natureh.510 on 2013-03-09 02:29:08

[Beep6581]

No answer. I'll choose (1) then which will be easier to code.

Reported by natureh.510 on 2013-03-11 22:08:05

[Beep6581]

Hombre, could you have a look at Issue 1740, which is also about applying tool's settings.
I don't get ahead to it :(

Ingo

Reported by heckflosse@i-weyrich.de on 2013-03-11 22:29:48

[Beep6581]

:) yes 2.1 is good (use the internal default values) 

Reported by michaelezra000 on 2013-03-11 23:13:38

[Beep6581]

@ #6
If I don't "Load Partial Profile", then I'm loading the full profile (or that's my
intention). I can't know whether there are some values missing from the PP3 (omission
by choice, by accident, or by RT having more sliders now than when the PP3 was created).
The only sane behavior in my opinion is for the missing values to be set at neutral/0/off
settings (or whatever value the neutral setting would be, e.g. with Shadow Recovery
the neutral value is 50).
If my intention is to only load parts of a profile, I should use the Load Partial Profile
button. As agreed to in comment #4 point 1, it would be very helpful if the Partial
Load dialog would gray out checkboxes of tools which aren't present in the profile
being opened - this will make loading a partial profile immediately indicative of what's
in it without having to resort to notepad.

There remains the question of how incomplete tool sections should be handled when the
user clicks Load Partial Profile, e.g.
  [Sharpening]
  Enabled=true
I think it would make most sense to only load these incomplete values without neutralizing-out
the missing ones for that tool, since after all we are Loading Partial Profile.

In other words,
- when using Load Profile, replace parameters of image with those in the PP3 and neutralize
everything that's missing,
- when using Load Partial Profile, gray-out checkboxes of missing [tool] sections in
the PP3, and replace (even incomplete) parameters of image only with those selected
by the checkboxes.

Reported by entertheyoni on 2013-03-12 00:02:23

[Beep6581]

Unfortunately, i didn't managed to get the modifier key's state when selecting a new
entry in the combobox :(, so the last option i can think of is an additional toggle
button (the "reset" image).

If toggled on, all partial profile would be filled to default values first, would it
be by selecting a new entry of the combobox, loading a partial profile or partially
load a perhaps partial profile.

Do you agree on this solution?

About reflecting the content of the profile, i think that the partial profile dialog
window should be completely rewritted to show a collapsable tree, likewise to the one
of the Preferences/Batch list. The we would have the same granularity than the content
of the pp3 profile, and we could more easily reflect the content of the pp3 file.

So:
1. do you still want to fix this issue as "low" priority?
2. do you want to split it in 2 different issues with different priorities?

Reported by natureh.510 on 2013-03-15 15:17:22

• Labels added: Type-Enhancement
• Labels removed: Type-Defect

[Beep6581]

Hombre, I like this more than a (hidden) modifier key. A new thin toggle button can
be placed right next to the profile drop-down.

I think reflecting the partial profile contents can be a separate issue.
I more important one would be to change profile dropdown so that it could drill down
to sub-directories with more profiles, all organized in a nice hierarchical structure.
 This is also a separate issue of course!:D

Reported by michaelezra000 on 2013-03-15 15:56:08

[Beep6581]

Sorry, "would it be by selecting" I don't understand that sentence.

Partial Profile tree - its a very good idea, but a separate issue. We should discuss
it in another issue.

Reported by entertheyoni on 2013-03-15 22:28:12

[Beep6581]

i.e. it would be usable for all the enumerated case.

Reported by natureh.510 on 2013-03-15 22:30:00

[Beep6581]

Sounds good! Maybe its more descriptive to call it "Merge", and turning Merge off replaces
missing values with default (off) ones?

Reported by entertheyoni on 2013-03-15 22:36:28

[Beep6581]

Here is a patch that adds a button next to the combobox. It will only affect the profiles
from the combo, not the copy/paste buttons.

Since there are new images, you'll have to re-cmake your project to include them in
the installed files, or copy them manually.

Please note that the values are taken from the internal default values, not the Neutral
profiles. They should be in synch, but that may not be the case.

Reported by natureh.510 on 2013-04-06 23:01:28

• Status changed: PatchSubmitted

---

• Attachment: [1751-Filled Profiles.patch](https://storage.googleapis.com/google-code-attachments/rawtherapee/issue-1751/comment-16/1751-Filled Profiles.patch)

[Beep6581]

This is fantastic Hombre!

Reported by entertheyoni on 2013-04-06 23:23:55

[Beep6581]

Hombre, great job!:)
A couple suggestions: 
1. in profilepanel void modeToggled(); has a very generic name. Just to make sure it
is future proof, as we could come up with various other "modes" (E.g. profile browser
mode, etc) could this be renamed into profileFillModeToggled();

2. History shows identical entry for application of profile, regardless of the state
of the new toggle button. If profile is filled, it may be more informative to indicate
it somehow. One option I could come up with - add a prefix "(*)" to the profile name
in history.

Reported by michaelezra000 on 2013-04-07 00:25:50

[Beep6581]

Here is the patch with the requested modifications. Can i commit?

Reported by natureh.510 on 2013-04-07 21:59:02

---

• Attachment: [1751-Filled Profiles02.patch](https://storage.googleapis.com/google-code-attachments/rawtherapee/issue-1751/comment-19/1751-Filled Profiles02.patch)

[Beep6581]

Already compiling to test it:)

Reported by michaelezra000 on 2013-04-07 22:02:26

[Beep6581]

Don't be surprised, I've chosen the "+" char and not "*", i think it's more self explaining.

Reported by natureh.510 on 2013-04-07 22:04:10

[Beep6581]

This works as well. Suggestion for tooltip:

PROFILEPANEL_MODE_TIP;Button pressed: partial profiles are converted to full profiles;
the missing values are replaced with the defaults\n\nButton released: the profiles
will be applied unchanged, altering only those tools which are saved in the profiles.

If you don't mind, for consistency, could you find/replace
toggledOnImage  -> profileFillModeOnImage
toggledOffImage -> profileFillModeOffImage
or something like that

Everything seems to work, so ok to commit.

Reported by michaelezra000 on 2013-04-07 22:20:08

[Beep6581]

"Defaults" might confuse someone to think they're the values from Default.pp3. Let's
use the unambiguous term we use in the manual, "hard-coded defaults". Secondly, if
we use "values" in the first line we should also use "values" in the second. The same
goes for the verbs tense - "will be" or "are".

PROFILEPANEL_MODE_TIP;Button pressed: partial profiles will be converted to full profiles;
the missing values will be replaced with hard-coded defaults.\n\nButton released: profiles
will be applied as they are, altering only those values which they contain.

1751-Filled Profiles02.patch works fine here.

Reported by entertheyoni on 2013-04-07 23:28:24

[Beep6581]

Are everybodies ok with #23? Give me the final go plz.

Reported by natureh.510 on 2013-04-08 07:41:00

[Beep6581]

Go!:)

Reported by michaelezra000 on 2013-04-08 10:38:22

[Beep6581]

Committed to Default.

Reported by natureh.510 on 2013-04-08 20:26:59

• Status changed: FixedPendingConfirmation

[Beep6581]

Thanks, Hombre, this is a very useful patch

Reported by michaelezra000 on 2013-04-08 22:16:46

[Beep6581]

Reported by natureh.510 on 2013-04-11 12:51:06

• Status changed: Fixed

[Beep6581]

I think that tooltip of the button should have a title of the button, so it could be
referred to in the manual, etc. Something like this:

 Profile Fill Mode\n\n(and the rest of the tooltip)

Reported by michaelezra000 on 2013-04-12 03:04:53

[Beep6581]

Good idea! I quickly added it, before tagging.

Reported by entertheyoni on 2013-04-12 20:30:46

[Beep6581]

DrSlony - before tagging, let me commit a profile or 2:)
I will be on IRC

Reported by michaelezra000 on 2013-04-12 20:45:11

[Beep6581]

Here is a patch to use the FillMode button with the Load button too.

Reported by natureh.510 on 2013-04-12 22:31:28

• Status changed: PatchSubmitted

[Beep6581]

This one will work flawlessly, even with the PartialPaste dialog box.

Reported by natureh.510 on 2013-04-12 22:48:10

---

• Attachment: [1751-Filled Profiles04.patch](https://storage.googleapis.com/google-code-attachments/rawtherapee/issue-1751/comment-33/1751-Filled Profiles04.patch)

[Beep6581]

Hombre it works flawlessly. :]
Thank you!

Reported by entertheyoni on 2013-04-12 23:38:08

[Beep6581]

When committing, should i add the language string modifications as requested in comment
#29?

Reported by natureh.510 on 2013-04-13 09:29:04

[Beep6581]

I guess so, I did not see DrSlony committing this in prior changesets

Reported by michaelezra000 on 2013-04-13 18:54:37

[Beep6581]

In fact he did it, just saw that now. I'll update the French strings then and close
this issue.

Reported by natureh.510 on 2013-04-13 21:06:21

[Beep6581]

Committed to default.

Reported by natureh.510 on 2013-04-14 00:08:09

• Status changed: Fixed