Fix headless visualizer runtime in SimulationContext and DirectRLEnvWarp#5061
Fix headless visualizer runtime in SimulationContext and DirectRLEnvWarp#5061IamHBW wants to merge 1 commit intoisaac-sim:developfrom
Conversation
Mirror resolved visualizer state to the legacy setting so visualizer-only runs continue rendering in existing env paths. Also require a real GUI before DirectRLEnvWarp creates a Kit UI window and harden BaseEnvWindow teardown after partial initialization failures. Add regression tests and update extension changelogs and versions.
IamHBW
requested review from
kellyguo11,
matthewtrepte,
ooctipus and
ossamaAhmed
as code owners
github-actions
bot
added
bug
Greptile SummaryThis PR fixes a regression (follow-up to #4948) where headless visualizer-only runs would skip rendering because some code paths still read the legacy Key changes:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[SimulationContext.__init__] --> B[_apply_render_cfg_settings]
B --> C[_sync_legacy_visualizer_setting]
C --> D{resolve_visualizer_types}
D -->|returns types list| E[set /isaaclab/visualizer = bool result]
F[set_setting called with\n/isaaclab/visualizer/* key] -->|not max_worlds| C
G[DirectRLEnvWarp.__init__] --> H{_should_create_ui_window?}
H -->|sim.has_gui AND\nui_window_class_type != None| I[Create Kit UI Window]
H -->|otherwise| J[self._window = None]
K[DirectRLEnvWarp.step] --> L[is_rendering = sim.is_rendering]
L --> M{sim.is_rendering}
M --> N[has_gui\nor offscreen_render\nor rtx_sensors setting\nor visualizers\nor xr_enabled]
N -->|True| O[sim.render called in loop]
N -->|False| P[skip render]
Q[BaseEnvWindow.__del__] --> R[ui_window = getattr self ui_window None]
R -->|not None| S[destroy window]
R -->|None| T[no-op safe teardown]
Last reviewed commit: "Fix headless visuali..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # note: checked here once to avoid multiple checks within the loop | ||
| _has_rtx = hasattr(self.sim, "has_rtx_sensors") and self.sim.has_rtx_sensors() | ||
| is_rendering = bool(self.sim.settings.get("/isaaclab/visualizer")) or _has_rtx | ||
| is_rendering = self.sim.is_rendering |
There was a problem hiding this comment.
The old code used a dynamic self.sim.has_rtx_sensors() call to detect active RTX cameras at runtime:
_has_rtx = hasattr(self.sim, "has_rtx_sensors") and self.sim.has_rtx_sensors()
is_rendering = bool(self.sim.settings.get("/isaaclab/visualizer")) or _has_rtxThe new self.sim.is_rendering property checks self.get_setting("/isaaclab/render/rtx_sensors") (a static setting) rather than has_rtx_sensors(). Importantly, simulation_context.py explicitly notes that has_rtx_sensors is not cached because it changes when camera sensors are created dynamically. If the /isaaclab/render/rtx_sensors setting is not updated when cameras are added at runtime, is_rendering could return False even though has_rtx_sensors() would return True, causing the per-step sim.render() call to be skipped.
For this PR to be fully safe for RTX-sensor workflows, it's worth verifying that the setting is reliably set to True whenever has_rtx_sensors() would return True, or alternatively consider including a has_rtx_sensors() guard in is_rendering itself.
| def test_direct_rl_env_warp_ui_window_requires_gui(): | ||
| env = object.__new__(DirectRLEnvWarp) | ||
| env.sim = type("Sim", (), {"has_gui": False})() | ||
| env.cfg = type("Cfg", (), {"ui_window_class_type": object})() | ||
|
|
||
| assert env._should_create_ui_window() is False |
There was a problem hiding this comment.
_should_create_ui_window not covered
The test only exercises the path where has_gui=False, confirming the method returns False. It doesn't test the complementary case (has_gui=True, ui_window_class_type is not None) to confirm it returns True, nor the case where has_gui=True but ui_window_class_type is None. Adding these assertions would give better confidence that the guard logic is fully correct:
def test_direct_rl_env_warp_ui_window_requires_gui_true():
env = object.__new__(DirectRLEnvWarp)
env.sim = type("Sim", (), {"has_gui": True})()
env.cfg = type("Cfg", (), {"ui_window_class_type": object})()
assert env._should_create_ui_window() is True
def test_direct_rl_env_warp_ui_window_none_class_type():
env = object.__new__(DirectRLEnvWarp)
env.sim = type("Sim", (), {"has_gui": True})()
env.cfg = type("Cfg", (), {"ui_window_class_type": None})()
assert env._should_create_ui_window() is False| if name.startswith("/isaaclab/visualizer/") and name != "/isaaclab/visualizer/max_worlds": | ||
| self._sync_legacy_visualizer_setting() |
There was a problem hiding this comment.
explicit before types
When callers update the CLI visualizer state with two sequential set_setting calls — first "/isaaclab/visualizer/explicit" then "/isaaclab/visualizer/types" — the legacy /isaaclab/visualizer key is briefly False between the two calls (because resolve_visualizer_types() sees explicit=True but types="" after the first call). Any code that reads the legacy key in between would get a stale value.
This is unlikely to matter in practice since both settings are typically written atomically before the simulation loop starts, but it could cause confusion in tests or tooling that writes them separately. Consider documenting this ordering assumption, or accepting explicit and types as a single atomic update.
1 participant
Description
Follow-up to #4948.
This PR fixes a regression in headless visualizer-only runs. After the
visualizer resolution changes, some existing environment paths still read the
legacy
/isaaclab/visualizerboolean instead of the resolved visualizerstate. As a result, rendering could remain disabled even when a visualizer was
active.
This change:
/isaaclab/visualizerinSimulationContext, preserving compatibility for existing code paths thatstill read the legacy key;
DirectRLEnvWarpto useSimulationContext.is_renderingfor renderdecisions;
DirectRLEnvWarpfrom creating a Kit UI window unless a real GUIis available;
BaseEnvWindowteardown so partial initialization failures do notraise
AttributeError;UI-window behavior.
User-visible impact:
paths;
DirectRLEnvWarpno longer attempts to create a Kit UI window duringheadless visualizer-only runs;
Related to #5015 and #5056.
Type of change
Test plan
./isaaclab.sh -p -m pytest source/isaaclab/test/sim/ test_simulation_context_visualizers.py./isaaclab.sh -p -m pytest source/isaaclab/test/envs/ test_direct_rl_env_warp_ui.pyManual verification:
still updates.