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

RGB (scene) by default in masks blending options if scene-referred is the default workflow. #10080

Open
difrkaguilar opened this issue Sep 25, 2021 · 5 comments
Labels
difficulty: good first contribution easy enough for first-time contributors willing to dip their toes into darktable feature: enhancement current features to improve scope: image processing correcting pixels scope: UI user interface and interactions
Milestone

Comments

@difrkaguilar
Copy link

Is your feature request related to a problem? Please describe.
Every time I create a mask in a module, then I've to select RGB (scene) in blending options too, others modules like Color calibration and color balance rgb have this option selected by default. I think that maybe some modules aren't optimized for the new scene-referred workflow. But It's suposed that if in darktable preferences the default workflow is selected as scene-referred, then all modules, optimized or not must change their masks blending options to RGB (scene)

Describe the solution you'd like
If scene-referred is selected as default workflow, then all masks blending options in modules must be RGB (scene) by default too.

Additional context
two sample images
Lab selected by default.
Screenshot-20210925005103-434x1148

RGB (scene) by default.
Screenshot-20210925011559-437x834

@Nilvus Nilvus added feature: enhancement current features to improve scope: image processing correcting pixels scope: UI user interface and interactions labels Sep 25, 2021
@Nilvus Nilvus added this to the 3.8 milestone Sep 25, 2021
@Nilvus Nilvus added the difficulty: good first contribution easy enough for first-time contributors willing to dip their toes into darktable label Sep 25, 2021
@TurboGit TurboGit modified the milestones: 3.8, 4.0 Nov 26, 2021
TurboGit pushed a commit that referenced this issue Jan 18, 2022
TurboGit pushed a commit that referenced this issue Jan 18, 2022
@TurboGit TurboGit modified the milestones: 4.0, 4.2 May 7, 2022
@TurboGit TurboGit modified the milestones: 4.2, 4.4 Nov 13, 2022
@jmoric
Copy link
Contributor

jmoric commented Apr 5, 2023

@TurboGit : To get this right. Should all modules be set to use RGB color space by default in scene-referred workspace. Apart from RAW modules of course.

That is int default_colorspace(...) return IOP_CS_RGB in scene-referred workspace. Even if they are not optimized to use RGB color space?

@TurboGit
Copy link
Member

TurboGit commented Apr 5, 2023

@jmoric : I'm not sure about all the implications. Some module are Lab should we use RGB (display or scene-referred).

Let's ping @flannelhead who knows lot more about color than me.

@ralfbrown
Copy link
Collaborator

I don't think just having default_colorspace always return IOP_CS_RGB will work properly, since it provides the module's expected input colorspace and produced output colorspace when input_colorspace or output_colorspace are not implemented. The pipeline code then uses that information to insert colorspace conversions as needed to adapt the output of one module to the input of the next.

In the specific case of contrast equalizer, that module operates in Lab, so using an RGB blend mode implies a colorspace conversion before the blending. The module happens to be in a position in the pipe where the following module is probably expecting RGB input, in which case a conversion would be needed either way. But if the following modules expect Lab input, an RGB blend mode adds two conversions.

(BTW, that reminds me that I want to raise an RFC about changing colorin's output colorspace from Lab to RGB, since it will almost certainly be followed by a module taking RGB input.)

@jmoric
Copy link
Contributor

jmoric commented Apr 6, 2023

I don't think just having default_colorspace always return IOP_CS_RGB will work properly, since it provides the module's expected input colorspace and produced output colorspace when input_colorspace or output_colorspace are not implemented. The pipeline code then uses that information to insert colorspace conversions as needed to adapt the output of one module to the input of the next.

Yes, it also seemed way too easy considering this problem has been delayed since 2021 (milestone 3.8). Thanks for the explanation.

In the specific case of contrast equalizer, that module operates in Lab, so using an RGB blend mode implies a colorspace conversion before the blending. The module happens to be in a position in the pipe where the following module is probably expecting RGB input, in which case a conversion would be needed either way. But if the following modules expect Lab input, an RGB blend mode adds two conversions.

(BTW, that reminds me that I want to raise an RFC about changing colorin's output colorspace from Lab to RGB, since it will almost certainly be followed by a module taking RGB input.)

@ralfbrown : To summarize: We should leave this as is until further module interface decisions are specified.

@jenshannoschwalm jenshannoschwalm modified the milestones: 4.4, 5.0 Aug 18, 2024
@jenshannoschwalm
Copy link
Collaborator

There has been done some work on module colorspace so we might want to check again about this idea.

@TurboGit TurboGit modified the milestones: 5.0, 5.2 Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: good first contribution easy enough for first-time contributors willing to dip their toes into darktable feature: enhancement current features to improve scope: image processing correcting pixels scope: UI user interface and interactions
Projects
None yet
Development

No branches or pull requests

6 participants