fix/raise_window - #1290
fix/raise_window#1290wyzula-jan wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
BECGuiClient.get_display_info() can incorrectly time out when attaching to an already-running GUI server due to relying on wait_for_server() without ensuring the started-event is set.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses a Qt/Wayland (GNOME 40, Qt 6.11.1) window-raising regression by changing the GUI RPC “raise” behavior to recreate native window resources on Wayland while preserving the underlying QWidget instances and BEC connections. It also introduces a system.get_display_info RPC and a corresponding client API (gui.get_display_info()) for collecting read-only display/window diagnostics from the running GUI process.
Changes:
- Update RPC window raising to be backend-aware: keep the existing X11/XWayland (“xcb”) GNOME/RHEL workaround while adding a Wayland-specific native-resource recreation path.
- Add GUI-side display diagnostics (
get_display_info) and expose them through a newsystem.get_display_infosystem RPC plus a CLI client method. - Add unit/e2e coverage and documentation for the new raise/show semantics and display diagnostics.
File summaries
| File | Description |
|---|---|
bec_widgets/utils/rpc_server.py |
Routes "raise" RPC calls for QWidget instances through a new backend-aware _raise_window() and exposes system.get_display_info. |
bec_widgets/utils/display_info.py |
Adds GUI-process diagnostics collection for Qt backend, screens, windows, and selected environment variables. |
bec_widgets/cli/client_utils.py |
Makes show() delegate to raise_window() and adds BECGuiClient.get_display_info() client API. |
tests/unit_tests/test_rpc_server.py |
Adds comprehensive tests covering raise/show behavior across platforms, minimized/hidden states, and thread-safety via dispatcher callbacks. |
tests/unit_tests/test_display_info.py |
Adds tests verifying diagnostics content, JSON-serializability, and non-invasive behavior (no native window creation). |
tests/unit_tests/test_client_utils.py |
Adds tests for show/raise_window behavior and get_display_info() client RPC behavior. |
tests/end-2-end/test_bec_gui_ipython.py |
Adds an end-to-end test validating visibility toggling and diagnostics retrieval from the running GUI server. |
README.md |
Documents the new visibility/raise behavior and the display diagnostics feature. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| with wait_for_server(self): | ||
| return self.launcher._run_rpc("system.get_display_info") |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
64ca15e to
e4dec96
Compare
Benchmark comparisonThreshold: 20% (lower is better). No benchmark regression exceeded the configured threshold. No benchmark improvement exceeded the configured threshold. All benchmark results
|
Description
On native Wayland with GNOME 40 and Qt 6.11.1, raising a BEC window could hide it instead of bringing it forward. This change works around the issue by recreating the native window resources while preserving the QWidget instances, their data, and BEC connections.