Document edge case for image nodes

[jstone-lucasfilm]

This changelist documents an edge case in the handling of image node channels, improving the robustness of the MaterialX specification, and aligning it with the current behavior of the MaterialX codebase.

[meshula]

Question about the behavior, as it doesn't match what we do in USD imaging. If the original image was RGBA, and the alt is RGB, I think RGB1 makes more sense than RGB0, especially if one is treating the image as premultiplied. In other words, we fill in channels with zero yes, but alpha is filled with one. Principle of least surprise, and more correct.

[jstone-lucasfilm]

That's a really interesting proposal, @meshula.

If we were to follow the line of thought you suggest here, then perhaps it would be more consistent to state that referenced image data is processed through a convert node when its type differs from that of the image node?

The rule set for convert nodes handles the RGB1 case that you mention here, but it also contains several additional rules for consistent reinterpretation of data between types.

These rules are stated here in the specification, and I'll copy the relevant details below:

The following input/output data type conversions are supported by convert:

float to colorN/vectorN: copy the input value to all channels of the output
colorN to vectorN / vectorN to colorN, where N is the same for in and out: straight copy of channel values
color3 to color4: copy RGB, set output alpha to 1.0
color4 to color3: drop alpha channel
boolean or integer to float: output is 0.0 or 1.0
vector2 to vector3, or vector3 to vector4: copy incoming channels and append an additional channel with value 1.0 (e.g. convert from non-homogeneous to homogeneous vector)
vector3 to vector2, or vector4 to vector3: drop the last channel

[meshula]

That sounds preferable indeed.

[jstone-lucasfilm]

@meshula I'd be fine with moving over to that approach in MaterialX, as long as these rules would be acceptable to the USD team as well. Does that sound like a path forward that would work for both teams?

[meshula]

Here is the algorithm that we have, maybe we can work to reconcile differences. Note that UINT textures have an odd behavior, but I think that specific behavior does not matter to MaterialX?

The fundamental difference we have is that we are propagating missing channels and your rules are appending a 1. I will send you a note as to why this is.

    // if the image has more channels than actually read, and any of the channels 
    // is missing, fill them in by propagating the channel to the left if 
    // possible. If not, fill with zero, unless it is the alpha channel. Alpha is always filled with one.

 if (img->channelCount > readChannelCount) {
        if (img->channelCount == 4 && alphaMissing) {
            // fill the alpha channel with 1.0
                ..... details omitted
            else if (img->pixelType == EXR_PIXEL_UINT) {
                // We're treating uint data as data, not rgba, so fill with zero
                .... details
            }
        }
        if (img->channelCount > 2 && blueMissing) {
            // if G exists, propagate it, else if R exists, propagate it, else fill with zero
            .... details
        }
        if (img->channelCount > 1 && green missing) {
            // if R exists, propagate it, else fill with zero
            .... details
        }
        if (redMissing) {
           // no R, fill with zero
           .... details
        }
  }

[jstone-lucasfilm]

@meshula I'd recommend that we take this update in two steps, first aligning the specification and codebase for MaterialX, and then updating both in tandem to propose a new expected behavior for image nodes. If that sounds fine to you, then I'll merge this existing change, and add a new GitHub Issue for the additional change proposed here.

[jstone-lucasfilm]

I've added a corresponding GitHub Issue for this proposal here:

#1797