Skip to content

Avoid continuous rendering between video captures - #7642

Open
AntoineRichard wants to merge 2 commits into
isaac-sim:developfrom
AntoineRichard:fix/video-idle-rendering-overhead
Open

Avoid continuous rendering between video captures#7642
AntoineRichard wants to merge 2 commits into
isaac-sim:developfrom
AntoineRichard:fix/video-idle-rendering-overhead

Conversation

@AntoineRichard

Copy link
Copy Markdown
Collaborator

Description

--video enabled a headless Kit visualizer and continuous PhysX Fabric transform updates for the full training run, even though frames are only needed during capture windows. This caused a substantial FPS drop between recordings.

This change treats headless visualizers as on-demand rather than continuous renderers, disables continuous Fabric synchronization after PhysX initialization, and synchronizes transforms immediately before each visualizer frame capture. RTX camera sensors explicitly retain continuous Fabric updates.

RTX 5090 benchmark

Isaac-Humanoid-Direct, PhysX, 4,096 environments, RSL-RL, 16 iterations. Idle results exclude iterations 0 and 10, which contain the two scheduled 32-frame capture windows.

Configuration Mean idle FPS Difference
Plain headless 360,635
Offscreen RTX enabled, no video recorder 326,336 -9.5% vs. plain headless
Periodic --video with this fix 315,966 -3.2% vs. offscreen RTX control

The unfixed periodic-video run reproduced approximately 214,300 FPS between captures, about 34% below its plain-headless control. With this fix, most of that avoidable idle-window regression is removed. Capture iterations still drop as expected; the fixed periodic run measured 76,127 and 135,151 FPS during its two capture iterations, with 289,675 FPS overall including both captures.

No new dependencies are required.

Type of change

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

Release backport

  • Backport this pull request to the active release branch after it merges into develop

Screenshots

Not applicable; this is a performance fix. A frame probe on the RTX 5090 verified that all 20 captured frames were non-black and changed over time (temporal_std=72.21, mean consecutive delta 12.46).

Checklist

Docker and GPU tests run on demand. Push the commits you want tested, then
comment run-ci on the pull request.

  • 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 (not applicable; no public API or user workflow changed)
  • 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

Validation

  • uv run --frozen --extra test --extra rerun python -m pytest source/isaaclab/test/sim/test_simulation_context_visualizers.py source/isaaclab/test/envs/test_video_recorder.py -q — 74 passed
  • uv run --frozen isaaclab -f — all hooks passed
  • source/isaaclab/test/sensors/test_camera.py::test_camera_init — passed on RTX 5090

@AntoineRichard
AntoineRichard requested a review from a team September 8, 2026 14:56
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Sep 8, 2026
@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR prevents capture-only headless visualizers from forcing rendering and PhysX Fabric transform synchronization throughout a training run. It:

  • Distinguishes continuous visualizers from headless, on-demand visualizers when determining whether simulation rendering is active.
  • Synchronizes physics transforms immediately before an on-demand visualizer frame is captured.
  • Keeps continuous Fabric updates enabled when RTX camera sensors require them.
  • Adds focused regression coverage for headless visualizer classification, frame synchronization, and RTX camera initialization.
  • Documents the performance fix in a changelog fragment.

Confidence Score: 5/5

The PR appears safe to merge, with the on-demand capture flow synchronized without disrupting continuous RTX camera rendering.

The changed rendering classification, Fabric-setting lifecycle, and capture-time synchronization are mutually consistent and covered by focused regression tests; no actionable correctness, security, or repository-rule issue remains.

Important Files Changed

Filename Overview
source/isaaclab/isaaclab/envs/utils/video_recorder.py Synchronizes physics kinematics immediately before frames from an on-demand visualizer are captured.
source/isaaclab/isaaclab/sim/simulation_context.py Excludes headless capture-only visualizers from continuous rendering and initializes Fabric synchronization accordingly.
source/isaaclab/isaaclab/sensors/camera/camera.py Ensures RTX camera creation restores continuous Fabric transform updates.
source/isaaclab/test/envs/test_video_recorder.py Verifies transform synchronization occurs before on-demand visualizer capture.
source/isaaclab/test/sim/test_simulation_context_visualizers.py Verifies a headless configured visualizer does not activate continuous rendering.
source/isaaclab/test/sensors/test_camera.py Verifies RTX camera initialization enables both RTX sensor rendering and Fabric synchronization.

Sequence Diagram

sequenceDiagram
    participant Training as Training loop
    participant Sim as SimulationContext
    participant Recorder as VideoRecorder
    participant Physics as PhysicsManager
    participant Viz as Headless visualizer

    Note over Sim: Headless capture-only visualizer<br/>does not enable continuous rendering
    loop Between capture windows
        Training->>Sim: Step physics
        Note over Sim,Physics: Continuous Fabric transform<br/>synchronization remains disabled
    end
    rect rgb(235, 245, 255)
        Note over Recorder,Viz: Scheduled capture window
        Training->>Recorder: Request frame
        Recorder->>Sim: Check is_rendering
        Sim-->>Recorder: false
        Recorder->>Sim: forward()
        Sim->>Physics: Update kinematics
        Physics-->>Sim: Transforms synchronized
        Recorder->>Viz: render_rgb_array()
        Viz-->>Recorder: Current frame
    end
Loading

Reviews (1): Last reviewed commit: "Avoid continuous rendering between video..." | Re-trigger Greptile

@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

Reviewed the change that classifies headless capture-only visualizers as on-demand, disables continuous PhysX Fabric synchronization for non-rendering simulations, restores it for Isaac RTX cameras, and synchronizes immediately before visualizer capture. The supplied findings do not establish a broken rendering path from the available context.

  • Design and architecture: The continuous-versus-on-demand rendering split is coherent for periodic headless video capture. Synchronization is intentionally performed at the visualizer capture boundary, while continuously rendered configurations retain per-step updates. The remaining coupling between SimulationContext settings, camera initialization, and VideoRecorder capture is a maintenance tradeoff but is not shown here to cause a concrete defect.
  • API: No public signature changed. The narrowed is_rendering behavior is described as continuous rendering, and its docstring already distinguishes headless offscreen, on-demand rendering from per-step rendering, so the proposed documentation finding is not supported as a contradiction.
  • Implementation: Traced the Fabric setting from SimulationContext initialization through the Isaac RTX camera override and VideoRecorder's guarded sim.forward() call. The claim that OVRTX or generic rgb_array rendering now receives stale transforms lacks the renderer and render-path context needed to demonstrate that those consumers depend on this PhysX Fabric setting or lack their own synchronization.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

@kellyguo11 kellyguo11 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.

@matthewtrepte for review


settings = get_settings_manager()
settings.set_bool("/isaaclab/render/rtx_sensors", True)
settings.set_bool("/physics/fabricUpdateTransformations", True)

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.

why is this needed here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

SimulationContext sets /physics/fabricUpdateTransformations during initialization, before scene-owned Camera objects are constructed. An Isaac RTX camera subsequently makes is_rendering true by setting the adjacent rtx_sensors flag, but that property does not mutate the Fabric setting. Without this line, Fabric remains disabled and camera transforms can be stale. Camera already owns the RTX sensor flag for the same lifecycle reason; the TODO above tracks moving both settings to a renderer pre-reset hook. test_camera_init verifies this path on the RTX 5090.


viz = candidates[0]
if not sim.is_rendering:
sim.forward()

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.

should this be the responsibility of the video recorder?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes—the current placement in VideoRecorder is intentional. It is the component that knows an on-demand frame is about to be requested. Moving the synchronization into KitVisualizer would couple the visualizer to the active physics backend and would also affect non-recorder consumers. sim.forward() delegates to the configured PhysicsManager and only runs here when continuous rendering is disabled. The dual-recorder integration test added in d319ed5 exercises this boundary with two simultaneous capture requests.

@matthewtrepte

Copy link
Copy Markdown
Contributor

ah nice find Antoine.

for an edge case, can you test if we launch multiple headless kit video recordings at once (with multiple VideoRecordingCfg files), if the on demand method doesn't cause any issues.

@AntoineRichard

Copy link
Copy Markdown
Collaborator Author

@matthewtrepte I tested one explicit KitVisualizerCfg(headless=True) shared by two simultaneous VideoRecorderCfg(source="visualizer:kit") entries. Both captured 20/20 non-black, moving frames on the RTX 5090; temporal standard deviations were 37.50 and 37.43. I added the GPU integration coverage in d319ed5. It mocks only the MoviePy encoding boundary and exercises the real PhysX and headless Kit capture path.

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.

3 participants