Skip to content

Fix rendering shadow hand environment to preserve object scale - #7010

Merged
pbarejko merged 3 commits into
isaac-sim:developfrom
pbarejko:pbarejko/scale-fix
Aug 19, 2026
Merged

Fix rendering shadow hand environment to preserve object scale#7010
pbarejko merged 3 commits into
isaac-sim:developfrom
pbarejko:pbarejko/scale-fix

Conversation

@pbarejko

Copy link
Copy Markdown
Collaborator

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (existing functionality will not work without user modification)
  • Documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump extension.toml — CI handles that)
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Too many files changed for review (3000 files, 100 file limit).

@pbarejko
pbarejko changed the base branch from release/3.0.0-beta2 to develop August 10, 2026 23:08
@github-actions github-actions Bot added bug Something isn't working asset New asset feature or request isaac-sim Related to Isaac Sim team isaac-mimic Related to Isaac Mimic team infrastructure labels Aug 10, 2026

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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/.

@github-actions github-actions Bot added the isaac-lab Related to Isaac Lab team label Aug 10, 2026
Comment thread source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer_kernels.py Outdated
@huidongc

Copy link
Copy Markdown
Collaborator

PR title: does -> doesn't

@pbarejko pbarejko changed the title Rendering shadow hand environment does preserve object scale Fix rendering shadow hand environment to preserve object scale Aug 11, 2026
@pbarejko
pbarejko force-pushed the pbarejko/scale-fix branch from d3b0335 to 0cc9065 Compare August 11, 2026 16:19
@pbarejko
pbarejko force-pushed the pbarejko/scale-fix branch from 0cc9065 to 358f1da Compare August 12, 2026 21:38
@pbarejko
pbarejko requested a review from marcodiiga as a code owner August 12, 2026 21:38
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.
@pbarejko
pbarejko force-pushed the pbarejko/scale-fix branch from 358f1da to 924fc22 Compare August 18, 2026 22:48
@pbarejko
pbarejko merged commit 0469d0e into isaac-sim:develop Aug 19, 2026
76 of 80 checks passed
kellyguo11 added a commit that referenced this pull request Sep 3, 2026
# 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`
isaaclab-bot Bot pushed a commit that referenced this pull request Sep 3, 2026
# 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)
kellyguo11 pushed a commit that referenced this pull request Sep 4, 2026
# 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`
kellyguo11 added a commit that referenced this pull request Sep 5, 2026
#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

asset New asset feature or request bug Something isn't working infrastructure isaac-lab Related to Isaac Lab team isaac-mimic Related to Isaac Mimic team isaac-sim Related to Isaac Sim team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants