Skip to content
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

Add more widgets to QAP by default #16656

Merged
merged 9 commits into from
Apr 27, 2024

Conversation

ralfbrown
Copy link
Collaborator

And before everyone jumps on me for making the panel require scrolling, I'm aware of that but think it's warranted to get all of the main editing functions that new users might end up using into one place. It's probably also better to err a little on the side of adding too many (e.g. having both Filmic and Sigmoid by default) since it's easier to remove widgets than to add them -- plus most users of any software never change defaults. The scrolling issue could be mitigated by switching the default preference to scrolling the panel instead of adjusting sliders; in that case we should consider adding a hint about using Alt-Scroll to the slider tooltips.

I've separated the additions into a bunch of small commits to permit easy cherrypicking. If all of them are included, we get full color and tonal control (including global and local contrast) as well as denoising control right from the panel.

Two other things I would like to add: binding the Escape key to activating the QAP (to return there after linking out to a full module), and adding the preset menu for each module next to the link button. I haven't figured out how to do the former in code yet, though it works quite well when done manually in the UI via the mapping function (changing the action from the default "toggle" to "on"). I haven't had a chance to look into the latter yet.

A dedicated return to the exact position in the QAP from which one jumped to a full module would be better (though more work) than a shortcut to activate the QAP, since the latter action always leaves you at the top instead of preserving the scroll position.

@ralfbrown ralfbrown added wip pull request in making, tests and feedback needed scope: UI user interface and interactions feature: enhancement current features to improve labels Apr 20, 2024
@TurboGit
Copy link
Member

@ralfbrown : A screenshot please?

@TurboGit TurboGit added this to the 4.8 milestone Apr 20, 2024
@ralfbrown
Copy link
Collaborator Author

QAP-1
QAP-2

@TurboGit
Copy link
Member

I don't think it is good to propose here Sigmoid & Filmic at the same time. Either Filmic or Sigmoid should be added depending on the current workflow setting.

Also I would reorder the modules to be on the order it should be used. For example in the scene-referred workflow we recommend to set the exposure early, but the module is far down here. Same for color calibration (at least WB) should come early.

That being said I'm quite happy to see this proposal, I like the idea of having a ready to use for basic development covering most needs.

@TurboGit
Copy link
Member

Two other things I would like to add: binding the Escape key to activating the QAP (to return there after linking out to a full module),

Sounds like a good addition to me.

@ralfbrown
Copy link
Collaborator Author

I don't think it is good to propose here Sigmoid & Filmic at the same time. Either Filmic or Sigmoid should be added depending on the current workflow setting.

That's fine, I just need to figure out how to distinguish which is in the workflow.

Also I would reorder the modules to be on the order it should be used. For example in the scene-referred workflow we recommend to set the exposure early, but the module is far down here. Same for color calibration (at least WB) should come early.

As with all of the right-hand panels, the display order reflects the pipeline order....

also add global brilliance slider from colorbalancergb.

Note that a restart is required for a changed workflow setting to show in the QAP.
@ralfbrown ralfbrown removed the wip pull request in making, tests and feedback needed label Apr 22, 2024
@TurboGit
Copy link
Member

As with all of the right-hand panels, the display order reflects the pipeline order....

Oh my bad, I really thought that the order was forced in this panel. Makes sense...

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor code style changes please.

src/libs/modulegroups.c Outdated Show resolved Hide resolved
src/libs/modulegroups.c Outdated Show resolved Hide resolved
Also, I was mistaken about needing a restart on changing workflows -
it is only necessary to leave and then re-enter darkroom view.
@ralfbrown ralfbrown dismissed TurboGit’s stale review April 27, 2024 15:46

requested changes applied

@ralfbrown ralfbrown changed the title RFC: Add more widgets to QAP by default Add more widgets to QAP by default Apr 27, 2024
@TurboGit TurboGit added documentation-pending a documentation work is required release notes: pending labels Apr 27, 2024
Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@TurboGit TurboGit merged commit cab9e84 into darktable-org:master Apr 27, 2024
6 checks passed
@ralfbrown ralfbrown deleted the qap_def_widgets branch April 27, 2024 19:08
@elstoc elstoc added documentation-pending a documentation work is required and removed documentation-pending a documentation work is required labels Sep 19, 2024
@ralfbrown
Copy link
Collaborator Author

Updated QAP screenshot in darktable-org/dtdocs#697.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation-pending a documentation work is required feature: enhancement current features to improve scope: UI user interface and interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants