Reuse existing visual material in randomization#395
Open
yuecideng wants to merge 18 commits into
Open
Conversation
Design for randomize_visual_material to reuse the object's existing dexsim-parsed material (instance-swap + pre-created textures) instead of creating a new material each time. Preserves asset original appearance (three-tier: original/library/solid) and removes per-reset material creation and clean_materials overhead. Old behavior retained behind a fallback_to_new flag. Co-Authored-By: Claude <noreply@anthropic.com>
7-task TDD plan for instance-swap + pre-created-texture reuse path in randomize_visual_material, with fallback_to_new legacy preservation. Co-Authored-By: Claude <noreply@anthropic.com>
Pre-flight fix: solid/library tier logic was duplicated between rigid and articulation paths. Helpers now take link_name and share _apply_inst. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…_material_inst Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…_material_inst Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
… swap Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ure_key Co-Authored-By: Claude <noreply@anthropic.com>
yuecideng
added
enhancement
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors randomize_visual_material to reuse dexsim-parsed per-object materials (via instance swapping) instead of creating and replacing a new material each reset, adds a three-tier texture selection policy (original/library/solid) with configurable probabilities, and keeps the prior behavior behind a fallback_to_new flag. It also introduces supporting sim APIs (ReuseSegmentState, “get existing material” helpers) and adds mocked unit tests to validate the new code paths.
Changes:
- Extend
VisualMaterialInst.set_base_color_texture(...)to accept a pre-created dexsimTextureobject (skip re-upload). - Add
ReuseSegmentStateplusget_existing_visual_material(...)/apply_render_material_inst(...)helpers onRigidObjectandArticulation. - Implement reuse-mode init/call paths in
randomize_visual_material, including tier-probability resolution and sim-level texture object caching; add extensive mocked tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
embodichain/lab/gym/envs/managers/randomization/visual.py |
Implements reuse-mode initialization and per-reset swapping, tier sampling, and cached texture objects; retains legacy path behind flag. |
embodichain/lab/sim/material.py |
Adds texture_obj support for base-color maps and introduces ReuseSegmentState. |
embodichain/lab/sim/objects/rigid_object.py |
Adds helpers to capture existing segment materials and swap MaterialInst back onto render bodies. |
embodichain/lab/sim/objects/articulation.py |
Adds link-aware equivalents of existing-material capture and render-body instance swapping. |
embodichain/lab/sim/__init__.py |
Exports ReuseSegmentState and defines an explicit package __all__. |
tests/gym/envs/managers/test_randomize_visual_material.py |
Adds characterization and reuse-mode tests (mocked dexsim) for legacy/reuse/compat scenarios. |
tests/sim/test_material_texture_obj.py |
Tests texture_obj path binds Texture without upload. |
tests/sim/objects/test_rigid_object_reuse_material.py |
Tests rigid-object reuse-state building and render-body swapping. |
tests/sim/objects/test_articulation_reuse_material.py |
Tests articulation reuse-state building per link and swapping per link segment. |
docs/superpowers/specs/2026-07-14-reuse-existing-visual-material-design.md |
Design write-up for the reuse-material approach and constraints. |
docs/superpowers/plans/2026-07-14-reuse-existing-visual-material.md |
Detailed implementation plan and checklist for the feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+640
to
+644
| logger.log_warning( | ||
| f"randomize_visual_material: reuse-existing-material unavailable for " | ||
| f"'{self.entity_cfg.uid}' ({e}); falling back to new-material path." | ||
| ) | ||
| self._new_mode = False |
Comment on lines
+872
to
+875
| else: | ||
| for reuse_i in range(num_reuse): | ||
| _apply(reuse_i, int(env_ids[reuse_i])) | ||
|
|
Comment on lines
+191
to
+193
| seg = _seg(0, MagicMock(name="orig"), MagicMock(name="tmpl")) | ||
| obj.get_existing_visual_material = MagicMock(return_value=[[seg]]) | ||
| cfg = _make_cfg({"entity_cfg": SceneEntityCfg(uid="obj")}) |
| functor(env, torch.arange(env.num_envs), entity_cfg=SceneEntityCfg(uid="obj")) | ||
|
|
||
| env.sim.env.clean_materials.assert_not_called() | ||
| obj.apply_render_material_inst.assert_called() |
Comment on lines
+253
to
256
| texture_path: str = None, | ||
| texture_data: torch.Tensor | None = None, | ||
| texture_obj=None, | ||
| ) -> None: |
Comment on lines
299
to
+303
| if texture_path is not None: | ||
| self.base_color_texture = texture_path | ||
| inst = self._mat.get_inst(self.uid) | ||
| inst.set_base_color_map(texture_path) | ||
| elif texture_obj is not None: |
Comment on lines
+819
to
+838
| if self._new_mode: | ||
| return self._call_reuse( | ||
| env, | ||
| env_ids, | ||
| base_color_range, | ||
| metallic_range, | ||
| roughness_range, | ||
| ior_range, | ||
| ) | ||
| clean = bool(self._fallback_to_new) | ||
| return self._call_legacy( | ||
| env, | ||
| env_ids, | ||
| random_texture_prob, | ||
| base_color_range, | ||
| metallic_range, | ||
| roughness_range, | ||
| ior_range, | ||
| clean=clean, | ||
| ) |
Comment on lines
+626
to
+628
| self._texture_key = ( | ||
| os.path.basename(texture_path) if texture_path is not None else "" | ||
| ) |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
embodichain/lab/sim/material.py:306
- set_base_color_texture() bypasses the VisualMaterialInst.mat property and always calls self._mat.get_inst(self.uid). That ignores the _mat_inst path introduced by from_existing(), so updates may be applied to a looked-up/new instance instead of the wrapped existing MaterialInst.
if texture_path is not None:
self.base_color_texture = texture_path
inst = self._mat.get_inst(self.uid)
inst.set_base_color_map(texture_path)
elif texture_obj is not None:
self.base_color_texture = texture_obj
inst = self._mat.get_inst(self.uid)
inst.set_base_color_map(texture_obj)
embodichain/lab/sim/objects/rigid_object.py:956
- get_existing_visual_material() assumes get_render_body() is always non-None and will raise an AttributeError on render_body.get_mesh_count() otherwise. The docstring says this method raises ValueError for invalid reuse state; render-body absence should be handled explicitly and raised as ValueError (so callers like randomize_visual_material can degrade cleanly).
for env_idx in local_env_ids:
render_body = self._entities[env_idx].get_render_body()
mesh_count = render_body.get_mesh_count()
segments: List[ReuseSegmentState] = []
Comment on lines
+718
to
724
| def _init_legacy(self, env: EmbodiedEnv) -> None: | ||
| """Legacy init: create a new material and replace the object's material.""" | ||
| if self.entity_cfg.uid == "default_plane": | ||
| pass | ||
|
|
||
| else: | ||
| # TODO: we may need to get the default material instance from the asset itself. | ||
| mat: VisualMaterial = env.sim.create_visual_material( | ||
| cfg=VisualMaterialCfg( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR makes
randomize_visual_materialreuse each object's existing dexsim-parsed material (instance-swap + pre-created textures) instead of creating a new material each reset. It adds a three-tier texture choice (original / library / solid) with configurable probabilities, and preserves the old create-new-material behavior behind afallback_to_newflag.Motivation and context:
Env.clean_materials()is no longer called on the new path.Key changes:
material.py:VisualMaterialInst.set_base_color_texture(texture_obj=...)to bind a pre-createdTexture;ReuseSegmentStatedataclass.rigid_object.py/articulation.py:get_existing_visual_material/apply_render_material_inst.visual.py: reuse path (_init_reuse/_call_reuse, three-tier swap, pre-created textures, tier-probability backward-compat derivation) +fallback_to_newlegacy preservation + aget_data_path(None)guard so omittingtexture_pathno longer crashes.Backward compatibility:
fallback_to_new=Truepreserves the exact legacy path. Whenp_*are unset, they derive from the existingrandom_texture_prob(p_original=0), so existing configs run on the new path with zero changes.default_planekeeps its in-place behavior (noclean_materialsin new mode).Dependencies: none (uses existing dexsim APIs).
Design spec:
docs/superpowers/specs/2026-07-14-reuse-existing-visual-material-design.mdImplementation plan:
docs/superpowers/plans/2026-07-14-reuse-existing-visual-material.mdKnown limitations / follow-ups (Minor, logged in
.superpowers/sdd/progress.md): type hints on private helpers;_call_legacydocstring; test-quality polish. Non-shared_call_reuseassumes full-resetlen(env_ids)==num_reuse(matches the legacy path's assumption).Type of change
Screenshots
N/A — simulation material behavior, verified via 24 unit tests with mocked dexsim (no GPU required).
Checklist
black .command to format the code base.🤖 Generated with Claude Code