AcademySoftwareFoundation / OpenColorIO

A color management framework for visual effects and animation.
https://opencolorio.org
BSD 3-Clause "New" or "Revised" License
1.74k stars 431 forks source link

PR: ocioview - Chromaticities Inspector #1914

Closed KelSolaar closed 1 month ago

KelSolaar commented 7 months ago

A temporary PR to park and discuss about the work on the Chromaticity Inspector.

image

Most of the base pieces are here but there are a few questions, the principal one being:

In order to plot chromaticities with colours, we need to be able to know which RGB colourspace we are coming from to do: $RGB \rightarrow CIE\ XYZ \rightarrow CIE\ xy(Y)$ The issue is OCIO cannot really do that transformation, Colour can do it but this assumes that there is a matching RGB colourspace builtin (or a corresponding one added). If only we had primaries and whitepoints... :)

I'm assuming ACES2065-1 in the above image. Suffice to say that we are interested in both the chromaticities of the original image and that of the transformed one.

Food for thoughts!

Paging @remia, @michdolan and @doug-walker!

KelSolaar commented 7 months ago

We discussed about using the CIE XYZ Interchange space with @doug-walker et al. this morning. This would work but would not give the final encoded chromaticities. That might be an acceptable compromise.

KelSolaar commented 7 months ago

Having put quite a bit of thoughts into that one here are some suggestions:

  1. Output Transform Passthrough: Convert from Input Color Space to CIE XYZ D65 Interchange and display chromaticities.
  2. Output Transform Active: Convert from Input Color Space to Output Transform to CIE XYZ D65 Interchange and display chromaticities.

Now some questions for @michdolan:

The current MessageRouter implementation can return a processor that could handle doing 2., partially. I would need to add the conversion to CIE XYZ D65 Interchange which requires knowledge of what is the Output Transform from the ImageViewer and I don't see a clean API to find that out. I cannot also find a way to get the Input Color Space from the ImageViewer which would be required to do 1.

remia commented 7 months ago

Looks good @KelSolaar (and nice choice of books)! I assume you are going to add settings to choose the chromaticity space to use, switch between 2D / 3D representation, optional gamut triangle (and maybe more) later?

KelSolaar commented 6 months ago

Looks good @KelSolaar (and nice choice of books)! I assume you are going to add settings to choose the chromaticity space to use, switch between 2D / 3D representation, optional gamut triangle (and maybe more) later?

Yes, that is the plan! I haven't had too much time to look into it but was slowly getting back to that. Might be worth a discussion as per my previous comment about the new API needs I have. I could implement them from that PR too.

@michdolan : Any thoughts?

michdolan commented 3 months ago

Sorry for the long delay in replying. This looks awesome @KelSolaar !

I think for getting the needed data (input color space, output transform) we could wrap the processor being passed to the message router in a data class that contains info about the processor's construction. That would be useful elsewhere too.

michdolan commented 3 months ago

In #1966 I added a new ProcessorContext interface that is passed through the message router with the processor, which will give you the input color space name, output transform item type, output transform item name, and the transform direction.

ProcessorContext(input_color_space='ACES - ACES2065-1', transform_item_type=<class 'PyOpenColorIO.ColorSpace'>, transform_item_name='Input - Generic - sRGB - Texture', transform_direction=<TransformDirection.TRANSFORM_DIR_FORWARD: 0>)

KelSolaar commented 2 months ago

Did some good progress over the weekend and tonight: https://www.youtube.com/watch?v=x5wqywe5mSA

I spent quite a lot of time finding out what would be a useful UX for the inspector and went with 3 layers of transformation, the benefit is that it allows to show the fully processed chromaticities compared to my suggestion above: https://github.com/AcademySoftwareFoundation/OpenColorIO/pull/1914#issuecomment-1829550722

  1. Apply current active processor
  2. Convert from chromaticities input space to "CIE-XYZ-D65" interchange
  3. Convert from "CIE-XYZ-D65" to "VisualRGBScatter3D" working space

The user HAS to choose the chromaticities input space, it might looks cumbersome but the benefit is that one could load the ACES 1.0 config, define the ROLE_INTERCHANGE_DISPLAY, apply the sRGB ODT, use the Linear sRGB as chromaticities input space and see the final image chromaticities.

I implemented some new convenient functions in Colour to help the CIE 1960 UCS and CIE 1976 UCS Chromaticity Diagrams.

KelSolaar commented 2 months ago

I think that this is ready for a first review, at least to discuss about it :)

KelSolaar commented 2 months ago

I was also thinking that adding a RGB mode should not be too hard but maybe a dedicated viewer is better. When I have some cycles, I would also like to add a RGB parade and possibly other type of scopes:

image image

linux-foundation-easycla[bot] commented 1 month ago

CLA Signed

The committers listed above are authorized under a signed CLA.

KelSolaar commented 1 month ago

I committed the suggestion but seems like EasyCLA is being a prick... Do we have a process for that or does it look like I will need to rebase/reword the commits?

michdolan commented 1 month ago

Sorry about the EasyCLA issue... You may need to amend the commit and force push again.

KelSolaar commented 1 month ago

Trimmed yourself from the commits @michdolan!

remia commented 1 month ago

Great work @KelSolaar!

Convert from "CIE-XYZ-D65" to "VisualRGBScatter3D" working space

Am I understanding correctly that this transformation is currently a no-op as the latter is also XYZ D65?

The user HAS to choose the chromaticities input space, it might looks cumbersome but the benefit is that one could load the ACES 1.0 config, define the ROLE_INTERCHANGE_DISPLAY, apply the sRGB ODT, use the Linear sRGB as chromaticities input space and see the final image chromaticities.

When applying the sRGB ODT, shouldn't the Chromaticities input space be Display sRGB in that case (sRGB EOTF encoded instead of Linear)?

KelSolaar commented 1 month ago

Am I understanding correctly that this transformation is currently a no-op as the latter is also XYZ D65?

As of right now, indeed, so we can optimise it but I left it as a test case for now, allows verifying that any working space produce the same stuff.

When applying the sRGB ODT, shouldn't the Chromaticities input space be Display sRGB in that case (sRGB EOTF encoded instead of Linear)?

Depends if one wants to see the encoded or linear chromaticities, both would be valid options, e.g., you want to ignore the EOTF inverse effect.

KelSolaar commented 1 month ago

Merging this one down!

mmdanggg2 commented 1 month ago

Merged code is broken, you didn't update the color_space_to_rgb_colourspace name in chromaticities_inspector.py

KelSolaar commented 1 month ago

Thanks @mmdanggg2! I thought I had checked that the branch was fully working after I rebased but looks like I did not. This should be fixed in #1985 whenever it is merged.

KelSolaar commented 3 weeks ago

@mmdanggg2 : This should be good now!