Fixed material overrides (#51)

We now follow rviz & urdfviewer where embedded OBJ/DAE materials take priority over URDF materials

Author: ft-lab

docs/concept_mapping.md

@@ -594,10 +594,12 @@ Mapping the material & shader concepts of these reference formats is outside the
 
 Whether supporting a partial or a full material conversion, they should be converted to appropriate `UsdShadeMaterial` and `UsdShadeShader` prims, stored alongside the native URDF materials in a library, and bound to the UsdGeom prims using an applied `MaterialBindingAPI`.
 
-If there are also direct URDF materials assigned to the same geometry, the native URDF assignments should take precedence (see [material](#material) for guidance).
-
 In USD, the subset concept maps to `UsdGeomSubset` Prims, which should be children of the `UsdGeomMesh` (see [mesh](#element-mesh)). Each `UsdGeomSubset` can have a `MaterialBindingAPI` applied to it, targeting a different `UsdShadeMaterial`. There is some runtime cost associated with Subsets, so if only one material is applied to the entire mesh, it is preferable to avoid Subset prims & recommended to use a material binding to the mesh itself.
 
+If embedded materials are assigned to meshes or a subset of faces in the OBJ/DAE files, they should be converted to `UsdShadeMaterials` and bound to the appropriate `UsdGeomMesh` or `UsdGeomSubset`. If URDF materials also exist for these meshes/subsets, they should be ignored. Native URDF materials should only be bound to meshes or subsets which do not already receive material bindings from the embedded OBJ/DAE files.
+
+See [material](#material) for further guidance.
+
 ### joint
 
 In URDF joints define the relative motion of links & describe the kinematic hierarchy of the robot, but defining motion degrees of freedom between a specified parent link and a child link, as well as the initial 3D position & orientation of the child link.

tests/data/material_mesh_override.urdf

@@ -20,10 +20,27 @@
       <material name="blue"/>
     </visual>
   </link>
+  <link name="link_dae">
+    <visual>
+      <geometry>
+        <!-- This dae file has a material: Material. -->
+        <mesh filename="assets/box.dae" scale="0.5 0.5 0.5"/>
+      </geometry>
+      <!-- Override the material with blue. -->
+      <!-- dae has only one mesh and already has a material assigned. -->
+      <!-- This URDF material specification will be skipped. -->
+      <material name="blue"/>
+    </visual>
+  </link>
 
   <joint name="joint_box_obj" type="fixed">
     <origin rpy="0 0 0" xyz="1 0 0"/>
     <parent link="link_box"/>
     <child link="link_obj"/>
   </joint>
+  <joint name="joint_obj_dae" type="fixed">
+    <origin rpy="0 0 0" xyz="1 0 0"/>
+    <parent link="link_obj"/>
+    <child link="link_dae"/>
+  </joint>
 </robot>

tests/testMaterial.py

@@ -534,23 +534,79 @@ def test_material_mesh_override(self):
         self.assertTrue(blue_material.GetPrim().HasAuthoredReferences())
 
         diffuse_color = self.get_material_diffuse_color(blue_material)
-        self.assertEqual(diffuse_color, Gf.Vec3f(0, 0, 1))
+        self.assertTrue(Gf.IsClose(diffuse_color, Gf.Vec3f(0, 0, 1), 1e-6))
         opacity = self.get_material_opacity(blue_material)
-        self.assertEqual(opacity, 1.0)
+        self.assertAlmostEqual(opacity, 1.0, places=6)
+
+        red_material_prim = material_scope_prim.GetChild("red_mat")
+        self.assertTrue(red_material_prim.IsValid())
+        self.assertTrue(red_material_prim.IsA(UsdShade.Material))
+
+        red_material = UsdShade.Material(red_material_prim)
+        self.assertTrue(red_material)
+        self.assertTrue(red_material.GetPrim().HasAuthoredReferences())
+
+        diffuse_color = self.get_material_diffuse_color(red_material)
+        self.assertTrue(Gf.IsClose(diffuse_color, Gf.Vec3f(1, 0, 0), 1e-6))
+        opacity = self.get_material_opacity(red_material)
+        self.assertAlmostEqual(opacity, 1.0, places=6)
+
+        green_material_prim = material_scope_prim.GetChild("green_mat")
+        self.assertTrue(green_material_prim.IsValid())
+        self.assertTrue(green_material_prim.IsA(UsdShade.Material))
+
+        green_material = UsdShade.Material(green_material_prim)
+        self.assertTrue(green_material)
+        self.assertTrue(green_material.GetPrim().HasAuthoredReferences())
+
+        diffuse_color = self.get_material_diffuse_color(green_material)
+        self.assertTrue(Gf.IsClose(diffuse_color, Gf.Vec3f(0, 1, 0), 1e-6))
+        opacity = self.get_material_opacity(green_material)
+        self.assertAlmostEqual(opacity, 1.0, places=6)
+
+        dae_material_prim = material_scope_prim.GetChild("Material")
+        self.assertTrue(dae_material_prim.IsValid())
+        self.assertTrue(dae_material_prim.IsA(UsdShade.Material))
+        dae_material = UsdShade.Material(dae_material_prim)
+        self.assertTrue(dae_material)
+        self.assertTrue(dae_material.GetPrim().HasAuthoredReferences())
+
+        diffuse_color = self.get_material_diffuse_color(dae_material)
+        diffuse_color = usdex.core.linearToSrgb(diffuse_color)
+        self.assertTrue(Gf.IsClose(diffuse_color, Gf.Vec3f(0.8, 0.8, 0.8), 1e-6))
+        opacity = self.get_material_opacity(dae_material)
+        self.assertAlmostEqual(opacity, 1.0, places=6)
 
+        # Check material bindings.
         default_prim = stage.GetDefaultPrim()
         geometry_scope_prim = default_prim.GetChild("Geometry")
         self.assertTrue(geometry_scope_prim.IsValid())
         self.assertTrue(geometry_scope_prim.IsA(UsdGeom.Scope))
 
-        two_boxes_prim = geometry_scope_prim.GetChild("link_box").GetChild("link_obj").GetChild("two_boxes")
+        link_obj_prim = geometry_scope_prim.GetChild("link_box").GetChild("link_obj")
+        self.assertTrue(link_obj_prim.IsValid())
+
+        two_boxes_prim = link_obj_prim.GetChild("two_boxes")
         self.assertTrue(two_boxes_prim.IsValid())
         self.assertTrue(two_boxes_prim.IsA(UsdGeom.Xform))
         self.assertTrue(two_boxes_prim.HasAuthoredReferences())
 
         # Check that the material bind is overwritten with blue_material.
         self.check_material_binding(two_boxes_prim, blue_material)
 
+        # Check that the material bind is overwritten with red_material.
+        cube_red_prim = two_boxes_prim.GetChild("Cube_Red")
+        self.assertTrue(cube_red_prim.IsValid())
+        self.assertTrue(cube_red_prim.IsA(UsdGeom.Mesh))
+        self.check_material_binding(cube_red_prim, red_material)
+
+        # Check that the material bind is dae_material.
+        # Since the material by dae is already assigned, it will not be overwritten by blue_material.
+        dae_box_prim = link_obj_prim.GetChild("link_dae").GetChild("box")
+        self.assertTrue(dae_box_prim.IsValid())
+        self.assertTrue(dae_box_prim.IsA(UsdGeom.Mesh))
+        self.check_material_binding(dae_box_prim, dae_material)
+
     def test_dae_materials(self):
         input_path = "tests/data/dae_materials.urdf"
         output_dir = self.tmpDir()

urdf_usd_converter/_impl/geometry.py

@@ -46,12 +46,12 @@ def convert_geometry(parent: Usd.Prim, name: str, safe_name: str, geometry: Elem
         apply_physics_collision(prim.GetPrim(), data)
 
     # If the visual has a material, bind the material.
-    # If the Visual element does not have a material, bind a material per mesh.
+    # After binding materials to each mesh, bind the URDF material to any shapes that did not receive materials from the embedded mesh files.
     if isinstance(geometry, ElementVisual):
+        if isinstance(geometry.geometry.shape, ElementMesh):
+            bind_mesh_material(prim.GetPrim(), geometry.geometry.shape.filename, data)
         if geometry.material and geometry.material.name:
             bind_material(prim.GetPrim(), None, geometry.material.name, data)
-        elif isinstance(geometry.geometry.shape, ElementMesh):
-            bind_mesh_material(prim.GetPrim(), geometry.geometry.shape.filename, data)
 
     return prim
 

urdf_usd_converter/_impl/material.py

@@ -432,6 +432,15 @@ def bind_material(geom_prim: Usd.Prim, mesh_file_path: pathlib.Path | None, mate
         Tf.Warn(f"Material '{material_name}' not found in Material Library {data.libraries[Tokens.Materials].GetRootLayer().identifier}")
         return
 
+    geom_over = data.content[Tokens.Materials].OverridePrim(geom_prim.GetPath())
+
+    # If the geometry already has a material binding, skip the binding.
+    material_binding = UsdShade.MaterialBindingAPI(geom_over)
+    if material_binding:
+        binding_rel = material_binding.GetDirectBindingRel()
+        if len(binding_rel.GetTargets()) > 0:
+            return
+
     # If the material does not exist in the Material layer, define the reference.
     material_prim = UsdShade.Material(local_materials.GetChild(ref_material.GetPrim().GetName()))
     if not material_prim:
@@ -448,7 +457,6 @@ def bind_material(geom_prim: Usd.Prim, mesh_file_path: pathlib.Path | None, mate
                 break
 
     # Bind the material to the geometry.
-    geom_over = data.content[Tokens.Materials].OverridePrim(geom_prim.GetPath())
     usdex.core.bindMaterial(geom_over, material_prim)
 
 
@@ -479,9 +487,9 @@ def _get_material_name_from_prim(prim: Usd.Prim, resolved_file_path: pathlib.Pat
         return None
 
     prim_name = prim.GetName()
-    if prim.IsA(UsdGeom.Mesh):
+    if prim.IsA(UsdGeom.Mesh) and len(prim.GetAllChildrenNames()) == 0:
         # If there are no child prims, it is a single mesh with no GeomSubset.
-        if prim.HasAuthoredReferences() and len(prim.GetAllChildrenNames()) == 0:
+        if prim.HasAuthoredReferences():
             # Get the first material name list.
             # In this case, a prim reference exists, and it refers to the single mesh itself.
             # When referencing, the prim_name may not match the name stored in 'data.mesh_material_references',