Fix incorrect transform in genosl

[jstone-lucasfilm]

This changelist updates the transform(matrix, vector4) method in genosl to match a corresponding fix in the OSL codebase. See AcademySoftwareFoundation/OpenShadingLanguage#1513 for more details.

[rasmusbonnedal]

@jstone-lucasfilm
I think this code depends on the old behaviour and will break with this change:
https://github.com/jstone-lucasfilm/MaterialX/blob/e47a3b3f14860d1b7e19fa49fa899fed973a73ea/libraries/stdlib/genosl/mx_rotate_vector3.osl#L18
https://github.com/jstone-lucasfilm/MaterialX/blob/e47a3b3f14860d1b7e19fa49fa899fed973a73ea/libraries/stdlib/genosl/mx_ap1_to_rec709_color3.osl#L5
https://github.com/jstone-lucasfilm/MaterialX/blob/e47a3b3f14860d1b7e19fa49fa899fed973a73ea/libraries/stdlib/genosl/mx_ap1_to_rec709_color4.osl#L5
https://github.com/jstone-lucasfilm/MaterialX/blob/e47a3b3f14860d1b7e19fa49fa899fed973a73ea/libraries/stdlib/genosl/mx_g22_ap1_to_lin_rec709_color3.osl#L8
https://github.com/jstone-lucasfilm/MaterialX/blob/e47a3b3f14860d1b7e19fa49fa899fed973a73ea/libraries/stdlib/genosl/mx_g22_ap1_to_lin_rec709_color4.osl#L8

[kwokcb]

Hi @rasmusbonnedal,
I replied in Slack, but yes, I believe these will break. Running the test suite for OSL rendering before and after should see what's changed (in case there is more). Also I'd suggest we change these to be vec3 computations as AFAIK no input colorspace transform modifies the 4th channel (alpha) (at least not in the OCIO implementations).

[rasmusbonnedal]

@kwokcb I agree! I searched for transform in all *.{osl,h} in the repo and these were all which uses vector4.

[jstone-lucasfilm]

Good points, @rasmusbonnedal and @kwokcb, and let's double check these visual behaviors before merging the fix. In some cases, we may have been transposing the matrix to compensate for this issue, and in those situations we should remove the unneeded transpose calls.

[jstone-lucasfilm]

Attaching reference renders with the latest proposed changes:

MaterialX_ReferenceRenders_05_30_2022.pdf

[rasmusbonnedal]

I think rotate3d needs to add a transpose: https://github.com/jstone-lucasfilm/MaterialX/blob/e47a3b3f14860d1b7e19fa49fa899fed973a73ea/libraries/stdlib/genosl/mx_rotate_vector3.osl#L18

This is before the change in transpose(matrix, vector4):

This is after:

[jstone-lucasfilm]

@rasmusbonnedal I was curious about this as well, and I couldn't find an existing test material that would confirm this one way or another. If you're confident that the original version was correct, and a transpose is now needed for parity, would you have the bandwidth to write up a pull request for this change to the MaterialX GitHub?

[rasmusbonnedal]

I compared the results with GLSL and it corresponds to the "after" result. It seems rotate3d was different before between OSL and GLSL and are the same now. So all good!

[jstone-lucasfilm]

That's great news, @rasmusbonnedal, and thanks for confirming!