Skip to content

Conversation

@matthewtrepte
Copy link
Contributor

Description

Add USD path sanitization for OV Visualizer Viewport Path

Type of change

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

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 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

@github-actions github-actions bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Jan 13, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 13, 2026

Greptile Overview

Greptile Summary

This PR adds USD path sanitization to fix an issue where viewport names containing spaces would result in invalid camera paths. The fix replaces spaces with underscores when constructing the camera prim path from the viewport name.

Context: The default viewport_name is "Visualizer Viewport" (containing a space), which when used directly in the path /World/Cameras/{viewport_name}_Camera creates an invalid USD path. USD paths cannot contain spaces, causing issues when creating or accessing camera prims.

Solution: Apply .replace(" ", "_") to sanitize the camera path, transforming "/World/Cameras/Visualizer Viewport_Camera" into the valid "/World/Cameras/Visualizer_Viewport_Camera".

Impact: This is a targeted bug fix that ensures the camera creation and assignment work correctly when the viewport name contains spaces. The change is minimal and follows the existing sanitization pattern used elsewhere in the codebase (e.g., in sim/utils/prims.py for semantic labels).

Confidence Score: 4/5

  • Safe to merge - this is a targeted bug fix that addresses invalid USD paths caused by spaces in viewport names
  • The change is minimal and correct, fixing a bug where spaces in viewport names would create invalid USD camera paths. The sanitization approach (replacing spaces with underscores) is consistent with patterns used elsewhere in the codebase. The fix is narrow in scope and only affects camera path construction. However, the sanitization is incomplete (only handles spaces, not other potential invalid characters) and there's a typo in the comment, but these are non-blocking style issues.
  • No files require special attention - the single changed file has a straightforward, localized fix

Important Files Changed

File Analysis

Filename Score Overview
source/isaaclab/isaaclab/visualizers/ov_visualizer.py 4/5 Added space-to-underscore sanitization for USD camera paths to fix invalid path issues when viewport names contain spaces

Sequence Diagram

sequenceDiagram
    participant User
    participant OVVisualizer
    participant USD Stage
    participant Viewport API
    
    User->>OVVisualizer: initialize(scene_data)
    OVVisualizer->>OVVisualizer: _setup_viewport(usd_stage)
    
    alt create_viewport == True
        OVVisualizer->>OVVisualizer: _create_and_assign_camera(usd_stage)
        Note over OVVisualizer: Sanitize viewport_name<br/>(replace spaces with underscores)
        OVVisualizer->>OVVisualizer: camera_path = "/World/Cameras/{name}_Camera".replace(" ", "_")
        OVVisualizer->>USD Stage: GetPrimAtPath(camera_path)
        
        alt Camera doesn't exist
            OVVisualizer->>USD Stage: UsdGeom.Camera.Define(camera_path)
            USD Stage-->>OVVisualizer: camera prim created
        else Camera exists
            USD Stage-->>OVVisualizer: existing camera prim
        end
        
        OVVisualizer->>Viewport API: set_active_camera(camera_path)
        Viewport API-->>OVVisualizer: camera assigned
    end
    
    OVVisualizer-->>User: initialization complete
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

try:
# Create camera prim path based on viewport name
camera_path = f"/World/Cameras/{self.cfg.viewport_name}_Camera"
# Create camera prim path based on viewport name (sanitize to enure valid USD path)
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo in comment: "enure" should be "ensure"

# Create camera prim path based on viewport name
camera_path = f"/World/Cameras/{self.cfg.viewport_name}_Camera"
# Create camera prim path based on viewport name (sanitize to enure valid USD path)
camera_path = f"/World/Cameras/{self.cfg.viewport_name}_Camera".replace(" ", "_")
Copy link
Contributor

Choose a reason for hiding this comment

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

The sanitization only handles spaces, but USD paths can contain other invalid characters (brackets, parentheses, colons, etc.). If viewport_name contains these characters, the path will still be invalid. Consider using a more comprehensive sanitization function or regex like re.sub(r'[^A-Za-z0-9_/-]', '_', viewport_name) to handle all invalid characters, consistent with USD path requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant