Skip to content

clean up visualizer lifecycles #1

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.

Sign up for GitHub

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

Merged
ooctipus merged 1 commit into from
Mar 14, 2026
Merged

clean up visualizer lifecycles #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 25 additions & 22 deletions source/isaaclab/isaaclab/sim/simulation_context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,12 @@ def resolve_visualizer_types(self) -> list[str]:
return [cfg.visualizer_type for cfg in visualizer_cfgs if getattr(cfg, "visualizer_type", None)]

def _resolve_visualizer_cfgs(self) -> list[Any]:
"""Resolve final visualizer configs from cfg and optional CLI override."""
"""Resolve final visualizer configs from cfg and optional CLI override.

When visualizers are explicitly requested via ``--visualizer`` CLI flag,
a :class:`RuntimeError` is raised if any requested type cannot be
resolved (unknown type or missing package).
"""
visualizer_cfgs: list[Any] = []
if self.cfg.visualizer_cfgs is not None:
visualizer_cfgs = (
Expand Down Expand Up @@ -492,6 +497,21 @@ def _resolve_visualizer_cfgs(self) -> list[Any]:
existing_types.add(viz_type)
self._apply_visualizer_cli_overrides(resolved)

# When visualizers were explicitly requested via CLI, verify all
# requested types were resolved. This catches unknown types and
# missing packages that _create_default_visualizer_configs silently
# skips.
if cli_explicit and cli_requested:
resolved_types = {getattr(cfg, "visualizer_type", None) for cfg in resolved}
missing = [t for t in cli_requested if t not in resolved_types]
if missing:
raise RuntimeError(
f"Explicitly requested visualizer(s) {missing} could not be configured. "
f"Valid types: {', '.join(repr(t) for t in _VISUALIZER_TYPES)}. "
"Ensure the required package is installed "
"(e.g., pip install isaaclab_visualizers[<type>])."
)

# XR auto-start: auto-inject a KitVisualizer when XR is active and no
# Kit visualizer is already present. The KitVisualizer pumps
# app.update() and triggers forward() (via requires_forward_before_step)
Expand Down Expand Up @@ -525,27 +545,11 @@ def initialize_visualizers(self) -> None:
self._viz_dt = (physics_dt if physics_dt is not None else self.cfg.dt) * self.cfg.render_interval

visualizer_cfgs = self._resolve_visualizer_cfgs()

cli_explicit = self._is_cli_visualizer_explicit()

# When visualizers were explicitly requested via CLI, verify all requested
# types were resolved to configs. This catches unknown types and missing
# packages that _create_default_visualizer_configs silently skips.
if cli_explicit:
cli_requested = self._get_cli_visualizer_types()
resolved_types = {getattr(cfg, "visualizer_type", None) for cfg in visualizer_cfgs}
missing = [t for t in cli_requested if t not in resolved_types]
if missing:
raise RuntimeError(
f"Explicitly requested visualizer(s) {missing} could not be configured. "
f"Valid types: {', '.join(repr(t) for t in _VISUALIZER_TYPES)}. "
"Ensure the required package is installed "
"(e.g., pip install isaaclab_visualizers[<type>])."
)

if not visualizer_cfgs:
return

cli_explicit = self._is_cli_visualizer_explicit()

# Resolve visualizer-driven requirements once and keep optional artifact payload untouched.
visualizer_types = [
cfg.visualizer_type for cfg in visualizer_cfgs if getattr(cfg, "visualizer_type", None) is not None
Expand All @@ -555,8 +559,6 @@ def initialize_visualizers(self) -> None:
self.initialize_scene_data_provider()
self._visualizers = []

cli_explicit = self._is_cli_visualizer_explicit()

for cfg in visualizer_cfgs:
try:
visualizer = cfg.create_visualizer()
Expand All @@ -565,7 +567,8 @@ def initialize_visualizers(self) -> None:
except Exception as exc:
if cli_explicit:
raise RuntimeError(
f"Visualizer '{cfg.visualizer_type}' was explicitly requested but failed to initialize: {exc}"
f"Visualizer '{cfg.visualizer_type}' was explicitly requested "
f"but failed to create or initialize: {exc}"
) from exc
logger.exception(
"Failed to initialize visualizer '%s' (%s): %s",
Expand Down
114 changes: 63 additions & 51 deletions source/isaaclab/test/sim/test_simulation_context_visualizers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,48 +346,8 @@ def test_get_cli_visualizer_types_handles_non_string_setting_without_crashing():
assert ctx._get_cli_visualizer_types() == []


def test_is_rendering_true_when_only_cfg_visualizer_is_set():
cfg_visualizer = type("CfgVisualizer", (), {"visualizer_type": "newton"})()
cfg = type("Cfg", (), {"visualizer_cfgs": [cfg_visualizer]})()

ctx = object.__new__(SimulationContext)
ctx._has_gui = False
ctx._has_offscreen_render = False
ctx.cfg = cfg

settings = {
"/isaaclab/render/rtx_sensors": False,
"/isaaclab/visualizer/types": "",
"/isaaclab/visualizer/explicit": False,
"/isaaclab/visualizer/disable_all": False,
}
ctx.get_setting = lambda name: settings.get(name)

assert ctx.is_rendering is True


def test_is_rendering_false_when_cli_disable_all_even_with_cfg_visualizer():
cfg_visualizer = type("CfgVisualizer", (), {"visualizer_type": "newton"})()
cfg = type("Cfg", (), {"visualizer_cfgs": [cfg_visualizer]})()

ctx = object.__new__(SimulationContext)
ctx._has_gui = False
ctx._has_offscreen_render = False
ctx.cfg = cfg

settings = {
"/isaaclab/render/rtx_sensors": False,
"/isaaclab/visualizer/types": "",
"/isaaclab/visualizer/explicit": True,
"/isaaclab/visualizer/disable_all": True,
}
ctx.get_setting = lambda name: settings.get(name)

assert ctx.is_rendering is False


# ---------------------------------------------------------------------------
# Tests for explicit CLI visualizer error handling
# Shared helpers for config-resolution and initialize_visualizers tests
# ---------------------------------------------------------------------------


Expand All @@ -410,8 +370,19 @@ def initialize(self, provider):
raise RuntimeError("init failed")


def _make_context_for_init(settings: dict, visualizer_cfgs=None):
"""Build a minimal SimulationContext suitable for testing initialize_visualizers."""
def _make_context_with_settings(
settings: dict,
visualizer_cfgs=None,
*,
has_gui: bool = False,
has_offscreen_render: bool = False,
):
"""Build a minimal SimulationContext suitable for testing is_rendering, _resolve_visualizer_cfgs,
and initialize_visualizers.

Centralises the ``object.__new__`` construction so new internal attributes only need to be added
in one place when the production code changes.
"""
cfg = type(
"Cfg",
(),
Expand All @@ -424,6 +395,9 @@ def _make_context_for_init(settings: dict, visualizer_cfgs=None):
)()
ctx = object.__new__(SimulationContext)
ctx.cfg = cfg
ctx._has_gui = has_gui
ctx._has_offscreen_render = has_offscreen_render
ctx._xr_enabled = False
ctx._visualizers = []
ctx._scene_data_provider = _FakeProvider()
ctx._scene_data_requirements = None
Expand All @@ -434,6 +408,30 @@ def _make_context_for_init(settings: dict, visualizer_cfgs=None):
return ctx


def test_is_rendering_true_when_only_cfg_visualizer_is_set():
cfg_visualizer = type("CfgVisualizer", (), {"visualizer_type": "newton"})()
settings = {
"/isaaclab/render/rtx_sensors": False,
"/isaaclab/visualizer/types": "",
"/isaaclab/visualizer/explicit": False,
"/isaaclab/visualizer/disable_all": False,
}
ctx = _make_context_with_settings(settings, visualizer_cfgs=[cfg_visualizer])
assert ctx.is_rendering is True


def test_is_rendering_false_when_cli_disable_all_even_with_cfg_visualizer():
cfg_visualizer = type("CfgVisualizer", (), {"visualizer_type": "newton"})()
settings = {
"/isaaclab/render/rtx_sensors": False,
"/isaaclab/visualizer/types": "",
"/isaaclab/visualizer/explicit": True,
"/isaaclab/visualizer/disable_all": True,
}
ctx = _make_context_with_settings(settings, visualizer_cfgs=[cfg_visualizer])
assert ctx.is_rendering is False


def test_explicit_unknown_visualizer_type_raises():
"""Requesting an unknown visualizer type via CLI raises RuntimeError."""
settings = {
Expand All @@ -442,7 +440,7 @@ def test_explicit_unknown_visualizer_type_raises():
"/isaaclab/visualizer/disable_all": False,
"/isaaclab/visualizer/max_worlds": None,
}
ctx = _make_context_for_init(settings)
ctx = _make_context_with_settings(settings)

with pytest.raises(RuntimeError, match="bogus_viz"):
ctx.initialize_visualizers()
Expand All @@ -456,7 +454,7 @@ def test_explicit_missing_package_raises(monkeypatch: pytest.MonkeyPatch):
"/isaaclab/visualizer/disable_all": False,
"/isaaclab/visualizer/max_worlds": None,
}
ctx = _make_context_for_init(settings)
ctx = _make_context_with_settings(settings)

# Force import to fail for the rerun visualizer module
import builtins
Expand All @@ -483,13 +481,13 @@ def test_explicit_visualizer_create_failure_raises(monkeypatch: pytest.MonkeyPat
"/isaaclab/visualizer/disable_all": False,
"/isaaclab/visualizer/max_worlds": None,
}
ctx = _make_context_for_init(settings, visualizer_cfgs=[failing_cfg])
ctx = _make_context_with_settings(settings, visualizer_cfgs=[failing_cfg])

import isaaclab.sim.simulation_context as sc_mod

monkeypatch.setattr(sc_mod, "resolve_scene_data_requirements", lambda **kwargs: type("R", (), {})())

with pytest.raises(RuntimeError, match="failed to initialize"):
with pytest.raises(RuntimeError, match="failed to create or initialize"):
ctx.initialize_visualizers()


Expand All @@ -502,13 +500,27 @@ def test_explicit_visualizer_init_failure_raises(monkeypatch: pytest.MonkeyPatch
"/isaaclab/visualizer/disable_all": False,
"/isaaclab/visualizer/max_worlds": None,
}
ctx = _make_context_for_init(settings, visualizer_cfgs=[failing_cfg])
ctx = _make_context_with_settings(settings, visualizer_cfgs=[failing_cfg])

import isaaclab.sim.simulation_context as sc_mod

monkeypatch.setattr(sc_mod, "resolve_scene_data_requirements", lambda **kwargs: type("R", (), {})())

with pytest.raises(RuntimeError, match="failed to initialize"):
with pytest.raises(RuntimeError, match="failed to create or initialize"):
ctx.initialize_visualizers()


def test_explicit_partial_valid_types_raises_for_invalid():
"""Requesting 'newton,bogus_viz' via CLI raises for the unknown type even though newton is valid."""
settings = {
"/isaaclab/visualizer/types": "newton,bogus_viz",
"/isaaclab/visualizer/explicit": True,
"/isaaclab/visualizer/disable_all": False,
"/isaaclab/visualizer/max_worlds": None,
}
ctx = _make_context_with_settings(settings)

with pytest.raises(RuntimeError, match="bogus_viz"):
ctx.initialize_visualizers()


Expand All @@ -520,7 +532,7 @@ def test_non_explicit_unknown_type_silently_skipped(caplog):
"/isaaclab/visualizer/disable_all": False,
"/isaaclab/visualizer/max_worlds": None,
}
ctx = _make_context_for_init(settings)
ctx = _make_context_with_settings(settings)

# Non-explicit: should not raise
ctx.initialize_visualizers()
Expand All @@ -536,7 +548,7 @@ def test_non_explicit_create_failure_silently_logged(monkeypatch: pytest.MonkeyP
"/isaaclab/visualizer/disable_all": False,
"/isaaclab/visualizer/max_worlds": None,
}
ctx = _make_context_for_init(settings, visualizer_cfgs=[failing_cfg])
ctx = _make_context_with_settings(settings, visualizer_cfgs=[failing_cfg])

import isaaclab.sim.simulation_context as sc_mod

Expand Down