Incorrect texture display could happen when TextureDescriptor.texturePrim is used as Hash in Storm's HdStMaterial::Sync(...)

[lilike-adsk]

Description of Issue

There's a performance optimization in HdStMaterial::Sync(), where the texture prim path is used as the hash code when HdStMaterial is already initialized (_isInitialized is true), see https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/imaging/hdSt/material.cpp#L159, however, this optimization can cause problem in some workflows as not like texture file path, the texture prim path can easily collide with others.

// Note about batching hashes:
        // If this is our first sync, try to hash using the asset path.
        // If we're on our 2nd+ sync, just use the texture prim path.
        //
        // This will aggressively batch textured prims together as long as
        // they are 100% static; if they are dynamic, we assume that the
        // textures are dynamic, and we split the batches of textured prims.
        //
        // This tries to balance two competing concerns:
        // 1.) Static textured simple geometry (like billboard placeholders)
        //     really need to be batched together; otherwise, the render
        //     cost is dominated by the switching cost between batches.
        // 2.) Objects with animated textures will change their texture hash
        //     every frame.  If the hash is based on asset path, we need to
        //     rebuild batches every frame, which is untenable. If it's
        //     based on scene graph path (meaning split into its own batch),
        //     we don't need to update batching.
        //
        // Better batch invalidation (i.e., not global) would really help
        // us here, as would hints from the scene about how likely the
        // textures are to change.  Maybe in the future...
        
        texturesFromStorm->push_back(
            { desc.name,
              desc.type,
              textureHandle,
              _isInitialized
                  ? hash_value(desc.texturePrim)
                  : _GetTextureHandleHash(textureHandle) });

Steps to Reproduce

One problematic case we're having:

E.g., there're lots of texture prim named as /full_color_texture in the ALab scene, when different USD prims with materials refer to its own /full_color_texture are put together into the scene, and the problem happens when we do "texture off" and "texture on" mode switch on the scene:

1. When switch to "texture off" mode, we dirty the scene by disconnect texture inputs nodes of all materials via HdDataSourceMaterialNetworkInterface::DeleteNodeInputConnection(), then some prims might be drawn in a batch now as the texture inputs are disconnected.
2. When we switch to "texture on" mode by reconnect texture inputs nodes of all materials, then in HdStMaterial::Sync(...), the previous batched draw prims are still not broken as now the texture prim path are all /full_color_texture, which produces same hash code due to that perf optimization code, and the drawing result is incorrect as those prims are actually refering to different texture files.

One proposal to fix is, when _isInitialized is true, adding another check on that TextureDescriptor to see if the old texture prim path is same as the current texture prim path, if yes, we can still use texturePrim as hash.

System Information (OS, Hardware)

Windows

Package Versions

USD 24.08

Build Flags

[jesschimein]

Filed as internal issue #USD-10420

[lilike-adsk]

Another solution might be use an absolute node path as TextureDescriptor.texturePrim? Currently, seems a relative node path is used at https://github.com/PixarAnimationStudios/OpenUSD/blob/release/pxr/imaging/hdSt/materialNetwork.cpp#L684