From 8fc077b771425715e4bafde282a72411201cacbb Mon Sep 17 00:00:00 2001 From: Rafael Wiltz Date: Thu, 22 Oct 2026 18:05:46 -0400 Subject: [PATCH 1/6] Add a keyboard input API to visualizers BaseVisualizer.key_event_source exposes keys typed into a visualizer's own window, or None when it has none. A KeyEventSource reports each press and release as a W3C KeyboardEvent.code string, plus a focus loss when the window stops delivering keys (blur, the visualizer's UI taking the keyboard, or close). add_key_listener returns a KeyboardSubscription, an idempotent handle the caller closes. Events are forwarded as the window reports them, so a listener tracks the keys it holds. Subscribing, closing and callbacks all happen on the thread that steps the visualizer. Closing the source is terminal: every listener gets one final focus loss. BaseVisualizer.set_camera_keys_enabled lets a keyboard consumer turn off the visualizer's keyboard camera movement while it uses those keys; it does nothing by default. --- docs/source/api/lab/isaaclab.visualizers.rst | 8 + .../rwiltz-visualizer-key-events.added.rst | 8 + .../rwiltz-visualizer-key-events.minor | 0 .../isaaclab/visualizers/__init__.pyi | 3 + .../isaaclab/visualizers/base_visualizer.py | 24 +++ .../isaaclab/visualizers/key_event_source.py | 163 +++++++++++++++ .../test/visualizers/test_key_event_source.py | 191 ++++++++++++++++++ 7 files changed, 397 insertions(+) create mode 100644 source/isaaclab/changelog.d/rwiltz-visualizer-key-events.added.rst create mode 100644 source/isaaclab/changelog.d/rwiltz-visualizer-key-events.minor create mode 100644 source/isaaclab/isaaclab/visualizers/key_event_source.py create mode 100644 source/isaaclab/test/visualizers/test_key_event_source.py diff --git a/docs/source/api/lab/isaaclab.visualizers.rst b/docs/source/api/lab/isaaclab.visualizers.rst index 7408647471e5..2ebb4b75f9b8 100644 --- a/docs/source/api/lab/isaaclab.visualizers.rst +++ b/docs/source/api/lab/isaaclab.visualizers.rst @@ -21,6 +21,8 @@ The following classes are part of the public :mod:`isaaclab.visualizers` API. :nosignatures: BaseVisualizer + KeyEventSource + KeyboardSubscription VisualizerCfg VisualizerBackendCfg WindowCfg @@ -32,6 +34,12 @@ The following classes are part of the public :mod:`isaaclab.visualizers` API. .. autoclass:: BaseVisualizer :show-inheritance: +.. autoclass:: KeyEventSource + :members: closed, add_key_listener, close + +.. autoclass:: KeyboardSubscription + :members: closed, close + .. autoclass:: VisualizerCfg :show-inheritance: diff --git a/source/isaaclab/changelog.d/rwiltz-visualizer-key-events.added.rst b/source/isaaclab/changelog.d/rwiltz-visualizer-key-events.added.rst new file mode 100644 index 000000000000..5c5e9f5ee952 --- /dev/null +++ b/source/isaaclab/changelog.d/rwiltz-visualizer-key-events.added.rst @@ -0,0 +1,8 @@ +* Added :attr:`~isaaclab.visualizers.BaseVisualizer.key_event_source` and + :class:`~isaaclab.visualizers.KeyEventSource`, keyboard input from a focused visualizer window. + :meth:`~isaaclab.visualizers.KeyEventSource.add_key_listener` returns a + :class:`~isaaclab.visualizers.KeyboardSubscription` that receives each key press and release as a + W3C ``KeyboardEvent.code`` string, and a focus loss when the window stops delivering keys. + Visualizers without a local window that can report keys return ``None``, the default. +* Added :meth:`~isaaclab.visualizers.BaseVisualizer.set_camera_keys_enabled`, which turns a + visualizer's keyboard camera movement off while a keyboard consumer uses those keys. diff --git a/source/isaaclab/changelog.d/rwiltz-visualizer-key-events.minor b/source/isaaclab/changelog.d/rwiltz-visualizer-key-events.minor new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab/isaaclab/visualizers/__init__.pyi b/source/isaaclab/isaaclab/visualizers/__init__.pyi index 376601822994..a98acaa7cda8 100644 --- a/source/isaaclab/isaaclab/visualizers/__init__.pyi +++ b/source/isaaclab/isaaclab/visualizers/__init__.pyi @@ -7,6 +7,8 @@ __all__ = [ "BaseVisualizer", "ImageView", "ImageViewCfg", + "KeyEventSource", + "KeyboardSubscription", "PerspectiveCameraCfg", "SceneCameraCfg", "VisualizerCfg", @@ -16,4 +18,5 @@ __all__ = [ from .base_visualizer import BaseVisualizer from .image_view import ImageView +from .key_event_source import KeyboardSubscription, KeyEventSource from .visualizer_cfg import WindowCfg, ImageViewCfg, PerspectiveCameraCfg, SceneCameraCfg, VisualizerCfg, VisualizerBackendCfg diff --git a/source/isaaclab/isaaclab/visualizers/base_visualizer.py b/source/isaaclab/isaaclab/visualizers/base_visualizer.py index 258ef56282a9..321c4bc4bdcf 100644 --- a/source/isaaclab/isaaclab/visualizers/base_visualizer.py +++ b/source/isaaclab/isaaclab/visualizers/base_visualizer.py @@ -35,6 +35,7 @@ from ..renderers.base_renderer import VisualMaterialBatch from ..sensors import Camera from ..sim import SimulationContext + from .key_event_source import KeyEventSource logger = logging.getLogger(__name__) @@ -231,6 +232,29 @@ def physics_backend(self) -> str | None: """ return FactoryBase._get_backend(sim=self._sim) if self._sim is not None else None + @property + def key_event_source(self) -> KeyEventSource | None: + """Keyboard input from this visualizer's local window, or ``None`` when there is none. + + Consumers subscribe to the keys pressed and released while the window has focus, e.g. for + keyboard teleoperation or script hotkeys. The source is created on first access after the + visualizer is initialized, and the visualizer closes it when the visualizer closes. ``None`` + before initialization, after close, and for backends or modes with no local window that can + report keys (the default). + """ + return None + + def set_camera_keys_enabled(self, enabled: bool) -> None: + """Enable or disable moving this visualizer's camera with the keyboard. + + A keyboard consumer such as keyboard teleoperation disables it, so the motion keys it uses + (e.g. WASD/QE and the arrow keys) do not also move the camera. Mouse camera control is + unaffected. Visualizers whose camera takes no keyboard input ignore it (the default). + + Args: + enabled: Whether keyboard camera movement is enabled. + """ + def supports_markers(self) -> bool: """Check if visualizer supports VisualizationMarkers. diff --git a/source/isaaclab/isaaclab/visualizers/key_event_source.py b/source/isaaclab/isaaclab/visualizers/key_event_source.py new file mode 100644 index 000000000000..81b32b167e82 --- /dev/null +++ b/source/isaaclab/isaaclab/visualizers/key_event_source.py @@ -0,0 +1,163 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Keyboard input from a focused visualizer window, shared by the visualizer backends.""" + +from __future__ import annotations + +import logging +from collections.abc import Callable + +logger = logging.getLogger(__name__) + +KeyCallback = Callable[[str, bool], None] +"""Called with a W3C ``KeyboardEvent.code`` string (e.g. ``"KeyW"``) and ``True`` on press, ``False`` on release.""" + +FocusLostCallback = Callable[[], None] +"""Called when the window stops delivering keys: treat every held key as released.""" + + +class KeyboardSubscription: + """A listener subscribed with :meth:`KeyEventSource.add_key_listener`. + + Close it (or use it as a context manager) to unsubscribe. Closing is idempotent and delivers no + final focus loss: the listener chose to leave. + """ + + def __init__(self, source: KeyEventSource, on_key: KeyCallback, on_focus_lost: FocusLostCallback) -> None: + self._source = source + self._on_key = on_key + self._on_focus_lost = on_focus_lost + self._closed = False + + @property + def closed(self) -> bool: + """Whether the subscription no longer receives callbacks.""" + return self._closed + + def close(self) -> None: + """Unsubscribe.""" + if not self._closed: + self._closed = True + self._source._remove_subscription(self) + + def __enter__(self) -> KeyboardSubscription: + return self + + def __exit__(self, *exc) -> None: + self.close() + + +class KeyEventSource: + """Keyboard input from a focused visualizer window. + + Obtained from :attr:`~isaaclab.visualizers.BaseVisualizer.key_event_source`. For every backend: + + - Subscribing, closing and every callback happen on the thread that steps the visualizer. + - ``on_key`` receives the window's presses and releases as W3C ``KeyboardEvent.code`` strings. + Key repeat can arrive as further presses, and a release can arrive without its press (e.g. a + key pressed in the visualizer's UI), so a listener tracks the keys it holds. + - ``on_focus_lost`` runs when the window stops delivering keys: it lost focus, the visualizer's + UI took the keyboard, or the source closed. The listener treats every held key as released. + - A listener that raises is logged and does not affect other listeners. + - Closing the source delivers a final focus loss to every listener; afterwards + :meth:`add_key_listener` raises :class:`RuntimeError`. + + A backend subclasses this, implements :meth:`_start_listening`, :meth:`_stop_listening` and + :meth:`_release`, and reports events through :meth:`_emit_key` and :meth:`_emit_focus_lost`. + """ + + def __init__(self) -> None: + self._subscriptions: list[KeyboardSubscription] = [] + self._listening = False + self._closed = False + + @property + def closed(self) -> bool: + """Whether the source was closed.""" + return self._closed + + def add_key_listener(self, on_key: KeyCallback, on_focus_lost: FocusLostCallback) -> KeyboardSubscription: + """Subscribe to key presses and releases; the backend starts listening with the first listener. + + Args: + on_key: Called with the key's W3C ``KeyboardEvent.code`` and whether it was pressed. + on_focus_lost: Called when the window stops delivering keys. + + Returns: + The subscription. Close it to unsubscribe. + + Raises: + RuntimeError: If the source is closed. + """ + if self._closed: + raise RuntimeError(f"{type(self).__name__} is closed") + if not self._listening: + self._start_listening() + self._listening = True + subscription = KeyboardSubscription(self, on_key, on_focus_lost) + self._subscriptions.append(subscription) + return subscription + + def close(self) -> None: + """Stop listening, release the backend, and deliver a final focus loss to every listener.""" + if self._closed: + return + self._closed = True + subscriptions, self._subscriptions = self._subscriptions, [] + try: + if self._listening: + self._listening = False + self._stop_listening() + finally: + try: + self._release() + finally: + for subscription in subscriptions: + subscription._closed = True + _call(subscription._on_focus_lost) + + # Backend hooks --------------------------------------------------------------------------------- + + def _start_listening(self) -> None: + """Hook the window's key events. Called when the first listener subscribes.""" + + def _stop_listening(self) -> None: + """Unhook the window's key events. Called when the last listener unsubscribes, and on close.""" + + def _release(self) -> None: + """Drop references to the backend's window and viewer. Called once, on close.""" + + # Event reporting ------------------------------------------------------------------------------- + + def _emit_key(self, code: str, pressed: bool) -> None: + """Report a key press or release to every listener.""" + for subscription in list(self._subscriptions): + if not subscription.closed: + _call(subscription._on_key, code, pressed) + + def _emit_focus_lost(self) -> None: + """Tell every listener that the window stopped delivering keys.""" + for subscription in list(self._subscriptions): + if not subscription.closed: + _call(subscription._on_focus_lost) + + # Internals ------------------------------------------------------------------------------------- + + def _remove_subscription(self, subscription: KeyboardSubscription) -> None: + if subscription not in self._subscriptions: + return + self._subscriptions.remove(subscription) + if not self._subscriptions and self._listening: + # stays listening if unhooking raises, so close or the next listener does not hook twice + self._stop_listening() + self._listening = False + + +def _call(callback: Callable, *args) -> None: + try: + callback(*args) + except Exception: + logger.exception("Keyboard listener %r failed", callback) diff --git a/source/isaaclab/test/visualizers/test_key_event_source.py b/source/isaaclab/test/visualizers/test_key_event_source.py new file mode 100644 index 000000000000..4158877e14bf --- /dev/null +++ b/source/isaaclab/test/visualizers/test_key_event_source.py @@ -0,0 +1,191 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +# pyright: reportPrivateUsage=none + +"""Tests for the :class:`~isaaclab.visualizers.KeyEventSource` contract shared by every backend. + +A backend double records its hooks and can fail any of them. +""" + +from __future__ import annotations + +import logging + +import pytest + +from isaaclab.visualizers import KeyEventSource + +pytestmark = [pytest.mark.unit] + + +class _Recorder: + """Listener that records what a source reports.""" + + def __init__(self) -> None: + self.events: list[tuple[str, bool]] = [] + self.focus_lost = 0 + + def on_key(self, code: str, pressed: bool) -> None: + self.events.append((code, pressed)) + + def on_focus_lost(self) -> None: + self.focus_lost += 1 + + +def _listen(source: KeyEventSource): + recorder = _Recorder() + return recorder, source.add_key_listener(recorder.on_key, recorder.on_focus_lost) + + +class _Source(KeyEventSource): + """Backend double that records its hooks and fails the ones listed in ``fail`` once each.""" + + def __init__(self, fail: tuple[str, ...] = ()) -> None: + super().__init__() + self.calls: list[str] = [] + self.fail = list(fail) + + def _hook(self, name: str) -> None: + self.calls.append(name) + if name in self.fail: + self.fail.remove(name) + raise RuntimeError(name) + + def _start_listening(self) -> None: + self._hook("start") + + def _stop_listening(self) -> None: + self._hook("stop") + + def _release(self) -> None: + self._hook("release") + + +def test_backend_listens_while_any_listener_is_subscribed(): + source = _Source() + first, first_subscription = _listen(source) + second, second_subscription = _listen(source) + assert source.calls == ["start"] + + source._emit_key("KeyW", True) + assert first.events == second.events == [("KeyW", True)] + + first_subscription.close() + first_subscription.close() # idempotent + assert first_subscription.closed + assert source.calls == ["start"] + with second_subscription: + pass + assert source.calls == ["start", "stop"] + + _listen(source) + assert source.calls == ["start", "stop", "start"] + + +def test_failed_start_registers_nothing_and_can_be_retried(): + source = _Source(fail=("start",)) + recorder = _Recorder() + with pytest.raises(RuntimeError, match="start"): + source.add_key_listener(recorder.on_key, recorder.on_focus_lost) + source._emit_key("KeyW", True) + assert recorder.events == [] + + retried, _ = _listen(source) + source._emit_key("KeyW", True) + assert source.calls == ["start", "start"] + assert retried.events == [("KeyW", True)] + + +def test_failed_stop_keeps_the_backend_listening(): + source = _Source(fail=("stop",)) + _, subscription = _listen(source) + with pytest.raises(RuntimeError, match="stop"): + subscription.close() + assert subscription.closed + + # still hooked, so the next listener does not start it again and close unhooks it + _listen(source) + source.close() + assert source.calls == ["start", "stop", "stop", "release"] + + +def test_events_are_forwarded_as_the_window_reports_them(): + """Listeners track held keys themselves: repeats and a release without its press reach them.""" + source = _Source() + recorder, _ = _listen(source) + source._emit_key("KeyA", False) + source._emit_key("KeyW", True) + source._emit_key("KeyW", True) + source._emit_key("KeyW", False) + assert recorder.events == [("KeyA", False), ("KeyW", True), ("KeyW", True), ("KeyW", False)] + + +def test_focus_loss_reaches_every_listener(): + source = _Source() + first, _ = _listen(source) + second, _ = _listen(source) + source._emit_focus_lost() + assert first.focus_lost == second.focus_lost == 1 + + +def test_listener_unsubscribed_during_dispatch_gets_no_more_callbacks(): + source = _Source() + later = _Recorder() + subscriptions = [] + source.add_key_listener(lambda code, pressed: subscriptions[0].close(), lambda: None) + subscriptions.append(source.add_key_listener(later.on_key, later.on_focus_lost)) + + source._emit_key("KeyW", True) + assert later.events == [] + + +def test_failing_listener_does_not_affect_others(caplog): + source = _Source() + + def fail(*args): + raise ValueError("listener bug") + + source.add_key_listener(fail, fail) + recorder, _ = _listen(source) + with caplog.at_level(logging.ERROR): + source._emit_key("KeyW", True) + source._emit_focus_lost() + assert recorder.events == [("KeyW", True)] + assert recorder.focus_lost == 1 + assert "listener bug" in caplog.text + + +def test_close_is_terminal(): + source = _Source() + recorder, subscription = _listen(source) + source._emit_key("KeyW", True) + + source.close() + source.close() # idempotent + assert source.closed and subscription.closed + assert source.calls == ["start", "stop", "release"] + assert recorder.focus_lost == 1 + + source._emit_key("KeyW", False) + source._emit_focus_lost() + subscription.close() + assert recorder.events == [("KeyW", True)] + assert recorder.focus_lost == 1 + assert source.calls == ["start", "stop", "release"] + with pytest.raises(RuntimeError, match="closed"): + source.add_key_listener(recorder.on_key, recorder.on_focus_lost) + + +def test_close_releases_and_notifies_even_when_unhooking_fails(): + source = _Source(fail=("stop",)) + recorder, _ = _listen(source) + + with pytest.raises(RuntimeError, match="stop"): + source.close() + assert source.calls == ["start", "stop", "release"] + assert recorder.focus_lost == 1 + source.close() # already closed: the failed step is not retried + assert source.calls == ["start", "stop", "release"] From d4fb5e816b811a3b91b2df13466bea6492a1fcab Mon Sep 17 00:00:00 2001 From: Rafael Wiltz Date: Thu, 22 Oct 2026 18:05:46 -0400 Subject: [PATCH 2/6] Report keys from the Kit and Newton visualizer windows KitVisualizer reports physical keys from the Kit app window and a focus loss on window blur. Kit has no Python query for UI keyboard ownership, so keys typed into its UI are reported too. Its viewport camera keys only act while the right mouse button is held, so it keeps the default set_camera_keys_enabled. NewtonGLVisualizer and NewtonRTXVisualizer report layout-mapped keys from their shared pyglet viewer window. They withhold presses typed into its ImGui widgets, checking ImGui's keyboard ownership after every frame. set_camera_keys_enabled(False) stops the window's WASD/QE and arrow-key camera movement, including for a window opened later. Each visualizer closes its keyboard source first in its teardown, while the window is still open, and finishes that teardown when the source fails to close. Headless runs and the Viser and Rerun visualizers have no keyboard source. --- docs/source/concepts/visualization.rst | 57 ++ .../rwiltz-visualizer-key-events.added.rst | 7 + .../rwiltz-visualizer-key-events.minor | 0 .../kit/kit_key_event_source.py | 165 +++++ .../kit/kit_visualizer.py | 20 + .../newton/newton_key_event_source.py | 187 ++++++ .../newton/newton_visualizer.py | 52 ++ .../test/test_key_event_sources.py | 616 ++++++++++++++++++ .../test_newton_visualizer_viewer_release.py | 29 + 9 files changed, 1133 insertions(+) create mode 100644 source/isaaclab_visualizers/changelog.d/rwiltz-visualizer-key-events.added.rst create mode 100644 source/isaaclab_visualizers/changelog.d/rwiltz-visualizer-key-events.minor create mode 100644 source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py create mode 100644 source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py create mode 100644 source/isaaclab_visualizers/test/test_key_event_sources.py diff --git a/docs/source/concepts/visualization.rst b/docs/source/concepts/visualization.rst index cd48d74688bb..4af32b34b1d1 100644 --- a/docs/source/concepts/visualization.rst +++ b/docs/source/concepts/visualization.rst @@ -706,6 +706,57 @@ Rerun cannot. See :doc:`/source/how-to/record_video` for the full guide and tutorial. +Keyboard Input +~~~~~~~~~~~~~~ + +A visualizer with a local window reports the keys typed into it through +:attr:`~isaaclab.visualizers.BaseVisualizer.key_event_source`, for example to drive a robot from +the keyboard. Each key press and release arrives as a W3C ``KeyboardEvent.code`` string such as +``"KeyW"``: + +- :class:`~isaaclab_visualizers.kit.KitVisualizer` reports physical key positions, independent of + the keyboard layout. Keys typed into Kit's own UI, such as a text field, are reported too. +- :class:`~isaaclab_visualizers.newton.NewtonGLVisualizer` and + :class:`~isaaclab_visualizers.newton.NewtonRTXVisualizer` report layout-mapped keys: a code names + the key that types the same character on a US layout. Presses typed into the window's UI are + withheld. Presses and releases pair up reliably on a US layout; on other layouts, changing Shift + while holding a digit or punctuation key can leave it held until the window loses focus. + +The Viser and Rerun visualizers, and any visualizer running headless, have no keyboard source and +return ``None``. A headless Viser server still takes input from connected browsers, but not through +``key_event_source``. + +Each Newton window has its own source. Kit visualizers share the Kit application window, so each of +them reports the same keys; a consumer that listens to every visualizer must accept a key from more +than one source. + +Close the subscription when you are done with it. A consumer that uses the camera's motion keys +also turns off keyboard camera movement with +:meth:`~isaaclab.visualizers.BaseVisualizer.set_camera_keys_enabled`; a Newton window then stops +moving its camera with WASD/QE and the arrow keys, and Kit's viewport, whose camera keys only act +while the right mouse button is held, is unaffected: + +.. code-block:: python + + visualizer = sim.visualizers[0] + source = visualizer.key_event_source + if source is not None: + held = set() + + def on_key(code: str, pressed: bool) -> None: + (held.add if pressed else held.discard)(code) + + visualizer.set_camera_keys_enabled(False) + with source.add_key_listener(on_key, held.clear): + ... + visualizer.set_camera_keys_enabled(True) + +Subscribe, unsubscribe and close on the thread that steps the visualizer; callbacks run there too, +in event order. Events arrive as the window reports them: key repeat can arrive as further presses, +and a release can arrive without its press, so a listener tracks the keys it holds. +``on_focus_lost`` runs when the window stops delivering keys, because it lost focus, its UI took +the keyboard, or the visualizer closed; treat every held key as released. + .. _visualization-configuration: @@ -945,6 +996,12 @@ Limitations - ✓ - ✓ - ✓ + * - Keyboard input, ``key_event_source`` + - ✓ + - ✓ + - ✗ + - ✗ + - ✓ **Lighting differences across visualizers** diff --git a/source/isaaclab_visualizers/changelog.d/rwiltz-visualizer-key-events.added.rst b/source/isaaclab_visualizers/changelog.d/rwiltz-visualizer-key-events.added.rst new file mode 100644 index 000000000000..89958f0fc564 --- /dev/null +++ b/source/isaaclab_visualizers/changelog.d/rwiltz-visualizer-key-events.added.rst @@ -0,0 +1,7 @@ +* Added ``key_event_source`` to :class:`~isaaclab_visualizers.kit.KitVisualizer`, which reports + physical keys, and to :class:`~isaaclab_visualizers.newton.NewtonGLVisualizer` and + :class:`~isaaclab_visualizers.newton.NewtonRTXVisualizer`, which report layout-mapped keys and + withhold presses typed into their UI. Headless runs and the Viser and Rerun visualizers return + ``None``. +* Added ``set_camera_keys_enabled`` to the Newton GL and RTX visualizers, which turns their WASD/QE + and arrow-key camera movement off and on. diff --git a/source/isaaclab_visualizers/changelog.d/rwiltz-visualizer-key-events.minor b/source/isaaclab_visualizers/changelog.d/rwiltz-visualizer-key-events.minor new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py new file mode 100644 index 000000000000..b5e4983907cf --- /dev/null +++ b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py @@ -0,0 +1,165 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Keyboard event surface for the Kit application window.""" + +from __future__ import annotations + +import functools +import logging +from types import SimpleNamespace +from typing import Any + +from isaaclab.visualizers import KeyEventSource + +logger = logging.getLogger(__name__) + + +@functools.cache +def _carb_key_name_to_w3c() -> dict[str, str]: + """Map ``carb.input.KeyboardInput`` names to W3C ``KeyboardEvent.code`` strings.""" + table: dict[str, str] = {} + for letter in "ABCDEFGHIJKLMNOPQRSTUVWXYZ": + table[letter] = f"Key{letter}" + for digit in "0123456789": + table[f"KEY_{digit}"] = f"Digit{digit}" + table[f"NUMPAD_{digit}"] = f"Numpad{digit}" + for n in range(1, 13): + table[f"F{n}"] = f"F{n}" + table.update( + { + "SPACE": "Space", + "ENTER": "Enter", + "ESCAPE": "Escape", + "TAB": "Tab", + "BACKSPACE": "Backspace", + "DEL": "Delete", + "INSERT": "Insert", + "HOME": "Home", + "END": "End", + "PAGE_UP": "PageUp", + "PAGE_DOWN": "PageDown", + "UP": "ArrowUp", + "DOWN": "ArrowDown", + "LEFT": "ArrowLeft", + "RIGHT": "ArrowRight", + "LEFT_SHIFT": "ShiftLeft", + "RIGHT_SHIFT": "ShiftRight", + "LEFT_CONTROL": "ControlLeft", + "RIGHT_CONTROL": "ControlRight", + "LEFT_ALT": "AltLeft", + "RIGHT_ALT": "AltRight", + "LEFT_SUPER": "MetaLeft", + "RIGHT_SUPER": "MetaRight", + "CAPS_LOCK": "CapsLock", + "MINUS": "Minus", + "EQUAL": "Equal", + "LEFT_BRACKET": "BracketLeft", + "RIGHT_BRACKET": "BracketRight", + "SEMICOLON": "Semicolon", + "APOSTROPHE": "Quote", + "GRAVE_ACCENT": "Backquote", + "BACKSLASH": "Backslash", + "COMMA": "Comma", + "PERIOD": "Period", + "SLASH": "Slash", + "NUMPAD_ADD": "NumpadAdd", + "NUMPAD_SUBTRACT": "NumpadSubtract", + "NUMPAD_MULTIPLY": "NumpadMultiply", + "NUMPAD_DIVIDE": "NumpadDivide", + "NUMPAD_DEL": "NumpadDecimal", + "NUMPAD_ENTER": "NumpadEnter", + "NUMPAD_EQUAL": "NumpadEqual", + } + ) + return table + + +def _kit() -> SimpleNamespace: + """The Kit modules the source uses, imported only when it starts listening.""" + import carb.input + import omni.appwindow + from carb.eventdispatcher import get_eventdispatcher + + return SimpleNamespace(input=carb.input, appwindow=omni.appwindow, dispatcher=get_eventdispatcher()) + + +class KitKeyEventSource(KeyEventSource): + """Keys typed into the Kit application window. + + Reads the default app window's ``carb.input`` keyboard, whose codes name physical key positions, + while a listener is subscribed, and reports a focus loss when the window loses OS focus. The raw + keyboard stream also carries keys typed into Kit's own UI (text fields included), and Kit has no + Python query for UI keyboard ownership, so they are reported too. + """ + + def __init__(self) -> None: + super().__init__() + self._input: Any = None + self._keyboard: Any = None + self._keyboard_subscription: Any = None + self._focus_observer: Any = None + self._event_types: Any = None + + def _start_listening(self) -> None: + kit = _kit() + app_window = kit.appwindow.get_default_app_window() + input_interface = kit.input.acquire_input_interface() + keyboard = app_window.get_keyboard() + subscription = input_interface.subscribe_to_keyboard_events(keyboard, self._on_keyboard_event) + try: + self._focus_observer = kit.dispatcher.observe_event( + filter=app_window.get_event_key(), + event_name=kit.appwindow.GLOBAL_EVENT_WINDOW_FOCUS, + on_event=self._on_focus_event, + observer_name="isaaclab_visualizers.KitKeyEventSource", + ) + except BaseException: + input_interface.unsubscribe_to_keyboard_events(keyboard, subscription) + raise + self._input, self._keyboard, self._keyboard_subscription = input_interface, keyboard, subscription + self._event_types = kit.input.KeyboardEventType + + def _stop_listening(self) -> None: + # Unhook the keyboard first: if that raises, both hooks stay in place and a retry can finish. + if self._keyboard_subscription is not None: + self._input.unsubscribe_to_keyboard_events(self._keyboard, self._keyboard_subscription) + self._keyboard_subscription = None + observer, self._focus_observer = self._focus_observer, None + if observer is not None: + try: + observer.reset() + except Exception: + # The keyboard is already unhooked, so the source is no longer listening either way. + logger.exception("[KitKeyEventSource] Failed to remove the window focus observer") + + def _release(self) -> None: + # A focus observer is still here only if unhooking failed on close: drop it rather than leak it. + observer, self._focus_observer = self._focus_observer, None + self._keyboard_subscription = None + self._input = self._keyboard = self._event_types = None + if observer is not None: + observer.reset() + + def _on_keyboard_event(self, event, *args, **kwargs) -> bool: + event_types = self._event_types + if event_types is None: + return True + # KeyboardEvent is a union: CHAR events carry a character, not a key. + if event.type in (event_types.KEY_PRESS, event_types.KEY_REPEAT): + pressed = True + elif event.type == event_types.KEY_RELEASE: + pressed = False + else: + return True + code = _carb_key_name_to_w3c().get(event.input.name) + if code is not None: + self._emit_key(code, pressed) + # let Kit keep processing the event + return True + + def _on_focus_event(self, event) -> None: + if not event["isFocused"]: + self._emit_focus_lost() diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py index db43c1896a97..4695b0a5d2a5 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py @@ -27,6 +27,8 @@ from isaaclab_visualizers.desktop_entry import write_desktop_entry +from .kit_key_event_source import KitKeyEventSource + logger = logging.getLogger(__name__) if TYPE_CHECKING: @@ -83,9 +85,17 @@ def __init__(self, cfg: VisualizerCfg): # Camera tracking state (replaces ViewportCameraController) self._interactive_scene = None # set from SimulationContext._interactive_scene in initialize() self._viewer_origin: torch.Tensor | None = None # world-space origin offset for eye/lookat + self._key_event_source: KitKeyEventSource | None = None # ---- Lifecycle ------------------------------------------------------------------------ + @property + def key_event_source(self) -> KitKeyEventSource | None: + """Keys typed into the Kit window; ``None`` before initialization, after close, or when Kit runs headless.""" + if self._key_event_source is None and self._is_initialized and not self._runtime_headless: + self._key_event_source = KitKeyEventSource() + return self._key_event_source + @property def visual_material_writer(self): """Write material channels directly through Fabric.""" @@ -198,6 +208,15 @@ def close(self) -> None: """Close viewport resources and restore temporary state.""" if not self._is_initialized: return + # Keep the reference until close finishes: a focus-loss callback reading key_event_source + # during shutdown gets the closing source back instead of creating one that is never closed. + key_event_source = self._key_event_source + if key_event_source is not None: + try: + key_event_source.close() + except Exception: + # keyboard input must not keep the viewport resources below from being released + logger.exception("[KitVisualizer] Keyboard input cleanup failed") try: if self._owns_kit_timeline: self.stop() @@ -223,6 +242,7 @@ def close(self) -> None: self._rgb_render_product = None self._controls_kit_timeline = False self._is_initialized = False + self._key_event_source = None super().close() def play(self) -> None: diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py new file mode 100644 index 000000000000..0c81347b0a58 --- /dev/null +++ b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py @@ -0,0 +1,187 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Keyboard event surface for the Newton viewer window, shared by the GL and RTX visualizers.""" + +from __future__ import annotations + +import functools +from typing import Any + +from isaaclab.visualizers import KeyEventSource + + +@functools.cache +def _pyglet_symbol_to_w3c() -> dict[int, str]: + """Map pyglet key symbols to W3C ``KeyboardEvent.code`` strings (imports ``pyglet.window``).""" + from pyglet.window import key + + return _symbol_table(key) + + +def _symbol_table(key: Any) -> dict[int, str]: + """Map the symbols of a ``pyglet.window.key``-like namespace to W3C ``KeyboardEvent.code`` strings. + + pyglet reports layout-mapped symbols rather than physical positions, so a symbol maps to the key + that types the same character on a US layout. + """ + table: dict[int, str] = {} + for letter in "ABCDEFGHIJKLMNOPQRSTUVWXYZ": + table[getattr(key, letter)] = f"Key{letter}" + for digit in "0123456789": + table[getattr(key, f"_{digit}")] = f"Digit{digit}" + table[getattr(key, f"NUM_{digit}")] = f"Numpad{digit}" + for n in range(1, 13): + table[getattr(key, f"F{n}")] = f"F{n}" + named = { + "SPACE": "Space", + "RETURN": "Enter", + "ENTER": "Enter", + "ESCAPE": "Escape", + "TAB": "Tab", + "BACKSPACE": "Backspace", + "DELETE": "Delete", + "INSERT": "Insert", + "HOME": "Home", + "END": "End", + "PAGEUP": "PageUp", + "PAGEDOWN": "PageDown", + "UP": "ArrowUp", + "DOWN": "ArrowDown", + "LEFT": "ArrowLeft", + "RIGHT": "ArrowRight", + "LSHIFT": "ShiftLeft", + "RSHIFT": "ShiftRight", + "LCTRL": "ControlLeft", + "RCTRL": "ControlRight", + "LALT": "AltLeft", + "RALT": "AltRight", + "LMETA": "MetaLeft", + "RMETA": "MetaRight", + "CAPSLOCK": "CapsLock", + "MINUS": "Minus", + "EQUAL": "Equal", + "BRACKETLEFT": "BracketLeft", + "BRACKETRIGHT": "BracketRight", + "SEMICOLON": "Semicolon", + "APOSTROPHE": "Quote", + "GRAVE": "Backquote", + "BACKSLASH": "Backslash", + "COMMA": "Comma", + "PERIOD": "Period", + "SLASH": "Slash", + "NUM_ADD": "NumpadAdd", + "NUM_SUBTRACT": "NumpadSubtract", + "NUM_MULTIPLY": "NumpadMultiply", + "NUM_DIVIDE": "NumpadDivide", + "NUM_DECIMAL": "NumpadDecimal", + "NUM_ENTER": "NumpadEnter", + # X11 reports Shift+digit or punctuation as the shifted symbol (Shift+= as PLUS), so a key + # pressed before Shift changes is released under the other symbol. Both name the same key. + "EXCLAMATION": "Digit1", + "AT": "Digit2", + "HASH": "Digit3", + "DOLLAR": "Digit4", + "PERCENT": "Digit5", + "ASCIICIRCUM": "Digit6", + "AMPERSAND": "Digit7", + "ASTERISK": "Digit8", + "PARENLEFT": "Digit9", + "PARENRIGHT": "Digit0", + "UNDERSCORE": "Minus", + "PLUS": "Equal", + "BRACELEFT": "BracketLeft", + "BRACERIGHT": "BracketRight", + "COLON": "Semicolon", + "DOUBLEQUOTE": "Quote", + "ASCIITILDE": "Backquote", + "BAR": "Backslash", + "LESS": "Comma", + "GREATER": "Period", + "QUESTION": "Slash", + } + for name, code in named.items(): + symbol = getattr(key, name, None) + if symbol is not None: + table[symbol] = code + return table + + +def viewer_window(viewer: Any) -> Any | None: + """The pyglet window of a Newton viewer, or ``None`` when it has none (headless).""" + return None if viewer.renderer.headless else viewer.renderer.window + + +class NewtonKeyEventSource(KeyEventSource): + """Keys typed into a Newton viewer window. + + Hooks the viewer's pyglet window while a listener is subscribed. pyglet reports layout-mapped key + symbols, so codes name the key that types the same character on a US layout rather than the + physical key. Presses and releases pair up reliably on a US layout, including when Shift + changes while a key is held; elsewhere this is best effort: a key whose shifted character + belongs to another US key (German ``+``/``*``, French ``&``/``1``) can stay held until the + window loses focus. Presses typed into the viewer's ImGui widgets are not forwarded, and when + ImGui takes the keyboard, listeners see a focus loss, as when the window loses focus. + """ + + def __init__(self, viewer: Any, window: Any) -> None: + """Initialize the source. + + Args: + viewer: The :class:`~isaaclab_visualizers.newton.newton_visualizer.NewtonViewerGL` whose + ImGui UI may take the keyboard. + window: The viewer's pyglet window (see :func:`viewer_window`). + """ + super().__init__() + self._viewer = viewer + self._window = window + self._ui_has_keyboard = False + # pushed and removed as one handler frame on the window + self._handlers = { + "on_key_press": self._on_key_press, + "on_key_release": self._on_key_release, + "on_deactivate": self._on_deactivate, + } + + def _start_listening(self) -> None: + self._window.push_handlers(**self._handlers) + self._viewer.keyboard_frame_hook = self._poll_ui_keyboard + + def _stop_listening(self) -> None: + # Remove the handlers first: if that raises, the frame hook stays with them. + self._window.remove_handlers(**self._handlers) + self._viewer.keyboard_frame_hook = None + + def _release(self) -> None: + # The frame hook is still set only if unhooking failed on close: never leave it on the viewer. + if self._viewer.keyboard_frame_hook == self._poll_ui_keyboard: + self._viewer.keyboard_frame_hook = None + self._viewer = self._window = None + + def _poll_ui_keyboard(self) -> bool: + """Track whether ImGui owns the keyboard, reporting a focus loss when it takes it.""" + viewer = self._viewer + if viewer is None: # released, with handlers left behind by a failed unhook + return False + has_keyboard = viewer.gui is not None and bool(viewer.gui.is_keyboard_capturing()) + if has_keyboard and not self._ui_has_keyboard: + self._emit_focus_lost() + self._ui_has_keyboard = has_keyboard + return has_keyboard + + def _on_key_press(self, symbol: int, modifiers: int) -> None: + if self._poll_ui_keyboard(): + return + code = _pyglet_symbol_to_w3c().get(symbol) + if code is not None: + self._emit_key(code, True) + + def _on_key_release(self, symbol: int, modifiers: int) -> None: + code = _pyglet_symbol_to_w3c().get(symbol) + if code is not None: + self._emit_key(code, False) + + def _on_deactivate(self) -> None: + self._emit_focus_lost() diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py index bf108eff6f9f..12d6671421e9 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py @@ -53,6 +53,8 @@ from isaaclab_visualizers.desktop_entry import write_desktop_entry from isaaclab_visualizers.newton.newton_visualization_markers import render_newton_visualization_markers +from .newton_key_event_source import NewtonKeyEventSource, viewer_window + logger = logging.getLogger(__name__) @@ -95,6 +97,10 @@ class NewtonViewerGL(ViewerGL): _contacts_available = True CAMERA_SPEED_BOOST_MULTIPLIER = 2.0 + camera_keys_suspended = False + """Set while keyboard camera movement is disabled (see :meth:`NewtonVisualizerBase.set_camera_keys_enabled`).""" + keyboard_frame_hook: Callable[[], object] | None = None + """Called after every frame while a :class:`NewtonKeyEventSource` listens, to track ImGui keyboard ownership.""" def __init__(self, *args, window_cfg: WindowCfg, metadata: dict | None = None, **kwargs): super().__init__(*args, **kwargs) @@ -389,6 +395,8 @@ def end_frame(self) -> None: super().end_frame() finally: self._fullscreen_image_active = False + if self.keyboard_frame_hook is not None: + self.keyboard_frame_hook() if self._close_requested: self.close() @@ -408,6 +416,12 @@ def on_mouse_press(self, x, y, button, modifiers): if not self._fullscreen_image_active: super().on_mouse_press(x, y, button, modifiers) + def _update_camera(self, dt: float): + """Move the camera from held keys, unless keyboard camera movement is disabled.""" + if self.camera_keys_suspended: + return + super()._update_camera(dt) + def log_points(self, name, points, radii=None, colors=None, hidden=False): """Apply configured model-particle appearance while preserving Newton's point logging. @@ -487,6 +501,17 @@ def __init__(self, cfg): self._last_present_time = -math.inf self._last_render_step = -1 self._pending_mesh_submissions: dict[str, dict[str, Any]] = {} + self._key_event_source: NewtonKeyEventSource | None = None + self._camera_keys_enabled = True + + @property + def key_event_source(self) -> NewtonKeyEventSource | None: + """Keys typed into the viewer window; ``None`` before initialization, after close, or when headless.""" + if self._key_event_source is None and self._viewer is not None: + window = viewer_window(self._viewer) + if window is not None: + self._key_event_source = NewtonKeyEventSource(self._viewer, window) + return self._key_event_source def initialize(self, sim: SimulationContext, *, cameras: list[PerspectiveCameraCfg | Camera]) -> None: """Bind camera sources and the display mode shared by both windows.""" @@ -508,6 +533,7 @@ def initialize(self, sim: SimulationContext, *, cameras: list[PerspectiveCameraC window_cfg=self.cfg.window, ) # fmt: skip self._viewer.marker_groups = sim.vis_marker_registry.get_groups().values() + self._viewer.camera_keys_suspended = not self._camera_keys_enabled self._viewer.register_ui_callback(self._draw_streaming_view_controls, position="side") def log_mesh( @@ -663,6 +689,7 @@ def reset(self, soft: bool = False) -> None: def close(self) -> None: """Detach perspective producers before releasing the visualizer's borrowed views.""" + self._key_event_source = None self._pending_mesh_submissions.clear() for view in self._image_views: if view.render == self._capture_frame: @@ -670,6 +697,29 @@ def close(self) -> None: view.invalidate() super().close() + def set_camera_keys_enabled(self, enabled: bool) -> None: + """Enable or disable the window's WASD/QE and arrow-key camera movement. + + Mouse camera control and the viewer's other hotkeys keep working. The setting also applies to a + window opened later by :meth:`initialize`. + """ + self._camera_keys_enabled = enabled + if self._viewer is not None: + self._viewer.camera_keys_suspended = not enabled + + def _close_key_event_source(self) -> None: + """Unhook keyboard input while the window is still open; subclasses call this first in ``close()``. + + The reference stays until :meth:`close` finishes, so a focus-loss callback reading + :attr:`key_event_source` during shutdown gets the closing source instead of a new one. + """ + if self._key_event_source is not None: + try: + self._key_event_source.close() + except Exception: + # keyboard input must not keep the viewer resources from being released + logger.exception("[%s] Keyboard input cleanup failed", type(self).__name__) + def supports_markers(self) -> bool: """Return whether visualization markers are enabled for this viewer.""" return self.cfg.enable_markers @@ -936,6 +986,7 @@ def close(self) -> None: """Release picking and scene references without destroying Kit's shared GL context.""" if self._is_closed: return + self._close_key_event_source() try: if self._picking_enabled: # Captured graphs retain neutral picking inputs until their last replay. @@ -1195,6 +1246,7 @@ def close(self) -> None: """Close the window; scene cameras retain their render products and shared renderer.""" if self._is_closed: return + self._close_key_event_source() try: if self._viewer is not None: self._viewer.close() diff --git a/source/isaaclab_visualizers/test/test_key_event_sources.py b/source/isaaclab_visualizers/test/test_key_event_sources.py new file mode 100644 index 000000000000..69cd46d5be94 --- /dev/null +++ b/source/isaaclab_visualizers/test/test_key_event_sources.py @@ -0,0 +1,616 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +# pyright: reportPrivateUsage=none + +"""Tests for the Kit and Newton keyboard sources and the visualizers that own them. + +Every backend runs against window, viewer and Kit doubles, so no display, Kit app or GPU is needed. +The contract they share is tested in ``source/isaaclab/test/visualizers/test_key_event_source.py``. +""" + +from __future__ import annotations + +import ast +import importlib.util +import logging +import os +import pathlib +import subprocess +import sys +from types import SimpleNamespace + +import isaaclab_visualizers.kit.kit_key_event_source as kit_key_event_source +import isaaclab_visualizers.newton.newton_key_event_source as newton_key_event_source +import isaaclab_visualizers.newton.newton_visualizer as newton_visualizer +import pytest +from isaaclab_visualizers.kit.kit_key_event_source import KitKeyEventSource, _carb_key_name_to_w3c +from isaaclab_visualizers.kit.kit_visualizer import KitVisualizer +from isaaclab_visualizers.newton import ( + NewtonGLBackendCfg, + NewtonGLVisualizer, + NewtonRTXBackendCfg, + NewtonRTXVisualizer, +) +from isaaclab_visualizers.newton.newton_key_event_source import NewtonKeyEventSource, _symbol_table, viewer_window + +from isaaclab.visualizers import KeyEventSource, VisualizerCfg + +pytestmark = [pytest.mark.unit] + + +class _Recorder: + """Listener that records what a source reports.""" + + def __init__(self) -> None: + self.events: list[tuple[str, bool]] = [] + self.focus_lost = 0 + + def on_key(self, code: str, pressed: bool) -> None: + self.events.append((code, pressed)) + + def on_focus_lost(self) -> None: + self.focus_lost += 1 + + +def _listen(source: KeyEventSource): + recorder = _Recorder() + return recorder, source.add_key_listener(recorder.on_key, recorder.on_focus_lost) + + +# --------------------------------------------------------------------------- +# Kit +# --------------------------------------------------------------------------- + + +class _CarbKeyEvent: + def __init__(self, event_type: int, key: str | None = None) -> None: + self.type = event_type + self._key = key + + @property + def input(self): + if self._key is None: + raise AssertionError("read the key of a CHAR event") + return SimpleNamespace(name=self._key) + + +class _FakeKit: + """``carb.input``, ``omni.appwindow`` and the event dispatcher, as the Kit source uses them.""" + + KEY_PRESS, KEY_RELEASE, KEY_REPEAT, CHAR = range(4) + + def __init__(self) -> None: + self.keyboard_callbacks: list = [] + self.focus_callbacks: list = [] + self.unsubscribed = 0 + self.observers_reset = 0 + self.fail_unsubscribe = False + self.fail_observe = False + self.fail_reset = False + kit = self + keyboard = object() + + class _Observer: + def __init__(self, callback) -> None: + self._callback = callback + + def reset(self) -> None: + if kit.fail_reset: + raise RuntimeError("reset") + kit.observers_reset += 1 + kit.focus_callbacks.remove(self._callback) + + def subscribe(device, callback): + assert device is keyboard + kit.keyboard_callbacks.append(callback) + return callback + + def unsubscribe(device, subscription): + assert device is keyboard + if kit.fail_unsubscribe: + raise RuntimeError("unsubscribe") + kit.unsubscribed += 1 + kit.keyboard_callbacks.remove(subscription) + + def observe_event(*, filter, event_name, on_event, observer_name): + assert (filter, event_name) == ("app-window", "window-focus") + if kit.fail_observe: + raise RuntimeError("observe") + kit.focus_callbacks.append(on_event) + return _Observer(on_event) + + interface = SimpleNamespace(subscribe_to_keyboard_events=subscribe, unsubscribe_to_keyboard_events=unsubscribe) + window = SimpleNamespace(get_keyboard=lambda: keyboard, get_event_key=lambda: "app-window") + self.namespace = SimpleNamespace( + input=SimpleNamespace( + acquire_input_interface=lambda: interface, + KeyboardEventType=SimpleNamespace( + KEY_PRESS=self.KEY_PRESS, KEY_RELEASE=self.KEY_RELEASE, KEY_REPEAT=self.KEY_REPEAT, CHAR=self.CHAR + ), + ), + appwindow=SimpleNamespace(get_default_app_window=lambda: window, GLOBAL_EVENT_WINDOW_FOCUS="window-focus"), + dispatcher=SimpleNamespace(observe_event=observe_event), + ) + + def key(self, event_type: int, key: str | None = None) -> list[bool]: + return [callback(_CarbKeyEvent(event_type, key)) for callback in self.keyboard_callbacks] + + def focus(self, focused: bool) -> None: + for callback in list(self.focus_callbacks): + callback({"isFocused": focused}) + + +@pytest.fixture +def kit(monkeypatch) -> _FakeKit: + fake = _FakeKit() + monkeypatch.setattr(kit_key_event_source, "_kit", lambda: fake.namespace) + return fake + + +def test_kit_hooks_keyboard_and_focus_once(kit): + source = KitKeyEventSource() + _, first = _listen(source) + _, second = _listen(source) + assert len(kit.keyboard_callbacks) == len(kit.focus_callbacks) == 1 + + first.close() + assert len(kit.keyboard_callbacks) == 1 + second.close() + assert kit.keyboard_callbacks == kit.focus_callbacks == [] + assert (kit.unsubscribed, kit.observers_reset) == (1, 1) + + +def test_kit_reports_physical_key_presses_and_releases(kit): + source = KitKeyEventSource() + recorder, _ = _listen(source) + results = [ + kit.key(kit.KEY_PRESS, "W"), + kit.key(kit.KEY_REPEAT, "W"), + kit.key(kit.KEY_RELEASE, "W"), + kit.key(kit.KEY_PRESS, "UNKNOWN"), + ] + assert recorder.events == [("KeyW", True), ("KeyW", True), ("KeyW", False)] + assert results == [[True]] * 4 # Kit keeps processing every event + + +def test_kit_ignores_char_events_without_reading_the_key(kit): + source = KitKeyEventSource() + recorder, _ = _listen(source) + assert kit.key(kit.CHAR) == [True] + assert recorder.events == [] + + +def test_kit_window_blur_is_a_focus_loss(kit): + source = KitKeyEventSource() + recorder, _ = _listen(source) + kit.key(kit.KEY_PRESS, "W") + kit.focus(True) + assert recorder.focus_lost == 0 + kit.focus(False) + assert recorder.focus_lost == 1 + assert recorder.events == [("KeyW", True)] + + +def test_kit_failed_unhook_keeps_both_hooks(kit): + source = KitKeyEventSource() + _, subscription = _listen(source) + kit.fail_unsubscribe = True + with pytest.raises(RuntimeError, match="unsubscribe"): + subscription.close() + assert len(kit.keyboard_callbacks) == len(kit.focus_callbacks) == 1 + + # still listening: a new listener gets blur without a second hook + recorder, _ = _listen(source) + kit.focus(False) + assert recorder.focus_lost == 1 + assert len(kit.keyboard_callbacks) == 1 + + +def test_kit_failed_focus_unhook_still_stops_listening(kit, caplog): + """A focus observer that fails to reset after the keyboard unhooks is logged, not left half-listening.""" + source = KitKeyEventSource() + _, subscription = _listen(source) + kit.fail_reset = True + with caplog.at_level(logging.ERROR): + subscription.close() + assert kit.keyboard_callbacks == [] + assert "window focus observer" in caplog.text + + kit.fail_reset = False + recorder, _ = _listen(source) # hooks the keyboard again + kit.key(kit.KEY_PRESS, "W") + assert recorder.events == [("KeyW", True)] + + +def test_kit_close_drops_the_focus_hook_when_unhooking_fails(kit): + source = KitKeyEventSource() + _listen(source) + kit.fail_unsubscribe = True + with pytest.raises(RuntimeError, match="unsubscribe"): + source.close() + assert kit.focus_callbacks == [] + assert kit.observers_reset == 1 + + +def test_kit_focus_hook_failure_rolls_back_the_keyboard_hook(kit): + source = KitKeyEventSource() + kit.fail_observe = True + with pytest.raises(RuntimeError, match="observe"): + _listen(source) + assert kit.keyboard_callbacks == [] + + kit.fail_observe = False + recorder, _ = _listen(source) + kit.key(kit.KEY_PRESS, "W") + assert recorder.events == [("KeyW", True)] + + +def test_kit_close_unhooks_and_releases_the_window(kit): + source = KitKeyEventSource() + recorder, _ = _listen(source) + source.close() + assert kit.keyboard_callbacks == kit.focus_callbacks == [] + assert recorder.focus_lost == 1 + assert source._input is None and source._keyboard is None + + +@pytest.mark.parametrize( + ("carb_name", "code"), + [ + ("W", "KeyW"), + ("KEY_7", "Digit7"), + ("NUMPAD_3", "Numpad3"), + ("F12", "F12"), + ("DEL", "Delete"), + ("NUMPAD_DEL", "NumpadDecimal"), + ("LEFT_SHIFT", "ShiftLeft"), + ("RIGHT_SUPER", "MetaRight"), + ("APOSTROPHE", "Quote"), + ("GRAVE_ACCENT", "Backquote"), + ], +) +def test_kit_key_names_map_to_w3c_codes(carb_name, code): + assert _carb_key_name_to_w3c()[carb_name] == code + + +# --------------------------------------------------------------------------- +# Newton +# --------------------------------------------------------------------------- + + +def _pyglet_keys() -> SimpleNamespace: + """The constants of ``pyglet/window/key.py``, read without importing ``pyglet.window`` (needs a display).""" + spec = importlib.util.find_spec("pyglet") + if spec is None or spec.origin is None: + pytest.skip("pyglet is not installed") + tree = ast.parse((pathlib.Path(spec.origin).parent / "window" / "key.py").read_text()) + constants = {} + for node in tree.body: + if isinstance(node, ast.Assign) and isinstance(node.value, ast.Constant) and type(node.value.value) is int: + for target in node.targets: + if isinstance(target, ast.Name): + constants[target.id] = node.value.value + return SimpleNamespace(**constants) + + +@pytest.fixture +def keys(monkeypatch) -> SimpleNamespace: + namespace = _pyglet_keys() + table = _symbol_table(namespace) + monkeypatch.setattr(newton_key_event_source, "_pyglet_symbol_to_w3c", lambda: table) + return namespace + + +class _FakeWindow: + """pyglet window double with a handler stack.""" + + def __init__(self) -> None: + self.frames: list[dict] = [] + + def push_handlers(self, **handlers) -> None: + self.frames.append(handlers) + + def remove_handlers(self, **handlers) -> None: + self.frames.remove(handlers) + + def dispatch(self, name: str, *args) -> None: + for frame in reversed(self.frames): + if name in frame: + frame[name](*args) + return + + +class _FakeGui: + def __init__(self) -> None: + self.capturing = False + + def is_keyboard_capturing(self) -> bool: + return self.capturing + + +def _newton_viewer(gui: _FakeGui | None = None) -> SimpleNamespace: + return SimpleNamespace(gui=gui, camera_keys_suspended=False, keyboard_frame_hook=None) + + +def test_newton_forwards_window_keys(keys): + window, viewer = _FakeWindow(), _newton_viewer(_FakeGui()) + source = NewtonKeyEventSource(viewer, window) + recorder, subscription = _listen(source) + assert len(window.frames) == 1 and viewer.keyboard_frame_hook is not None + + window.dispatch("on_key_press", keys.W, 0) + window.dispatch("on_key_release", keys.W, 0) + window.dispatch("on_key_press", keys.RETURN, 0) + assert recorder.events == [("KeyW", True), ("KeyW", False), ("Enter", True)] + + subscription.close() + assert window.frames == [] and viewer.keyboard_frame_hook is None + + +def test_newton_ui_taking_the_keyboard_is_a_focus_loss_on_the_next_frame(keys): + window, gui = _FakeWindow(), _FakeGui() + viewer = _newton_viewer(gui) + source = NewtonKeyEventSource(viewer, window) + recorder, _ = _listen(source) + window.dispatch("on_key_press", keys.W, 0) + + gui.capturing = True # e.g. a text field was clicked: no key press needed + viewer.keyboard_frame_hook() + viewer.keyboard_frame_hook() + assert recorder.focus_lost == 1 + + window.dispatch("on_key_press", keys.A, 0) # typed into the UI + gui.capturing = False + viewer.keyboard_frame_hook() + window.dispatch("on_key_release", keys.A, 0) + assert recorder.events == [("KeyW", True), ("KeyA", False)] # the press stayed in the UI + + +def test_newton_viewer_without_gui_forwards_keys(keys): + window = _FakeWindow() + source = NewtonKeyEventSource(_newton_viewer(gui=None), window) + recorder, _ = _listen(source) + window.dispatch("on_key_press", keys.W, 0) + assert recorder.events == [("KeyW", True)] + + +@pytest.mark.parametrize( + ("unshifted", "shifted", "code"), + [("EQUAL", "PLUS", "Equal"), ("_1", "EXCLAMATION", "Digit1"), ("SLASH", "QUESTION", "Slash")], +) +@pytest.mark.parametrize("shift_first", [False, True], ids=["shift-after-press", "shift-before-press"]) +def test_newton_key_released_after_shift_changes(keys, unshifted, shifted, code, shift_first): + """X11 names a digit or punctuation key by its shifted symbol while Shift is held.""" + window = _FakeWindow() + source = NewtonKeyEventSource(_newton_viewer(_FakeGui()), window) + recorder, _ = _listen(source) + press, release = (shifted, unshifted) if shift_first else (unshifted, shifted) + + window.dispatch("on_key_press", getattr(keys, press), 0) + window.dispatch("on_key_release", getattr(keys, release), 0) + + assert recorder.events == [(code, True), (code, False)] + + +def test_newton_window_deactivation_is_a_focus_loss(keys): + window = _FakeWindow() + source = NewtonKeyEventSource(_newton_viewer(_FakeGui()), window) + recorder, _ = _listen(source) + window.dispatch("on_key_press", keys.W, 0) + window.dispatch("on_deactivate") + assert recorder.focus_lost == 1 + assert recorder.events == [("KeyW", True)] + + +def test_newton_failed_unhook_keeps_the_frame_hook(keys): + window, viewer = _FakeWindow(), _newton_viewer(_FakeGui()) + source = NewtonKeyEventSource(viewer, window) + _, subscription = _listen(source) + + def fail(**handlers): + raise RuntimeError("remove_handlers") + + window.remove_handlers = fail + with pytest.raises(RuntimeError, match="remove_handlers"): + subscription.close() + assert len(window.frames) == 1 and viewer.keyboard_frame_hook is not None + + with pytest.raises(RuntimeError, match="remove_handlers"): + source.close() + assert viewer.keyboard_frame_hook is None # the closed source is never polled again + + +def test_newton_close_unhooks_and_releases_the_window(keys): + window, viewer = _FakeWindow(), _newton_viewer(_FakeGui()) + source = NewtonKeyEventSource(viewer, window) + recorder, _ = _listen(source) + + source.close() + assert window.frames == [] and viewer.keyboard_frame_hook is None + assert recorder.focus_lost == 1 + assert source._viewer is None and source._window is None + + +def test_viewer_window_is_none_without_a_window(): + window = object() + assert viewer_window(SimpleNamespace(renderer=SimpleNamespace(headless=False, window=window))) is window + assert viewer_window(SimpleNamespace(renderer=SimpleNamespace(headless=True, window=window))) is None + + +def test_gl_viewer_polls_keyboard_ownership_after_each_frame(monkeypatch): + events: list[str] = [] + monkeypatch.setattr(newton_visualizer.ViewerGL, "end_frame", lambda self: events.append("frame")) + viewer = object.__new__(newton_visualizer.NewtonViewerGL) + viewer._close_requested = False + viewer.keyboard_frame_hook = lambda: events.append("poll") + + viewer.end_frame() + viewer.keyboard_frame_hook = None + viewer.end_frame() + assert events == ["frame", "poll", "frame"] + + +@pytest.mark.parametrize( + ("pyglet_name", "code"), + [ + ("W", "KeyW"), + ("_7", "Digit7"), + ("NUM_3", "Numpad3"), + ("F12", "F12"), + ("RETURN", "Enter"), + ("LSHIFT", "ShiftLeft"), + ("RMETA", "MetaRight"), + ("BRACKETLEFT", "BracketLeft"), + ("APOSTROPHE", "Quote"), + ("GRAVE", "Backquote"), + ], +) +def test_pyglet_symbols_map_to_w3c_codes(pyglet_name, code): + keys = _pyglet_keys() + assert _symbol_table(keys)[getattr(keys, pyglet_name)] == code + + +# --------------------------------------------------------------------------- +# Visualizer ownership +# --------------------------------------------------------------------------- + + +def _newton_gl_viewer(headless: bool) -> newton_visualizer.NewtonViewerGL: + viewer = object.__new__(newton_visualizer.NewtonViewerGL) + viewer.renderer = SimpleNamespace(headless=headless, window=_FakeWindow()) + viewer.gui = None + viewer.camera_keys_suspended = False + viewer.keyboard_frame_hook = None + viewer.close = lambda: None # the RTX visualizer closes its window + return viewer + + +_NEWTON_VISUALIZERS = [(NewtonGLVisualizer, NewtonGLBackendCfg), (NewtonRTXVisualizer, NewtonRTXBackendCfg)] + + +def _newton_visualizer(viewer, visualizer_type=NewtonGLVisualizer, backend_cfg=NewtonGLBackendCfg): + visualizer = visualizer_type(VisualizerCfg(backend=backend_cfg())) + visualizer._viewer = viewer + return visualizer + + +@pytest.mark.parametrize(("visualizer_type", "backend_cfg"), _NEWTON_VISUALIZERS) +def test_newton_visualizer_key_event_source_lifecycle(keys, visualizer_type, backend_cfg): + assert _newton_visualizer(None, visualizer_type, backend_cfg).key_event_source is None # not initialized + headless = _newton_visualizer(_newton_gl_viewer(headless=True), visualizer_type, backend_cfg) + assert headless.key_event_source is None + + viewer = _newton_gl_viewer(headless=False) + visualizer = _newton_visualizer(viewer, visualizer_type, backend_cfg) + source = visualizer.key_event_source + assert isinstance(source, NewtonKeyEventSource) and visualizer.key_event_source is source + recorder, _ = _listen(source) + + visualizer.close() + assert source.closed and recorder.focus_lost == 1 + assert viewer.keyboard_frame_hook is None + assert visualizer._viewer is None and visualizer.key_event_source is None + + +@pytest.mark.parametrize(("visualizer_type", "backend_cfg"), _NEWTON_VISUALIZERS) +def test_viewer_skips_camera_keys_while_disabled(monkeypatch, visualizer_type, backend_cfg): + moved: list[float] = [] + monkeypatch.setattr(newton_visualizer.ViewerGL, "_update_camera", lambda self, dt: moved.append(dt)) + viewer = _newton_gl_viewer(headless=False) + visualizer = _newton_visualizer(viewer, visualizer_type, backend_cfg) + + viewer._update_camera(0.1) + visualizer.set_camera_keys_enabled(False) + viewer._update_camera(0.2) + visualizer.set_camera_keys_enabled(True) + viewer._update_camera(0.3) + assert moved == [0.1, 0.3] + + +@pytest.mark.parametrize(("visualizer_type", "backend_cfg"), _NEWTON_VISUALIZERS) +def test_newton_focus_loss_during_shutdown_cannot_create_a_new_source(keys, visualizer_type, backend_cfg): + visualizer = _newton_visualizer(_newton_gl_viewer(headless=False), visualizer_type, backend_cfg) + source = visualizer.key_event_source + seen = [] + source.add_key_listener(lambda code, pressed: None, lambda: seen.append(visualizer.key_event_source)) + + visualizer.close() + assert seen == [source] and source.closed # the closing source, not a new one + assert visualizer.key_event_source is None + + +def _kit_visualizer(monkeypatch, *, initialized: bool, headless: bool) -> KitVisualizer: + visualizer = object.__new__(KitVisualizer) + visualizer._is_initialized = initialized + visualizer._is_closed = False + visualizer._runtime_headless = headless + visualizer._key_event_source = None + visualizer._camera_choices = [] + visualizer._viewport_camera_xform_ops = {} + visualizer._viewport_camera_pose_cache = {} + visualizer._camera_image_provider = None + visualizer._camera_image_window = None + visualizer._rgb_annotator = None + visualizer._rgb_render_product = None + visualizer._owns_kit_timeline = False + visualizer._image_views = [] + visualizer.torn_down = [] + monkeypatch.setattr(visualizer, "_teardown_backend_menubar_label", lambda: visualizer.torn_down.append("menu")) + monkeypatch.setattr(visualizer, "_restore_env_visibility", lambda: visualizer.torn_down.append("visibility")) + return visualizer + + +def test_kit_visualizer_key_event_source_lifecycle(monkeypatch, kit): + assert _kit_visualizer(monkeypatch, initialized=False, headless=False).key_event_source is None + assert _kit_visualizer(monkeypatch, initialized=True, headless=True).key_event_source is None + + visualizer = _kit_visualizer(monkeypatch, initialized=True, headless=False) + source = visualizer.key_event_source + assert isinstance(source, KitKeyEventSource) and visualizer.key_event_source is source + recorder, _ = _listen(source) + + visualizer.close() + assert source.closed and recorder.focus_lost == 1 + assert kit.keyboard_callbacks == [] + assert not visualizer._is_initialized and visualizer.key_event_source is None + + +def test_kit_focus_loss_during_shutdown_cannot_create_a_new_source(monkeypatch, kit): + visualizer = _kit_visualizer(monkeypatch, initialized=True, headless=False) + source = visualizer.key_event_source + seen = [] + source.add_key_listener(lambda code, pressed: None, lambda: seen.append(visualizer.key_event_source)) + + visualizer.close() + assert seen == [source] and source.closed # the closing source, not a new one + assert visualizer.key_event_source is None + assert kit.keyboard_callbacks == [] + + +def test_kit_visualizer_close_survives_a_failing_key_source(monkeypatch, kit, caplog): + visualizer = _kit_visualizer(monkeypatch, initialized=True, headless=False) + _listen(visualizer.key_event_source) + kit.fail_unsubscribe = True + + with caplog.at_level(logging.ERROR): + visualizer.close() + assert visualizer.torn_down == ["menu", "visibility"] + assert visualizer._is_closed and visualizer.key_event_source is None + assert "Keyboard input cleanup failed" in caplog.text + + +def test_backends_import_without_a_display_or_kit(): + code = ( + "import sys\n" + "from isaaclab_visualizers.kit.kit_key_event_source import KitKeyEventSource\n" + "from isaaclab_visualizers.newton.newton_key_event_source import NewtonKeyEventSource\n" + "KitKeyEventSource(); NewtonKeyEventSource(object(), object())\n" + "assert 'pyglet.window' not in sys.modules, 'pyglet.window'\n" + "assert 'carb' not in sys.modules, 'carb'\n" + ) + env = {k: v for k, v in os.environ.items() if k not in ("DISPLAY", "WAYLAND_DISPLAY")} + result = subprocess.run([sys.executable, "-c", code], env=env, capture_output=True, text=True, timeout=120) + assert result.returncode == 0, result.stderr diff --git a/source/isaaclab_visualizers/test/test_newton_visualizer_viewer_release.py b/source/isaaclab_visualizers/test/test_newton_visualizer_viewer_release.py index aac47d862479..9ebfc587a0dd 100644 --- a/source/isaaclab_visualizers/test/test_newton_visualizer_viewer_release.py +++ b/source/isaaclab_visualizers/test/test_newton_visualizer_viewer_release.py @@ -72,3 +72,32 @@ def test_close_releases_owned_resources_even_on_failure(failure): assert visualizer._sim is visualizer.current_image_view is None assert not visualizer._image_views assert visualizer._is_closed + + +@pytest.mark.parametrize( + ("visualizer_type", "backend_cfg"), + [(NewtonGLVisualizer, NewtonGLBackendCfg), (NewtonRTXVisualizer, NewtonRTXBackendCfg)], +) +@pytest.mark.parametrize("failure", [False, True]) +def test_close_unhooks_keyboard_before_releasing_the_viewer(visualizer_type, backend_cfg, failure, caplog): + """Keyboard input closes while the window is open, and a failure there must not leak the viewer.""" + visualizer = visualizer_type(VisualizerCfg(backend=backend_cfg())) + viewer = visualizer._viewer = Mock() + events: list[str] = [] + key_event_source = visualizer._key_event_source = Mock() + + def close_keys() -> None: + events.append(f"keys closed, viewer bound: {visualizer._viewer is viewer}") + if failure: + raise RuntimeError("keyboard cleanup failed") + + key_event_source.close.side_effect = close_keys + + visualizer.close() + + assert events == ["keys closed, viewer bound: True"] + assert ("Keyboard input cleanup failed" in caplog.text) == failure + assert visualizer._viewer is None + assert visualizer._key_event_source is None + assert visualizer.key_event_source is None + assert visualizer._is_closed From 14428a537755838c38c16f8601f41b00c3316ba3 Mon Sep 17 00:00:00 2001 From: ooctipus Date: Fri, 23 Oct 2026 14:36:14 -0700 Subject: [PATCH 3/6] Bind keyboard input to visualizer window lifetime --- docs/source/concepts/visualization.rst | 12 +- .../minimal-window-keyboard.changed.rst | 2 + .../isaaclab/visualizers/base_visualizer.py | 4 +- .../isaaclab/visualizers/key_event_source.py | 55 +--- .../test/visualizers/test_key_event_source.py | 139 +++------ .../minimal-window-keyboard.changed.rst | 2 + .../kit/kit_key_event_source.py | 78 ++--- .../kit/kit_visualizer.py | 6 +- .../newton/newton_key_event_source.py | 40 +-- .../newton/newton_visualizer.py | 8 +- .../test/test_key_event_sources.py | 291 ++++-------------- .../test_newton_visualizer_viewer_release.py | 16 +- .../test/visualizer_integration_utils.py | 2 + 13 files changed, 199 insertions(+), 456 deletions(-) create mode 100644 source/isaaclab/changelog.d/minimal-window-keyboard.changed.rst create mode 100644 source/isaaclab_visualizers/changelog.d/minimal-window-keyboard.changed.rst diff --git a/docs/source/concepts/visualization.rst b/docs/source/concepts/visualization.rst index 4af32b34b1d1..0514878abb40 100644 --- a/docs/source/concepts/visualization.rst +++ b/docs/source/concepts/visualization.rst @@ -730,6 +730,10 @@ Each Newton window has its own source. Kit visualizers share the Kit application them reports the same keys; a consumer that listens to every visualizer must accept a key from more than one source. +The visualizer connects native keyboard and focus events when its window initializes and +disconnects them before the window closes. A subscription only adds or removes a listener; +it does not initialize window resources. The source owns no held-key state or camera policy. + Close the subscription when you are done with it. A consumer that uses the camera's motion keys also turns off keyboard camera movement with :meth:`~isaaclab.visualizers.BaseVisualizer.set_camera_keys_enabled`; a Newton window then stops @@ -747,9 +751,11 @@ while the right mouse button is held, is unaffected: (held.add if pressed else held.discard)(code) visualizer.set_camera_keys_enabled(False) - with source.add_key_listener(on_key, held.clear): - ... - visualizer.set_camera_keys_enabled(True) + try: + with source.add_key_listener(on_key, held.clear): + ... # use held while stepping the simulation + finally: + visualizer.set_camera_keys_enabled(True) Subscribe, unsubscribe and close on the thread that steps the visualizer; callbacks run there too, in event order. Events arrive as the window reports them: key repeat can arrive as further presses, diff --git a/source/isaaclab/changelog.d/minimal-window-keyboard.changed.rst b/source/isaaclab/changelog.d/minimal-window-keyboard.changed.rst new file mode 100644 index 000000000000..d3c7b3870788 --- /dev/null +++ b/source/isaaclab/changelog.d/minimal-window-keyboard.changed.rst @@ -0,0 +1,2 @@ +* Simplified keyboard sources to listener registration and event forwarding; native window hooks + were initialized and closed by visualizers instead of changing with listener counts. diff --git a/source/isaaclab/isaaclab/visualizers/base_visualizer.py b/source/isaaclab/isaaclab/visualizers/base_visualizer.py index 321c4bc4bdcf..71d1cb8c2b18 100644 --- a/source/isaaclab/isaaclab/visualizers/base_visualizer.py +++ b/source/isaaclab/isaaclab/visualizers/base_visualizer.py @@ -237,8 +237,8 @@ def key_event_source(self) -> KeyEventSource | None: """Keyboard input from this visualizer's local window, or ``None`` when there is none. Consumers subscribe to the keys pressed and released while the window has focus, e.g. for - keyboard teleoperation or script hotkeys. The source is created on first access after the - visualizer is initialized, and the visualizer closes it when the visualizer closes. ``None`` + keyboard teleoperation or script hotkeys. The source connects when the visualizer initializes + its window, and the visualizer closes it before releasing that window. ``None`` before initialization, after close, and for backends or modes with no local window that can report keys (the default). """ diff --git a/source/isaaclab/isaaclab/visualizers/key_event_source.py b/source/isaaclab/isaaclab/visualizers/key_event_source.py index 81b32b167e82..a85ffe03b221 100644 --- a/source/isaaclab/isaaclab/visualizers/key_event_source.py +++ b/source/isaaclab/isaaclab/visualizers/key_event_source.py @@ -41,7 +41,8 @@ def close(self) -> None: """Unsubscribe.""" if not self._closed: self._closed = True - self._source._remove_subscription(self) + if not self._source.closed: + self._source._subscriptions.remove(self) def __enter__(self) -> KeyboardSubscription: return self @@ -65,13 +66,13 @@ class KeyEventSource: - Closing the source delivers a final focus loss to every listener; afterwards :meth:`add_key_listener` raises :class:`RuntimeError`. - A backend subclasses this, implements :meth:`_start_listening`, :meth:`_stop_listening` and - :meth:`_release`, and reports events through :meth:`_emit_key` and :meth:`_emit_focus_lost`. + The visualizer connects native events when its window initializes and closes the source before + the window closes. Backend bridges report events through :meth:`_emit_key` and + :meth:`_emit_focus_lost`; adding or removing a listener never changes native window resources. """ def __init__(self) -> None: self._subscriptions: list[KeyboardSubscription] = [] - self._listening = False self._closed = False @property @@ -80,7 +81,7 @@ def closed(self) -> bool: return self._closed def add_key_listener(self, on_key: KeyCallback, on_focus_lost: FocusLostCallback) -> KeyboardSubscription: - """Subscribe to key presses and releases; the backend starts listening with the first listener. + """Subscribe to key presses and releases from the initialized window. Args: on_key: Called with the key's W3C ``KeyboardEvent.code`` and whether it was pressed. @@ -94,43 +95,20 @@ def add_key_listener(self, on_key: KeyCallback, on_focus_lost: FocusLostCallback """ if self._closed: raise RuntimeError(f"{type(self).__name__} is closed") - if not self._listening: - self._start_listening() - self._listening = True subscription = KeyboardSubscription(self, on_key, on_focus_lost) self._subscriptions.append(subscription) return subscription def close(self) -> None: - """Stop listening, release the backend, and deliver a final focus loss to every listener.""" + """Close every subscription and deliver a final focus loss to its listener.""" if self._closed: return self._closed = True subscriptions, self._subscriptions = self._subscriptions, [] - try: - if self._listening: - self._listening = False - self._stop_listening() - finally: - try: - self._release() - finally: - for subscription in subscriptions: - subscription._closed = True - _call(subscription._on_focus_lost) - - # Backend hooks --------------------------------------------------------------------------------- - - def _start_listening(self) -> None: - """Hook the window's key events. Called when the first listener subscribes.""" - - def _stop_listening(self) -> None: - """Unhook the window's key events. Called when the last listener unsubscribes, and on close.""" - - def _release(self) -> None: - """Drop references to the backend's window and viewer. Called once, on close.""" - - # Event reporting ------------------------------------------------------------------------------- + for subscription in subscriptions: + if not subscription.closed: + subscription._closed = True + _call(subscription._on_focus_lost) def _emit_key(self, code: str, pressed: bool) -> None: """Report a key press or release to every listener.""" @@ -144,17 +122,6 @@ def _emit_focus_lost(self) -> None: if not subscription.closed: _call(subscription._on_focus_lost) - # Internals ------------------------------------------------------------------------------------- - - def _remove_subscription(self, subscription: KeyboardSubscription) -> None: - if subscription not in self._subscriptions: - return - self._subscriptions.remove(subscription) - if not self._subscriptions and self._listening: - # stays listening if unhooking raises, so close or the next listener does not hook twice - self._stop_listening() - self._listening = False - def _call(callback: Callable, *args) -> None: try: diff --git a/source/isaaclab/test/visualizers/test_key_event_source.py b/source/isaaclab/test/visualizers/test_key_event_source.py index 4158877e14bf..7f6d6c3a90c7 100644 --- a/source/isaaclab/test/visualizers/test_key_event_source.py +++ b/source/isaaclab/test/visualizers/test_key_event_source.py @@ -5,10 +5,8 @@ # pyright: reportPrivateUsage=none -"""Tests for the :class:`~isaaclab.visualizers.KeyEventSource` contract shared by every backend. -A backend double records its hooks and can fail any of them. -""" +"""Window-independent event forwarding; native hook ownership is tested at each backend.""" from __future__ import annotations @@ -40,81 +38,9 @@ def _listen(source: KeyEventSource): return recorder, source.add_key_listener(recorder.on_key, recorder.on_focus_lost) -class _Source(KeyEventSource): - """Backend double that records its hooks and fails the ones listed in ``fail`` once each.""" - - def __init__(self, fail: tuple[str, ...] = ()) -> None: - super().__init__() - self.calls: list[str] = [] - self.fail = list(fail) - - def _hook(self, name: str) -> None: - self.calls.append(name) - if name in self.fail: - self.fail.remove(name) - raise RuntimeError(name) - - def _start_listening(self) -> None: - self._hook("start") - - def _stop_listening(self) -> None: - self._hook("stop") - - def _release(self) -> None: - self._hook("release") - - -def test_backend_listens_while_any_listener_is_subscribed(): - source = _Source() - first, first_subscription = _listen(source) - second, second_subscription = _listen(source) - assert source.calls == ["start"] - - source._emit_key("KeyW", True) - assert first.events == second.events == [("KeyW", True)] - - first_subscription.close() - first_subscription.close() # idempotent - assert first_subscription.closed - assert source.calls == ["start"] - with second_subscription: - pass - assert source.calls == ["start", "stop"] - - _listen(source) - assert source.calls == ["start", "stop", "start"] - - -def test_failed_start_registers_nothing_and_can_be_retried(): - source = _Source(fail=("start",)) - recorder = _Recorder() - with pytest.raises(RuntimeError, match="start"): - source.add_key_listener(recorder.on_key, recorder.on_focus_lost) - source._emit_key("KeyW", True) - assert recorder.events == [] - - retried, _ = _listen(source) - source._emit_key("KeyW", True) - assert source.calls == ["start", "start"] - assert retried.events == [("KeyW", True)] - - -def test_failed_stop_keeps_the_backend_listening(): - source = _Source(fail=("stop",)) - _, subscription = _listen(source) - with pytest.raises(RuntimeError, match="stop"): - subscription.close() - assert subscription.closed - - # still hooked, so the next listener does not start it again and close unhooks it - _listen(source) - source.close() - assert source.calls == ["start", "stop", "stop", "release"] - - def test_events_are_forwarded_as_the_window_reports_them(): """Listeners track held keys themselves: repeats and a release without its press reach them.""" - source = _Source() + source = KeyEventSource() recorder, _ = _listen(source) source._emit_key("KeyA", False) source._emit_key("KeyW", True) @@ -124,26 +50,32 @@ def test_events_are_forwarded_as_the_window_reports_them(): def test_focus_loss_reaches_every_listener(): - source = _Source() + source = KeyEventSource() first, _ = _listen(source) second, _ = _listen(source) source._emit_focus_lost() assert first.focus_lost == second.focus_lost == 1 -def test_listener_unsubscribed_during_dispatch_gets_no_more_callbacks(): - source = _Source() +@pytest.mark.parametrize("event", ["key", "focus", "close"]) +def test_listener_unsubscribed_during_dispatch_gets_no_more_callbacks(event): + source = KeyEventSource() later = _Recorder() subscriptions = [] - source.add_key_listener(lambda code, pressed: subscriptions[0].close(), lambda: None) + source.add_key_listener(lambda code, pressed: subscriptions[0].close(), lambda: subscriptions[0].close()) subscriptions.append(source.add_key_listener(later.on_key, later.on_focus_lost)) - source._emit_key("KeyW", True) - assert later.events == [] + if event == "key": + source._emit_key("KeyW", True) + elif event == "focus": + source._emit_focus_lost() + else: + source.close() + assert later.events == [] and later.focus_lost == 0 def test_failing_listener_does_not_affect_others(caplog): - source = _Source() + source = KeyEventSource() def fail(*args): raise ValueError("listener bug") @@ -159,14 +91,13 @@ def fail(*args): def test_close_is_terminal(): - source = _Source() + source = KeyEventSource() recorder, subscription = _listen(source) source._emit_key("KeyW", True) source.close() source.close() # idempotent assert source.closed and subscription.closed - assert source.calls == ["start", "stop", "release"] assert recorder.focus_lost == 1 source._emit_key("KeyW", False) @@ -174,18 +105,34 @@ def test_close_is_terminal(): subscription.close() assert recorder.events == [("KeyW", True)] assert recorder.focus_lost == 1 - assert source.calls == ["start", "stop", "release"] with pytest.raises(RuntimeError, match="closed"): source.add_key_listener(recorder.on_key, recorder.on_focus_lost) -def test_close_releases_and_notifies_even_when_unhooking_fails(): - source = _Source(fail=("stop",)) - recorder, _ = _listen(source) - - with pytest.raises(RuntimeError, match="stop"): - source.close() - assert source.calls == ["start", "stop", "release"] - assert recorder.focus_lost == 1 - source.close() # already closed: the failed step is not retried - assert source.calls == ["start", "stop", "release"] +def test_subscriptions_are_independent_and_close_is_idempotent(): + source = KeyEventSource() + first, subscription = _listen(source) + second, _ = _listen(source) + source._emit_key("KeyW", True) + subscription.close() + subscription.close() + source._emit_key("KeyW", False) + assert first.events == [("KeyW", True)] + assert second.events == [("KeyW", True), ("KeyW", False)] + assert subscription.closed and first.focus_lost == 0 + + +def test_source_does_not_own_backend_lifecycle_or_controller_state(): + """Window setup belongs to initialize/close; held keys and camera policy belong to consumers.""" + source = KeyEventSource() + forbidden = ( + "_start_listening", + "_stop_listening", + "_release", + "_listening", + "_held", + "_captures", + "_lock", + "capture_keyboard", + ) + assert all(not hasattr(source, name) for name in forbidden) diff --git a/source/isaaclab_visualizers/changelog.d/minimal-window-keyboard.changed.rst b/source/isaaclab_visualizers/changelog.d/minimal-window-keyboard.changed.rst new file mode 100644 index 000000000000..51e7128ae0c6 --- /dev/null +++ b/source/isaaclab_visualizers/changelog.d/minimal-window-keyboard.changed.rst @@ -0,0 +1,2 @@ +* Bound Kit and Newton keyboard input once when their windows initialized and disconnected it on + close, removing the separate first-listener and last-listener lifecycle. diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py index b5e4983907cf..7227087c28db 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py @@ -8,14 +8,10 @@ from __future__ import annotations import functools -import logging -from types import SimpleNamespace from typing import Any from isaaclab.visualizers import KeyEventSource -logger = logging.getLogger(__name__) - @functools.cache def _carb_key_name_to_w3c() -> dict[str, str]: @@ -77,71 +73,49 @@ def _carb_key_name_to_w3c() -> dict[str, str]: return table -def _kit() -> SimpleNamespace: - """The Kit modules the source uses, imported only when it starts listening.""" - import carb.input - import omni.appwindow - from carb.eventdispatcher import get_eventdispatcher - - return SimpleNamespace(input=carb.input, appwindow=omni.appwindow, dispatcher=get_eventdispatcher()) - - class KitKeyEventSource(KeyEventSource): """Keys typed into the Kit application window. - Reads the default app window's ``carb.input`` keyboard, whose codes name physical key positions, - while a listener is subscribed, and reports a focus loss when the window loses OS focus. The raw + Reads the supplied app window's ``carb.input`` keyboard, whose codes name physical key positions, + for the window's lifetime, and reports a focus loss when the window loses OS focus. The raw keyboard stream also carries keys typed into Kit's own UI (text fields included), and Kit has no Python query for UI keyboard ownership, so they are reported too. """ - def __init__(self) -> None: - super().__init__() - self._input: Any = None - self._keyboard: Any = None - self._keyboard_subscription: Any = None - self._focus_observer: Any = None - self._event_types: Any = None + def __init__(self, app_window: Any) -> None: + """Connect the initialized Kit window's keyboard and focus events.""" + import carb.input + import omni.appwindow + from carb.eventdispatcher import get_eventdispatcher - def _start_listening(self) -> None: - kit = _kit() - app_window = kit.appwindow.get_default_app_window() - input_interface = kit.input.acquire_input_interface() - keyboard = app_window.get_keyboard() - subscription = input_interface.subscribe_to_keyboard_events(keyboard, self._on_keyboard_event) + super().__init__() + self._input = carb.input.acquire_input_interface() + self._keyboard = app_window.get_keyboard() + self._event_types = carb.input.KeyboardEventType + self._keyboard_subscription = self._input.subscribe_to_keyboard_events(self._keyboard, self._on_keyboard_event) try: - self._focus_observer = kit.dispatcher.observe_event( + self._focus_observer = get_eventdispatcher().observe_event( filter=app_window.get_event_key(), - event_name=kit.appwindow.GLOBAL_EVENT_WINDOW_FOCUS, + event_name=omni.appwindow.GLOBAL_EVENT_WINDOW_FOCUS, on_event=self._on_focus_event, observer_name="isaaclab_visualizers.KitKeyEventSource", ) except BaseException: - input_interface.unsubscribe_to_keyboard_events(keyboard, subscription) + self._input.unsubscribe_to_keyboard_events(self._keyboard, self._keyboard_subscription) raise - self._input, self._keyboard, self._keyboard_subscription = input_interface, keyboard, subscription - self._event_types = kit.input.KeyboardEventType - def _stop_listening(self) -> None: - # Unhook the keyboard first: if that raises, both hooks stay in place and a retry can finish. - if self._keyboard_subscription is not None: - self._input.unsubscribe_to_keyboard_events(self._keyboard, self._keyboard_subscription) - self._keyboard_subscription = None - observer, self._focus_observer = self._focus_observer, None - if observer is not None: + def close(self) -> None: + """Disconnect the window and notify listeners, including when native cleanup fails.""" + if self.closed: + return + try: try: - observer.reset() - except Exception: - # The keyboard is already unhooked, so the source is no longer listening either way. - logger.exception("[KitKeyEventSource] Failed to remove the window focus observer") - - def _release(self) -> None: - # A focus observer is still here only if unhooking failed on close: drop it rather than leak it. - observer, self._focus_observer = self._focus_observer, None - self._keyboard_subscription = None - self._input = self._keyboard = self._event_types = None - if observer is not None: - observer.reset() + self._input.unsubscribe_to_keyboard_events(self._keyboard, self._keyboard_subscription) + finally: + self._focus_observer.reset() + finally: + self._input = self._keyboard = self._keyboard_subscription = self._focus_observer = self._event_types = None + super().close() def _on_keyboard_event(self, event, *args, **kwargs) -> bool: event_types = self._event_types diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py index 4695b0a5d2a5..7744772011a0 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py @@ -92,8 +92,6 @@ def __init__(self, cfg: VisualizerCfg): @property def key_event_source(self) -> KitKeyEventSource | None: """Keys typed into the Kit window; ``None`` before initialization, after close, or when Kit runs headless.""" - if self._key_event_source is None and self._is_initialized and not self._runtime_headless: - self._key_event_source = KitKeyEventSource() return self._key_event_source @property @@ -166,6 +164,10 @@ def initialize(self, sim: SimulationContext, *, cameras: list[PerspectiveCameraC self._controls_kit_timeline = sim.has_gui and self.physics_backend not in ("physx", "ovphysx") if self._controls_kit_timeline: self.play() + if not self._runtime_headless: + import omni.appwindow + + self._key_event_source = KitKeyEventSource(omni.appwindow.get_default_app_window()) def step(self, dt: float) -> None: """Advance visualizer/UI updates for one simulation step. diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py index 0c81347b0a58..5dc8caf96951 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py @@ -18,15 +18,6 @@ def _pyglet_symbol_to_w3c() -> dict[int, str]: """Map pyglet key symbols to W3C ``KeyboardEvent.code`` strings (imports ``pyglet.window``).""" from pyglet.window import key - return _symbol_table(key) - - -def _symbol_table(key: Any) -> dict[int, str]: - """Map the symbols of a ``pyglet.window.key``-like namespace to W3C ``KeyboardEvent.code`` strings. - - pyglet reports layout-mapped symbols rather than physical positions, so a symbol maps to the key - that types the same character on a US layout. - """ table: dict[int, str] = {} for letter in "ABCDEFGHIJKLMNOPQRSTUVWXYZ": table[getattr(key, letter)] = f"Key{letter}" @@ -109,15 +100,10 @@ def _symbol_table(key: Any) -> dict[int, str]: return table -def viewer_window(viewer: Any) -> Any | None: - """The pyglet window of a Newton viewer, or ``None`` when it has none (headless).""" - return None if viewer.renderer.headless else viewer.renderer.window - - class NewtonKeyEventSource(KeyEventSource): """Keys typed into a Newton viewer window. - Hooks the viewer's pyglet window while a listener is subscribed. pyglet reports layout-mapped key + Hooks the initialized viewer's pyglet window until close. pyglet reports layout-mapped key symbols, so codes name the key that types the same character on a US layout rather than the physical key. Presses and releases pair up reliably on a US layout, including when Shift changes while a key is held; elsewhere this is best effort: a key whose shifted character @@ -126,17 +112,16 @@ class NewtonKeyEventSource(KeyEventSource): ImGui takes the keyboard, listeners see a focus loss, as when the window loses focus. """ - def __init__(self, viewer: Any, window: Any) -> None: + def __init__(self, viewer: Any) -> None: """Initialize the source. Args: viewer: The :class:`~isaaclab_visualizers.newton.newton_visualizer.NewtonViewerGL` whose ImGui UI may take the keyboard. - window: The viewer's pyglet window (see :func:`viewer_window`). """ super().__init__() self._viewer = viewer - self._window = window + self._window = viewer.renderer.window self._ui_has_keyboard = False # pushed and removed as one handler frame on the window self._handlers = { @@ -145,20 +130,19 @@ def __init__(self, viewer: Any, window: Any) -> None: "on_deactivate": self._on_deactivate, } - def _start_listening(self) -> None: self._window.push_handlers(**self._handlers) self._viewer.keyboard_frame_hook = self._poll_ui_keyboard - def _stop_listening(self) -> None: - # Remove the handlers first: if that raises, the frame hook stays with them. - self._window.remove_handlers(**self._handlers) - self._viewer.keyboard_frame_hook = None - - def _release(self) -> None: - # The frame hook is still set only if unhooking failed on close: never leave it on the viewer. - if self._viewer.keyboard_frame_hook == self._poll_ui_keyboard: + def close(self) -> None: + """Disconnect the window and notify listeners, including when native cleanup fails.""" + if self.closed: + return + try: + self._window.remove_handlers(**self._handlers) + finally: self._viewer.keyboard_frame_hook = None - self._viewer = self._window = None + self._viewer = self._window = None + super().close() def _poll_ui_keyboard(self) -> bool: """Track whether ImGui owns the keyboard, reporting a focus loss when it takes it.""" diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py index 12d6671421e9..59666d7b45d0 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py @@ -53,7 +53,7 @@ from isaaclab_visualizers.desktop_entry import write_desktop_entry from isaaclab_visualizers.newton.newton_visualization_markers import render_newton_visualization_markers -from .newton_key_event_source import NewtonKeyEventSource, viewer_window +from .newton_key_event_source import NewtonKeyEventSource logger = logging.getLogger(__name__) @@ -507,10 +507,6 @@ def __init__(self, cfg): @property def key_event_source(self) -> NewtonKeyEventSource | None: """Keys typed into the viewer window; ``None`` before initialization, after close, or when headless.""" - if self._key_event_source is None and self._viewer is not None: - window = viewer_window(self._viewer) - if window is not None: - self._key_event_source = NewtonKeyEventSource(self._viewer, window) return self._key_event_source def initialize(self, sim: SimulationContext, *, cameras: list[PerspectiveCameraCfg | Camera]) -> None: @@ -535,6 +531,8 @@ def initialize(self, sim: SimulationContext, *, cameras: list[PerspectiveCameraC self._viewer.marker_groups = sim.vis_marker_registry.get_groups().values() self._viewer.camera_keys_suspended = not self._camera_keys_enabled self._viewer.register_ui_callback(self._draw_streaming_view_controls, position="side") + if not self._runtime_headless: + self._key_event_source = NewtonKeyEventSource(self._viewer) def log_mesh( self, diff --git a/source/isaaclab_visualizers/test/test_key_event_sources.py b/source/isaaclab_visualizers/test/test_key_event_sources.py index 69cd46d5be94..5ea388fbdcf5 100644 --- a/source/isaaclab_visualizers/test/test_key_event_sources.py +++ b/source/isaaclab_visualizers/test/test_key_event_sources.py @@ -5,7 +5,7 @@ # pyright: reportPrivateUsage=none -"""Tests for the Kit and Newton keyboard sources and the visualizers that own them. +"""Tests for the Kit and Newton keyboard sources over their native windows' lifetimes. Every backend runs against window, viewer and Kit doubles, so no display, Kit app or GPU is needed. The contract they share is tested in ``source/isaaclab/test/visualizers/test_key_event_source.py``. @@ -15,28 +15,18 @@ import ast import importlib.util -import logging import os import pathlib import subprocess import sys from types import SimpleNamespace -import isaaclab_visualizers.kit.kit_key_event_source as kit_key_event_source import isaaclab_visualizers.newton.newton_key_event_source as newton_key_event_source -import isaaclab_visualizers.newton.newton_visualizer as newton_visualizer import pytest from isaaclab_visualizers.kit.kit_key_event_source import KitKeyEventSource, _carb_key_name_to_w3c -from isaaclab_visualizers.kit.kit_visualizer import KitVisualizer -from isaaclab_visualizers.newton import ( - NewtonGLBackendCfg, - NewtonGLVisualizer, - NewtonRTXBackendCfg, - NewtonRTXVisualizer, -) -from isaaclab_visualizers.newton.newton_key_event_source import NewtonKeyEventSource, _symbol_table, viewer_window +from isaaclab_visualizers.newton.newton_key_event_source import NewtonKeyEventSource -from isaaclab.visualizers import KeyEventSource, VisualizerCfg +from isaaclab.visualizers import KeyEventSource pytestmark = [pytest.mark.unit] @@ -124,6 +114,7 @@ def observe_event(*, filter, event_name, on_event, observer_name): interface = SimpleNamespace(subscribe_to_keyboard_events=subscribe, unsubscribe_to_keyboard_events=unsubscribe) window = SimpleNamespace(get_keyboard=lambda: keyboard, get_event_key=lambda: "app-window") + self.window = window self.namespace = SimpleNamespace( input=SimpleNamespace( acquire_input_interface=lambda: interface, @@ -146,12 +137,18 @@ def focus(self, focused: bool) -> None: @pytest.fixture def kit(monkeypatch) -> _FakeKit: fake = _FakeKit() - monkeypatch.setattr(kit_key_event_source, "_kit", lambda: fake.namespace) + monkeypatch.setitem(sys.modules, "carb", SimpleNamespace(input=fake.namespace.input)) + monkeypatch.setitem(sys.modules, "carb.input", fake.namespace.input) + monkeypatch.setitem( + sys.modules, "carb.eventdispatcher", SimpleNamespace(get_eventdispatcher=lambda: fake.namespace.dispatcher) + ) + monkeypatch.setitem(sys.modules, "omni", SimpleNamespace(appwindow=fake.namespace.appwindow)) + monkeypatch.setitem(sys.modules, "omni.appwindow", fake.namespace.appwindow) return fake def test_kit_hooks_keyboard_and_focus_once(kit): - source = KitKeyEventSource() + source = KitKeyEventSource(kit.window) _, first = _listen(source) _, second = _listen(source) assert len(kit.keyboard_callbacks) == len(kit.focus_callbacks) == 1 @@ -159,12 +156,17 @@ def test_kit_hooks_keyboard_and_focus_once(kit): first.close() assert len(kit.keyboard_callbacks) == 1 second.close() + assert len(kit.keyboard_callbacks) == len(kit.focus_callbacks) == 1 + recorder, _ = _listen(source) + kit.key(kit.KEY_PRESS, "W") + assert recorder.events == [("KeyW", True)] + source.close() assert kit.keyboard_callbacks == kit.focus_callbacks == [] assert (kit.unsubscribed, kit.observers_reset) == (1, 1) def test_kit_reports_physical_key_presses_and_releases(kit): - source = KitKeyEventSource() + source = KitKeyEventSource(kit.window) recorder, _ = _listen(source) results = [ kit.key(kit.KEY_PRESS, "W"), @@ -177,14 +179,14 @@ def test_kit_reports_physical_key_presses_and_releases(kit): def test_kit_ignores_char_events_without_reading_the_key(kit): - source = KitKeyEventSource() + source = KitKeyEventSource(kit.window) recorder, _ = _listen(source) assert kit.key(kit.CHAR) == [True] assert recorder.events == [] def test_kit_window_blur_is_a_focus_loss(kit): - source = KitKeyEventSource() + source = KitKeyEventSource(kit.window) recorder, _ = _listen(source) kit.key(kit.KEY_PRESS, "W") kit.focus(True) @@ -194,39 +196,8 @@ def test_kit_window_blur_is_a_focus_loss(kit): assert recorder.events == [("KeyW", True)] -def test_kit_failed_unhook_keeps_both_hooks(kit): - source = KitKeyEventSource() - _, subscription = _listen(source) - kit.fail_unsubscribe = True - with pytest.raises(RuntimeError, match="unsubscribe"): - subscription.close() - assert len(kit.keyboard_callbacks) == len(kit.focus_callbacks) == 1 - - # still listening: a new listener gets blur without a second hook - recorder, _ = _listen(source) - kit.focus(False) - assert recorder.focus_lost == 1 - assert len(kit.keyboard_callbacks) == 1 - - -def test_kit_failed_focus_unhook_still_stops_listening(kit, caplog): - """A focus observer that fails to reset after the keyboard unhooks is logged, not left half-listening.""" - source = KitKeyEventSource() - _, subscription = _listen(source) - kit.fail_reset = True - with caplog.at_level(logging.ERROR): - subscription.close() - assert kit.keyboard_callbacks == [] - assert "window focus observer" in caplog.text - - kit.fail_reset = False - recorder, _ = _listen(source) # hooks the keyboard again - kit.key(kit.KEY_PRESS, "W") - assert recorder.events == [("KeyW", True)] - - def test_kit_close_drops_the_focus_hook_when_unhooking_fails(kit): - source = KitKeyEventSource() + source = KitKeyEventSource(kit.window) _listen(source) kit.fail_unsubscribe = True with pytest.raises(RuntimeError, match="unsubscribe"): @@ -236,20 +207,20 @@ def test_kit_close_drops_the_focus_hook_when_unhooking_fails(kit): def test_kit_focus_hook_failure_rolls_back_the_keyboard_hook(kit): - source = KitKeyEventSource() kit.fail_observe = True with pytest.raises(RuntimeError, match="observe"): - _listen(source) + KitKeyEventSource(kit.window) assert kit.keyboard_callbacks == [] kit.fail_observe = False + source = KitKeyEventSource(kit.window) recorder, _ = _listen(source) kit.key(kit.KEY_PRESS, "W") assert recorder.events == [("KeyW", True)] def test_kit_close_unhooks_and_releases_the_window(kit): - source = KitKeyEventSource() + source = KitKeyEventSource(kit.window) recorder, _ = _listen(source) source.close() assert kit.keyboard_callbacks == kit.focus_callbacks == [] @@ -257,6 +228,17 @@ def test_kit_close_unhooks_and_releases_the_window(kit): assert source._input is None and source._keyboard is None +def test_kit_failed_focus_cleanup_still_closes_and_notifies(kit): + source = KitKeyEventSource(kit.window) + recorder, subscription = _listen(source) + kit.fail_reset = True + with pytest.raises(RuntimeError, match="reset"): + source.close() + assert kit.keyboard_callbacks == [] + assert source.closed and subscription.closed and recorder.focus_lost == 1 + source.close() + + @pytest.mark.parametrize( ("carb_name", "code"), [ @@ -299,9 +281,10 @@ def _pyglet_keys() -> SimpleNamespace: @pytest.fixture def keys(monkeypatch) -> SimpleNamespace: namespace = _pyglet_keys() - table = _symbol_table(namespace) - monkeypatch.setattr(newton_key_event_source, "_pyglet_symbol_to_w3c", lambda: table) - return namespace + monkeypatch.setitem(sys.modules, "pyglet.window", SimpleNamespace(key=namespace)) + newton_key_event_source._pyglet_symbol_to_w3c.cache_clear() + yield namespace + newton_key_event_source._pyglet_symbol_to_w3c.cache_clear() class _FakeWindow: @@ -331,13 +314,14 @@ def is_keyboard_capturing(self) -> bool: return self.capturing -def _newton_viewer(gui: _FakeGui | None = None) -> SimpleNamespace: - return SimpleNamespace(gui=gui, camera_keys_suspended=False, keyboard_frame_hook=None) +def _newton_viewer(window: _FakeWindow, gui: _FakeGui | None = None) -> SimpleNamespace: + return SimpleNamespace(renderer=SimpleNamespace(window=window), gui=gui, keyboard_frame_hook=None) def test_newton_forwards_window_keys(keys): - window, viewer = _FakeWindow(), _newton_viewer(_FakeGui()) - source = NewtonKeyEventSource(viewer, window) + window = _FakeWindow() + viewer = _newton_viewer(window, _FakeGui()) + source = NewtonKeyEventSource(viewer) recorder, subscription = _listen(source) assert len(window.frames) == 1 and viewer.keyboard_frame_hook is not None @@ -347,13 +331,18 @@ def test_newton_forwards_window_keys(keys): assert recorder.events == [("KeyW", True), ("KeyW", False), ("Enter", True)] subscription.close() + assert len(window.frames) == 1 and viewer.keyboard_frame_hook is not None + recorder, _ = _listen(source) + window.dispatch("on_key_press", keys.W, 0) + assert recorder.events == [("KeyW", True)] + source.close() assert window.frames == [] and viewer.keyboard_frame_hook is None def test_newton_ui_taking_the_keyboard_is_a_focus_loss_on_the_next_frame(keys): window, gui = _FakeWindow(), _FakeGui() - viewer = _newton_viewer(gui) - source = NewtonKeyEventSource(viewer, window) + viewer = _newton_viewer(window, gui) + source = NewtonKeyEventSource(viewer) recorder, _ = _listen(source) window.dispatch("on_key_press", keys.W, 0) @@ -371,7 +360,7 @@ def test_newton_ui_taking_the_keyboard_is_a_focus_loss_on_the_next_frame(keys): def test_newton_viewer_without_gui_forwards_keys(keys): window = _FakeWindow() - source = NewtonKeyEventSource(_newton_viewer(gui=None), window) + source = NewtonKeyEventSource(_newton_viewer(window, gui=None)) recorder, _ = _listen(source) window.dispatch("on_key_press", keys.W, 0) assert recorder.events == [("KeyW", True)] @@ -385,7 +374,7 @@ def test_newton_viewer_without_gui_forwards_keys(keys): def test_newton_key_released_after_shift_changes(keys, unshifted, shifted, code, shift_first): """X11 names a digit or punctuation key by its shifted symbol while Shift is held.""" window = _FakeWindow() - source = NewtonKeyEventSource(_newton_viewer(_FakeGui()), window) + source = NewtonKeyEventSource(_newton_viewer(window, _FakeGui())) recorder, _ = _listen(source) press, release = (shifted, unshifted) if shift_first else (unshifted, shifted) @@ -397,7 +386,7 @@ def test_newton_key_released_after_shift_changes(keys, unshifted, shifted, code, def test_newton_window_deactivation_is_a_focus_loss(keys): window = _FakeWindow() - source = NewtonKeyEventSource(_newton_viewer(_FakeGui()), window) + source = NewtonKeyEventSource(_newton_viewer(window, _FakeGui())) recorder, _ = _listen(source) window.dispatch("on_key_press", keys.W, 0) window.dispatch("on_deactivate") @@ -405,27 +394,28 @@ def test_newton_window_deactivation_is_a_focus_loss(keys): assert recorder.events == [("KeyW", True)] -def test_newton_failed_unhook_keeps_the_frame_hook(keys): - window, viewer = _FakeWindow(), _newton_viewer(_FakeGui()) - source = NewtonKeyEventSource(viewer, window) - _, subscription = _listen(source) +def test_newton_failed_unhook_still_closes_and_notifies(keys): + window = _FakeWindow() + viewer = _newton_viewer(window, _FakeGui()) + source = NewtonKeyEventSource(viewer) + recorder, subscription = _listen(source) def fail(**handlers): raise RuntimeError("remove_handlers") window.remove_handlers = fail - with pytest.raises(RuntimeError, match="remove_handlers"): - subscription.close() - assert len(window.frames) == 1 and viewer.keyboard_frame_hook is not None - with pytest.raises(RuntimeError, match="remove_handlers"): source.close() assert viewer.keyboard_frame_hook is None # the closed source is never polled again + assert source.closed and subscription.closed and recorder.focus_lost == 1 + window.dispatch("on_key_press", keys.W, 0) # a leftover native hook is inert + assert recorder.events == [] def test_newton_close_unhooks_and_releases_the_window(keys): - window, viewer = _FakeWindow(), _newton_viewer(_FakeGui()) - source = NewtonKeyEventSource(viewer, window) + window = _FakeWindow() + viewer = _newton_viewer(window, _FakeGui()) + source = NewtonKeyEventSource(viewer) recorder, _ = _listen(source) source.close() @@ -434,25 +424,6 @@ def test_newton_close_unhooks_and_releases_the_window(keys): assert source._viewer is None and source._window is None -def test_viewer_window_is_none_without_a_window(): - window = object() - assert viewer_window(SimpleNamespace(renderer=SimpleNamespace(headless=False, window=window))) is window - assert viewer_window(SimpleNamespace(renderer=SimpleNamespace(headless=True, window=window))) is None - - -def test_gl_viewer_polls_keyboard_ownership_after_each_frame(monkeypatch): - events: list[str] = [] - monkeypatch.setattr(newton_visualizer.ViewerGL, "end_frame", lambda self: events.append("frame")) - viewer = object.__new__(newton_visualizer.NewtonViewerGL) - viewer._close_requested = False - viewer.keyboard_frame_hook = lambda: events.append("poll") - - viewer.end_frame() - viewer.keyboard_frame_hook = None - viewer.end_frame() - assert events == ["frame", "poll", "frame"] - - @pytest.mark.parametrize( ("pyglet_name", "code"), [ @@ -468,146 +439,20 @@ def test_gl_viewer_polls_keyboard_ownership_after_each_frame(monkeypatch): ("GRAVE", "Backquote"), ], ) -def test_pyglet_symbols_map_to_w3c_codes(pyglet_name, code): - keys = _pyglet_keys() - assert _symbol_table(keys)[getattr(keys, pyglet_name)] == code +def test_pyglet_symbols_map_to_w3c_codes(keys, pyglet_name, code): + assert newton_key_event_source._pyglet_symbol_to_w3c()[getattr(keys, pyglet_name)] == code # --------------------------------------------------------------------------- -# Visualizer ownership +# Import boundary # --------------------------------------------------------------------------- -def _newton_gl_viewer(headless: bool) -> newton_visualizer.NewtonViewerGL: - viewer = object.__new__(newton_visualizer.NewtonViewerGL) - viewer.renderer = SimpleNamespace(headless=headless, window=_FakeWindow()) - viewer.gui = None - viewer.camera_keys_suspended = False - viewer.keyboard_frame_hook = None - viewer.close = lambda: None # the RTX visualizer closes its window - return viewer - - -_NEWTON_VISUALIZERS = [(NewtonGLVisualizer, NewtonGLBackendCfg), (NewtonRTXVisualizer, NewtonRTXBackendCfg)] - - -def _newton_visualizer(viewer, visualizer_type=NewtonGLVisualizer, backend_cfg=NewtonGLBackendCfg): - visualizer = visualizer_type(VisualizerCfg(backend=backend_cfg())) - visualizer._viewer = viewer - return visualizer - - -@pytest.mark.parametrize(("visualizer_type", "backend_cfg"), _NEWTON_VISUALIZERS) -def test_newton_visualizer_key_event_source_lifecycle(keys, visualizer_type, backend_cfg): - assert _newton_visualizer(None, visualizer_type, backend_cfg).key_event_source is None # not initialized - headless = _newton_visualizer(_newton_gl_viewer(headless=True), visualizer_type, backend_cfg) - assert headless.key_event_source is None - - viewer = _newton_gl_viewer(headless=False) - visualizer = _newton_visualizer(viewer, visualizer_type, backend_cfg) - source = visualizer.key_event_source - assert isinstance(source, NewtonKeyEventSource) and visualizer.key_event_source is source - recorder, _ = _listen(source) - - visualizer.close() - assert source.closed and recorder.focus_lost == 1 - assert viewer.keyboard_frame_hook is None - assert visualizer._viewer is None and visualizer.key_event_source is None - - -@pytest.mark.parametrize(("visualizer_type", "backend_cfg"), _NEWTON_VISUALIZERS) -def test_viewer_skips_camera_keys_while_disabled(monkeypatch, visualizer_type, backend_cfg): - moved: list[float] = [] - monkeypatch.setattr(newton_visualizer.ViewerGL, "_update_camera", lambda self, dt: moved.append(dt)) - viewer = _newton_gl_viewer(headless=False) - visualizer = _newton_visualizer(viewer, visualizer_type, backend_cfg) - - viewer._update_camera(0.1) - visualizer.set_camera_keys_enabled(False) - viewer._update_camera(0.2) - visualizer.set_camera_keys_enabled(True) - viewer._update_camera(0.3) - assert moved == [0.1, 0.3] - - -@pytest.mark.parametrize(("visualizer_type", "backend_cfg"), _NEWTON_VISUALIZERS) -def test_newton_focus_loss_during_shutdown_cannot_create_a_new_source(keys, visualizer_type, backend_cfg): - visualizer = _newton_visualizer(_newton_gl_viewer(headless=False), visualizer_type, backend_cfg) - source = visualizer.key_event_source - seen = [] - source.add_key_listener(lambda code, pressed: None, lambda: seen.append(visualizer.key_event_source)) - - visualizer.close() - assert seen == [source] and source.closed # the closing source, not a new one - assert visualizer.key_event_source is None - - -def _kit_visualizer(monkeypatch, *, initialized: bool, headless: bool) -> KitVisualizer: - visualizer = object.__new__(KitVisualizer) - visualizer._is_initialized = initialized - visualizer._is_closed = False - visualizer._runtime_headless = headless - visualizer._key_event_source = None - visualizer._camera_choices = [] - visualizer._viewport_camera_xform_ops = {} - visualizer._viewport_camera_pose_cache = {} - visualizer._camera_image_provider = None - visualizer._camera_image_window = None - visualizer._rgb_annotator = None - visualizer._rgb_render_product = None - visualizer._owns_kit_timeline = False - visualizer._image_views = [] - visualizer.torn_down = [] - monkeypatch.setattr(visualizer, "_teardown_backend_menubar_label", lambda: visualizer.torn_down.append("menu")) - monkeypatch.setattr(visualizer, "_restore_env_visibility", lambda: visualizer.torn_down.append("visibility")) - return visualizer - - -def test_kit_visualizer_key_event_source_lifecycle(monkeypatch, kit): - assert _kit_visualizer(monkeypatch, initialized=False, headless=False).key_event_source is None - assert _kit_visualizer(monkeypatch, initialized=True, headless=True).key_event_source is None - - visualizer = _kit_visualizer(monkeypatch, initialized=True, headless=False) - source = visualizer.key_event_source - assert isinstance(source, KitKeyEventSource) and visualizer.key_event_source is source - recorder, _ = _listen(source) - - visualizer.close() - assert source.closed and recorder.focus_lost == 1 - assert kit.keyboard_callbacks == [] - assert not visualizer._is_initialized and visualizer.key_event_source is None - - -def test_kit_focus_loss_during_shutdown_cannot_create_a_new_source(monkeypatch, kit): - visualizer = _kit_visualizer(monkeypatch, initialized=True, headless=False) - source = visualizer.key_event_source - seen = [] - source.add_key_listener(lambda code, pressed: None, lambda: seen.append(visualizer.key_event_source)) - - visualizer.close() - assert seen == [source] and source.closed # the closing source, not a new one - assert visualizer.key_event_source is None - assert kit.keyboard_callbacks == [] - - -def test_kit_visualizer_close_survives_a_failing_key_source(monkeypatch, kit, caplog): - visualizer = _kit_visualizer(monkeypatch, initialized=True, headless=False) - _listen(visualizer.key_event_source) - kit.fail_unsubscribe = True - - with caplog.at_level(logging.ERROR): - visualizer.close() - assert visualizer.torn_down == ["menu", "visibility"] - assert visualizer._is_closed and visualizer.key_event_source is None - assert "Keyboard input cleanup failed" in caplog.text - - def test_backends_import_without_a_display_or_kit(): code = ( "import sys\n" "from isaaclab_visualizers.kit.kit_key_event_source import KitKeyEventSource\n" "from isaaclab_visualizers.newton.newton_key_event_source import NewtonKeyEventSource\n" - "KitKeyEventSource(); NewtonKeyEventSource(object(), object())\n" "assert 'pyglet.window' not in sys.modules, 'pyglet.window'\n" "assert 'carb' not in sys.modules, 'carb'\n" ) diff --git a/source/isaaclab_visualizers/test/test_newton_visualizer_viewer_release.py b/source/isaaclab_visualizers/test/test_newton_visualizer_viewer_release.py index 9ebfc587a0dd..552554c5dc4f 100644 --- a/source/isaaclab_visualizers/test/test_newton_visualizer_viewer_release.py +++ b/source/isaaclab_visualizers/test/test_newton_visualizer_viewer_release.py @@ -38,6 +38,7 @@ def test_gl_close_request_closes_after_frame(monkeypatch: pytest.MonkeyPatch, re monkeypatch.setattr(newton_visualizer.ViewerGL, "close", lambda self: events.append("close")) viewer = object.__new__(newton_visualizer.NewtonViewerGL) viewer._close_requested = False + viewer.keyboard_frame_hook = lambda: events.append("keyboard") if requested: viewer.request_close() viewer.renderer = Mock() @@ -45,7 +46,7 @@ def test_gl_close_request_closes_after_frame(monkeypatch: pytest.MonkeyPatch, re viewer.end_frame() - assert events == (["frame", "context", "close"] if requested else ["frame"]) + assert events == (["frame", "keyboard", "context", "close"] if requested else ["frame", "keyboard"]) @pytest.mark.parametrize("failure", [False, True]) @@ -101,3 +102,16 @@ def close_keys() -> None: assert visualizer._key_event_source is None assert visualizer.key_event_source is None assert visualizer._is_closed + + +def test_keyboard_camera_movement_can_be_disabled(monkeypatch): + moved = [] + monkeypatch.setattr(newton_visualizer.ViewerGL, "_update_camera", lambda self, dt: moved.append(dt)) + viewer = object.__new__(newton_visualizer.NewtonViewerGL) + visualizer = NewtonGLVisualizer(VisualizerCfg(backend=NewtonGLBackendCfg())) + visualizer._viewer = viewer + visualizer.set_camera_keys_enabled(False) + viewer._update_camera(0.1) + visualizer.set_camera_keys_enabled(True) + viewer._update_camera(0.2) + assert moved == [0.2] diff --git a/source/isaaclab_visualizers/test/visualizer_integration_utils.py b/source/isaaclab_visualizers/test/visualizer_integration_utils.py index 791d9ad54102..5ff2140ec23c 100644 --- a/source/isaaclab_visualizers/test/visualizer_integration_utils.py +++ b/source/isaaclab_visualizers/test/visualizer_integration_utils.py @@ -1718,6 +1718,7 @@ def _step_env() -> None: kit_visualizers = [viz for viz in env.sim.visualizers if isinstance(viz, KitVisualizer)] assert kit_visualizers, "Expected an initialized Kit visualizer." + assert kit_visualizers[0].key_event_source is not None, "Window input is missing." assert omni.timeline.get_timeline_interface().is_playing(), ( "The Kit toolbar must show Pause while the simulation is running." ) @@ -1731,6 +1732,7 @@ def _step_env() -> None: if isinstance(viz, NewtonVisualizer) and not isinstance(viz, NewtonRTXVisualizer) ] assert newton_visualizers, "Expected an initialized Newton visualizer." + assert newton_visualizers[0].key_event_source is not None, "Window input is missing." viewer = getattr(newton_visualizers[0], "_viewer", None) assert viewer is not None, "Newton viewer was not created." From 5119499dfbc68568419b40d463fe427ec7c583a0 Mon Sep 17 00:00:00 2001 From: ooctipus Date: Fri, 23 Oct 2026 17:29:36 -0700 Subject: [PATCH 4/6] Consolidate visualizer keyboard ownership --- .../minimal-window-keyboard.changed.rst | 2 + .../isaaclab/visualizers/base_visualizer.py | 14 ++- .../isaaclab/visualizers/key_event_source.py | 14 +-- .../test/visualizers/test_key_event_source.py | 2 + .../test/visualizers/test_visualizer.py | 18 ++- .../minimal-window-keyboard.changed.rst | 2 + .../kit/kit_key_event_source.py | 116 ++++++++---------- .../kit/kit_visualizer.py | 17 +-- .../newton/newton_key_event_source.py | 29 ++--- .../newton/newton_visualizer.py | 20 --- .../test/test_key_event_sources.py | 106 +++++++--------- 11 files changed, 148 insertions(+), 192 deletions(-) diff --git a/source/isaaclab/changelog.d/minimal-window-keyboard.changed.rst b/source/isaaclab/changelog.d/minimal-window-keyboard.changed.rst index d3c7b3870788..4369dda8fb1a 100644 --- a/source/isaaclab/changelog.d/minimal-window-keyboard.changed.rst +++ b/source/isaaclab/changelog.d/minimal-window-keyboard.changed.rst @@ -1,2 +1,4 @@ * Simplified keyboard sources to listener registration and event forwarding; native window hooks were initialized and closed by visualizers instead of changing with listener counts. +* Centralized the optional keyboard source and teardown in the base visualizer and removed + redundant subscription state. diff --git a/source/isaaclab/isaaclab/visualizers/base_visualizer.py b/source/isaaclab/isaaclab/visualizers/base_visualizer.py index 71d1cb8c2b18..df00eaacef97 100644 --- a/source/isaaclab/isaaclab/visualizers/base_visualizer.py +++ b/source/isaaclab/isaaclab/visualizers/base_visualizer.py @@ -22,6 +22,7 @@ from ..utils import validate from ..utils.backend_utils import FactoryBase from .image_view import ImageView +from .key_event_source import KeyEventSource from .visualizer_cfg import ( ImageViewCfg, PerspectiveCameraCfg, @@ -35,7 +36,6 @@ from ..renderers.base_renderer import VisualMaterialBatch from ..sensors import Camera from ..sim import SimulationContext - from .key_event_source import KeyEventSource logger = logging.getLogger(__name__) @@ -72,6 +72,7 @@ def __init__(self, cfg: VisualizerCfg): self._sim_time = 0.0 self.current_image_view: ImageView | None = None self._image_views: list[ImageView] = [] + self._key_event_source: KeyEventSource | None = None @property def visual_material_writer(self) -> Callable[[tuple[VisualMaterialBatch, ...]], Any] | None: @@ -242,7 +243,7 @@ def key_event_source(self) -> KeyEventSource | None: before initialization, after close, and for backends or modes with no local window that can report keys (the default). """ - return None + return self._key_event_source def set_camera_keys_enabled(self, enabled: bool) -> None: """Enable or disable moving this visualizer's camera with the keyboard. @@ -446,3 +447,12 @@ def pause(self) -> None: def stop(self) -> None: """Handle simulation stop. No-op by default.""" pass + + def _close_key_event_source(self) -> None: + """Close window input before native teardown; cleanup failures must not retain the window.""" + source, self._key_event_source = self._key_event_source, None + if source is not None: + try: + source.close() + except Exception: + logger.exception("[%s] Keyboard input cleanup failed", type(self).__name__) diff --git a/source/isaaclab/isaaclab/visualizers/key_event_source.py b/source/isaaclab/isaaclab/visualizers/key_event_source.py index a85ffe03b221..d2de10de7565 100644 --- a/source/isaaclab/isaaclab/visualizers/key_event_source.py +++ b/source/isaaclab/isaaclab/visualizers/key_event_source.py @@ -27,22 +27,20 @@ class KeyboardSubscription: """ def __init__(self, source: KeyEventSource, on_key: KeyCallback, on_focus_lost: FocusLostCallback) -> None: - self._source = source + self._source: KeyEventSource | None = source self._on_key = on_key self._on_focus_lost = on_focus_lost - self._closed = False @property def closed(self) -> bool: """Whether the subscription no longer receives callbacks.""" - return self._closed + return self._source is None def close(self) -> None: """Unsubscribe.""" - if not self._closed: - self._closed = True - if not self._source.closed: - self._source._subscriptions.remove(self) + source, self._source = self._source, None + if source is not None and not source.closed: + source._subscriptions.remove(self) def __enter__(self) -> KeyboardSubscription: return self @@ -107,7 +105,7 @@ def close(self) -> None: subscriptions, self._subscriptions = self._subscriptions, [] for subscription in subscriptions: if not subscription.closed: - subscription._closed = True + subscription.close() _call(subscription._on_focus_lost) def _emit_key(self, code: str, pressed: bool) -> None: diff --git a/source/isaaclab/test/visualizers/test_key_event_source.py b/source/isaaclab/test/visualizers/test_key_event_source.py index 7f6d6c3a90c7..e91ed52e25bc 100644 --- a/source/isaaclab/test/visualizers/test_key_event_source.py +++ b/source/isaaclab/test/visualizers/test_key_event_source.py @@ -136,3 +136,5 @@ def test_source_does_not_own_backend_lifecycle_or_controller_state(): "capture_keyboard", ) assert all(not hasattr(source, name) for name in forbidden) + subscription = source.add_key_listener(lambda code, pressed: None, lambda: None) + assert not hasattr(subscription, "_closed"), "Subscription lifetime is owned by its source reference." diff --git a/source/isaaclab/test/visualizers/test_visualizer.py b/source/isaaclab/test/visualizers/test_visualizer.py index e3c53129463e..123409f1d9fb 100644 --- a/source/isaaclab/test/visualizers/test_visualizer.py +++ b/source/isaaclab/test/visualizers/test_visualizer.py @@ -16,6 +16,7 @@ from isaaclab.utils import validate from isaaclab.utils.string import ResolvableString from isaaclab.visualizers.base_visualizer import BaseVisualizer +from isaaclab.visualizers.key_event_source import KeyEventSource from isaaclab.visualizers.visualizer_cfg import ( ImageViewCfg, PerspectiveCameraCfg, @@ -104,7 +105,8 @@ def step(self, dt: float) -> None: return def close(self) -> None: - self._is_closed = True + self._close_key_event_source() + super().close() def is_running(self) -> bool: return True @@ -161,3 +163,17 @@ def test_physics_backend_returns_none_without_simulation_context(): """physics_backend is None when no SimulationContext is active.""" viz = _DummyVisualizer(VisualizerCfg()) assert viz.physics_backend is None + assert viz.key_event_source is None + + +def test_visualizer_retires_window_input_before_focus_loss_callbacks(): + viz = _DummyVisualizer(VisualizerCfg()) + source = viz._key_event_source = KeyEventSource() + observed = [] + subscription = source.add_key_listener(lambda code, pressed: None, lambda: observed.append(viz.key_event_source)) + assert viz.key_event_source is source + + viz.close() + viz.close() + assert viz.key_event_source is None and source.closed and subscription.closed + assert observed == [None] diff --git a/source/isaaclab_visualizers/changelog.d/minimal-window-keyboard.changed.rst b/source/isaaclab_visualizers/changelog.d/minimal-window-keyboard.changed.rst index 51e7128ae0c6..f860e4788520 100644 --- a/source/isaaclab_visualizers/changelog.d/minimal-window-keyboard.changed.rst +++ b/source/isaaclab_visualizers/changelog.d/minimal-window-keyboard.changed.rst @@ -1,2 +1,4 @@ * Bound Kit and Newton keyboard input once when their windows initialized and disconnected it on close, removing the separate first-listener and last-listener lifecycle. +* Reused pyglet's native handler registration and required its declared key constants instead of + silently omitting missing keys. diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py index 7227087c28db..472bf5adfe40 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_key_event_source.py @@ -7,70 +7,58 @@ from __future__ import annotations -import functools from typing import Any from isaaclab.visualizers import KeyEventSource - -@functools.cache -def _carb_key_name_to_w3c() -> dict[str, str]: - """Map ``carb.input.KeyboardInput`` names to W3C ``KeyboardEvent.code`` strings.""" - table: dict[str, str] = {} - for letter in "ABCDEFGHIJKLMNOPQRSTUVWXYZ": - table[letter] = f"Key{letter}" - for digit in "0123456789": - table[f"KEY_{digit}"] = f"Digit{digit}" - table[f"NUMPAD_{digit}"] = f"Numpad{digit}" - for n in range(1, 13): - table[f"F{n}"] = f"F{n}" - table.update( - { - "SPACE": "Space", - "ENTER": "Enter", - "ESCAPE": "Escape", - "TAB": "Tab", - "BACKSPACE": "Backspace", - "DEL": "Delete", - "INSERT": "Insert", - "HOME": "Home", - "END": "End", - "PAGE_UP": "PageUp", - "PAGE_DOWN": "PageDown", - "UP": "ArrowUp", - "DOWN": "ArrowDown", - "LEFT": "ArrowLeft", - "RIGHT": "ArrowRight", - "LEFT_SHIFT": "ShiftLeft", - "RIGHT_SHIFT": "ShiftRight", - "LEFT_CONTROL": "ControlLeft", - "RIGHT_CONTROL": "ControlRight", - "LEFT_ALT": "AltLeft", - "RIGHT_ALT": "AltRight", - "LEFT_SUPER": "MetaLeft", - "RIGHT_SUPER": "MetaRight", - "CAPS_LOCK": "CapsLock", - "MINUS": "Minus", - "EQUAL": "Equal", - "LEFT_BRACKET": "BracketLeft", - "RIGHT_BRACKET": "BracketRight", - "SEMICOLON": "Semicolon", - "APOSTROPHE": "Quote", - "GRAVE_ACCENT": "Backquote", - "BACKSLASH": "Backslash", - "COMMA": "Comma", - "PERIOD": "Period", - "SLASH": "Slash", - "NUMPAD_ADD": "NumpadAdd", - "NUMPAD_SUBTRACT": "NumpadSubtract", - "NUMPAD_MULTIPLY": "NumpadMultiply", - "NUMPAD_DIVIDE": "NumpadDivide", - "NUMPAD_DEL": "NumpadDecimal", - "NUMPAD_ENTER": "NumpadEnter", - "NUMPAD_EQUAL": "NumpadEqual", - } - ) - return table +_CARB_KEY_CODES = { + **{letter: f"Key{letter}" for letter in "ABCDEFGHIJKLMNOPQRSTUVWXYZ"}, + **{f"KEY_{digit}": f"Digit{digit}" for digit in "0123456789"}, + **{f"NUMPAD_{digit}": f"Numpad{digit}" for digit in "0123456789"}, + **{f"F{n}": f"F{n}" for n in range(1, 13)}, + "SPACE": "Space", + "ENTER": "Enter", + "ESCAPE": "Escape", + "TAB": "Tab", + "BACKSPACE": "Backspace", + "DEL": "Delete", + "INSERT": "Insert", + "HOME": "Home", + "END": "End", + "PAGE_UP": "PageUp", + "PAGE_DOWN": "PageDown", + "UP": "ArrowUp", + "DOWN": "ArrowDown", + "LEFT": "ArrowLeft", + "RIGHT": "ArrowRight", + "LEFT_SHIFT": "ShiftLeft", + "RIGHT_SHIFT": "ShiftRight", + "LEFT_CONTROL": "ControlLeft", + "RIGHT_CONTROL": "ControlRight", + "LEFT_ALT": "AltLeft", + "RIGHT_ALT": "AltRight", + "LEFT_SUPER": "MetaLeft", + "RIGHT_SUPER": "MetaRight", + "CAPS_LOCK": "CapsLock", + "MINUS": "Minus", + "EQUAL": "Equal", + "LEFT_BRACKET": "BracketLeft", + "RIGHT_BRACKET": "BracketRight", + "SEMICOLON": "Semicolon", + "APOSTROPHE": "Quote", + "GRAVE_ACCENT": "Backquote", + "BACKSLASH": "Backslash", + "COMMA": "Comma", + "PERIOD": "Period", + "SLASH": "Slash", + "NUMPAD_ADD": "NumpadAdd", + "NUMPAD_SUBTRACT": "NumpadSubtract", + "NUMPAD_MULTIPLY": "NumpadMultiply", + "NUMPAD_DIVIDE": "NumpadDivide", + "NUMPAD_DEL": "NumpadDecimal", + "NUMPAD_ENTER": "NumpadEnter", + "NUMPAD_EQUAL": "NumpadEqual", +} class KitKeyEventSource(KeyEventSource): @@ -114,13 +102,13 @@ def close(self) -> None: finally: self._focus_observer.reset() finally: - self._input = self._keyboard = self._keyboard_subscription = self._focus_observer = self._event_types = None + self._input = self._keyboard = self._keyboard_subscription = self._focus_observer = None super().close() def _on_keyboard_event(self, event, *args, **kwargs) -> bool: - event_types = self._event_types - if event_types is None: + if self.closed: return True + event_types = self._event_types # KeyboardEvent is a union: CHAR events carry a character, not a key. if event.type in (event_types.KEY_PRESS, event_types.KEY_REPEAT): pressed = True @@ -128,7 +116,7 @@ def _on_keyboard_event(self, event, *args, **kwargs) -> bool: pressed = False else: return True - code = _carb_key_name_to_w3c().get(event.input.name) + code = _CARB_KEY_CODES.get(event.input.name) if code is not None: self._emit_key(code, pressed) # let Kit keep processing the event diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py index 7744772011a0..d707377f3f21 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py @@ -85,15 +85,9 @@ def __init__(self, cfg: VisualizerCfg): # Camera tracking state (replaces ViewportCameraController) self._interactive_scene = None # set from SimulationContext._interactive_scene in initialize() self._viewer_origin: torch.Tensor | None = None # world-space origin offset for eye/lookat - self._key_event_source: KitKeyEventSource | None = None # ---- Lifecycle ------------------------------------------------------------------------ - @property - def key_event_source(self) -> KitKeyEventSource | None: - """Keys typed into the Kit window; ``None`` before initialization, after close, or when Kit runs headless.""" - return self._key_event_source - @property def visual_material_writer(self): """Write material channels directly through Fabric.""" @@ -210,15 +204,7 @@ def close(self) -> None: """Close viewport resources and restore temporary state.""" if not self._is_initialized: return - # Keep the reference until close finishes: a focus-loss callback reading key_event_source - # during shutdown gets the closing source back instead of creating one that is never closed. - key_event_source = self._key_event_source - if key_event_source is not None: - try: - key_event_source.close() - except Exception: - # keyboard input must not keep the viewport resources below from being released - logger.exception("[KitVisualizer] Keyboard input cleanup failed") + self._close_key_event_source() try: if self._owns_kit_timeline: self.stop() @@ -244,7 +230,6 @@ def close(self) -> None: self._rgb_render_product = None self._controls_kit_timeline = False self._is_initialized = False - self._key_event_source = None super().close() def play(self) -> None: diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py index 5dc8caf96951..ca681d4a6024 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_key_event_source.py @@ -29,7 +29,6 @@ def _pyglet_symbol_to_w3c() -> dict[int, str]: named = { "SPACE": "Space", "RETURN": "Enter", - "ENTER": "Enter", "ESCAPE": "Escape", "TAB": "Tab", "BACKSPACE": "Backspace", @@ -94,9 +93,7 @@ def _pyglet_symbol_to_w3c() -> dict[int, str]: "QUESTION": "Slash", } for name, code in named.items(): - symbol = getattr(key, name, None) - if symbol is not None: - table[symbol] = code + table[getattr(key, name)] = code return table @@ -123,14 +120,7 @@ def __init__(self, viewer: Any) -> None: self._viewer = viewer self._window = viewer.renderer.window self._ui_has_keyboard = False - # pushed and removed as one handler frame on the window - self._handlers = { - "on_key_press": self._on_key_press, - "on_key_release": self._on_key_release, - "on_deactivate": self._on_deactivate, - } - - self._window.push_handlers(**self._handlers) + self._window.push_handlers(self) self._viewer.keyboard_frame_hook = self._poll_ui_keyboard def close(self) -> None: @@ -138,7 +128,7 @@ def close(self) -> None: if self.closed: return try: - self._window.remove_handlers(**self._handlers) + self._window.remove_handlers(self) finally: self._viewer.keyboard_frame_hook = None self._viewer = self._window = None @@ -146,26 +136,29 @@ def close(self) -> None: def _poll_ui_keyboard(self) -> bool: """Track whether ImGui owns the keyboard, reporting a focus loss when it takes it.""" - viewer = self._viewer - if viewer is None: # released, with handlers left behind by a failed unhook + if self.closed: # a failed native unhook can leave callbacks behind return False + viewer = self._viewer has_keyboard = viewer.gui is not None and bool(viewer.gui.is_keyboard_capturing()) if has_keyboard and not self._ui_has_keyboard: self._emit_focus_lost() self._ui_has_keyboard = has_keyboard return has_keyboard - def _on_key_press(self, symbol: int, modifiers: int) -> None: + def on_key_press(self, symbol: int, modifiers: int) -> None: + """Forward a pyglet key press unless the UI owns keyboard input.""" if self._poll_ui_keyboard(): return code = _pyglet_symbol_to_w3c().get(symbol) if code is not None: self._emit_key(code, True) - def _on_key_release(self, symbol: int, modifiers: int) -> None: + def on_key_release(self, symbol: int, modifiers: int) -> None: + """Forward a pyglet key release.""" code = _pyglet_symbol_to_w3c().get(symbol) if code is not None: self._emit_key(code, False) - def _on_deactivate(self) -> None: + def on_deactivate(self) -> None: + """Release the listener's held keys when the window loses focus.""" self._emit_focus_lost() diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py index 59666d7b45d0..0eea9655ea86 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py @@ -501,14 +501,8 @@ def __init__(self, cfg): self._last_present_time = -math.inf self._last_render_step = -1 self._pending_mesh_submissions: dict[str, dict[str, Any]] = {} - self._key_event_source: NewtonKeyEventSource | None = None self._camera_keys_enabled = True - @property - def key_event_source(self) -> NewtonKeyEventSource | None: - """Keys typed into the viewer window; ``None`` before initialization, after close, or when headless.""" - return self._key_event_source - def initialize(self, sim: SimulationContext, *, cameras: list[PerspectiveCameraCfg | Camera]) -> None: """Bind camera sources and the display mode shared by both windows.""" super().initialize(sim, cameras=cameras) @@ -687,7 +681,6 @@ def reset(self, soft: bool = False) -> None: def close(self) -> None: """Detach perspective producers before releasing the visualizer's borrowed views.""" - self._key_event_source = None self._pending_mesh_submissions.clear() for view in self._image_views: if view.render == self._capture_frame: @@ -705,19 +698,6 @@ def set_camera_keys_enabled(self, enabled: bool) -> None: if self._viewer is not None: self._viewer.camera_keys_suspended = not enabled - def _close_key_event_source(self) -> None: - """Unhook keyboard input while the window is still open; subclasses call this first in ``close()``. - - The reference stays until :meth:`close` finishes, so a focus-loss callback reading - :attr:`key_event_source` during shutdown gets the closing source instead of a new one. - """ - if self._key_event_source is not None: - try: - self._key_event_source.close() - except Exception: - # keyboard input must not keep the viewer resources from being released - logger.exception("[%s] Keyboard input cleanup failed", type(self).__name__) - def supports_markers(self) -> bool: """Return whether visualization markers are enabled for this viewer.""" return self.cfg.enable_markers diff --git a/source/isaaclab_visualizers/test/test_key_event_sources.py b/source/isaaclab_visualizers/test/test_key_event_sources.py index 5ea388fbdcf5..6a6531a422c3 100644 --- a/source/isaaclab_visualizers/test/test_key_event_sources.py +++ b/source/isaaclab_visualizers/test/test_key_event_sources.py @@ -13,18 +13,19 @@ from __future__ import annotations -import ast -import importlib.util import os import pathlib +import runpy import subprocess import sys from types import SimpleNamespace import isaaclab_visualizers.newton.newton_key_event_source as newton_key_event_source +import pyglet import pytest -from isaaclab_visualizers.kit.kit_key_event_source import KitKeyEventSource, _carb_key_name_to_w3c +from isaaclab_visualizers.kit.kit_key_event_source import KitKeyEventSource from isaaclab_visualizers.newton.newton_key_event_source import NewtonKeyEventSource +from pyglet.event import EventDispatcher from isaaclab.visualizers import KeyEventSource @@ -254,8 +255,11 @@ def test_kit_failed_focus_cleanup_still_closes_and_notifies(kit): ("GRAVE_ACCENT", "Backquote"), ], ) -def test_kit_key_names_map_to_w3c_codes(carb_name, code): - assert _carb_key_name_to_w3c()[carb_name] == code +def test_kit_key_names_map_to_w3c_codes(kit, carb_name, code): + source = KitKeyEventSource(kit.window) + recorder, _ = _listen(source) + kit.key(kit.KEY_PRESS, carb_name) + assert recorder.events == [(code, True)] # --------------------------------------------------------------------------- @@ -263,47 +267,23 @@ def test_kit_key_names_map_to_w3c_codes(carb_name, code): # --------------------------------------------------------------------------- -def _pyglet_keys() -> SimpleNamespace: - """The constants of ``pyglet/window/key.py``, read without importing ``pyglet.window`` (needs a display).""" - spec = importlib.util.find_spec("pyglet") - if spec is None or spec.origin is None: - pytest.skip("pyglet is not installed") - tree = ast.parse((pathlib.Path(spec.origin).parent / "window" / "key.py").read_text()) - constants = {} - for node in tree.body: - if isinstance(node, ast.Assign) and isinstance(node.value, ast.Constant) and type(node.value.value) is int: - for target in node.targets: - if isinstance(target, ast.Name): - constants[target.id] = node.value.value - return SimpleNamespace(**constants) - - @pytest.fixture -def keys(monkeypatch) -> SimpleNamespace: - namespace = _pyglet_keys() +def keys(monkeypatch): + # Load the real constants without importing pyglet.window, which opens a display. + namespace = SimpleNamespace(**runpy.run_path(str(pathlib.Path(pyglet.__file__).parent / "window/key.py"))) monkeypatch.setitem(sys.modules, "pyglet.window", SimpleNamespace(key=namespace)) newton_key_event_source._pyglet_symbol_to_w3c.cache_clear() yield namespace newton_key_event_source._pyglet_symbol_to_w3c.cache_clear() -class _FakeWindow: - """pyglet window double with a handler stack.""" - - def __init__(self) -> None: - self.frames: list[dict] = [] - - def push_handlers(self, **handlers) -> None: - self.frames.append(handlers) +class _Window(EventDispatcher): + """Real pyglet event dispatch without a native display.""" - def remove_handlers(self, **handlers) -> None: - self.frames.remove(handlers) - def dispatch(self, name: str, *args) -> None: - for frame in reversed(self.frames): - if name in frame: - frame[name](*args) - return +_Window.register_event_type("on_key_press") +_Window.register_event_type("on_key_release") +_Window.register_event_type("on_deactivate") class _FakeGui: @@ -314,55 +294,55 @@ def is_keyboard_capturing(self) -> bool: return self.capturing -def _newton_viewer(window: _FakeWindow, gui: _FakeGui | None = None) -> SimpleNamespace: +def _newton_viewer(window: _Window, gui: _FakeGui | None = None) -> SimpleNamespace: return SimpleNamespace(renderer=SimpleNamespace(window=window), gui=gui, keyboard_frame_hook=None) def test_newton_forwards_window_keys(keys): - window = _FakeWindow() + window = _Window() viewer = _newton_viewer(window, _FakeGui()) source = NewtonKeyEventSource(viewer) recorder, subscription = _listen(source) - assert len(window.frames) == 1 and viewer.keyboard_frame_hook is not None + assert len(window._event_stack) == 1 and viewer.keyboard_frame_hook is not None - window.dispatch("on_key_press", keys.W, 0) - window.dispatch("on_key_release", keys.W, 0) - window.dispatch("on_key_press", keys.RETURN, 0) + window.dispatch_event("on_key_press", keys.W, 0) + window.dispatch_event("on_key_release", keys.W, 0) + window.dispatch_event("on_key_press", keys.RETURN, 0) assert recorder.events == [("KeyW", True), ("KeyW", False), ("Enter", True)] subscription.close() - assert len(window.frames) == 1 and viewer.keyboard_frame_hook is not None + assert len(window._event_stack) == 1 and viewer.keyboard_frame_hook is not None recorder, _ = _listen(source) - window.dispatch("on_key_press", keys.W, 0) + window.dispatch_event("on_key_press", keys.W, 0) assert recorder.events == [("KeyW", True)] source.close() - assert window.frames == [] and viewer.keyboard_frame_hook is None + assert window._event_stack == [] and viewer.keyboard_frame_hook is None def test_newton_ui_taking_the_keyboard_is_a_focus_loss_on_the_next_frame(keys): - window, gui = _FakeWindow(), _FakeGui() + window, gui = _Window(), _FakeGui() viewer = _newton_viewer(window, gui) source = NewtonKeyEventSource(viewer) recorder, _ = _listen(source) - window.dispatch("on_key_press", keys.W, 0) + window.dispatch_event("on_key_press", keys.W, 0) gui.capturing = True # e.g. a text field was clicked: no key press needed viewer.keyboard_frame_hook() viewer.keyboard_frame_hook() assert recorder.focus_lost == 1 - window.dispatch("on_key_press", keys.A, 0) # typed into the UI + window.dispatch_event("on_key_press", keys.A, 0) # typed into the UI gui.capturing = False viewer.keyboard_frame_hook() - window.dispatch("on_key_release", keys.A, 0) + window.dispatch_event("on_key_release", keys.A, 0) assert recorder.events == [("KeyW", True), ("KeyA", False)] # the press stayed in the UI def test_newton_viewer_without_gui_forwards_keys(keys): - window = _FakeWindow() + window = _Window() source = NewtonKeyEventSource(_newton_viewer(window, gui=None)) recorder, _ = _listen(source) - window.dispatch("on_key_press", keys.W, 0) + window.dispatch_event("on_key_press", keys.W, 0) assert recorder.events == [("KeyW", True)] @@ -373,34 +353,34 @@ def test_newton_viewer_without_gui_forwards_keys(keys): @pytest.mark.parametrize("shift_first", [False, True], ids=["shift-after-press", "shift-before-press"]) def test_newton_key_released_after_shift_changes(keys, unshifted, shifted, code, shift_first): """X11 names a digit or punctuation key by its shifted symbol while Shift is held.""" - window = _FakeWindow() + window = _Window() source = NewtonKeyEventSource(_newton_viewer(window, _FakeGui())) recorder, _ = _listen(source) press, release = (shifted, unshifted) if shift_first else (unshifted, shifted) - window.dispatch("on_key_press", getattr(keys, press), 0) - window.dispatch("on_key_release", getattr(keys, release), 0) + window.dispatch_event("on_key_press", getattr(keys, press), 0) + window.dispatch_event("on_key_release", getattr(keys, release), 0) assert recorder.events == [(code, True), (code, False)] def test_newton_window_deactivation_is_a_focus_loss(keys): - window = _FakeWindow() + window = _Window() source = NewtonKeyEventSource(_newton_viewer(window, _FakeGui())) recorder, _ = _listen(source) - window.dispatch("on_key_press", keys.W, 0) - window.dispatch("on_deactivate") + window.dispatch_event("on_key_press", keys.W, 0) + window.dispatch_event("on_deactivate") assert recorder.focus_lost == 1 assert recorder.events == [("KeyW", True)] def test_newton_failed_unhook_still_closes_and_notifies(keys): - window = _FakeWindow() + window = _Window() viewer = _newton_viewer(window, _FakeGui()) source = NewtonKeyEventSource(viewer) recorder, subscription = _listen(source) - def fail(**handlers): + def fail(*handlers): raise RuntimeError("remove_handlers") window.remove_handlers = fail @@ -408,18 +388,18 @@ def fail(**handlers): source.close() assert viewer.keyboard_frame_hook is None # the closed source is never polled again assert source.closed and subscription.closed and recorder.focus_lost == 1 - window.dispatch("on_key_press", keys.W, 0) # a leftover native hook is inert + window.dispatch_event("on_key_press", keys.W, 0) # a leftover native hook is inert assert recorder.events == [] def test_newton_close_unhooks_and_releases_the_window(keys): - window = _FakeWindow() + window = _Window() viewer = _newton_viewer(window, _FakeGui()) source = NewtonKeyEventSource(viewer) recorder, _ = _listen(source) source.close() - assert window.frames == [] and viewer.keyboard_frame_hook is None + assert window._event_stack == [] and viewer.keyboard_frame_hook is None assert recorder.focus_lost == 1 assert source._viewer is None and source._window is None From 840f67d2508423c7252936d438fab4a37390f5a5 Mon Sep 17 00:00:00 2001 From: Rafael Wiltz Date: Fri, 23 Oct 2026 23:01:30 -0400 Subject: [PATCH 5/6] Keep the visualizers starting when keyboard input fails Kit and Newton visualizers now connect keyboard input when their window initializes. Keyboard input is optional, so a failure there is logged and key_event_source stays None instead of stopping the visualizer from starting. --- .../isaaclab_visualizers/kit/kit_visualizer.py | 6 +++++- .../isaaclab_visualizers/newton/newton_visualizer.py | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py index d707377f3f21..25f87de8dc2d 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py @@ -161,7 +161,11 @@ def initialize(self, sim: SimulationContext, *, cameras: list[PerspectiveCameraC if not self._runtime_headless: import omni.appwindow - self._key_event_source = KitKeyEventSource(omni.appwindow.get_default_app_window()) + # keyboard input is optional: a failure here must not stop the visualizer from starting + try: + self._key_event_source = KitKeyEventSource(omni.appwindow.get_default_app_window()) + except Exception: + logger.exception("[KitVisualizer] Keyboard input is unavailable") def step(self, dt: float) -> None: """Advance visualizer/UI updates for one simulation step. diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py index 0eea9655ea86..7a8d5f69a4b5 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualizer.py @@ -526,7 +526,11 @@ def initialize(self, sim: SimulationContext, *, cameras: list[PerspectiveCameraC self._viewer.camera_keys_suspended = not self._camera_keys_enabled self._viewer.register_ui_callback(self._draw_streaming_view_controls, position="side") if not self._runtime_headless: - self._key_event_source = NewtonKeyEventSource(self._viewer) + # keyboard input is optional: a failure here must not stop the visualizer from starting + try: + self._key_event_source = NewtonKeyEventSource(self._viewer) + except Exception: + logger.exception("[%s] Keyboard input is unavailable", type(self).__name__) def log_mesh( self, From d80d8c2c4e5819fcb8377ecbf4ff1f42d85229c3 Mon Sep 17 00:00:00 2001 From: Octi Zhang Date: Fri, 23 Oct 2026 22:54:09 -0700 Subject: [PATCH 6/6] Handle unavailable Kit keyboard imports during initialization --- .../changelog.d/optional-keyboard-import.fixed.rst | 1 + .../isaaclab_visualizers/kit/kit_visualizer.py | 4 ++-- .../test/test_kit_visualizer_scene_partitioning.py | 6 +++++- 3 files changed, 8 insertions(+), 3 deletions(-) create mode 100644 source/isaaclab_visualizers/changelog.d/optional-keyboard-import.fixed.rst diff --git a/source/isaaclab_visualizers/changelog.d/optional-keyboard-import.fixed.rst b/source/isaaclab_visualizers/changelog.d/optional-keyboard-import.fixed.rst new file mode 100644 index 000000000000..9a7e5690ff5c --- /dev/null +++ b/source/isaaclab_visualizers/changelog.d/optional-keyboard-import.fixed.rst @@ -0,0 +1 @@ +* Fixed Kit visualizer initialization failing when the optional window-input module was unavailable. diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py index 25f87de8dc2d..4ad67d9d6c8e 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py @@ -159,10 +159,10 @@ def initialize(self, sim: SimulationContext, *, cameras: list[PerspectiveCameraC if self._controls_kit_timeline: self.play() if not self._runtime_headless: - import omni.appwindow - # keyboard input is optional: a failure here must not stop the visualizer from starting try: + import omni.appwindow + self._key_event_source = KitKeyEventSource(omni.appwindow.get_default_app_window()) except Exception: logger.exception("[KitVisualizer] Keyboard input is unavailable") diff --git a/source/isaaclab_visualizers/test/test_kit_visualizer_scene_partitioning.py b/source/isaaclab_visualizers/test/test_kit_visualizer_scene_partitioning.py index 86b4276bb7ca..849f804f9d61 100644 --- a/source/isaaclab_visualizers/test/test_kit_visualizer_scene_partitioning.py +++ b/source/isaaclab_visualizers/test/test_kit_visualizer_scene_partitioning.py @@ -206,7 +206,9 @@ def test_close_cleans_resources_when_timeline_stop_is_interrupted(monkeypatch, k ("physics_backend", "controls_timeline"), [("ovphysx", False), ("newton", True)], ) -def test_timeline_controls_only_non_physx_backends(monkeypatch, physics_backend, controls_timeline): +def test_timeline_controls_only_non_physx_backends(monkeypatch, physics_backend, controls_timeline, caplog): + """Timeline setup must survive unavailable optional window input.""" + monkeypatch.setitem(sys.modules, "omni.appwindow", None) visualizer = KitVisualizer(VisualizerCfg(backend=KitBackendCfg())) sim = MagicMock(has_gui=True, stage=Usd.Stage.CreateInMemory(), device="cpu") sim.get_scene_data_provider.return_value.num_envs = 0 @@ -228,6 +230,8 @@ def test_timeline_controls_only_non_physx_backends(monkeypatch, physics_backend, assert visualizer._controls_kit_timeline is controls_timeline assert play.call_count == int(controls_timeline) fabric.bind_transforms.assert_called_once_with(sim.get_scene_data_provider.return_value) + assert visualizer.is_initialized and visualizer.key_event_source is None + assert "Keyboard input is unavailable" in caplog.text @pytest.mark.parametrize("color", [(0.1, 0.2, 0.3), None])