Repository navigation
Use the simulation rendering dt for OVRTX render steps - #8973
blessedefe4real wants to merge 1 commit into
Conversation
OVRTXRenderer submitted every step with a hardcoded delta_time of 1/60 s. OVRTX uses this delta for temporal effects such as motion blur and motion vectors, so any scene not rendering at 60 Hz produced temporal output that did not match the simulated time between frames. Use SimulationContext.get_rendering_dt() for the synchronous, asynchronous and OVStage submission paths. Fixes isaac-sim#6545
 [Medium impact] The PR appears safe to merge. Summary
No actionable issues were found. Tests were not run during this review. Reviews (2) · Last reviewed commit: "Pass the simulation rendering dt to OVRT..." · Reviewed by Greptile |
There was a problem hiding this comment.
Isaac Lab Review Bot
Needs attention: Style consistency (1). Other reviewed categories look good.
- PR description: The title, scope, and validation match the diff; replace “every render step” with “capture submissions” because the transform-initialization step intentionally remains at zero delta.
- Design and architecture: Architecture looks good.
- Scope and over-engineering: No over-engineering issues.
- API: API changes look good.
- Compatibility and deprecation: No breaking changes identified.
- Implementation: Implementation looks good.
- Style consistency: Move
_RENDERING_DTabove helper functions with_SPAWNto preserve the required imports-constants-functions file order. - Test quality: Test quality looks good.
Minor fixes needed. Posted 1 inline finding.
Automated review; human maintainers own approval decisions.
| return renderer | ||
|
|
||
|
|
||
| _RENDERING_DT = 0.025 |
There was a problem hiding this comment.
🟡 Warning · Style Consistency — Constant declared after helper functions
File order is imports, constants, then functions, and _SPAWN already sits above the helpers. Move _RENDERING_DT into that constant block so shared test setup stays in one place.
Description
OVRTXRenderersubmitted every render step with a hardcodeddelta_timeof 1/60 s. OVRTX uses this delta for temporal effects such as motion blur and motion vectors, so any scene not rendering at 60 Hz produced temporal output that did not match the simulated time between frames.This passes
SimulationContext.get_rendering_dt()to the synchronousstep, the asynchronousstep_async, and the OVStagestep(..., ordinal=...)paths, and removes the_RENDER_DELTA_TIMEconstant.Fixes #6545
Type of change
Validation
test_ovrtx_renderer_contract.pyandtest_ovrtx_visual_material_sync.py: 62 passed, 12 skipped with the fix.main.Checklist
pre-commitcheckssource/<pkg>/changelog.d/for every touched packageCONTRIBUTORS.mdor my name already exists there