AcademySoftwareFoundation
Adsk Contrib - Adding color space attribs: interop_id, amf_transform & icc_profile_name
- Addressing the issues #1975, #2152 and #2153
- Bumped the config version to 2.5
- newly added attributes require v2.5 config both for serialization and de-serialization and will throw if they appear in older configs.
- Hardened multiple existing functions against null parameters. Previously those functions were crashing if null is passed, as assigning null to std::string is undefined behavior.
- Expanded tests to some affected functions' unit tests to include null passing. Setters will take null as empty string. compare functions will throw.
I'm extremely curious to hear if @cozdas feels like the copilot review comments are helpful and correct.
Not a fan of seeing all those interchange attributes stuffed at the base level of the ColorSpace transform. Feels like a lot of pollution induced by secondary non-critical information. Could we consider them to be nested in a new bag-like field, e.g.:
- !<ColorSpace>
name: raw
family: raw
equalitygroup: ""
bitdepth: 32f
description: Some text.
isdata: true
allocation: uniform
interchange:
interop_id: foo
amf_transform_id:
bar
baz
icc_profile_name: bam
I'm extremely curious to hear if @cozdas feels like the copilot review comments are helpful and correct.
Do we have an idea on the associated cost?
I'm extremely curious to hear if @cozdas feels like the copilot review comments are helpful and correct.
Not too bad, it caught a valid issue and a typo. For this one the signal to noise ratio was not too bad but I'm curious to see it in a more involved PR. Sometimes this "vibe coding" is eating more time that it saves.
Do we have an idea on the associated cost?
Nothing, this is just a feature of GitHub.
Or did you mean some metaphorical cost to the world or our souls or something?
Nothing, this is just a feature of GitHub.
Or did you mean some metaphorical cost to the world or our souls or something?
Good to know, just wanted to check whether or not it was on a shared ASWF budget.
Not a fan of seeing all those interchange attributes stuffed at the base level of the
ColorSpacetransform. Feels like a lot of pollution induced by secondary non-critical information. Could we consider them to be nested in a new bag-like field, e.g.:- !<ColorSpace> name: raw family: raw equalitygroup: "" bitdepth: 32f description: Some text. isdata: true allocation: uniform interchange: interop_id: foo amf_transform_id: bar baz icc_profile_name: bam
When looking at the ACES configs, I found what was bothering me but could not put my finger on: AMF (amf_transform_id) is something very specific to ACES, as much as I like ACES and the work we do there, I feel like this is probably too much to have an attribute added to OCIO just to support ACES. I already felt that the hardcoded ACES interchange role was too much and this is continuing the trend. We should strive for agnosticism as much as possible.
Hey Thomas - it's a good point you make around being agnostic, and one we definitely should keep in mind.
We definitely put our eggs in the ACES basket for configs, but the library is less so (other than the fact that all the ACES transforms are built-in). To me, I was looking at this as a way to help us clean up the configs a bit, make it easier for folks using AMF & OCIO to operate. It's not a required attribute, so it's not locking anyone into an ACES workflow that doesn't want to be.
However, your point is fair and we should discuss at the TSC today.
Per the discussion at the 2025-08-25 working group meeting, the interop_id will stay as is but the amf_transform_id and icc_profile_name will move under a sub-group. The items in the sub-group will all use a generic getter/setter that takes an attribute name.
In addition, we are looking at allowing the interop_id to be written in profile versions >= 2.0.
Cuneyt will be making another commit in the coming days with these changes.