diff --git a/README.md b/README.md index 3c72105a..5f173a06 100644 --- a/README.md +++ b/README.md @@ -106,12 +106,16 @@ when a window is behind another application. `gui.raise_window()` uses the same `gui.flomni.raise_window()` targets one window. Already-visible, active windows are left unchanged. On XCB (X11/XWayland), BEC retains its GNOME/RHEL window-raising workaround and restores the original -window flags afterwards. On native Wayland, BEC briefly hides and shows visible, inactive windows -before requesting activation, because GNOME can refuse to activate an existing window. This keeps -the window's contents and flags, but may cause flicker or let the compositor reposition it. Each -call finishes with the window shown; already-active windows are left untouched. On macOS, BEC -requests activation without remapping the window. Activation is still subject to the window -manager/compositor's focus policy; a Redis RPC is not a local mouse or keyboard event. +window flags afterwards. On native Wayland, the experimental workaround hides a visible, inactive +window and schedules showing it on a subsequent Qt event-loop turn. Repeated requests share one +pending show; an explicit RPC hide/show of that window, or a close, cancels it. Hidden windows are +shown immediately. +The workaround preserves contents and flags, but may flicker or let the compositor reposition the +window. It does not guarantee foreground activation. macOS uses Qt's normal activation request. + +Wayland's supported focus transfer uses an XDG activation token from the focused application. +The terminal's input event/token is not carried by BEC's Redis RPC, so GNOME may reject activation. +See [GNOME's explanation of focus stealing prevention](https://blogs.gnome.org/shell-dev/2024/09/20/understanding-gnome-shells-focus-stealing-prevention/). To inspect the **running GUI server process**, from the BEC IPython client: diff --git a/bec_widgets/cli/client_utils.py b/bec_widgets/cli/client_utils.py index f0b6a0f4..2680cf00 100644 --- a/bec_widgets/cli/client_utils.py +++ b/bec_widgets/cli/client_utils.py @@ -414,7 +414,7 @@ class BECGuiClient(RPCBase): logger.warning("Using .start() is deprecated, use .show() instead.") return self._start(wait=wait) - def show(self, wait=True) -> None: + def show(self, wait: bool = True) -> None: """ Show GUI windows and request foreground activation, including for visible windows behind another application. Already-active windows are left unchanged. @@ -423,9 +423,7 @@ class BECGuiClient(RPCBase): Args: wait(bool): Whether to wait for the server to start. Defaults to True. """ - if self._check_if_server_is_alive(): - return self._raise_all() - return self._start(wait=wait) + return self.raise_window(wait=wait) def hide(self): """Hide the GUI window.""" @@ -749,18 +747,15 @@ class BECGuiClient(RPCBase): for window in self._top_level.values(): window.hide() - def _do_raise_all(self): - """Bring GUI windows to the front.""" - if self.launcher and len(self._top_level) == 0: - self.launcher._run_rpc("raise") # pylint: disable=protected-access - for window in self._top_level.values(): - window.raise_window() - def _raise_all(self): + """Bring GUI windows to the front.""" with wait_for_server(self): if self._killed: return - return self._do_raise_all() + if self.launcher and not self._top_level: + self.launcher._run_rpc("raise") # pylint: disable=protected-access + for window in self._top_level.values(): + window.raise_window() def _update_dynamic_namespace(self, server_registry: dict): """ diff --git a/bec_widgets/utils/rpc_server.py b/bec_widgets/utils/rpc_server.py index 7f6607db..1288719e 100644 --- a/bec_widgets/utils/rpc_server.py +++ b/bec_widgets/utils/rpc_server.py @@ -11,7 +11,7 @@ from bec_lib.client import BECClient from bec_lib.endpoints import MessageEndpoints from bec_lib.logger import bec_logger from bec_lib.utils.import_utils import lazy_import -from qtpy.QtCore import Qt, QTimer +from qtpy.QtCore import QEvent, QObject, Qt, QTimer from qtpy.QtWidgets import QApplication, QWidget from redis.exceptions import RedisError @@ -28,7 +28,6 @@ from bec_widgets.widgets.containers.main_window.main_window import BECMainWindow if TYPE_CHECKING: # pragma: no cover from bec_lib import messages - from qtpy.QtCore import QObject else: messages = lazy_import("bec_lib.messages") logger = bec_logger.logger @@ -41,6 +40,24 @@ class RegistryNotReadyError(Exception): """Raised when trying to access an object from the RPC registry that is not yet registered.""" +class _WindowRaiser(QObject): + """Finish a Wayland hide/show across event-loop turns; the window owns the timer.""" + + def __init__(self, window: QWidget) -> None: + super().__init__(window) + self.timer = QTimer(self) + self.timer.setSingleShot(True) + self.timer.timeout.connect(window.show) + self.timer.timeout.connect(window.raise_) + self.timer.timeout.connect(window.activateWindow) + window.installEventFilter(self) + + def eventFilter(self, watched: QObject, event: QEvent) -> bool: + if event.type() == QEvent.Close: + self.timer.stop() + return False + + @contextmanager def rpc_exception_hook(err_func): """This context replaces the popup message box for error display with a specific hook""" @@ -238,14 +255,12 @@ class RPCServer: if method == "raise" and isinstance(obj, QWidget): self._raise_window(obj.window()) res = None - elif method == "show" and isinstance(obj, QWidget): - if obj.isMinimized(): - # Preserve maximized/full-screen state when restoring a minimized window. - obj.setWindowState(obj.windowState() & ~Qt.WindowMinimized) - if not obj.isVisible(): - obj.show() - res = None else: + if method in ("hide", "show") and isinstance(obj, QWidget) and obj.isWindow(): + # Explicit visibility requests supersede a deferred Wayland raise. + raiser = obj.findChild(_WindowRaiser, options=Qt.FindDirectChildrenOnly) + if raiser is not None: + raiser.timer.stop() target_obj, method_obj = self._resolve_rpc_target(obj, method) # check if the method accepts args and kwargs if not callable(method_obj): @@ -264,6 +279,10 @@ class RPCServer: if window.isVisible() and window.isActiveWindow() and not window.isMinimized(): return + raiser = window.findChild(_WindowRaiser, options=Qt.FindDirectChildrenOnly) + if raiser is not None and raiser.timer.isActive(): + return + state = window.windowState() & ~Qt.WindowMinimized platform = QApplication.platformName() if platform == "xcb": @@ -284,9 +303,13 @@ class RPCServer: if window.isMinimized(): window.setWindowState(state) if platform.startswith("wayland") and window.isVisible(): - # GNOME Wayland can refuse activation of an existing surface. Remap the - # inactive window using hide/show, which brings it forward on those consoles. + # Give Qt an event-loop turn between hiding and showing. A synchronous hide/show + # can leave the native Wayland surface hidden until the next RPC call. + if raiser is None: + raiser = _WindowRaiser(window) window.hide() + raiser.timer.start(0) + return if not window.isVisible(): window.show() window.raise_() diff --git a/tests/unit_tests/test_client_utils.py b/tests/unit_tests/test_client_utils.py index a9c5f623..94e3d8d8 100644 --- a/tests/unit_tests/test_client_utils.py +++ b/tests/unit_tests/test_client_utils.py @@ -158,6 +158,18 @@ def test_client_show_and_raise_request_foreground_activation(method, window_name window._run_rpc.assert_not_called() +@pytest.mark.parametrize("method", ["show", "raise_window"]) +@pytest.mark.parametrize("wait", [False, True]) +def test_client_show_and_raise_start_gui_with_requested_wait(method, wait): + gui = BECGuiClient() + with ( + mock.patch.object(gui, "_check_if_server_is_alive", return_value=False), + mock.patch.object(gui, "_start") as start, + ): + getattr(gui, method)(wait=wait) + start.assert_called_once_with(wait=wait) + + def test_client_display_info_comes_from_server_without_starting_gui(): gui = BECGuiClient() with ( diff --git a/tests/unit_tests/test_rpc_server.py b/tests/unit_tests/test_rpc_server.py index 4e386d20..72e12947 100644 --- a/tests/unit_tests/test_rpc_server.py +++ b/tests/unit_tests/test_rpc_server.py @@ -3,7 +3,7 @@ from unittest.mock import MagicMock, patch import pytest from bec_lib.service_config import ServiceConfig -from qtpy.QtCore import QEvent, QObject, Qt +from qtpy.QtCore import QCoreApplication, QEvent, QObject, Qt from qtpy.QtWidgets import QWidget from bec_widgets.applications import companion_app as companion_app_module @@ -136,21 +136,6 @@ def test_repeated_show_and_raise_preserve_visibility_and_flags( assert events.hide_events == events.show_events == expected_remaps -def test_widget_show_visible_window_does_not_request_activation(rpc_server, dummy_widget): - dummy_widget.show() - with ( - patch.object(dummy_widget, "raise_") as raise_window, - patch.object(dummy_widget, "activateWindow") as activate_window, - patch.object(dummy_widget, "show") as show_window, - patch.object(dummy_widget, "setWindowState") as set_state, - ): - rpc_server.run_rpc(dummy_widget, "show", [], {}) - raise_window.assert_not_called() - activate_window.assert_not_called() - show_window.assert_not_called() - set_state.assert_not_called() - - @pytest.mark.parametrize("method", ["show", "raise"]) def test_show_and_raise_restore_hidden_window(rpc_server, dummy_widget, method): dummy_widget.hide() @@ -158,20 +143,19 @@ def test_show_and_raise_restore_hidden_window(rpc_server, dummy_widget, method): assert dummy_widget.isVisible() -@pytest.mark.parametrize("method", ["show", "raise"]) @pytest.mark.parametrize("state", [Qt.WindowNoState, Qt.WindowMaximized, Qt.WindowFullScreen]) @pytest.mark.parametrize("platform", ["wayland", "xcb"]) -def test_show_and_raise_restore_minimized_window_state( - rpc_server, dummy_widget, monkeypatch, method, state, platform +def test_raise_restores_minimized_window_state( + rpc_server, dummy_widget, monkeypatch, qtbot, state, platform ): monkeypatch.setattr(rpc_server_module.QApplication, "platformName", lambda: platform) dummy_widget.setWindowState(state | Qt.WindowMinimized) dummy_widget.show() assert dummy_widget.isMinimized() - rpc_server.run_rpc(dummy_widget, method, [], {}) + rpc_server.run_rpc(dummy_widget, "raise", [], {}) - assert dummy_widget.isVisible() + qtbot.waitUntil(dummy_widget.isVisible) assert not dummy_widget.isMinimized() assert dummy_widget.windowState() & (Qt.WindowMaximized | Qt.WindowFullScreen) == state @@ -208,7 +192,7 @@ def test_raise_active_window_is_noop(rpc_server, dummy_widget, monkeypatch, plat @pytest.mark.parametrize("platform", ["wayland", "wayland-egl", "cocoa"]) def test_raise_visible_inactive_window_requests_activation( - rpc_server, dummy_widget, monkeypatch, platform + rpc_server, dummy_widget, monkeypatch, qtbot, platform ): dummy_widget.show() monkeypatch.setattr(rpc_server_module.QApplication, "platformName", lambda: platform) @@ -218,14 +202,15 @@ def test_raise_visible_inactive_window_requests_activation( patch.object(dummy_widget, "setWindowFlags") as flags, ): rpc_server.run_rpc(dummy_widget, "raise", [], {}) + qtbot.waitUntil(lambda: activate.called) activate.assert_called_once_with() flags.assert_not_called() @pytest.mark.parametrize("platform", ["wayland", "wayland-egl"]) @pytest.mark.parametrize("initially_visible", [False, True]) -def test_wayland_raise_finishes_showing_before_requesting_activation( - rpc_server, dummy_widget, monkeypatch, platform, initially_visible +def test_wayland_raise_defers_remapping_but_shows_hidden_windows_immediately( + rpc_server, dummy_widget, monkeypatch, qtbot, platform, initially_visible ): monkeypatch.setattr(rpc_server_module.QApplication, "platformName", lambda: platform) monkeypatch.setattr(dummy_widget, "isActiveWindow", lambda: False) @@ -239,6 +224,12 @@ def test_wayland_raise_finishes_showing_before_requesting_activation( side_effect=lambda: visible_during_activation.append(dummy_widget.isVisible()), ): rpc_server.run_rpc(dummy_widget, "raise", [], {}) + if initially_visible: + assert not dummy_widget.isVisible() + assert visible_during_activation == [] + else: + assert dummy_widget.isVisible() + qtbot.waitUntil(lambda: bool(visible_during_activation)) assert events.hide_events == int(initially_visible) assert events.show_events == 1 @@ -246,6 +237,75 @@ def test_wayland_raise_finishes_showing_before_requesting_activation( assert dummy_widget.isVisible() +def test_repeated_wayland_raises_share_pending_remap(rpc_server, dummy_widget, monkeypatch, qtbot): + monkeypatch.setattr(rpc_server_module.QApplication, "platformName", lambda: "wayland") + monkeypatch.setattr(dummy_widget, "isActiveWindow", lambda: False) + dummy_widget.show() + events = WindowEvents(dummy_widget) + + for _ in range(3): + rpc_server.run_rpc(dummy_widget, "raise", [], {}) + assert not dummy_widget.isVisible() + + qtbot.waitUntil(dummy_widget.isVisible) + assert events.hide_events == events.show_events == 1 + + +@pytest.mark.parametrize("method", ["hide", "close", "show"]) +def test_visibility_request_cancels_pending_wayland_raise( + rpc_server, dummy_widget, monkeypatch, qapp, method +): + monkeypatch.setattr(rpc_server_module.QApplication, "platformName", lambda: "wayland") + monkeypatch.setattr(dummy_widget, "isActiveWindow", lambda: False) + dummy_widget.show() + + with patch.object(dummy_widget, "activateWindow") as activate: + rpc_server.run_rpc(dummy_widget, "raise", [], {}) + if method == "close": + dummy_widget.close() # Also cover a close that does not arrive through RPC. + else: + rpc_server.run_rpc(dummy_widget, method, [], {}) + qapp.processEvents() + + assert dummy_widget.isVisible() == (method == "show") + activate.assert_not_called() + + +def test_destroying_window_cancels_pending_wayland_raise(rpc_server, qapp, monkeypatch): + monkeypatch.setattr(rpc_server_module.QApplication, "platformName", lambda: "wayland") + window = QWidget() # Deleted explicitly below, before the timer can fire. + monkeypatch.setattr(window, "isActiveWindow", lambda: False) + window.show() + + with patch.object(window, "activateWindow") as activate: + rpc_server.run_rpc(window, "raise", [], {}) + window.deleteLater() + QCoreApplication.sendPostedEvents(None, QEvent.DeferredDelete) + qapp.processEvents() + + activate.assert_not_called() + + +@pytest.mark.parametrize("method", ["hide", "show"]) +def test_child_visibility_does_not_cancel_pending_window_raise( + rpc_server, dummy_widget, qtbot, qapp, monkeypatch, method +): + window = QWidget() + qtbot.addWidget(window) + dummy_widget.setParent(window) + dummy_widget.setVisible(method == "hide") + window.show() + monkeypatch.setattr(rpc_server_module.QApplication, "platformName", lambda: "wayland") + monkeypatch.setattr(window, "isActiveWindow", lambda: False) + + rpc_server.run_rpc(window, "raise", [], {}) + rpc_server.run_rpc(dummy_widget, method, [], {}) + qapp.processEvents() + + assert window.isVisible() + assert dummy_widget.isVisible() == (method == "show") + + def test_wayland_remap_preserves_dock_contents(rpc_server, qtbot, qapp, monkeypatch, tmp_path): monkeypatch.setenv("BECWIDGETS_PROFILE_DIR", str(tmp_path)) dock_area = rpc_server._launch_dock_area(name="flomni", startup_profile="skip")