Moving SplitShaderIdentifier from UsdShadeShaderDefUtils down to Ndr …

…as NdrFsHelpersSplitShaderIdentifier.

The intention is to make this usable as a default by NdrFsHelpersDiscoverNodes in an upcoming change.

(Internal change: 2216700)
(Internal change: 2216912)

pxr/usd/ndr/filesystemDiscoveryHelpers.cpp

@@ -31,11 +31,10 @@
 #include "pxr/usd/ndr/filesystemDiscoveryHelpers.h"
 
 #include <algorithm>
+#include <cctype>
 
 PXR_NAMESPACE_OPEN_SCOPE
 
-namespace {
-
 // Examines the specified set of files, and determines if any of the files
 // are candidates for being parsed into a node. If a file is determined
 // to be a candidate, it is appended to \p foundNodes and
@@ -47,7 +46,7 @@ namespace {
 // \param[in]  dirFileNames The file names in the \p dirPath dir to test
 // \return `true` if the search should continue on to other paths in the
 //         search path
-bool
+static bool
 FsHelpersExamineFiles(
     NdrNodeDiscoveryResultVec* foundNodes,
     NdrStringSet* foundNodesWithTypes,
@@ -116,7 +115,74 @@ FsHelpersExamineFiles(
     return true;
 }
 
-} // anonymous namespace
+static bool 
+_IsNumber(const std::string& s)
+{
+    return !s.empty() &&
+        std::find_if(s.begin(), s.end(),
+                     [](unsigned char c) { return !std::isdigit(c); })
+        == s.end();
+}
+
+bool
+NdrFsHelpersSplitShaderIdentifier(
+    const TfToken &identifier, 
+    TfToken *family,
+    TfToken *name,
+    NdrVersion *version)
+{
+    const std::vector<std::string> tokens = 
+        TfStringTokenize(identifier.GetString(), "_");
+
+    if (tokens.empty()) {
+        return false;
+    }
+
+    *family = TfToken(tokens[0]);
+
+    if (tokens.size() == 1) {
+        *family = identifier;
+        *name = identifier;
+        *version = NdrVersion();
+        return true;
+    }
+
+    if (tokens.size() == 2) {
+        if (_IsNumber(tokens.back())) {
+            const int major = std::stoi(tokens.back());
+            *version = NdrVersion(major);
+            *name = *family;
+        } else {
+            *version = NdrVersion();
+            *name = identifier;
+        }
+        return true;
+    } 
+
+    const bool lastTokenIsNumber = _IsNumber(*(tokens.end() - 1));
+    const bool penultimateTokenIsNumber = _IsNumber(*(tokens.end() - 2));
+
+    if (penultimateTokenIsNumber) {
+        if (!lastTokenIsNumber) {
+            TF_WARN("Invalid shader identifier '%s'.", identifier.GetText()); 
+            return false;
+        }
+        // Has a major and minor version
+        *version = NdrVersion(std::stoi(*(tokens.end() - 2)), 
+                              std::stoi(*(tokens.end() - 1)));
+        *name = TfToken(TfStringJoin(tokens.begin(), tokens.end() - 2, "_"));
+    } else if (lastTokenIsNumber) {
+        // Has just a major version
+        *version = NdrVersion(std::stoi(tokens[tokens.size()-1]));
+        *name = TfToken(TfStringJoin(tokens.begin(), tokens.end() - 1, "_"));
+    } else {
+        // No version information is available. 
+        *name = identifier;
+        *version = NdrVersion();
+    }
+
+    return true;
+}
 
 NdrNodeDiscoveryResultVec
 NdrFsHelpersDiscoverNodes(

pxr/usd/ndr/filesystemDiscoveryHelpers.h

@@ -43,6 +43,31 @@ class NdrDiscoveryPluginContext;
 /// in a large chunk of the functionality.
 ///
 
+/// Given a shader's \p identifier token, computes the corresponding 
+/// NdrNode's family name, implementation name and shader version 
+/// (as NdrVersion).
+/// 
+/// * \p family is the prefix of \p identifier up to and not 
+/// including the first underscore. 
+/// * \p version is the suffix of \p identifier comprised of one or 
+/// two integers representing the major and minor version numbers.
+/// * \p name is the string we get by joining 
+/// <i>family</i> with everything that's in between <i>family</i> 
+/// and <i>version</i> with an underscore.
+/// 
+/// Returns true if \p identifier is valid and was successfully split 
+/// into the different components. 
+/// 
+/// \note The python version of this function returns a tuple containing
+/// (famiyName, implementationName, version).
+NDR_API 
+bool
+NdrFsHelpersSplitShaderIdentifier(
+    const TfToken &identifier, 
+    TfToken *family,
+    TfToken *name,
+    NdrVersion *version);
+
 /// Walks the specified search paths, optionally following symlinks. Paths
 /// are walked recursively, and each directory has `FsHelpersExamineFiles()`
 /// called on it. Only files that match one of the provided extensions (case

pxr/usd/ndr/testenv/testNdrFilesystemDiscovery.py

@@ -32,7 +32,7 @@
 os.environ["PXR_NDR_FS_PLUGIN_SEARCH_PATHS"] = os.getcwd()
 os.environ["PXR_NDR_FS_PLUGIN_ALLOWED_EXTS"] = "oso:args"
 
-from pxr.Ndr import _FilesystemDiscoveryPlugin
+from pxr import Ndr
 
 class TestNdrFilesystemDiscovery(unittest.TestCase):
     def test_NdrFilesystemDiscovery(self):
@@ -41,8 +41,8 @@ def test_NdrFilesystemDiscovery(self):
         directories and nodes with the same name.
         """
 
-        fsPlugin = _FilesystemDiscoveryPlugin()
-        context = _FilesystemDiscoveryPlugin.Context()
+        fsPlugin = Ndr._FilesystemDiscoveryPlugin()
+        context = Ndr._FilesystemDiscoveryPlugin.Context()
         discoveryResults = fsPlugin.DiscoverNodes(context)
         discoveredNodeNames = [result.name for result in discoveryResults]
 
@@ -52,5 +52,24 @@ def test_NdrFilesystemDiscovery(self):
             "TestNodeSameName"
         }
 
+    def test_testSplitShaderIdentifier(self):
+        self.assertEqual(
+            Ndr.FsHelpersSplitShaderIdentifier('Primvar'),
+            ('Primvar', 'Primvar', Ndr.Version()))
+        self.assertEqual(
+            Ndr.FsHelpersSplitShaderIdentifier('Primvar_float2'),
+            ('Primvar', 'Primvar_float2', Ndr.Version()))
+        self.assertEqual(
+            Ndr.FsHelpersSplitShaderIdentifier('Primvar_float2_3'),
+            ('Primvar', 'Primvar_float2', Ndr.Version(3, 0)))
+        self.assertEqual(
+            Ndr.FsHelpersSplitShaderIdentifier('Primvar_float_3_4'),
+            ('Primvar', 'Primvar_float', Ndr.Version(3, 4)))
+
+        self.assertIsNone(
+            Ndr.FsHelpersSplitShaderIdentifier('Primvar_float2_3_nonNumber'))
+        self.assertIsNone(
+            Ndr.FsHelpersSplitShaderIdentifier('Primvar_4_nonNumber'))
+
 if __name__ == '__main__':
     unittest.main()

pxr/usd/ndr/wrapFilesystemDiscoveryHelpers.cpp

@@ -27,14 +27,13 @@
 #include "pxr/base/tf/weakPtr.h"
 
 #include <boost/python/def.hpp>
+#include <boost/python/tuple.hpp>
 
 using namespace boost::python;
 
 PXR_NAMESPACE_USING_DIRECTIVE
 
-namespace {
-
-NdrNodeDiscoveryResultVec
+static NdrNodeDiscoveryResultVec
 _WrapFsHelpersDiscoverNodes(
     const NdrStringVec& searchPaths,
     const NdrStringVec& allowedExtensions,
@@ -47,10 +46,24 @@ _WrapFsHelpersDiscoverNodes(
                                      boost::get_pointer(context));
 }
 
+static object
+_WrapFsHelpersSplitShaderIdentifier(const TfToken &identifier)
+{
+    TfToken family;
+    TfToken name;
+    NdrVersion version;
+    if (NdrFsHelpersSplitShaderIdentifier(identifier,
+            &family, &name, &version)) {
+        return boost::python::make_tuple(family, name, version);
+    } else {
+        return object();
+    }
 }
 
 void wrapFilesystemDiscoveryHelpers()
 {
+    def("FsHelpersSplitShaderIdentifier", _WrapFsHelpersSplitShaderIdentifier,
+        arg("identifier"));
     def("FsHelpersDiscoverNodes", _WrapFsHelpersDiscoverNodes,
         (args("searchPaths"),
         args("allowedExtensions"),

pxr/usd/usdShade/shaderDefUtils.cpp

@@ -29,6 +29,8 @@
 
 #include "pxr/usd/ar/resolver.h"
 
+#include "pxr/usd/ndr/filesystemDiscoveryHelpers.h"
+
 #include "pxr/usd/sdf/assetPath.h"
 #include "pxr/usd/sdf/schema.h"
 #include "pxr/usd/sdf/types.h"
@@ -50,72 +52,6 @@ TF_DEFINE_PRIVATE_TOKENS(
     (implementationName)
 );
 
-static bool _IsNumber(const std::string& s)
-{
-    return !s.empty() && 
-        std::find_if(s.begin(), s.end(), 
-                     [](unsigned char c) { return !std::isdigit(c); })
-        == s.end();
-}
-
-/* static */ 
-bool
-UsdShadeShaderDefUtils::SplitShaderIdentifier(
-    const TfToken &identifier, 
-    TfToken *familyName,
-    TfToken *implementationName,
-    NdrVersion *version)
-{
-   std::vector<std::string> tokens = TfStringTokenize(identifier.GetString(), 
-        "_");
-
-    if (tokens.empty()) {
-        return false;
-    }
-
-    *familyName = TfToken(tokens[0]);
-
-    if (tokens.size() == 1) {
-        *familyName = identifier;
-        *implementationName = identifier;
-        *version = NdrVersion();
-    } else if (tokens.size() == 2) {
-        if (_IsNumber(tokens[tokens.size()-1])) {
-            int major = std::stoi(*tokens.rbegin());
-            *version = NdrVersion(major);
-            *implementationName = *familyName;
-        } else {
-            *version = NdrVersion();
-            *implementationName = identifier;
-        }
-    } else if (tokens.size() > 2) {
-        bool lastTokenIsNumber = _IsNumber(tokens[tokens.size()-1]);
-        bool penultimateTokenIsNumber = _IsNumber(tokens[tokens.size()-2]);
-
-        if (penultimateTokenIsNumber && !lastTokenIsNumber) {
-            TF_WARN("Invalid shader identifier '%s'.", identifier.GetText()); 
-            return false;
-        }
-
-        if (lastTokenIsNumber && penultimateTokenIsNumber) {
-            *version = NdrVersion(std::stoi(tokens[tokens.size()-2]), 
-                                  std::stoi(tokens[tokens.size()-1]));
-            *implementationName = TfToken(TfStringJoin(tokens.begin(), 
-                tokens.begin() + (tokens.size() - 2), "_"));
-        } else if (lastTokenIsNumber) {
-            *version = NdrVersion(std::stoi(tokens[tokens.size()-1]));
-            *implementationName  = TfToken(TfStringJoin(tokens.begin(), 
-                tokens.begin() + (tokens.size() - 1), "_"));
-        } else {
-            // No version information is available. 
-            *implementationName = identifier;
-            *version = NdrVersion();
-        }
-    }
-
-    return true;
-}
-
 /* static */
 NdrNodeDiscoveryResultVec 
 UsdShadeShaderDefUtils::GetNodeDiscoveryResults(
@@ -137,7 +73,7 @@ UsdShadeShaderDefUtils::GetNodeDiscoveryResults(
     TfToken family;
     TfToken name; 
     NdrVersion version; 
-    if (!SplitShaderIdentifier(shaderDefPrim.GetName(), 
+    if (!NdrFsHelpersSplitShaderIdentifier(shaderDefPrim.GetName(), 
                 &family, &name, &version)) {
         // A warning has already been issued by SplitShaderIdentifier.
         return result;

pxr/usd/usdShade/shaderDefUtils.h

@@ -43,29 +43,6 @@ class UsdShadeShader;
 ///
 class UsdShadeShaderDefUtils {
 public:
-    /// Given a shader's \p identifier token, computes the corresponding 
-    /// SdrShaderNode's family name, implementation name and shader version 
-    /// (as NdrVersion).
-    /// 
-    /// * \p familyName is the prefix of \p identifier up to and not 
-    /// including the first underscore. 
-    /// * \p version is the suffix of \p identifier comprised of one or 
-    /// two integers representing the major and minor version numbers.
-    /// * \p implementationName is the string we get by joining 
-    /// <i>familyName</i> with everything that's in between <i>familyName</i> 
-    /// and <i>version</i> with an underscore.
-    /// 
-    /// Returns true if \p identifier is valid and was successfully split 
-    /// into the different components. 
-    /// 
-    /// \note The python version of this function returns a tuple containing
-    /// (famiyName, implementationName, version).
-    USDSHADE_API
-    static bool SplitShaderIdentifier(const TfToken &identifier, 
-                TfToken *familyName,
-                TfToken *implementationName,
-                NdrVersion *version);
-
     /// Returns the list of NdrNodeDiscoveryResult objects that must be added 
     /// to the shader registry for the given shader \p shaderDef, assuming it 
     /// is found in a shader definition file found by an Ndr discovery plugin. 

pxr/usd/usdShade/testenv/testUsdShadeShaderDef.py

@@ -29,20 +29,6 @@
 import os
 
 class TestUsdShadeShaderDef(unittest.TestCase):
-    def test_testSplitShaderIdentifier(self):
-        SSI = UsdShade.ShaderDefUtils.SplitShaderIdentifier
-        self.assertEqual(SSI('Primvar'), 
-                ('Primvar', 'Primvar', Ndr.Version()))
-        self.assertEqual(SSI('Primvar_float2'), 
-                ('Primvar', 'Primvar_float2', Ndr.Version()))
-        self.assertEqual(SSI('Primvar_float2_3'), 
-                ('Primvar', 'Primvar_float2', Ndr.Version(3, 0)))
-        self.assertEqual(SSI('Primvar_float_3_4'), 
-                ('Primvar', 'Primvar_float', Ndr.Version(3, 4)))
-
-        self.assertIsNone(SSI('Primvar_float2_3_nonNumber'))
-        self.assertIsNone(SSI('Primvar_4_nonNumber'))
-
     def test_ShaderDefParser_NodeDefAPI(self):
         # Test the NodeDefAPI path.
         self.test_ShaderDefParser(useForwardedAPI=False)

pxr/usd/usdShade/wrapShaderDefUtils.cpp

@@ -37,29 +37,9 @@ using namespace boost::python;
 
 PXR_NAMESPACE_USING_DIRECTIVE
 
-namespace {
-
-static object 
-_WrapSplitShaderIdentifier(const TfToken &identifier)
-{
-    TfToken familyName, shaderName;
-    NdrVersion version;
-    if (UsdShadeShaderDefUtils::SplitShaderIdentifier(identifier, 
-            &familyName, &shaderName, &version)) {
-        return boost::python::make_tuple(familyName, shaderName, version);
-    } else {
-        return object();
-    }
-}
-
-}
-
 void wrapUsdShadeShaderDefUtils()
 {
     scope thisScope = class_<UsdShadeShaderDefUtils>("ShaderDefUtils", no_init)
-        .def("SplitShaderIdentifier", _WrapSplitShaderIdentifier,
-            arg("identifier"))
-        .staticmethod("SplitShaderIdentifier")
         .def("GetNodeDiscoveryResults", 
              &UsdShadeShaderDefUtils::GetNodeDiscoveryResults,
              (arg("shaderDef"), arg("sourceUri")),