Only emit texture colorspaces in OSL 1.14+

[boberfly]

Added oslFileTextureColorspace GenOptions to omit the colorspace optional argument to the texture function.

I commented in #2127 (comment) about it, but I thought I'd figure it out myself.

I couldn't think of another way to approach it but this seems to allow generating the nodes without the optional argument to texture() which causes errors in some renderers.

eg. ERROR : RenderMan : S20006 OSL: Unknown texture optional argument: "colorspace", (shaders\__mtlx\__ND_image_color3.osl:35)

Kind regards
-Alex

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: boberfly / name: Alex Fuller (fdad61b)
• ✅ login: jstone-lucasfilm / name: Jonathan Stone (3648ad7, 775af7c)

[jstone-lucasfilm]

@boberfly This looks like an important fix to implement in our OSL shader generation, but I wonder if it might be better to leverage the built-in OSL_VERSION_MAJOR and OSL_VERSION_MINOR defines to achieve this.

An example of this approach can be seen in this earlier pull request, which allows MaterialX to support OSL 1.14 without losing support for earlier versions:

#2204

[boberfly]

Hey @jstone-lucasfilm

I was thinking this, but I was looking through the source code and documentation of OSL and couldn't find colorspace as a valid optional argument for texture(), it might just be an Arnold-specific addition? Maybe future renderers will adopt it I'm not sure though.

[kwokcb]

This was indeed introduced as a Arnold request, but the proposal was to make this part of the OSL spec itself. I remember it was something being looked at but don't know what happened here. Adding @lgritz for comment.

[lgritz]

I think maybe we added it to the texture system but didn't wire up all of the OSL side? I'll check on it.

[jstone-lucasfilm]

@lgritz I don't see the colorspace input documented in the specification for OSL 1.13.10, and if it's not yet a standard feature, then I'd support @boberfly's suggestion to enable it with a shader generator option.

https://open-shading-language.readthedocs.io/en/v1.13.10.0/stdlib.html#texture

If it's perhaps a hidden OSL feature, though, then we could enable it based on the detected OSL version defines.

[jstone-lucasfilm]

From additional discussions with @lgritz, it sounds as if this proposal is indeed the right solution, as colorspace support isn't yet guaranteed above a specific version of OSL.

@boberfly Let's continue focusing on this PR as the recommended path forward, and I'll take a closer look at the code when time permits.

[ld-kerley]

Instead of adding a GenContext option, perhaps we should just use the OSL_VERSION guard @jstone-lucasfilm suggests above, but just set it to the next version of OSL that isn't released yet? I was in the OSL TSC meeting yesterday and @lgritz said he hoped to have this keyword arg wired up in pretty soon, so it should drop in to main quickly, and anyone building from there will get the new version.

[jstone-lucasfilm]

I'd be fine with that suggestion as well, @ld-kerley, as long as it doesn't make it too challenging for developers to access these colorspace attributes in their proprietary renderers.

By the time developers are working with MaterialX v1.39.4, perhaps it's reasonable to ask them to update to a newer version of OSL as well.

[dal]

At Pixar we're watching this PR closely because we've hit the OSL compile error trying to deploy mtlx at the studio. Both approaches seem fine but I think we favor the OSL_VERSION guard because it would involve minimal code changes on our end.

[boberfly]

Ok I see OSL 1.14.5.0 has the optional argument now from @lgritz 's changelog. I'll go the OSL_VERSION approach and re-push this to this PR soon and simplify things.

@dal another approach that I've taken short-term in Renderman 26.1 is to just swap all texture-related MaterialX OSL nodes to PxrTexture and just map to the input equivalents. Not perfect though but it's functional...

[jstone-lucasfilm]

This fix looks good to me, thanks @boberfly.

I'm additionally CC'ing @lfl-eholthouser for her thoughts, as this change potentially benefits upcoming work on OSL shader translation.

[lgritz]

I would suggest here

#if OSL_VERSION >= 11405

to make sure that anybody using a pre-release 1.14 (such as 1.14.3) won't get the errors.