Skip to content

Commit

Permalink
PyMJCF: Don't replace '/' -> '\' in attributes that may contain fil…
Browse files Browse the repository at this point in the history
…esystem path information.

PiperOrigin-RevId: 631419245
Change-Id: I317bae0b5b6d6041baaf1ca663cd5628012831a0
  • Loading branch information
erez-tom authored and copybara-github committed May 7, 2024
1 parent f3e1114 commit ad0824f
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 6 deletions.
11 changes: 11 additions & 0 deletions dm_control/mjcf/export_with_assets_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
_ASSETS_DIR = os.path.join(os.path.dirname(__file__), 'test_assets')
_TEST_MODEL_WITH_ASSETS = os.path.join(_ASSETS_DIR, 'model_with_assets.xml')
_TEST_MODEL_WITH_ASSETDIR = os.path.join(_ASSETS_DIR, 'model_with_assetdir.xml')
_TEST_MODEL_WITH_SEPARATORS = os.path.join(_ASSETS_DIR,
'model_with_separators.xml')
_TEST_MODEL_WITHOUT_ASSETS = os.path.join(_ASSETS_DIR, 'lego_brick.xml')


Expand Down Expand Up @@ -89,6 +91,15 @@ def test_default_model_filename(self):
expected_name = mjcf_model.model + '.xml'
self.assertTrue(os.path.isfile(os.path.join(out_dir, expected_name)))

def test_model_with_separators(self):
out_dir = self.create_tempdir().full_path
mjcf_model = mjcf.from_path(
_TEST_MODEL_WITH_SEPARATORS, escape_separators=True
)
mjcf.export_with_assets(mjcf_model, out_dir, out_file_name=None)
expected_name = mjcf_model.model + '.xml'
self.assertTrue(os.path.isfile(os.path.join(out_dir, expected_name)))

def test_exceptions(self):
out_dir = self.create_tempdir().full_path
mjcf_model = mjcf.from_path(_TEST_MODEL_WITH_ASSETS)
Expand Down
19 changes: 13 additions & 6 deletions dm_control/mjcf/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,19 @@ def _parse_children(xml_element, mjcf_element, escape_separators=False):
if escape_separators:
attributes = {}
for name, value in xml_child.attrib.items():
new_value = value.replace(
constants.PREFIX_SEPARATOR_ESCAPE,
constants.PREFIX_SEPARATOR_ESCAPE * 2)
new_value = new_value.replace(
constants.PREFIX_SEPARATOR, constants.PREFIX_SEPARATOR_ESCAPE)
attributes[name] = new_value
# skip flipping the slash for fields that may contain paths, like
# custom text and asset file.
if name in ['data', 'file', 'meshdir', 'assetdir', 'texturedir',
'content_type', 'fileleft', 'fileright', 'fileback',
'filefront', 'plugin', 'key', 'value']:
attributes[name] = value
else:
new_value = value.replace(
constants.PREFIX_SEPARATOR_ESCAPE,
constants.PREFIX_SEPARATOR_ESCAPE * 2)
new_value = new_value.replace(
constants.PREFIX_SEPARATOR, constants.PREFIX_SEPARATOR_ESCAPE)
attributes[name] = new_value
else:
attributes = dict(xml_child.attrib)
if child_spec.repeated or child_spec.on_demand:
Expand Down
19 changes: 19 additions & 0 deletions dm_control/mjcf/test_assets/model_with_separators.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<mujoco model="Textured cube and mesh">
<compiler meshdir="meshes" texturedir="textures"/>
<asset>
<mesh name="cube" file="cube.stl"/>
<mesh name="another_cube" file="more_meshes/cube.stl"/>
<mesh name="unused_asset_should_not_cause_problems" file="cube.stl"/>
<mesh name="cube_msh" file="cube.msh"/>
<texture name="texture" file="deepmind.png"/>
<material name="mat_texture" texture="texture"/>
<hfield name="hill" file="../textures/deepmind.png" size="0.5 0.5 0.5 0.1"/>
</asset>
<worldbody>
<light diffuse=".5 .5 .5" pos="0 0 3" dir="0 0 -1"/>
<geom name="left/cube" type="mesh" mesh="cube" material="mat_texture"/>
<geom name="right/cube" type="mesh" mesh="another_cube" material="mat_texture" pos="2.5 0. 0."/>
<geom type="mesh" mesh="cube_msh" material="mat_texture" pos="4. 0. 0."/>
<geom type="hfield" hfield="hill" pos="1.2 0. 0" rgba="0. 0.9 0. 1" size="40 40 0.1"/>
</worldbody>
</mujoco>

0 comments on commit ad0824f

Please sign in to comment.