Skip to content

Comments

Removes majority of dependencies of omni.usd#4685

Open
ooctipus wants to merge 1 commit intoisaac-sim:developfrom
ooctipus:feature/develop/omni_usd_refactor
Open

Removes majority of dependencies of omni.usd#4685
ooctipus wants to merge 1 commit intoisaac-sim:developfrom
ooctipus:feature/develop/omni_usd_refactor

Conversation

@ooctipus
Copy link
Collaborator

Description

This PR removes majority dependencies of relying on omni.usd to manage stage lifecylce. Instead we use pxr directly to manage stage lifecylce and only attach the stage to omni.usd's context when kit needs it.

This helps clean up our repo's dependencies by a lot.

Type of change

  • New feature (non-breaking change which adds functionality)

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

@ooctipus ooctipus force-pushed the feature/develop/omni_usd_refactor branch from f81ecbd to 9cd8ce4 Compare February 22, 2026 07:55
@github-actions github-actions bot added enhancement New feature or request isaac-lab Related to Isaac Lab team labels Feb 22, 2026
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 22, 2026

Greptile Summary

This PR significantly refactors the USD stage lifecycle management to use pure USD (pxr) APIs instead of omni.usd, reducing dependencies and enabling kitless operation mode. The changes introduce a has_kit() helper to detect Kit availability and conditionally attach stages to the Kit context only when needed.

Major changes:

  • Replaced create_new_stage_in_memory() with create_new_stage() using Usd.Stage.CreateInMemory()
  • Removed omni.usd dependencies from stage, prims, queries, and semantics utilities
  • Implemented pure USD versions of resolve_paths(), get_next_free_prim_path(), and _check_ancestral()
  • Refactored add_labels() to use UsdSemantics.LabelsAPI directly instead of Replicator API
  • Removed move_prim() function and is_stage_loading() function
  • Updated all test files to use create_new_stage() instead of omni.usd.get_context().new_stage()

Issues found:

  • Critical: get_current_stage() returns None when _context.stage is not set (line 500), which will cause runtime errors
  • clear_stage() calls omni.kit.app without checking Kit availability (line 468-469)

Breaking changes:

  • Public API change: create_new_stage_in_memory() effectively replaced by create_new_stage()
  • move_prim() function removed from public API
  • is_stage_loading() function removed

Confidence Score: 3/5

  • This PR has significant architectural improvements but contains critical bugs that will cause runtime failures
  • Score reflects the presence of critical logic errors in get_current_stage() and clear_stage() that will cause runtime failures. While the overall architecture is sound and most implementations are correct, these bugs must be fixed before merge. The refactoring is well-tested with comprehensive test coverage, but the identified issues are blockers.
  • Pay close attention to source/isaaclab/isaaclab/sim/utils/stage.py - contains critical bugs in get_current_stage() and clear_stage() functions

Important Files Changed

Filename Overview
source/isaaclab/isaaclab/sim/utils/stage.py Major refactor to use pure USD instead of omni.usd. Contains critical bug in get_current_stage() function that returns None when stage is not set.
source/isaaclab/isaaclab/sim/simulation_context.py Updates stage creation to use create_new_stage() and adds Kit context attachment logic. Logic appears sound.
source/isaaclab/isaaclab/utils/version.py Adds has_kit() helper function to detect Kit availability. Implementation is correct with proper exception handling.
source/isaaclab/isaaclab/sim/utils/queries.py Replaces omni.usd.get_stage_next_free_path with pure USD implementation using regex-based path incrementing.
source/isaaclab/isaaclab/sim/utils/semantics.py Refactors add_labels() to use UsdSemantics.LabelsAPI directly instead of Replicator API, avoiding stage reference issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Application Start] --> B{has_kit?}
    B -->|Yes| C[Kit Mode]
    B -->|No| D[Kitless Mode]
    
    C --> E[create_new_stage]
    D --> E
    
    E --> F[Usd.Stage.CreateInMemory]
    F --> G[Insert in StageCache]
    G --> H[Set _context.stage]
    
    H --> I{Kit Mode?}
    I -->|Yes| J[Attach to omni.usd context]
    I -->|No| K[Skip Kit attachment]
    
    J --> L[SimulationContext ready]
    K --> L
    
    L --> M[Use pure USD APIs]
    M --> N{Operation needs Kit?}
    N -->|Yes| O[has_kit check]
    N -->|No| P[Direct USD operation]
    
    O -->|True| Q[Use Kit API]
    O -->|False| R[Skip or fallback]
    
    Q --> S[Complete]
    P --> S
    R --> S
Loading

Last reviewed commit: 9cd8ce4

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.

48 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

return stage

return stage

Copy link
Contributor

Choose a reason for hiding this comment

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

get_current_stage() returns None when _context.stage is not set, which will cause runtime errors. Should raise an exception or fall back to Kit context.

Suggested change
raise RuntimeError("No current stage set. Call create_new_stage() or open_stage() first.")

Comment on lines 468 to 469
omni.kit.app.get_app_interface().update()

Copy link
Contributor

Choose a reason for hiding this comment

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

clear_stage() calls omni.kit.app without checking if Kit is available (kitless mode). Use has_kit() check.

Suggested change
omni.kit.app.get_app_interface().update()
if has_kit() and builtins.ISAAC_LAUNCHED_FROM_TERMINAL is False: # type: ignore
omni.kit.app.get_app_interface().update()

@ooctipus ooctipus force-pushed the feature/develop/omni_usd_refactor branch from 9cd8ce4 to 0d9bd21 Compare February 22, 2026 22:30
@ooctipus ooctipus force-pushed the feature/develop/omni_usd_refactor branch from 0d9bd21 to c778f10 Compare February 22, 2026 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant