-
Notifications
You must be signed in to change notification settings - Fork 3k
Add USD path sanitization for OV Visualizer Viewport Path #4370
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
base: feature/newton
Are you sure you want to change the base?
Add USD path sanitization for OV Visualizer Viewport Path #4370
Conversation
Greptile OverviewGreptile SummaryThis 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 Solution: Apply 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 Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
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
|
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.
2 files reviewed, 2 comments
| 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) |
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.
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(" ", "_") |
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.
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.
Description
Add USD path sanitization for OV Visualizer Viewport Path
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there