Fixed incorrect camera pose in :class:~isaaclab_newton.sim.views.NewtonSiteFrameView

[huidongc]

Description

Fixed incorrect camera pose in :class:~isaaclab_newton.sim.views.NewtonSiteFrameView.

The code change is cherry-picked from #5979 .

Type of change

• Bug fix (non-breaking change which fixes an issue)

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 updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot — PR #6047

Title: Fixed incorrect camera pose in NewtonSiteFrameView
Author: @huidongc | Base: develop | SHA: a5d810e

---

Summary

This PR fixes a bug in _resolve_source_prim() where the reference prim used for computing the camera frame pose was incorrectly set to the full source_root path. In multi-environment setups where destination_template contains a {} placeholder, the source_root may point to a path deeper than the environment instance root (e.g., /World/envs/env_0/Robot/Camera). The old code passed this full path to stage.GetPrimAtPath() as the reference prim, which would resolve to the wrong transform when computing the relative camera pose.

The fix correctly strips the source_root path down to the environment instance root (e.g., /World/envs/env_0) by:

1. Splitting the destination_template at {} to get the prefix
2. Extracting just the instance identifier from source_root
3. Using prefix + instance as the reference path

Findings

#	Severity	Finding
1	ℹ️ Info	The fix only activates when destination_template contains {} — safe fallback for non-templated paths
2	⚠️ Minor	If source_root does NOT start with destination_prefix, ref_path remains as source_root (original behavior preserved) — good defensive design
3	ℹ️ Info	No unit test added for this fix (checkbox unchecked in PR template). Consider adding a test case for multi-env camera pose resolution.

Analysis

• Correctness: ✅ The path manipulation logic correctly extracts the environment instance root. The split("/", 1)[0] after stripping the prefix isolates the instance ID segment (e.g., env_0), and concatenating with the prefix reconstructs the instance root path.
• Coordinate conventions: ✅ No changes to quaternion/rotation math — the fix only affects which prim is used as the reference frame. resolve_prim_pose and wp.transform remain unchanged.
• Tensor shapes: ✅ No tensor shape changes — the returned wp.transform(pos, quat) structure is identical.
• Downstream impact: ✅ Camera-based sensors that consume frame poses from NewtonSiteFrameView will now receive correct transforms in multi-instance environments.
• Changelog: ✅ Present at changelog.d/huidongc-fix-incorrect-camera-pose.rst
• CI: ⏳ Pre-commit passed; Docker/installation/docs builds still pending.

Verdict

✅ LGTM — Clean, minimal, well-scoped bug fix. The logic is sound and the defensive guards prevent regressions for non-templated configurations. Would be ideal to add a unit test for the multi-env scenario in a follow-up.

---

🔄 Update (127add9): The path-stripping logic has been refactored to use the existing get_suffix() utility from isaaclab.cloner.cloner_utils instead of manual string splitting. Changes:

• Import added: get_suffix now imported alongside iter_clone_plan_matches
• Logic refactored: Instead of manually splitting destination_template at {} and checking prefix/extracting instance, now uses instance_template = destination_template.partition('{}')[0] + '{}' to build the pattern, then calls get_suffix(source_root, instance_template) to get the trailing portion after the instance root
• Path reconstruction: Uses source_root[:-len(source_suffix)] to strip the suffix, which is cleaner than prefix + instance concatenation
• Condition relaxed: No longer requires {} to be present via in check — get_suffix handles this gracefully by returning None when pattern doesn't match

Assessment: ✅ This is a quality improvement — reuses a tested utility function rather than reimplementing pattern matching inline. The logic is equivalent but more robust (handles edge cases like nested {} or unusual path structures that the manual approach might miss). No new concerns. Previous verdict stands: LGTM.

[greptile-apps]

Greptile Summary

This PR fixes a bug where NewtonSiteFrameView computed incorrect world-space poses for camera/sensor frames that are children of the environment root (rather than a rigid body) in a cloned-environment setup. The fix derives the proper per-instance environment root path from the destination_template and source_root when no parent RigidBodyAPI ancestor is found.

• Core change: In _resolve_source_prim, when the prim has no rigid-body ancestor, the reference prim used for resolve_prim_pose is now the environment-instance root (e.g., /World/Env_0) rather than the full source root (e.g., /World/Env_0/Robot), so that the subsequent per-world transform multiplication in _initialize_from_specs produces the correct absolute pose.
• Changelog: A new huidongc-fix-incorrect-camera-pose.rst fragment is added under changelog.d/.

Confidence Score: 4/5

The change is small, targeted, and the core fix is mathematically correct. The one edge-case scenario (partial path-component prefix match) degrades gracefully rather than crashing, so no data corruption or hard failure results from it.

The fix correctly derives the environment-instance root from the destination template and applies it as the reference prim for pose resolution. The only open question is partial-path-component prefix matching, which falls back silently to an absolute pose instead of crashing — low risk in practice but worth tightening.

source/isaaclab_newton/isaaclab_newton/sim/views/newton_site_frame_view.py — specifically the new ref_path derivation block (lines 324–330).

Important Files Changed

Filename	Overview
source/isaaclab_newton/isaaclab_newton/sim/views/newton_site_frame_view.py	Core fix: ref_path now resolves to the environment-instance root when a destination template is present, correcting world-pose computation for non-rigid-body frames. Logic is sound for typical template patterns, with a minor robustness concern around partial path-component prefix matching.
source/isaaclab_newton/changelog.d/huidongc-fix-incorrect-camera-pose.rst	New changelog fragment describing the bug fix; content is accurate and consistent with the code change.

Sequence Diagram

sequenceDiagram
    participant C as Caller
    participant V as NewtonSiteFrameView
    participant S as _resolve_source_prim
    participant USD as USD Stage

    C->>V: __init__(prim_path)
    V->>V: _resolve_site_specs(stage)
    V->>S: _resolve_source_prim(prim, source_root, destination_template, ...)

    note over S: Walk ancestors for RigidBodyAPI
    S->>S: "body_prim = prim.GetParent()"
    alt Parent has RigidBodyAPI
        S->>USD: resolve_prim_pose(prim, body_prim)
        S-->>V: (body_patterns, wp.transform(pos, quat), False, env_ids)
    else No RigidBody ancestor (e.g. world-frame camera)
        note over S: BUG FIX: derive env instance root from destination_template + source_root
        S->>S: "destination_prefix = template.split("{}")[0]"
        S->>S: "source_instance = source_root[len(prefix):].split("/")[0]"
        S->>S: "ref_path = prefix + source_instance"
        S->>USD: GetPrimAtPath(ref_path)
        S->>USD: resolve_prim_pose(prim, ref_prim)
        S-->>V: "(None, wp.transform(pos, quat), per_world=True, env_ids)"
    end

    V->>V: _initialize_from_specs(model)
    note over V: per_world=True: apply world_xform[env_id] * local_pose = correct absolute world pose per env

Loading

_{Reviews (1): Last reviewed commit: "Fixed incorrect camera pose in :class:`~..." | Re-trigger Greptile}