From 09a6fb7d4dea43236b04e831940ef42c483ed073 Mon Sep 17 00:00:00 2001 From: Google DeepMind Date: Tue, 13 Jan 2026 05:54:06 -0800 Subject: [PATCH] #mjspec Enforce that material textures array is always of length mjNTEXROLE. PiperOrigin-RevId: 855684213 Change-Id: Id8a3734c75841dfb91bd2252114249f08461b7f2 --- .../mujoco/codegen/generate_spec_bindings.py | 24 ++++++++++++++ python/mujoco/specs_test.py | 31 ++++++++++++++++--- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/python/mujoco/codegen/generate_spec_bindings.py b/python/mujoco/codegen/generate_spec_bindings.py index 52763fc7..a0026e55 100644 --- a/python/mujoco/codegen/generate_spec_bindings.py +++ b/python/mujoco/codegen/generate_spec_bindings.py @@ -239,6 +239,30 @@ def _ptr_binding_code( }} }}, py::return_value_policy::move);""" elif vartype == 'mjStringVec': + # Special case for material.textures: must be exactly mjNTEXROLE size + if classname == 'mjsMaterial' and varname == 'textures': + return f"""\ + {classname}.def_property( + "{varname}", + []({rawclassname}& self) -> MjTypeVec {{ + return MjTypeVec(self.{fullvarname}->data(), + self.{fullvarname}->size()); + }}, + []({rawclassname}& self, py::object rhs) {{ + if (py::len(rhs) != mjNTEXROLE) {{ + throw pybind11::value_error( + "material.textures must have exactly " + std::to_string(mjNTEXROLE) + + " elements, got " + std::to_string(py::len(rhs)) + ". " + + "Assign a list of " + std::to_string(mjNTEXROLE) + " texture names " + + "(use empty strings '' for unused slots)."); + }} + self.{fullvarname}->clear(); + self.{fullvarname}->reserve(mjNTEXROLE); + for (auto val : rhs) {{ + self.{fullvarname}->push_back(py::cast(val)); + }} + }}, py::return_value_policy::move);""" + # Default case for other mjStringVec properties return f"""\ {classname}.def_property( "{varname}", diff --git a/python/mujoco/specs_test.py b/python/mujoco/specs_test.py index e92ddb59..e9ee5cc5 100644 --- a/python/mujoco/specs_test.py +++ b/python/mujoco/specs_test.py @@ -1002,23 +1002,44 @@ class SpecsTest(absltest.TestCase): material.textures[texture_index] = 'texture_name' self.assertEqual(material.textures[texture_index], 'texture_name') - # Assign a complete list - material.textures = ['', 'new_name', '', '', ''] + # Assign a complete list (must be mjNTEXROLE = 10 elements) + material.textures = ['', 'new_name', '', '', '', '', '', '', '', ''] self.assertEqual(material.textures[texture_index], 'new_name') # textures is iterable self.assertEqual('new_name', ''.join(material.textures)) - # supports `len` - self.assertLen(material.textures, 5) + # supports `len` - should always be mjNTEXROLE (10) + self.assertLen(material.textures, mujoco.mjtTextureRole.mjNTEXROLE) # field checks for out-of-bound access on read and on write with self.assertRaises(IndexError): - material.textures[5] = 'x' + material.textures[10] = 'x' with self.assertRaises(IndexError): material.textures[-1] = 'x' + def test_textures(self): + """Tests that partial texture list assignment raises ValueError.""" + + spec = mujoco.MjSpec() + material = spec.add_material(name='mat') + texture = spec.add_texture( + name='tex', builtin=mujoco.mjtBuiltin.mjBUILTIN_FLAT, width=2, height=2 + ) + + # Should raise ValueError for incorrect size (only 1 element instead of 10) + with self.assertRaises(ValueError) as cm: + material.textures = ['tex'] + self.assertIn('must have exactly 10 elements', str(cm.exception)) + self.assertIn('got 1', str(cm.exception)) + + # Should succeed with correct size (mjNTEXROLE = 10) + material.textures = ['tex', '', '', '', '', '', '', '', '', ''] + spec.worldbody.add_geom(size=[0.1, 0.1, 0.1], material='mat') + model = spec.compile() + self.assertIsNotNone(model) + def test_assign_texture(self): spec = mujoco.MjSpec() texture = spec.add_texture(name='texture', height=2, width=2)