From 81d16b7ccc4eb1165d452727efffc84afbfc4963 Mon Sep 17 00:00:00 2001
From: Google DeepMind <noreply@deepmind.com>
Date: Wed, 25 Jun 2025 02:21:51 -0700
Subject: [PATCH] Add some comments to the material construction code

Grouped similar lines together, since they have become separated.

PiperOrigin-RevId: 775595031
Change-Id: Id75e87e07eb2e6b745698a57ea048daaff34f35c
---
 .../usd/plugins/mjcf/mujoco_to_usd.cc         | 43 +++++++++++--------
 .../usd/plugins/mjcf/mjcf_file_format_test.cc |  8 ++++
 2 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/src/experimental/usd/plugins/mjcf/mujoco_to_usd.cc b/src/experimental/usd/plugins/mjcf/mujoco_to_usd.cc
index c91d91e2..1a813c4f 100644
--- a/src/experimental/usd/plugins/mjcf/mujoco_to_usd.cc
+++ b/src/experimental/usd/plugins/mjcf/mujoco_to_usd.cc
@@ -78,7 +78,7 @@ TF_DEFINE_PRIVATE_TOKENS(kTokens,
                          ((light, "Light"))
                          ((meshScope, "MeshSources"))
                          ((materialsScope, "Materials"))
-                         ((surface, "PreviewSurface"))
+                         ((previewSurface, "PreviewSurface"))
                          ((world, "World"))
                          ((xformOpTransform, "xformOp:transform"))
                          ((xformOpScale, "xformOp:scale"))
@@ -710,30 +710,49 @@ class ModelWriter {
   }
 
   void WriteMaterial(mjsMaterial *material, const pxr::SdfPath &parent_path) {
+    // Create a Material prim.
     auto name = GetAvailablePrimName(
         *material->name, pxr::UsdShadeTokens->Material, parent_path);
     pxr::SdfPath material_path =
         CreatePrimSpec(data_, parent_path, name, pxr::UsdShadeTokens->Material);
 
-    // Shader "PreviewSurface"
-    pxr::SdfPath preview_surface_shader_path = CreatePrimSpec(
-        data_, material_path, kTokens->surface, pxr::UsdShadeTokens->Shader);
+    // Create a Shader prim "PreviewSurface" under the Material prim.
+    pxr::SdfPath preview_surface_shader_path =
+        CreatePrimSpec(data_, material_path, kTokens->previewSurface,
+                       pxr::UsdShadeTokens->Shader);
 
+    // Set the Shader'sinfoId attribute to UsdPreviewSurface, a standard surface
+    // shader.
     pxr::SdfPath info_id_attr = CreateAttributeSpec(
         data_, preview_surface_shader_path, pxr::UsdShadeTokens->infoId,
         pxr::SdfValueTypeNames->Token, pxr::SdfVariabilityUniform);
     SetAttributeDefault(data_, info_id_attr,
                         pxr::UsdImagingTokens->UsdPreviewSurface);
 
+    // Connect material's surface output to the preview surface's surface
+    // output.
     pxr::SdfPath surface_output_attr = CreateAttributeSpec(
         data_, preview_surface_shader_path, pxr::UsdShadeTokens->outputsSurface,
         pxr::SdfValueTypeNames->Token);
+    pxr::SdfPath material_surface_output_attr = CreateAttributeSpec(
+        data_, material_path, pxr::UsdShadeTokens->outputsSurface,
+        pxr::SdfValueTypeNames->Token);
+    AddAttributeConnection(data_, material_surface_output_attr,
+                           surface_output_attr);
 
+    // Connect material's displacement output to the preview surface's
+    // displacement output.
     pxr::SdfPath displacement_output_attr =
         CreateAttributeSpec(data_, preview_surface_shader_path,
                             pxr::UsdShadeTokens->outputsDisplacement,
                             pxr::SdfValueTypeNames->Token);
+    pxr::SdfPath material_displacement_output_attr = CreateAttributeSpec(
+        data_, material_path, pxr::UsdShadeTokens->outputsDisplacement,
+        pxr::SdfValueTypeNames->Token);
+    AddAttributeConnection(data_, material_displacement_output_attr,
+                           displacement_output_attr);
 
+    // Add an st (uv) Shader, a prim var reader for the UV coordinates.
     const pxr::SdfPath &uvmap_st_output_attr =
         AddUVTextureShader(material_path, pxr::TfToken("uvmap"));
     const mjStringVec &textures = *(material->textures);
@@ -815,6 +834,8 @@ class ModelWriter {
       }
     }
 
+    // Set the value of diffuse color. This can come from a diffuse texture
+    // or as a value defined in mjsMaterial_.
     pxr::SdfPath diffuse_color_attr = CreateAttributeSpec(
         data_, preview_surface_shader_path, kTokens->inputsDiffuseColor,
         pxr::SdfValueTypeNames->Color3f);
@@ -840,20 +861,6 @@ class ModelWriter {
                           pxr::GfVec3f(material->rgba[0], material->rgba[1],
                                        material->rgba[2]));
     }
-
-    pxr::SdfPath material_surface_output_attr = CreateAttributeSpec(
-        data_, material_path, pxr::UsdShadeTokens->outputsSurface,
-        pxr::SdfValueTypeNames->Token);
-
-    AddAttributeConnection(data_, material_surface_output_attr,
-                           surface_output_attr);
-
-    pxr::SdfPath material_displacement_output_attr = CreateAttributeSpec(
-        data_, material_path, pxr::UsdShadeTokens->outputsDisplacement,
-        pxr::SdfValueTypeNames->Token);
-
-    AddAttributeConnection(data_, material_displacement_output_attr,
-                           displacement_output_attr);
   }
 
   void WriteMaterials() {
diff --git a/test/experimental/usd/plugins/mjcf/mjcf_file_format_test.cc b/test/experimental/usd/plugins/mjcf/mjcf_file_format_test.cc
index a9e5d8c1..9628199e 100644
--- a/test/experimental/usd/plugins/mjcf/mjcf_file_format_test.cc
+++ b/test/experimental/usd/plugins/mjcf/mjcf_file_format_test.cc
@@ -143,6 +143,14 @@ TEST_F(MjcfSdfFileFormatPluginTest, TestMaterials) {
       stage,
       "/mesh_test/Materials/material_red/PreviewSurface.inputs:diffuseColor",
       pxr::GfVec3f(0.8, 0, 0));
+  ExpectAttributeHasConnection(
+    stage,
+    "/mesh_test/Materials/material_red.outputs:surface",
+    "/mesh_test/Materials/material_red/PreviewSurface.outputs:surface");
+  ExpectAttributeHasConnection(
+    stage,
+    "/mesh_test/Materials/material_red.outputs:displacement",
+    "/mesh_test/Materials/material_red/PreviewSurface.outputs:displacement");
 
   EXPECT_PRIM_VALID(stage, "/mesh_test/Materials/material_texture");
   EXPECT_PRIM_VALID(stage,