Fix rendering shadow hand environment to preserve object scale - #7010
Conversation
|
Too many files changed for review (3000 files, 100 file limit). |
There was a problem hiding this comment.
Isaac Lab Review Bot
The OVRTX Newton transform path now preserves authored USD scale across both legacy and ovstage bindings. The implementation is coherent, but this user-visible isaaclab_ov fix lacks the required package changelog fragment.
- Design and architecture: Capturing composed scale before stage export and aligning it with Newton body paths provides a shared solution for both OVRTX binding paths without introducing a public abstraction or dependency.
- API: The internal Warp kernel gains a scale-array parameter, and both in-repository call sites, initialization guards, and cleanup paths were updated consistently. No public API break is evident.
- Implementation: Scale-array ordering follows the existing object-path/Newton-index ordering, missing entries default to unit scale, and the kernel composition preserves translation while applying body-frame scale. Before merge, add a past-tense release-note fragment under
source/isaaclab_ov/changelog.d/as required for this user-visible package change.
Minor fixes needed. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
| self._exported_usd_string = export_stage_to_string( | ||
| stage, | ||
| num_envs, | ||
| source_paths=self._clone_plan.sources, | ||
| keep_env_roots=not self._use_ovstage, | ||
| ) | ||
|
|
||
| def _capture_object_scales(self, stage: Any) -> None: |
There was a problem hiding this comment.
🟡 Warning · Implementation — Missing isaaclab_ov changelog fragment
This user-visible rendering fix changes the isaaclab_ov package, but the only fragment added is an empty .skip under source/isaaclab_tasks/changelog.d/ named for an unrelated xfail removal. Repository rules require one fragment per changed source package for user-visible changes, so the scale-preservation fix would ship without a release note. Add a past-tense fragment under source/isaaclab_ov/changelog.d/.
|
PR title: does -> doesn't |
d3b0335 to
0cc9065
Compare
0cc9065 to
358f1da
Compare
Newton body transforms carry only translation and rotation, so writing them into OVRTX overwrote any authored USD scale with unit scale and rendered scaled assets (for example Shadow Hand) at the wrong size.
358f1da to
924fc22
Compare
# Description Newton's Kit/RTX Fabric synchronization rebuilt every rigid-body world matrix from a Newton `transformf`, which contains translation and rotation but no scale. The first render sync therefore overwrote composed USD scale with unit scale, making scaled rigid assets appear oversized. This change captures each body's initialized Fabric world scale by Newton body index and reapplies it when composing subsequent render matrices. It fixes the synchronization contract at the backend boundary, following the same root-fix approach used for OVRTX in #7010, and removes the need for the task-specific asset substitution proposed in #7476. No new dependencies are required. Related: #7010, #7476 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Screenshots Not applicable; the regression test checks the rendering-side Fabric world matrix directly. ## Validation - Ruff format and lint passed for the changed Python files. - `git diff --check` passed. - The changelog gate passed against current `upstream/develop`. - A Warp 1.16 CPU smoke test compiled and exercised the scale-capture and transform-composition kernels. - The Isaac Sim/CUDA regression test was not run locally because this host is macOS/arm; it is marked for Isaac Sim CI. ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [ ] I have run the full pre-commit checks with `uv run isaaclab -f` (unsupported on this macOS/arm host) - [x] No public API or documentation change is required - [x] My changes generate no new warnings in the available local checks - [x] I have added a regression test that proves the fix preserves non-uniform authored scale across pose synchronization - [x] I have added a changelog fragment under `source/isaaclab_newton/changelog.d/` - [x] My name already exists in `CONTRIBUTORS.md`
# Description Newton's Kit/RTX Fabric synchronization rebuilt every rigid-body world matrix from a Newton `transformf`, which contains translation and rotation but no scale. The first render sync therefore overwrote composed USD scale with unit scale, making scaled rigid assets appear oversized. This change captures each body's initialized Fabric world scale by Newton body index and reapplies it when composing subsequent render matrices. It fixes the synchronization contract at the backend boundary, following the same root-fix approach used for OVRTX in #7010, and removes the need for the task-specific asset substitution proposed in #7476. No new dependencies are required. Related: #7010, #7476 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Screenshots Not applicable; the regression test checks the rendering-side Fabric world matrix directly. ## Validation - Ruff format and lint passed for the changed Python files. - `git diff --check` passed. - The changelog gate passed against current `upstream/develop`. - A Warp 1.16 CPU smoke test compiled and exercised the scale-capture and transform-composition kernels. - The Isaac Sim/CUDA regression test was not run locally because this host is macOS/arm; it is marked for Isaac Sim CI. ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [ ] I have run the full pre-commit checks with `uv run isaaclab -f` (unsupported on this macOS/arm host) - [x] No public API or documentation change is required - [x] My changes generate no new warnings in the available local checks - [x] I have added a regression test that proves the fix preserves non-uniform authored scale across pose synchronization - [x] I have added a changelog fragment under `source/isaaclab_newton/changelog.d/` - [x] My name already exists in `CONTRIBUTORS.md` (cherry picked from commit 0077d98)
# Description Revives #6308 on current `develop` and supersedes #3728 with a single backend-neutral contract for kinematic rigid-object rendering. ## Architecture `source/isaaclab/test/renderers/rigid_object_rendering_contract.py` is the composition root. It owns the cloned scene, kinematic pose sequence, depth measurements, and assertions. Package-local adapters own only availability checks, simulation/renderer selection, and backend cleanup: - Isaac RTX + PhysX on CPU and CUDA, with and without a coexisting articulation; - Newton Warp + PhysX on CUDA; - OVRTX + OVPhysX on CUDA through both legacy and OVStage scene ownership (OVStage runs when installed). The dependency direction is adapter -> shared test contract -> public Isaac Lab APIs. An AST architecture gate rejects backend imports in the shared contract and rejects scene, asset, sensor, or class ownership in adapters. The contract creates two cloned instanceable DexCubes with root-level nonuniform scale, verifies their depth silhouettes, moves both kinematic bodies through the public rigid-object tensor API, verifies the physics poses, and requires opposite rendered centroid displacement. ## Current-develop audit Most production changes in the old PR have since landed through newer ownership boundaries: Isaac RTX render-product lifetime in #6729, Newton shadow-state copying in #6773, OVRTX scale-aware transform writes in #7010, and Newton Fabric scale preservation in #7481. This revival removes those stale patches rather than carrying duplicate implementations. The revived contract exposed one remaining OVRTX bug: composed scale was captured only for clone-plan source paths, while OVRTX creates non-source destinations after exporting the host USD stage. Those destinations therefore defaulted to unit scale. As a deliberately temporary bridge, this PR projects only captured non-unit scales through the existing `isaaclab.cloner.query.path_env_ids` and `path_to_clone` boundary using the already-validated `ClonePlan`; real destination scales take precedence. It adds no plan fields, query APIs, renderer configuration, or per-body fallback, and the bridge can be deleted as one unit when SDP supplies composed scale aligned with canonical rigid-body paths. Historical context: [Isaac Sim forum report](https://forums.developer.nvidia.com/t/rigidbody-prim-is-not-updated-in-rendering-pipeline-if-set-to-kinematic/346608). ## Type of change - Bug fix - Shared regression coverage ## Testing - Isaac RTX contract: 4 passed (CUDA/CPU x articulation absent/present). - Newton Warp contract: 1 passed. - OVRTX contract: 2 passed (legacy and OVStage). - OVRTX renderer unit surface: 167 passed. - Core architecture and Newton visualization suites: 25 passed. - OVRTX clone-plan suite: 17 passed. - Cloner query and rendering-contract architecture suites: 87 passed. - Controlled OVRTX regression: failed before the production fix with clone silhouettes of 264 vs. 36 pixels; passed after the fix. - Incoming #7462 NumPy-backed `ClonePlan` query-boundary smoke check: passed unchanged, including non-dense environment ids. - `uv run isaaclab -f`: all hooks passed against the exact upstream `develop` base, including changelog validation. ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the pre-commit checks - [x] Documentation changes are not applicable - [x] I have added unit and integration regression coverage - [x] I have added changelog fragments for every touched package - [x] My name is already present in `CONTRIBUTORS.md`
#7587) # Description Backports #6308 to `release/3.0.0`. The canonical merged commit `ab34e8c5e3ee7a2c5f260d1511714e5be3bed3eb` was cherry-picked with `-x` provenance. Eleven of its twelve source paths replay exactly. `source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py` required a localized release-compatible conflict resolution, so this PR is intentionally a draft for release-maintainer review. | Field | Commit | |---|---| | Original merged change | `ab34e8c5e3ee7a2c5f260d1511714e5be3bed3eb` | | Release base used | `1c754876008f0806fdcfda8e3a6b2f593b34d6fc` | | Proposed backport | `740e3d7b2efe15c5a25f671583d29c034602e36f` | ## Conflict resolution The release renderer already contains the prerequisite work from #6729, #6773, #7010, and #7481, but differs from the source parent around clone-plan handling and method documentation. The resolution preserves the release branch's tensor-backed `ClonePlan` validation and existing renderer structure, then adds only #6308's semantic change: - imports the existing `isaaclab.cloner.query` API; - passes the validated release clone plan into `_capture_object_scales`; - projects captured non-unit source scales to active clone destinations with `path_env_ids` and `path_to_clone`; - retains real destination scales via `setdefault`. No paths outside the original PR are changed. ## Type of change - Bug fix - Shared regression coverage ## Validation - Repository backport candidate validation passed across all 12 original source paths. - Per-file stable patch IDs match on 11 paths; only the conflict-resolved renderer path differs. - Shared rendering-contract architecture tests — 2 passed. - Focused clone-query tests for `path_env_ids` and `path_to_clone` — 4 passed. - Python compilation passed for all changed Python files. - All changed-file pre-commit hooks passed, including changelog and Git LFS checks. - `git diff --check upstream/release/3.0.0...HEAD` passed. - The focused OVRTX runtime test was retried with the documented `ov` extra, but the release lock has no macOS/arm64 environment. Backend rendering tests and the canonical `uv run isaaclab -f` remain pending Linux CI. ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the available pre-commit checks - [x] Documentation changes are not applicable - [x] The original unit and integration regression coverage is preserved - [x] Changelog fragments are preserved for every touched package - [x] The original contributor is already listed in `CONTRIBUTORS.md` Co-authored-by: ooctipus <zhengyuz@nvidia.com>
Description
When OVRTX syncs rigid-body poses from Newton, authored USD scale is lost. Newton transformf carries only translation and rotation, so the per-frame transform write rebuilds a unit-scale matrix and overwrites any non-unit scale on the stage. Shadow Hand (and any other scaled assets) render at the wrong size in kitless OVRTX.
#6787
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there