-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
po/exposure next - exposure mapping + new collapsible section #11142
Conversation
when changing the lightness slider value while the color picker is active, update the computed exposure interactively without needing to push the button again.
Fix static naming for consistency.
| // get the saved params | ||
| dt_iop_gui_enter_critical_section(self); | ||
|
|
||
| float lightness = 50.f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'm missing something, but this does break the reset of the module. That is, even when clicking on reset on the module the lightness spot is not reset to 50%. Is that expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reset option applies only on module's params. These are not params but read/written from/to darktablerc, so the reset option does nothing here. The only way to reset is double click on sliders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed ! Thanks.
|
|
||
| float lightness = 50.f; | ||
| if(dt_conf_key_exists("darkroom/modules/exposure/lightness")) | ||
| lightness = dt_conf_get_float("darkroom/modules/exposure/lightness"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TurboGit @aurelienpierre Looking at this mechanically while rebasing #11257 and wondering why this isn't using darktableconfig.xml.in and setting the default there?
EDIT: same for similar patterns in channelmixerrgb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We keep the latest value set by the user, which is another option. The 50.f above could be the default (and it is) but this is used only the first time the module is run, so not very important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. Is this different from many other variables? The dt_conf infrastructure can deal with defaults and can be used to document the meaning of variables in darktablerc (which now, for these cases, contains mystery variables). I like consistency and copy/paste will always increase entropy, but as you say, it is not very important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, I missed the point, here you go #11279
Moved Aurélien branch to the new collapsible section support.