-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Restore remote asset URLs in stage dumps #7083
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
06c8655
f705caf
49b0eea
a39d972
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| Added | ||
| ^^^^^ | ||
|
|
||
| * Added :func:`~isaaclab.utils.assets.unmirror_file_path`, which maps a locally cached asset copy | ||
| written by :func:`~isaaclab.utils.assets.retrieve_file_path` back to the URL it was downloaded | ||
| from. Exports of a stage that references cached copies can use it to name the source assets | ||
| instead of machine-specific cache paths. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Rendering test stage dumps rewrite cached asset paths back to their source URLs, so a stage saved | ||
| through ``ISAAC_LAB_SAVE_STAGES`` no longer carries texture paths that only resolve on the machine | ||
| that ran the test. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,8 @@ | |
| from isaaclab.utils.warp import ProxyArray | ||
|
|
||
| if TYPE_CHECKING: | ||
| from pxr import Sdf | ||
|
|
||
| from isaaclab.sensors.camera import CameraData | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
@@ -510,6 +512,20 @@ def _sanitize_golden_stage_text(text: str) -> str: | |
| return text.rstrip("\n") + "\n" | ||
|
|
||
|
|
||
| def _restore_remote_asset_paths(layer: "Sdf.Layer") -> None: | ||
| """Point cached asset paths in ``layer`` back at the URLs they were downloaded from. | ||
|
|
||
| Remote USD assets are referenced through a local cache copy, so flattening resolves the | ||
| textures and materials they carry into absolute cache paths that exist only on the machine | ||
| that ran the test. Locally authored paths are left untouched. | ||
| """ | ||
| from pxr import UsdUtils # noqa: PLC0415 | ||
|
|
||
| from isaaclab.utils.assets import unmirror_file_path # noqa: PLC0415 | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be moved to the top?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have imports at the top of isaaclab.utils, I would assume yes? i wonder why would we want to delay this import? |
||
|
|
||
| UsdUtils.ModifyAssetPaths(layer, lambda asset_path: unmirror_file_path(asset_path) or asset_path) | ||
|
|
||
|
|
||
| def maybe_save_stage( | ||
| test_name: str, | ||
| physics_backend: str, | ||
|
|
@@ -551,6 +567,7 @@ def maybe_save_stage( | |
| flat_layer = opened_stage.Flatten() | ||
| if flat_layer is None: | ||
| pytest.fail(f"Could not flatten the saved stage at {stage_path}.") | ||
| _restore_remote_asset_paths(flat_layer) | ||
|
|
||
| if out_dir: | ||
| os.makedirs(out_dir, exist_ok=True) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 Suggestion · Api — Changelog omits git remote-path behavior change
The fragment documents only the new
unmirror_file_path, but the_is_git_remote_pathchange also alters user-visible behavior of the publicretrieve_git_asset_path: a Windows local checkout such asC:\assetswas previously treated as a remote repository and cloned into the cache, and is now used in place. Repository rules require changed behavior to appear in the release note; add aFixedentry covering it.