MAYA-108707 Share shader effect among similar-enough material networks #1043

huidong-chen · 2021-01-07T05:07:10Z

Replace the authored node paths with simplified paths in the form of "node#".
By doing so we will be able to reuse shader effects among material networks
which have the same node identifiers and relationships but different node
paths, reduce shader compilation overhead and enable material consolidation
for faster rendering.

Replace the authored node paths with simplified paths in the form of "node#". By doing so we will be able to reuse shader effects among material networks which have the same node identifiers and relationships but different node paths, reduce shader compilation overhead and enable material consolidation for faster rendering.

huidong-chen · 2021-01-07T05:14:23Z

lib/mayaUsd/render/vp2RenderDelegate/material.cpp

-//! - Add passthrough nodes to read vector component(s).
-//! - Fix UsdImagingMaterialAdapter issue for not producing primvar requirements.
-//! - Temporary workaround of missing support for normal map.
-void _ApplyVP2Fixes(HdMaterialNetwork& outNet, const HdMaterialNetwork& inNet)


The method is moved down into the class section because it is now a member of HdVP2Material. I need to do so for accessing new member of the class.

huidong-chen · 2021-01-07T05:15:55Z

lib/mayaUsd/render/vp2RenderDelegate/material.cpp

-
-        // We use concise relative paths to generate the same string for duplicate material
-        // networks.
-        const SdfPathVector conciseRelativePaths = SdfPath::GetConciseRelativePaths(paths);


Now that we've created the simplified paths which have only one unique token, we don't need to worry about name collision inside a material network so we can remove the part of the code here.

huidong-chen · 2021-01-07T05:21:45Z

lib/mayaUsd/render/vp2RenderDelegate/material.cpp

@@ -497,6 +385,9 @@ _LoadUdimTexture(const std::string& path, bool& isColorSpaceSRGB, MFloatArray& u
 MHWRender::MTexture*
 _LoadTexture(const std::string& path, bool& isColorSpaceSRGB, MFloatArray& uvScaleOffset)
 {
+    MProfilingScope profilingScope(


Adding some profiler event to help identifying bottlenecks.

In the NV attic scene, the biggest bottleneck is texture loading actually which takes 54 seconds. The optimization of this commit reduces material translation and compilation, improves consolidation, but won't help with texture loading overhead. The first frame drops from the original 70s to 54s, while HdSt takes only 30+ seconds. I think we need to understand why texture loading is slower than HdSt, maybe it is a task for next release.

FWIW, NV Attic scene contains 188 4k textures and a dozen of smaller textures.

huidong-chen · 2021-01-07T05:23:16Z

lib/mayaUsd/render/vp2RenderDelegate/material.cpp

+    size_t nodeCounter = 0;
+
+    _nodePathMap.clear();
+    _nodePathMap.reserve(numNodes);
+
+    HdMaterialNetwork tmpNet;
+    tmpNet.nodes.reserve(numNodes);
+    tmpNet.relationships.reserve(numRelationships);
+
+    // Replace the authored node paths with simplified paths in the form of "node#". By doing so
+    // we will be able to reuse shader effects among material networks which have the same node
+    // identifiers and relationships but different node paths, reduce shader compilation overhead
+    // and enable material consolidation for faster rendering.
+    for (const HdMaterialNode& node : inNet.nodes) {
+        tmpNet.nodes.push_back(node);
+
+        HdMaterialNode& outNode = tmpNet.nodes.back();
+        outNode.path = SdfPath(kNodePathPrefix + std::to_string(++nodeCounter));
+
+        _nodePathMap[node.path] = outNode.path;
+    }
+
+    // Update the relationships to use the new node paths.
+    for (const HdMaterialRelationship& rel : inNet.relationships) {
+        tmpNet.relationships.push_back(rel);
+
+        HdMaterialRelationship& outRel = tmpNet.relationships.back();
+        outRel.inputId = _nodePathMap[outRel.inputId];
+        outRel.outputId = _nodePathMap[outRel.outputId];
+    }


These lines are new code for the optimization.

I am still running more tests. Please challenge the code to catch bugs early.

kxl-adsk · 2021-01-07T19:04:11Z

lib/mayaUsd/render/vp2RenderDelegate/material.h

    MHWRender::MShaderInstance* _CreateShaderInstance(const HdMaterialNetwork& mat);
    void                        _UpdateShaderInstance(const HdMaterialNetwork& mat);
    const HdVP2TextureInfo&     _AcquireTexture(const std::string& path);

    HdVP2RenderDelegate* const
        _renderDelegate; //!< VP2 render delegate for which this material was created

+    std::unordered_map<SdfPath, SdfPath, SdfPath::Hash>


How long is this container needed during the lifetime of the material? In other words, does it have to be a member?

It is needed during the whole lifetime of the material. Each material needs the mapping info to find the converted node names which are used to get fragment parameters.

huidong-chen · 2021-01-08T01:54:10Z

@kxl-adsk Jerry is probably busy for his work and won't be able to review. I've done more testing and marked this ready for merge.

JGamache-autodesk · 2021-01-08T13:48:00Z

lib/mayaUsd/render/vp2RenderDelegate/material.cpp

+        tmpNet.nodes.push_back(node);
+
+        HdMaterialNode& outNode = tmpNet.nodes.back();
+        outNode.path = SdfPath(kNodePathPrefix + std::to_string(++nodeCounter));


That is a very good idea as it will collapse networks with similar topologies and is really safe to use in the case of UsdPreviewSurface since all nodes have dissimilar parameter names and will probably never result in ambiguity.

However, for MaterialX, where we can have many math nodes where all the inputs are called "input1" and " input2" and all outputs called "out" you will merge together networks that differ only by the use of an "add" instead of a "multiply" node. The fix is actually simple. Instead of using a single "node" string + number, you should be using info:id + number.

Good point. I was actually doing the approach with identifier + number and thought it was not necessary for UsdShade material. The change is very small, so we can do it when we implement MaterialX.

Hmm... Second thought. I can do it now because it is actually also safer than the "node" prefix.

Yes, we should not defer this work.

The follow-up of this conversation is at #1048 .

huidong-chen assigned ecp-maya-devops-adsk Jan 7, 2021

huidong-chen requested review from kxl-adsk, williamkrick and JGamache-autodesk January 7, 2021 05:07

clang-format change

0c5ba59

huidong-chen commented Jan 7, 2021

View reviewed changes

huidong-chen assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Jan 7, 2021

huidong-chen linked an issue Jan 7, 2021 that may be closed by this pull request

[MAYA-105968] Nvidia attic stage - super slow to load #460

Closed

huidong-chen mentioned this pull request Jan 7, 2021

[MAYA-105968] Nvidia attic stage - super slow to load #460

Closed

williamkrick approved these changes Jan 7, 2021

View reviewed changes

kxl-adsk approved these changes Jan 7, 2021

View reviewed changes

huidong-chen added the ready-for-merge Development process is finished, PR is ready for merge label Jan 8, 2021

kxl-adsk merged commit 8daa8d7 into dev Jan 8, 2021

kxl-adsk deleted the chenh/MAYA-108707/reuse-shader-effect-for-similar-enough-material-networks branch January 8, 2021 11:16

JGamache-autodesk reviewed Jan 8, 2021

View reviewed changes

huidong-chen mentioned this pull request Jan 8, 2021

MAYA-108707 Making shader reuse safer #1048

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAYA-108707 Share shader effect among similar-enough material networks #1043

MAYA-108707 Share shader effect among similar-enough material networks #1043

huidong-chen commented Jan 7, 2021

huidong-chen Jan 7, 2021

huidong-chen Jan 7, 2021

huidong-chen Jan 7, 2021

huidong-chen Jan 7, 2021

huidong-chen Jan 7, 2021

huidong-chen Jan 7, 2021

kxl-adsk Jan 7, 2021

huidong-chen Jan 7, 2021

huidong-chen commented Jan 8, 2021

JGamache-autodesk Jan 8, 2021

huidong-chen Jan 8, 2021

huidong-chen Jan 8, 2021

kxl-adsk Jan 8, 2021

huidong-chen Jan 8, 2021

MAYA-108707 Share shader effect among similar-enough material networks #1043

MAYA-108707 Share shader effect among similar-enough material networks #1043

Conversation

huidong-chen commented Jan 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huidong-chen commented Jan 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment