Align UsdUVTexture wrap modes with USD specification

[pixelsandpointers]

Closes #2899

The wrap mode enum values for ND_UsdUVTexture_23 did not match those used by UsdUVTexture in OpenUSD, making round-trip interchange lossy. This PR aligns the names, defaults, and fallback behavior with the USD specification.

Changes

libraries/bxdf/usd_preview_surface.mtlx

• Renamed enum values periodic → repeat and black is now explicitly included (was previously named constant)
• Changed the default for wrapS/wrapT from periodic to useMetaData, matching the USD default

source/MaterialXRender/ImageHandler.h

• Renamed AddressMode enum values: CONSTANT → BLACK, PERIODIC → REPEAT to match USD terminology

GL / Metal / Slang texture handlers

• Updated all references to the renamed enum values
• Changed the default (unspecified) address mode fallback from REPEAT (GL: GL_REPEAT, Metal: MTLSamplerAddressModeRepeat) to BLACK (GL: GL_CLAMP_TO_BORDER, Metal: MTLSamplerAddressModeClampToBorderColor), matching USD's fallback from useMetaData when no metadata is present

Not yet addressed

useMetaData is now accepted as the default enum value in the nodedef, but reading wrap modes from texture file metadata is not yet implemented — a follow-up is noted with a TODO (#2899) comment in both the nodedef and the AddressMode enum.

[pixelsandpointers]

I think this is up for review @ld-kerley and @jstone-lucasfilm.

I am not entirely sure if all the changes I introduced were necessary (still fairly new to contributing to MtlX). Therefore, I am also uncertain if the versioning code should still target 1.39, or 1.40.

The useMetaData is not targeted in this PR. I added fallthrough and fallback cases to black. I may need to read a bit more about how OpenUSD handles useMetaData to propose a solution.

Happy for feedback and comments.

[jstone-lucasfilm]

@pixelsandpointers I haven't yet done a full review, but on my first read-through, it looks like you're renaming the address modes of all texture nodes, including the core image node in MaterialX, when you probably mean to restrict this change to the UsdUVTexture node alone.

[pixelsandpointers]

@jstone-lucasfilm I was not sure which files concretely touch what. I grepped for the ND_UsdUVTexture23 and applied the changes there first and later looked into the ImageHandler defining the enumerations. Afterwards, I followed all the old enum values in all downstream definitions and changed them to use the new ones..

Concretely, that means I need to revert which changes? I assume I need to revert the gltf definitions, right?

[kwokcb]

• MTLX: Only the usd preview surface mtlx node should be modified. All others should be reverted.
• For ImageHandler and all the "Render" code : the enum can not be a replacement. constant is not equal to black. constant means the default color so USD is a restricted case of constant so maybe it should added as a new case to handle. Not sure.

[pixelsandpointers]

Unfortunately, a couple of changes sneaked in when running clang-format on the files.

I went down the additive approach now and added new enumerations and adjusted the handlers, respectively. Please @jstone-lucasfilm @kwokcb @ld-kerley let me know what you think.

[jstone-lucasfilm]

Thanks for iterating on this, @pixelsandpointers, and for your patience as this discussion has evolved. The additive approach is a clear improvement, and you've correctly preserved the important constant vs black distinction that @kwokcb brought up.

Before we go further down the current path, though, I'd like to suggest an alternative direction that I think is worth some community discussion, because it may let us solve this more cleanly than either renaming or extending the core image node.

The crux of the problem is that UsdUVTexture and the core image node use different vocabularies for the same address mode behaviors (repeat == periodic, and black is roughly equal to constant), and our shader generation currently can't bridge that gap. It resolves enum inputs purely by their position in the enum list, so a UsdUVTexture value like repeat flows straight into the image node and fails to validate against its enum. That's what's pushing your PR toward adding new values to the core node.

But MaterialX actually has an existing mechanism for precisely this kind of mapping: the specification defines enumvalues (with impltype) as a way to map enum labels to explicit underlying values, and the example discussed in the specification is, fittingly, the uaddressmode and vaddressmode inputs of the image node. The catch is that our shader generators don't yet implement this, and instead just use the positional index.

So a potential path forward would be:

• Implement enumvalues/impltype resolution in shader generation (positional index stays the default when they're absent, so existing nodes are unaffected by default).
• Then express the UsdUVTexture to image address mode mapping as pure data on UsdUVTexture, e.g. black/clamp/repeat/mirror mapping to the existing image sampler behaviors.

If that approach works out, it would let us:

• Keep the core image node and its public enum completely unchanged,
• Keep the UsdUVTexture implementation as a pure, language-agnostic graph (no C++ implementation, no new nodes),
• And give us a reusable way to handle the same vocabulary-mismatch problem for other interchange nodes down the road (glTF, future USD nodes, etc.).

There are some details to work through: where this mapping is best declared, how the exact transparent-black behavior is handled, and a likely small spec clarification, so this would want a bit of design discussion before anyone commits to it, and it implements a feature that touches the specification, so it's probably worthy of a TSC discussion.

I don't want to discard the work you've already done here, and at a very minimum your PR has surfaced an important missing feature in our shader generators. Would you (along with @ld-kerley and @kwokcb) be open to exploring this direction? I'm happy to extend this idea with more context if there is interest.

[kwokcb]

Hi @jstone-lucasfilm,

I agree that this can be approached as a remapping issue.

My initial suggestion had imprecise wording -- I thought that maybe "add" black mode would be a good option but couldn't think of how to do remapping... :)

Looking at the existing graph and it's implied indexing based on enumeration order

• black is inteted to map to constant, and the default "fallback" input is indeed a black (0,0,0,1) value.
• periodic maps to periodic.

This is the impl graph connections:

I think adding an explicit enumvalues attribute to the current image node + any other nodes using image should give you proper explicit mapping for hardware shading languages. Adding enumvalues should not affect any languages accepting modes defined as strings such as OSL.

I think fallback should be hidden from the interface if you want it to always be black.

So for USD it would look something like this:

<nodedef name="ND_UsdUVTexture_23" node="UsdUVTexture" nodegroup="texture2d" version="2.3" isdefaultversion="true">
    <input name="wrapS" type="string" value="periodic" enum="black,clamp,repeat,mirror" enumvalues="0,1,2,3,4" uniform="true" />
    <input name="wrapT" type="string" value="periodic" enum="black,clamp,repeat,mirror" enumvalues="0,1,2,3,4"uniform="true" />
    <!-- Remove this and set the value for "default" on the image nodes usde for the implementation graph to be black 
    <input name="fallback" type="color4" value="0, 0, 0, 1" />  -->

which maps to an image node which looks like this:

<nodedef name="ND_image_float" node="image" nodegroup="texture2d">
    <input name="uaddressmode" type="string" value="periodic" enum="constant,clamp,periodic,mirror" enumvalues="0,1,2,3,4" uiname="Address Mode U" uniform="true" />
    <input name="vaddressmode" type="string" value="periodic" enum="constant,clamp,periodic,mirror" enumvalues="0,1,2,3,4" uiname="Address Mode V" uniform="true" />

• 0 maps black to constant which means it will use a hard-coded black value
• 3 maps repeat to periodic.
• The rest map as the did before but explicitly now.

This should mean no C++ changes at all just definition changes.

[pixelsandpointers]

Sorry for the delay.

@kwokcb I am uncertain if you meant we have only 4 enum values. I implemented the version now on the 4 enum values. The fallbacks (default colours) were already in place and only changed the RGBA to have an alpha of 1, so we get an opaque black.

As you said, the only changes now are the enumvalues and the defaults.

@jstone-lucasfilm I don't mind iterating on this. I think this is also a good way to get into the intrinsics of MatX.