Align MaterialX to Color Interop recommendations #2514
Conversation
For more details, see the [Texture asset color space](https://github.com/AcademySoftwareFoundation/ColorInterop/blob/main/Recommendations/01_TextureAssetColorSpaces/TextureAssetColorSpaces.md) recommendations. - All names are now based on color interop short names - Deprecated names are now labeled as such - Unit tests still covers all names even older legacy ones - Introduced lin_ap0, lin_rec2020, and srgb_ap1 color spaces
| Declarations of the default color transforms in MaterialX. | ||
| --> | ||
|
|
||
| <!-- Functions that keep the same name in both 1.38 and color interop naming schemes --> |
There was a problem hiding this comment.
A lot of movement because I wanted to group NodeDefs by how they were affected by the change.
| <output name="out" type="color4" /> | ||
| </nodedef> | ||
|
|
||
| <nodedef name="ND_lin_ap0_to_lin_rec709_color3" node="lin_ap0_to_lin_rec709" nodegroup="colortransform"> |
There was a problem hiding this comment.
Introducing lin_ap0 and lin_rec2020
| <output name="out" type="color4" /> | ||
| </nodedef> | ||
|
|
||
| <!-- Note: not adding lin_ciexyzd65_scene --> |
There was a problem hiding this comment.
Can be added if requested.
| <!-- Functions introduced with color interop naming scheme --> | ||
|
|
||
| <implementation name="IMPL_lin_ap1_to_lin_rec709_color3" nodedef="ND_lin_ap1_to_lin_rec709_color3" nodegraph="NG_lin_ap1_to_lin_rec709_color3" /> | ||
| <implementation name="IMPL_acescg_to_lin_rec709_color3" nodedef="ND_acescg_to_lin_rec709_color3" nodegraph="NG_lin_ap1_to_lin_rec709_color3" /> |
There was a problem hiding this comment.
Backward compatibility with deprecated NodeDefs done via implementations.
| <input name="surfaceshader" type="surfaceshader" nodename="standard_surface1" /> | ||
| </surfacematerial> | ||
| <!-- acescg becomes lin_ap1 --> | ||
| <acescg_to_lin_rec709 name="deprecated13" type="color3"> |
There was a problem hiding this comment.
Testing every single deprecated node to make sure it can be referenced in older documents. Will double up as an upgrade test in 1.40. Should produce 2 spheres of uniform orange color.
| </convert> | ||
| <switch name="switch1" type="color4"> | ||
| <input name="in1" type="color4" value="0.5776, 0.1274, 0.0319, 1" /> | ||
| <input name="in2" type="color4" value="0.5776, 0.1274, 0.0319, 1" colorspace="lin_rec709" /> |
There was a problem hiding this comment.
Every single color space name from legacy gamma22 up to latest g22_adobergb_scene is tested in both color3 and color4 variants. Should produce 10 spheres of uniform orange color.
|
|
||
| if (sourceSpace == targetSpace) | ||
| { | ||
| return _document->getNodeDef("ND_dot_" + transform.type.getName()); |
There was a problem hiding this comment.
Discovered because we are testing every name, even lin_rec709 and lin_rec709_scene in the unit test.
|
Thanks @JGamache-autodesk, and I'd agree this makes the most sense as a contribution for 1.39.5. |
There was a problem hiding this comment.
I confirm that the matrices for Rec.2020, AP0, AP1, and AdobeRGB are correct.
The matrix for P3 D65 is not new in this PR, but I checked that one too and got a slightly different value:
1.224940176281, -0.042056954710, -0.019637554590,
-0.224940176281, 1.042056954710, -0.078636045551,
0.000000000000, -0.000000000000, 1.098273600141
And small tweak to the p3d65 matrix.
…autodesk-forks/MaterialX into adsk/full_color_interop_support
4 participants
I would recommend we wait for 1.39.5, but if there is a consensus to be bold, I don't see why it could not sneak in 1.39.4.
For more details, see the Texture asset color space recommendations.