Autodesk: [hdSt] Early parallel MaterialX codegen, launched by a scene index #3847

erikaharrison-adsk · 2025-10-09T22:12:58Z

Description of Change(s)

This is a refactoring of #3567 incorporating the code review feedback, namely:

Removed the HdDirtyBits-based mechanism to check if the filter task cached when the codegen was launched can be reused during material sync. Instead, we always derive the necessary data again, which is a minor overhead relative to the codegen process itself. This can be further optimized in the future using an alternative mechanism, e.g. based on the change tracker as suggested in the original code review.
Removed the new render delegate APIs for starting the early codegen and waiting for its results. Instead, the Storm render delegate injects a Storm-specific scene index which launches the parallel tasks when it receives material primitives. The waiting is exposed via a method of the scene index class.

Link to proposal (if applicable)

N/A

Fixes Issue(s)

N/A

Checklist

I have created this PR based on the dev branch
I have followed the coding conventions
I have added unit tests that exercise this functionality (Reference:
testing guidelines)
I have verified that all unit tests pass with the proposed changes
I have submitted a signed Contributor License Agreement (Reference:
Contributor License Agreement instructions)

jesschimein · 2025-10-10T17:03:45Z

Filed as internal issue #USD-11537

(This is an automated message. See here for more information.)

ppenenko · 2025-10-14T19:45:14Z

pxr/imaging/hd/sceneIndexAdapterSceneDelegate.cpp

+HdSceneIndexAdapterSceneDelegate::GetMaterialResourceFromSceneIndexPrim(
+    HdSceneIndexPrim& prim, const TfTokenVector& renderContexts)
 {
-    TRACE_FUNCTION();
-    HF_MALLOC_TAG_FUNCTION();
-    HdSceneIndexPrim prim = _GetInputPrim(id);
-
    HdMaterialSchema matSchema = HdMaterialSchema::GetFromParent(
            prim.dataSource);
    if (!matSchema.IsDefined()) {
        return VtValue();
    }

    // Query for a material network to match the requested render contexts
-    const TfTokenVector renderContexts =
-        GetRenderIndex().GetRenderDelegate()->GetMaterialRenderContexts();
-    HdMaterialNetworkSchema netSchema = matSchema.GetMaterialNetwork(renderContexts);
+    HdMaterialNetworkSchema netSchema =
+        matSchema.GetMaterialNetwork(renderContexts);
    if (!netSchema.IsDefined()) {
        return VtValue();
    }

    return VtValue(_ToMaterialNetworkMap(netSchema, renderContexts));
 }


Extracted this static function for code reuse between the old synchronous code path and the new scene index which gets scene index prims without any conversions.

ppenenko · 2025-10-14T19:45:47Z

pxr/imaging/hdSt/CMakeLists.txt

    list(APPEND optionalPrivateClasses
         materialXFilter
         materialXShaderGen
+         materialXSyncSceneIndex


The new scene index implementation, which replaces materialXSyncDispatcher in the original PR.

ppenenko · 2025-10-14T19:46:32Z

pxr/imaging/hdSt/material.cpp

+            _hdStMaterialNetwork.ProcessMaterialNetwork(GetId(), hdNetworkMap,
                                                    resourceRegistry.get());
-            fragmentSource = _networkProcessor.GetFragmentCode();
-            volumeSource = _networkProcessor.GetVolumeCode();
-            displacementSource = _networkProcessor.GetDisplacementCode();
-            materialMetadata = _networkProcessor.GetMetadata();
-            materialTag = _networkProcessor.GetMaterialTag();
-            params = _networkProcessor.GetMaterialParams();
-                textureDescriptors = _networkProcessor.GetTextureDescriptors();
+            fragmentSource = _hdStMaterialNetwork.GetFragmentCode();
+            volumeSource = _hdStMaterialNetwork.GetVolumeCode();
+            displacementSource = _hdStMaterialNetwork.GetDisplacementCode();
+            materialMetadata = _hdStMaterialNetwork.GetMetadata();
+            materialTag = _hdStMaterialNetwork.GetMaterialTag();
+            params = _hdStMaterialNetwork.GetMaterialParams();
+                textureDescriptors = _hdStMaterialNetwork.GetTextureDescriptors();


I propose to rename _networkProcessor to _hdStMaterialNetwork. IMHO "Processor" sounds confusing here because the object _hdStMaterialNetwork is simply initialized with those Process methods from hdNetworkMap, it doesn't process some external objects.

pxr/imaging/hdSt/material.cpp

+#ifdef PXR_MATERIALX_SUPPORT_ENABLED
+    {
+        HdStRenderDelegate* stormDelegate = static_cast<HdStRenderDelegate*>(
+            sceneDelegate->GetRenderIndex().GetRenderDelegate());
+
+        if (HdSt_MaterialXSyncSceneIndex* sceneIndex =
+            stormDelegate->GetMaterialXSyncSceneIndex()) {
+
+            sceneIndex->Wait();
+        }
+    }
+#endif


ppenenko · 2025-10-14T19:50:11Z

pxr/imaging/hdSt/materialNetwork.cpp

+HdMaterialNode2 const*
+HdSt_GetTerminalNode(


Now need to call it from outside this source file.

ppenenko · 2025-10-14T19:51:06Z

pxr/imaging/hdSt/materialNetwork.cpp

+        return;
+    }
+
+    ProcessFilterTask(materialId, filterTask, isVolume, resourceRegistry);


ProcessMaterialNetwork is now implemented via ProcessFilterTask for code reuse.

ppenenko · 2025-10-14T19:52:46Z

pxr/imaging/hdSt/materialNetwork.cpp

+    auto filterTask = std::make_shared<HdSt_MaterialFilterTask>();
+    filterTask->hdNetwork =
+        HdConvertToHdMaterialNetwork2(hdNetworkMap, &isVolume);


The state necessary for the codegen process is now encapsulated in the HdSt_MaterialFilterTask class, because it needs to be created earlier than the material sprim and to persist until the parallel codegen task is finished.

HdStMaterialNetwork::ProcessMaterialNetwork is called in the old, synchronous code path, but we create a HdSt_MaterialFilterTask here in order to share code between the two code paths.

ppenenko · 2025-10-14T19:54:39Z

pxr/imaging/hdSt/materialXFilter.cpp

+void
+HdSt_MaterialFilterTask::AddFallbackDomeLightTextureNode()


This and a few other functions are now methods of HdSt_MaterialFilterTask.

ppenenko · 2025-10-14T19:55:09Z

pxr/imaging/hdSt/materialXFilter.cpp

    const SdfPath domeTexturePath = 
        terminalNodePath.ReplaceName(_tokens->domeLightFallback);
-    hdNetwork->nodes.insert({domeTexturePath, hdDomeTextureNode});
+    hdNetwork.nodes.insert({domeTexturePath, hdDomeTextureNode});


This and a few other related pieces of data are now encapsulated as members of HdSt_MaterialFilterTask.

ppenenko · 2025-10-14T19:55:55Z

pxr/imaging/hdSt/materialXFilter.cpp

+size_t
+HdSt_MaterialFilterTask::BuildAnonymizedMaterialNetwork(
+    HdMaterialNetwork2* anonNetwork)


Unified the terminology around "anonymized". The input parameters are replaced by HdSt_MaterialFilterTask's members.

ppenenko · 2025-10-14T19:56:53Z

pxr/imaging/hdSt/materialXFilter.cpp

-static mx::ShaderPtr
-_GenerateMaterialXShader(
-    HdMaterialNetwork2 const& hdNetwork,
+HdSt_MaterialXGeneratorTask::HdSt_MaterialXGeneratorTask(


This is a different kind of "task" - the necessary data passed to the lambda executing in parallel in TBB and wrapping the MaterialX codegen.

ppenenko · 2025-10-14T19:58:07Z

pxr/imaging/hdSt/materialXFilter.cpp

+    HdSt_MaterialXGeneratorTask generatorTask(
+        filterTask,
+        materialPath,
+        *resourceRegistry->GetHgi());


Creating the generator task on the stack in the case when parallel codegen is off.

ppenenko · 2025-10-14T19:58:56Z

pxr/imaging/hdSt/materialXFilter.cpp

+    return std::make_unique<HdSt_MaterialXGeneratorTask>(
+        filterTask,
+        materialPath,
+        hgi);


Creating the generator task on the heap for parallel codegen.

One important change from the original implementation is that the generator task now gets a shared pointer to the filter task and holds on to it. The generation process reads some material network data which is part of the filter task, so it's important to ensure that it's not destroyed before codegen is complete. In the original implementation, the sprim held ownership of the filter task for the duration of codegen.

ppenenko · 2025-10-14T20:00:34Z

pxr/imaging/hdSt/materialXSyncSceneIndex.cpp

@@ -0,0 +1,180 @@
+//


Replaces pxr/imaging/hdSt/materialXSyncDispatcher.cpp in the original implementation

ppenenko · 2025-10-14T20:01:32Z

pxr/imaging/hdSt/materialXSyncSceneIndex.cpp

+    if (resourceRegistry->ContainsMaterialXShader(
+        generatorTask->GetShaderHash())) {
+        // We already have a shader for this topology.
+        return;
+    }


First check that the shader hasn't been registered before the early parallel init. This is a new method which, unlike RegisterMaterialXShader, doesn't add a new instance to the registry if an existing one isn't found.

Reusing RegisterMaterialXShader in combination with IsFirstInstance would be problematic because the intended usage pattern, in the case when there's no existing value, seems to be:

Insert a nullptr value and return a registry instance pointing to that value and holding a lock to the whole MaterialX registry.

The client code is supposed to populate the instance with a non-nullptr value while holding the lock. The instance's IsFirstInstance method returns true if the value is nullptr.

So the above pattern can't be used for parallel tasks, when the uniqueness check has to happen before launching the task, the non-nullptr value is known only when the task completes and the lock has to be released ASAP.

Makes sense.

ppenenko · 2025-10-14T20:02:01Z

pxr/imaging/hdSt/materialXSyncSceneIndex.cpp

+    // Use a separate concurrent container for tracking unique generator tasks.
+    // The generated shader will be registered in the resource registry when
+    // the task completes.
+    auto insertResult =
+        _generatorTaskSet.insert(generatorTask->GetShaderHash());


Another uniqueness check, this time between the different parallel tasks, using a dedicated data structure.

ppenenko · 2025-10-14T20:02:33Z

pxr/imaging/hdSt/renderDelegate.cpp

+#ifdef PXR_MATERIALX_SUPPORT_ENABLED
+TF_DEFINE_ENV_SETTING(HDST_ENABLE_PARALLEL_MTLX_CODEGEN, false,
+                      "Enable early parallelized MaterialX codegen");
+#endif


Moved this setting definition here for better encapsulation.

ppenenko · 2025-10-14T20:03:28Z

pxr/imaging/hdSt/renderDelegate.cpp

+void
+HdStRenderDelegate::SetTerminalSceneIndex(
+    const HdSceneIndexBaseRefPtr &terminalSceneIndex)
+{
+    if (!TfGetEnvSetting(HDST_ENABLE_PARALLEL_MTLX_CODEGEN)) {
+        return;
+    }
+
+    _materialXSyncSceneIndex = HdSt_MaterialXSyncSceneIndexRefPtr(
+        new HdSt_MaterialXSyncSceneIndex(terminalSceneIndex, *this));
+}


Injecting our scene index as the very last in the chain, and only if the optimization is enabled.

ppenenko · 2025-10-14T20:04:30Z

pxr/imaging/hdSt/renderDelegate.cpp

+HdSt_MaterialXSyncSceneIndex*
+HdStRenderDelegate::GetMaterialXSyncSceneIndex()
+{
+    return get_pointer(_materialXSyncSceneIndex);
+}
+#endif


This is currently only used in HdStMaterial::Sync to wait for the parallel codegen, but could be used in the future to reuse some per-material state.

ppenenko · 2025-10-14T20:05:07Z

pxr/imaging/hdSt/resourceRegistry.cpp

+bool
+HdStResourceRegistry::ContainsMaterialXShader(
+        HdInstance<MaterialX::ShaderPtr>::ID id)
+{
+    bool found = false;
+    _materialXShaderRegistry.FindInstance(id, &found);
+    return found;
+}


The new method just wraps existing functionality.

ppenenko · 2025-10-15T13:59:03Z

FYI @tcauchois

…terial sync

…llel_mtlx_codegen_si

ppenenko · 2025-10-31T18:03:49Z

pxr/imaging/hdSt/material.cpp

+            // Wait for all early parallel codegen tasks to complete and
+            // retrieve the state cached for this sprim when codegen was
+            // started
+            filterTask = sceneIndex->WaitAndExtractFilterTask(GetId());


Wait for the codegen tasks to complete. Previously, we were waiting by calling a new render delegate API before syncing the materials: https://github.com/PixarAnimationStudios/OpenUSD/pull/3567/files#r1995705092

Now, we wait for the generator task group in each material's sync. In practice, only the first material sprim would ever wait for any measurable duration, and the overhead of waiting for all subsequent sprims is negligible (around 1 microsecond).

This is also where we get the cached per-material state from the scene index. The filter task is removed from the scene index its ownership is transferred to the local variable.

Reusing the filter task allows us to avoid getting the material resource again below.

Is it possible instead to have the material prim datasource call wait on the scene index, so that Storm doesn't need to deal with the new scene index at all? I'll try to sketch out a proposal in the scene index code.

ppenenko · 2025-10-31T18:04:41Z

pxr/imaging/hdSt/material.cpp

+                _hdStMaterialNetwork.ProcessMaterialNetwork(GetId(),
+                    hdNetworkMap, resourceRegistry.get());
+
+                processedMaterialNetwork = true;


Once the material network has processed the filter task, we don't need the task any more, so we destroy it by letting it go out of scope.

ppenenko · 2025-10-31T18:18:20Z

pxr/imaging/hdSt/materialXSyncSceneIndex.cpp

+    HD_TRACE_FUNCTION();
+
+    for (const HdSceneIndexObserver::DirtiedPrimEntry& entry : entries) {
+        // If the dirtied prim is a material for which we've cached a filter
+        // task, remove the task since it's now stale. Any ongoing generator 
+        // tasks will keep their own shared pointers to the respective filter 
+        // tasks, so this is safe. If the element is not in the map, then the 
+        // lookup won't involve locking.
+        _FilterTaskMap::accessor accessor;
+        if (_filterTaskMap.find(accessor, entry.primPath)) {
+            _filterTaskMap.erase(accessor);
+        }
+    }


Deleting the filter task on dirtying.

ppenenko · 2025-10-31T18:19:13Z

pxr/imaging/hdSt/materialXSyncSceneIndex.cpp

+        {
+            _FilterTaskMap::accessor accessor;
+            _filterTaskMap.insert(accessor, id);
+            accessor->second = generatorTask->GetFilterTask();


Caching the filter task in the scene index.

ppenenko · 2025-10-31T18:32:52Z

Description of Change(s)

This is a refactoring of #3567 incorporating the code review feedback, namely:

Removed the HdDirtyBits-based mechanism to check if the filter task cached when the codegen was launched can be reused during material sync. Instead, we always derive the necessary data again, which is a minor overhead relative to the codegen process itself. This can be further optimized in the future using an alternative mechanism, e.g. based on the change tracker as suggested in the original code review.

FYI @tcauchois @klucknav I've pushed another change to this PR reintroducing the reuse of filter tasks in HdStMaterial::Sync, but with invalidation implemented via a simpler mechanism based on scene index dirty notifications instead of dirty flags.

This addresses the other major feedback item from #3567.

To recap, filter tasks can become stale if the material changes after parallel codegen started but before the sprim is synced. Animated materials is the only known case of this, and USD has tests for animated materials.

This should be the last change in this branch.

tcauchois

Hey Pavlo, this is great progress! Moving the task launching to Hydra 2 PrimsAdded looks like it cut out a bunch of the hd API changes, and it should hopefully be letting you launch the tasks slightly earlier as well!

I feel like if we fold the WaitAndExtractFilterTask call into some kind of datasource access call inside MaterialXSyncSceneIndex we can additionally get rid of some of the new render delegate API in hdSt/renderDelegate.h, and make the scene index more modular/reusable, which I think a bunch of folks would appreciate past just the Storm team!

Happy to expand on my thoughts in the checkin tomorrow or on slack but this is definitely a good direction.

tcauchois · 2025-12-03T23:26:05Z

pxr/imaging/hdSt/material.cpp

+            // Wait for all early parallel codegen tasks to complete and
+            // retrieve the state cached for this sprim when codegen was
+            // started
+            filterTask = sceneIndex->WaitAndExtractFilterTask(GetId());


Is it possible instead to have the material prim datasource call wait on the scene index, so that Storm doesn't need to deal with the new scene index at all? I'll try to sketch out a proposal in the scene index code.

tcauchois · 2025-12-03T23:35:06Z

pxr/imaging/hdSt/materialNetwork.cpp

-    SdfPath surfTerminalPath;
-    if (HdMaterialNode2 const* surfTerminal = 
-            _GetTerminalNode(surfaceNetwork, terminalName, &surfTerminalPath)) {
+    filterTask->terminalNode = 


Here & material.*, instead of bringing the "filterTask" API into a bunch of files in Storm, you could modularize things better by either:
(1) having the scene index produce a "glsl" material network that can be read by the non-materialx code; or
(2) having the scene index produce a digested material network as code snippets for each stage & params, and pass them through as datasources. If material.cpp finds that, it uses it, and otherwise it falls back to network processing.

This splits the code up a little better, which is nice since the MaterialX code is an external dependency with an evolving API and hidden behind a build flag, so having a big API interface between the two makes me a bit worried.

tcauchois · 2025-12-03T23:36:11Z

pxr/imaging/hdSt/materialNetwork.h

+using HdSt_MaterialFilterTaskSharedPtr =
+    std::shared_ptr<HdSt_MaterialFilterTask>;
+
+extern HdMaterialNode2 const*


Don't think this needs an extern, but if you want it to be publicly visible (which maybe we don't care?) you could throw an HDST_API on it...

tcauchois · 2025-12-03T23:37:02Z

pxr/imaging/hdSt/materialNetwork.h

+/// synchronously, or on the heap, owned by the respective Sprim, if the
+/// codegen happens in parallel tasks.
+///
+struct ARCH_EXPORT_TYPE HdSt_MaterialFilterTask final


This feels like an implementation detail of the MaterialX code, and I don't think this belongs in hdSt/materialNetwork.h. Also curious why we need ARCH_EXPORT_TYPE here.

tcauchois · 2025-12-03T23:41:31Z

pxr/imaging/hdSt/materialXFilter.h

+    HdSt_MaterialXGeneratorTask(
+        HdSt_MaterialFilterTaskSharedPtr filterTask,
+        SdfPath const& materialPath,
+        Hgi const& hgi);


I know you're improving on the status quo here (which is to pass in resource registry and pull hgi out of that), but it would be cleaner to just pass in the relevant info, namely bindless support and shading language.

tcauchois · 2025-12-03T23:45:27Z

pxr/imaging/hdSt/materialXSyncSceneIndex.cpp

+    if (resourceRegistry->ContainsMaterialXShader(
+        generatorTask->GetShaderHash())) {
+        // We already have a shader for this topology.
+        return;
+    }


Makes sense.

tcauchois · 2025-12-03T23:49:46Z

pxr/imaging/hdSt/materialXSyncSceneIndex.cpp

+HdSceneIndexPrim
+HdSt_MaterialXSyncSceneIndex::GetPrim(const SdfPath &primPath) const
+{
+    // Just forward to input scene index - we don't modify prim data


I was envisioning that you call WaitAndExtractFilterTask here, either when you call GetPrim() on a materialx prim, or potentially when you call primDataSource->Get("material"). That way, you don't need all of the complicated logic in renderDelegate.h to pass the scene index pointer through.

tcauchois · 2025-12-03T23:54:56Z

pxr/imaging/hdSt/renderDelegate.cpp


+#ifdef PXR_MATERIALX_SUPPORT_ENABLED
+void
+HdStRenderDelegate::SetTerminalSceneIndex(


As mentioned in the scene index, this bit breaks a bunch of encapsulation. We need GetMaterialXSyncSceneIndex so we can call WaitAndExtractFilterTask in material.cpp, but if we do that in the scene index GetPrim() or a datasource instead we don't need this call anymore. Meanwhile, rather than grafting the new scene index onto the terminal scene index like this, the more flexible/preferred way is to register a scene index plugin that inserts it somewhere relative to other scene index filters; it's a small bit of boilerplate you can grab from e.g. the dependencySceneIndexPlugin.* or something.

Then the only issue is your scene index's use of the render delegate, but it shouldn't really be getting a pointer to the render delegate anyway. It does need a pointer to a registry of MaterialX codegen results, but we could either find a lower key way to pass in the resource registry, or just have a local registry on the scene index instead.

[hdSt] Early parallel MaterialX codegen, launched by a scene index

c521391

ppenenko reviewed Oct 14, 2025

View reviewed changes

erikaharrison-adsk marked this pull request as ready for review October 14, 2025 23:59

Fix leaking the MaterialX Sync Scene Index

e93d4af

erikaharrison-adsk mentioned this pull request Oct 24, 2025

Autodesk: [hdSt] Early parallel MaterialX codegen #3567

Closed

5 tasks

ppenenko added 4 commits October 30, 2025 11:56

Reuse early parallel codegen state, cached in filter tasks, during ma…

0ae0ac5

…terial sync

Merge remote-tracking branch 'origin/dev' into penenkp/AGPMAT-84/resolve

74ce797

Post-merge fixes - compiles

118f955

Merge branch 'penenkp/AGPMAT-84/resolve' into adsk/feature/early_para…

73c061d

…llel_mtlx_codegen_si

ppenenko reviewed Oct 31, 2025

View reviewed changes

tcauchois reviewed Dec 4, 2025

View reviewed changes

		void
		HdSt_MaterialFilterTask::AddFallbackDomeLightTextureNode()

Autodesk: [hdSt] Early parallel MaterialX codegen, launched by a scene index #3847

Are you sure you want to change the base?

Autodesk: [hdSt] Early parallel MaterialX codegen, launched by a scene index #3847

Uh oh!

Conversation

erikaharrison-adsk commented Oct 9, 2025

Description of Change(s)

Link to proposal (if applicable)

Fixes Issue(s)

Checklist

Uh oh!

jesschimein commented Oct 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ppenenko Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ppenenko commented Oct 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ppenenko commented Oct 31, 2025

Description of Change(s)

Uh oh!

tcauchois left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ppenenko Oct 14, 2025 •

edited

Loading