[BUG FIX] Make Surface shortcut resolution idempotent

genesis/options/surfaces.py

@@ -1,5 +1,5 @@
 import math
-from typing import ClassVar, Literal
+from typing import Any, ClassVar, Literal
 from typing_extensions import Self
 
 import numpy as np
@@ -95,37 +95,37 @@ class Surface(Options):
     generate_foam: StrictBool = False
     foam_options: FoamOptions = Field(default_factory=FoamOptions)
 
-    @model_validator(mode="after")
-    def _resolve_shortcuts(self) -> Self:
-        color_target = type(self)._color_target
-        if self.color is not None:
-            if getattr(self, color_target) is not None:
-                gs.raise_exception(f"'color' and '{color_target}' cannot both be set.")
-            setattr(self, color_target, ColorTexture(color=tuple(self.color)))
-
+    @model_validator(mode="before")
+    @classmethod
+    def _resolve_shortcuts(cls, data: Any) -> Any:
+        # Route each shortcut into its texture counterpart. Subclasses that don't expose a given texture (e.g. Glass has
+        # no opacity_texture) are skipped via the model_fields guard, and class defaults like `Rough.roughness = 1.0`
+        # are honored.
         for shortcut, texture_field in (
+            ("color", cls._color_target),
             ("opacity", "opacity_texture"),
             ("roughness", "roughness_texture"),
             ("metallic", "metallic_texture"),
+            ("thickness", "thickness_texture"),
+            ("emissive", "emissive_texture"),
         ):
-            value = getattr(self, shortcut, None)
-            if value is not None:
-                if texture_field in self.model_fields:
-                    if getattr(self, texture_field) is not None:
-                        gs.raise_exception(f"'{shortcut}' and '{texture_field}' cannot both be set.")
-                    setattr(self, texture_field, ColorTexture(color=(float(value),)))
-
-        if self.emissive is not None:
-            if "emissive_texture" in self.model_fields:
-                if self.emissive_texture is not None:
-                    gs.raise_exception("'emissive' and 'emissive_texture' cannot both be set.")
-                self.emissive_texture = ColorTexture(color=tuple(self.emissive))
-
-        # Sync default_roughness with roughness shortcut unless explicitly set
-        if self.roughness is not None and "default_roughness" not in self.model_fields_set:
-            self.default_roughness = float(self.roughness)
-
-        return self
+            if texture_field not in cls.model_fields:
+                continue
+            field = cls.model_fields.get(shortcut)
+            value = data.get(shortcut, field.default if field is not None else None)
+            if value is None:
+                continue
+            if data.get(texture_field) is not None:
+                gs.raise_exception(f"'{shortcut}' and '{texture_field}' cannot both be set.")
+            data[texture_field] = ColorTexture(color=value)
+
+        # Mirror the roughness shortcut into default_roughness unless the user passed an explicit value.
+        if "default_roughness" not in data and "roughness" in cls.model_fields:
+            roughness = data.get("roughness", cls.model_fields["roughness"].default)
+            if roughness is not None:
+                data["default_roughness"] = float(roughness)
+
+        return data
 
     @property
     def texture(self) -> Texture | None:
@@ -317,12 +317,6 @@ class Glass(Surface):
 
     @model_validator(mode="after")
     def _post_init(self) -> Self:
-        # Handle thickness shortcut
-        if self.thickness is not None:
-            if self.thickness_texture is not None:
-                gs.raise_exception("'thickness' and 'thickness_texture' cannot both be set.")
-            self.thickness_texture = ColorTexture(color=(float(self.thickness),))
-
         # Truncate specular/emissive textures to 3 channels (discard alpha for Glass which has no opacity_texture)
         if self.specular_texture is not None:
             self.specular_texture.check_dim(3)

tests/test_misc.py

@@ -1,8 +1,11 @@
 """Tests for the entity naming system."""
 
 import pytest
+from pydantic import BaseModel
 
 import genesis as gs
+from genesis.options.surfaces import Surface
+from genesis.options.textures import ColorTexture
 
 
 @pytest.mark.required
@@ -120,3 +123,51 @@ def test_urdf_mjcf_names_from_file():
     urdf_entity2 = scene.add_entity(gs.morphs.URDF(file="urdf/plane/plane.urdf"))
     assert urdf_entity2.name.startswith("plane_")
     assert urdf_entity.name != urdf_entity2.name
+
+
+@pytest.mark.required
+def test_surface_shortcut_resolution():
+    # Plastic family: color resolves to diffuse_texture; the Rough subclass roughness default (1.0) feeds
+    # roughness_texture and default_roughness.
+    rough = gs.surfaces.Rough(color=(0.4, 0.4, 0.4))
+    assert rough.color == (0.4, 0.4, 0.4)
+    assert rough.roughness == 1.0
+    assert rough.diffuse_texture.color == (0.4, 0.4, 0.4)
+    assert rough.roughness_texture.color == (1.0,)
+    assert rough.default_roughness == 1.0
+
+    # Glass: color resolves to specular_texture and the thickness shortcut is honored on the same path.
+    glass = gs.surfaces.Glass(color=(0.6, 0.8, 1.0), thickness=0.02)
+    assert glass.specular_texture.color == (0.6, 0.8, 1.0)
+    assert glass.thickness_texture.color == (0.02,)
+
+    # BSDF exercises multiple shortcuts at once.
+    bsdf = gs.surfaces.BSDF(color=(0.2, 0.3, 0.4), roughness=0.3, metallic=0.5)
+    assert bsdf.diffuse_texture.color == (0.2, 0.3, 0.4)
+    assert bsdf.roughness_texture.color == (0.3,)
+    assert bsdf.metallic_texture.color == (0.5,)
+    assert bsdf.default_roughness == 0.3
+
+    # Emission: color resolves to emissive_texture.
+    emit = gs.surfaces.Emission(color=(1.0, 1.0, 0.0))
+    assert emit.emissive_texture.color == (1.0, 1.0, 0.0)
+
+    # Explicit default_roughness wins over the roughness shortcut.
+    override = gs.surfaces.Rough(roughness=0.7, default_roughness=0.5)
+    assert override.default_roughness == 0.5
+
+    # Nesting an already-resolved surface in another Pydantic model must not re-trigger resolution.
+    class Wrapper(BaseModel):
+        surface: Surface
+
+    for surface in (rough, glass, bsdf, emit):
+        Wrapper(surface=surface)
+    Wrapper(surface=rough)
+    assert rough.diffuse_texture.color == (0.4, 0.4, 0.4)
+    assert rough.roughness_texture.color == (1.0,)
+
+    # Passing both the shortcut and its resolved texture at construction is a user error.
+    with pytest.raises(Exception, match="'color' and 'diffuse_texture' cannot both be set"):
+        gs.surfaces.Rough(color=(1.0, 0.0, 0.0), diffuse_texture=ColorTexture(color=(0.0, 1.0, 0.0)))
+    with pytest.raises(Exception, match="'thickness' and 'thickness_texture' cannot both be set"):
+        gs.surfaces.Glass(thickness=0.02, thickness_texture=ColorTexture(color=(0.05,)))