search

AcademySoftwareFoundation / OpenColorIO-Config-ACES

https://opencolorio.org
BSD 3-Clause "New" or "Revised" License
263 stars 38 forks source link

Collated Feedback from the Studio Config #67

Open KelSolaar opened 2 years ago

KelSolaar commented 2 years ago

@flord

image

@MrLixm

Hello, great to see this discussion. A thing I'm wondering is why the unique identifier of the colourspace is also the pretty name used for interfaces? Shouldn't decoupling the UI and the logic a better idea to allow for more flexible configs that can be updated at any moment ?

SImple naive example :

  - !<ColorSpace>
    identifier: displaysrgb
    name: True-PRO sRGB Display
    family: Display
    equalitygroup: ""
    bitdepth: 32f
    description: Convert CIE XYZ (D65 white) to sRGB (piecewise EOTF)

Where anyone could just change the name and it wouldn't break anything in any software.

This could then open a lot of other discussions (identifiers formats, identifiers conventions, ...) but just wanna know your thought about this ? Cheers. Liam.

@anderslanglands

I understand that it may be too late but it would be really nice if compact, unique identifiers could be split from the “display name”, especially if the display name is forced to have extra context to be used in isolation.

For instance, some applications will bake the colour space name into file names (substance does this when exporting) and with the current ACES configs this results in some absolutely horrific file names. Plus all those spaces are just bugs waiting to happen.

As someone who likes to think he knows a little about colour, but doesn’t know OCIO very well, I find the current naming in the ACES configs extremely confusing - eg which sRGB is actually sRGB? Or are they both the same thing? (I still don’t know the answer to this).

@KelSolaar

@scoopxyz

@nick-shaw

Is there a reason that the ACES Reference Gamut compression is available as a look in the Reference config, but not the Studio config? I feel we should be encouraging people to start using it, and making it easy for them to do so.

On that subject, because the "Blue Highlight Fix" is alphabetically before the RGC, that comes up as the default look when the Reference config is used. That should really be deprecated in favour of the RGC.

And finally (not that I'm obsessed with the RGC at all!) but it brings up the point that support for aliases in looks could be useful, so you could simply use e.g. "RGC" instead of typing "ACES 1.3 Reference Gamut Compression" in full.

Bartłomiej Styczeń

@Thomas Mansencal so I finally got to this, and all seems to go well until I specify a BuiltinTransform Style in the CSV, I tried with these: ACEScc_to_ACES2065-1 ACEScct_to_ACES2065-1 ACEScg_to_ACES2065-1 ACESproxy10i_to_ACES2065-1 And they all crash the generator with INFO:opencolorio_config_aces.config.cg.generate.config:Creating a "Colorspace" transform for "ACEScc_to_ACES2065-1" style... Traceback (most recent call last): File "/home/aswf/OpenColorIO-Config-ACES/opencolorio_config_aces/config/studio/generate/config.py", line 274, in config, data = generate_config_studio( File "/home/aswf/OpenColorIO-Config-ACES/opencolorio_config_aces/config/studio/generate/config.py", line 228, in generate_config_studio _config, data = generate_config_cg( File "/home/aswf/OpenColorIO-Config-ACES/opencolorio_config_aces/config/cg/generate/config.py", line 997, in generate_config_cg colorspace = style_to_colorspace(**kwargs) File "/home/aswf/OpenColorIO-Config-ACES/opencolorio_config_aces/config/cg/generate/config.py", line 415, in style_to_colorspace [beautify_alias(signature["name"])] + signature["aliases"] KeyError: 'name' Before I delve into the source code, just wanted to ask if it’s something you recognise and maybe the problem is as trivial as error in the BuiltinTransform Style name

After some debugging and slight modification to the code and csv, I got a config, but not really sure about the correctness of it. The problem I mentioned above is due to a signature["name'] never being set. I have no clue where that name should come from so I just fed it style name. Later on there was a problem with the config version: PyOpenColorIO.Exception: Error building YAML: Only config version 2.1 (or higher) can have BuiltinTransform style 'ACES-LMT - ACES 1.3 Reference Gamut Compression'. It seems like the generation starts with the proper 2.1 version, but then it is changed midway to 2.0 in config/cd/generate/config.py: 1085: data.profile_version = VersionData(2, 0) If that was intended, can I just remove that line or will it mess up the further logic? (edited)

Sidenote: Dockerfile seems to be a bit out of date, it’s based on old ci-ocio that uses python 3.7 and one of the requirements requires python >= 3.8 (numpy 1.22 if I remember correctly). I’ve used FROM aswf/ci-ocio:2022 and it built fine, but maybe that can cause some issues with the generator?

@mdecaria

Hi, congratulations on the beta config release, just have some general questions/feedback on the Studio Config

@remia

I looked quickly at the studio config, nothing really important to report or probably already known:

Great work everyone!

@zachlewis

My quickie beef -- can we get rid of the " - Display" part of the Display names? Gets to be quite a bit to type.

I understand that we want to keep " - Display" in the Display Color Spaces names, and that's fine. But maybe instead of using Shared Views for all the ACES-based views, we can use plain-ol regular views.

Derek Flood

I see that the Canon CLFs have been updated for C-Log3 and its corresponding linear. https://github.com/AcademySoftwareFoundation/OpenColorIO-Config-ACES/pull/71

Is it planned to also support the C-Log2? Or is C-Log2 depreciated/unsupported?

Just seeking education/understanding, thanks!

MrLixm commented 2 years ago

Hello, thanks for porting this here. Perhaps me and Anders comments should be ported to the OpenColorIO repository?

KelSolaar commented 2 years ago

@MrLixm : We touched on a similar topic during the last meeting: https://wiki.aswf.io/display/OCIO/2022-08-02

We will get back to it soon.

Note that I ported the new slugification code from Colour to generate aliases automatically, maybe this could be used.

Cheers,

Thomas

nick-shaw commented 2 years ago

Is there a reason that the ACES Reference Gamut compression is available as a look in the Reference config, but not the Studio config? I feel we should be encouraging people to start using it, and making it easy for them to do so.

On that subject, because the "Blue Highlight Fix" is alphabetically before the RGC, that comes up as the default look when the Reference config is used. That should really be deprecated in favour of the RGC.

And finally (not that I'm obsessed with the RGC at all!) but it brings up the point that support for aliases in looks could be useful, so you could simply use e.g. "RGC" instead of typing "ACES 1.3 Reference Gamut Compression" in full.

nick-shaw commented 2 years ago

Correction. It's not alphabetically before it in terms of name, but it does come before in the Reference config. Is that ordered based on CTL file name? If they were in alphabetical order, "ACES 1.3 Reference Gamut Compression" would come first, and become the default. Problem solved.

KelSolaar commented 2 years ago

Hey @nick-shaw,

Is there a reason that the ACES Reference Gamut compression is available as a look in the Reference config, but not the Studio config? I feel we should be encouraging people to start using it, and making it easy for them to do so.

Great question with a simple answer, whilst the current VFX Reference Platform mandates OCIO 2.1 as the version for 2022. Many applications, e.g. Nuke, are still on the 2.0 version of the library which does not have the ACES 1.3 Reference Gamut Compression transform. So rather than having a config that cannot be loaded in those applications, we decided to only leave it in the Reference config.

On that subject, because the "Blue Highlight Fix" is alphabetically before the RGC, that comes up as the default look when the Reference config is used. That should really be deprecated in favour of the RGC.

It is because of the Ordering value it has in the spreadsheet and its position within it:

image

We should simply change the Ordering value of the Blue Light Artifact Fix to 315 and that would push it down.

And finally (not that I'm obsessed with the RGC at all!) but it brings up the point that support for aliases in looks could be useful, so you could simply use e.g. "RGC" instead of typing "ACES 1.3 Reference Gamut Compression" in full.

This is a good one, and we lost that when converting the Colorspace from 1.2 to a Look. There is unfortunately no API for that: https://opencolorio.readthedocs.io/en/latest/api/look.html. @doug-walker and @hodoulp for VIS.

nigelhadley-foundry commented 2 years ago

Hello @KelSolaar @nick-shaw

Nuke 14 (latest main) is at OCIO 2.1 The CG and the Studio configs are being added now (ie today!) so Nuke will have 2.1 OCIO and ACES 1.3 for the upcoming 14.0 Beta

KelSolaar commented 2 years ago

This is awesome @nigelhadley-foundry! Just to be sure, you picked the v0.3.1 release right? v0.3.0 had some incorrect directions set.

KelSolaar commented 2 years ago

Self-Reminder: Ensure that colorspaces version is updated when we update the generator.

scoopxyz commented 2 years ago

Just a small comment to include a link to the spreadsheets that are used as the "ground truth" for naming, etc. in the README.md if you can :)

KelSolaar commented 2 years ago

Thank you good point @scoopxyz !

remia commented 2 years ago

I looked quickly at the studio config, nothing really important to report or probably already known:

Great work everyone!

zachlewis commented 2 years ago

My quickie beef -- can we get rid of the " - Display" part of the Display names? Gets to be quite a bit to type.

I understand that we want to keep " - Display" in the Display Color Spaces names, and that's fine. But maybe instead of using Shared Views for all the ACES-based views, we can use plain-ol regular views.