Fix edge case for SdrShaderNodeMetadata items whose empty strings indicate the item should be cleared.

(Internal change: 2393119)

Author: anwang2009

pxr/usd/sdr/CMakeLists.txt

@@ -98,6 +98,17 @@ pxr_build_test_shared_lib(TestSdrRegistry
         testenv/TestSdrRegistry_oslParserPlugin.cpp
 )
 
+pxr_build_test(testSdrShaderMetadata
+    LIBRARIES
+        gf
+        sdf
+        sdr
+        tf
+        vt
+    CPPFILES
+        testenv/testSdrShaderMetadata.cpp
+)
+
 pxr_build_test(testSdrParseValue
     LIBRARIES
         gf
@@ -109,6 +120,11 @@ pxr_build_test(testSdrParseValue
         testenv/testSdrParseValue.cpp
 )
 
+pxr_register_test(testSdrShaderMetadata
+    COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testSdrShaderMetadata"
+    EXPECTED_RETURN_CODE 0
+)
+
 pxr_register_test(testSdrParseValue
     COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testSdrParseValue"
     EXPECTED_RETURN_CODE 0

pxr/usd/sdr/shaderNodeMetadata.cpp

@@ -180,6 +180,22 @@ SdrShaderNodeMetadata::SdrShaderNodeMetadata(const SdrTokenMap& legacyMetadata)
     }
 }
 
+SdrShaderNodeMetadata::SdrShaderNodeMetadata(const VtDictionary& items) {
+    // Run SetItem on each metadata item because some items may have
+    // exclusions from _HasSetItemExclusion.
+    for (const auto& kv : items) {
+        SetItem(TfToken(kv.first), kv.second);
+    }
+}
+
+SdrShaderNodeMetadata::SdrShaderNodeMetadata(VtDictionary&& items) {
+    // Run SetItem on each metadata item because some items may have
+    // exclusions from _HasSetItemExclusion.
+    for (const auto& kv : items) {
+        SetItem(TfToken(kv.first), kv.second);
+    }
+}
+
 bool
 SdrShaderNodeMetadata::HasItem(const TfToken& key) const
 {

pxr/usd/sdr/shaderNodeMetadata.h

@@ -94,11 +94,11 @@ class SdrShaderNodeMetadata
         const std::initializer_list<std::pair<TfToken, std::string>>& init
     ): SdrShaderNodeMetadata(_LegacyCtorFromInitializer(init)) {}
 
-    explicit SdrShaderNodeMetadata(const VtDictionary& items)
-        : _items(items) {}
+    SDR_API
+    explicit SdrShaderNodeMetadata(const VtDictionary& items);
 
-    explicit SdrShaderNodeMetadata(VtDictionary&& items)
-        : _items(std::move(items)) {}
+    SDR_API
+    explicit SdrShaderNodeMetadata(VtDictionary&& items);
 
     SdrShaderNodeMetadata() {}
 

pxr/usd/sdr/testenv/testSdrShaderMetadata.cpp

@@ -0,0 +1,110 @@
+//
+// Copyright 2026 Pixar
+//
+// Licensed under the terms set forth in the LICENSE.txt file available at
+// https://openusd.org/license.
+//
+
+#include "pxr/pxr.h"
+#include "pxr/base/vt/dictionary.h"
+#include "pxr/base/vt/value.h"
+#include "pxr/usd/sdr/declare.h"
+#include "pxr/usd/sdr/shaderNodeMetadata.h"
+
+PXR_NAMESPACE_USING_DIRECTIVE
+
+void
+TestNodeLabel()
+{
+    // Test the typical behavior for a token-valued metadata item
+    SdrShaderNodeMetadata m;
+    TF_VERIFY(!m.HasLabel());
+    TF_VERIFY(!m.HasItem(SdrNodeMetadata->Label));
+    m.SetItem(SdrNodeMetadata->Label, TfToken("foo"));
+    TF_VERIFY(m.HasLabel());
+    TF_VERIFY(m.HasItem(SdrNodeMetadata->Label));
+    TF_VERIFY(m.GetLabel() == TfToken("foo"));
+    TF_VERIFY(m.GetItemValueAs<TfToken>(SdrNodeMetadata->Label)
+              == m.GetLabel());
+    TF_VERIFY(m.GetItemValue(SdrNodeMetadata->Label)
+              == VtValue(TfToken("foo")));
+    m.SetItem(SdrNodeMetadata->Label, TfToken(""));
+    TF_VERIFY(m.HasLabel());
+    TF_VERIFY(m.HasItem(SdrNodeMetadata->Label));
+    m.ClearLabel();
+    TF_VERIFY(!m.HasLabel());
+
+    // Test that ingestion carries over the label value
+    VtDictionary d;
+    d[SdrNodeMetadata->Label] = TfToken("");
+    m = SdrShaderNodeMetadata(std::move(d));
+    TF_VERIFY(m.HasLabel());
+    TF_VERIFY(m.HasItem(SdrNodeMetadata->Label));
+    TF_VERIFY(m.GetItemValue(SdrNodeMetadata->Label) == VtValue(TfToken("")));
+}
+
+void
+TestNodeRole()
+{
+    // Test that an empty Token value clears Role. This is a
+    // special behavior of Role that doesn't apply to other token
+    // valued metadata.
+    SdrShaderNodeMetadata m;
+    TF_VERIFY(!m.HasRole());
+    TF_VERIFY(!m.HasItem(SdrNodeMetadata->Role));
+    m.SetRole(TfToken());
+    TF_VERIFY(!m.HasRole());
+    TF_VERIFY(!m.HasItem(SdrNodeMetadata->Role));
+    m.SetRole(TfToken("hi"));
+    TF_VERIFY(m.HasRole());
+    TF_VERIFY(m.HasItem(SdrNodeMetadata->Role));
+    m.SetRole(TfToken());
+    TF_VERIFY(!m.HasRole());
+    TF_VERIFY(!m.HasItem(SdrNodeMetadata->Role));
+
+    // Test that ingesting an empty TfToken Role means that
+    // the metadata "doesn't have" a Role item.
+    VtDictionary d;
+    d[SdrNodeMetadata->Role] = TfToken("");
+    m = SdrShaderNodeMetadata(d);
+    TF_VERIFY(!m.HasRole());
+    TF_VERIFY(!m.HasItem(SdrNodeMetadata->Role));
+    // "Get" returns the default constructed type
+    TF_VERIFY(m.GetRole() == TfToken());
+    TF_VERIFY(m.GetItemValue(SdrNodeMetadata->Role) == VtValue());
+
+    // Test the positive ingestion case
+    d[SdrNodeMetadata->Role] = TfToken("hi");
+    m = SdrShaderNodeMetadata(d);
+    TF_VERIFY(m.HasRole());
+    TF_VERIFY(m.HasItem(SdrNodeMetadata->Role));
+}
+
+void
+TestNodeOpenPages()
+{
+    // Tests the typical behavior for a metadata item with a complex type
+    SdrShaderNodeMetadata m;
+    m.SetItem(SdrNodeMetadata->OpenPages,
+              SdrTokenVec({TfToken("foo"), TfToken("bar")}));
+    TF_VERIFY(m.HasOpenPages());
+    TF_VERIFY(m.GetOpenPages().size() == 2);
+
+    // Test clearing the item
+    m.ClearItem(SdrNodeMetadata->OpenPages);
+    TF_VERIFY(!m.HasOpenPages());
+    TF_VERIFY(m.GetOpenPages().size() == 0);
+}
+
+void
+TestSdrShaderNodeMetadata()
+{
+    TestNodeLabel();
+    TestNodeRole();
+    TestNodeOpenPages();
+}
+
+int main()
+{
+    TestSdrShaderNodeMetadata();
+}