From 19031084f816393d0e6f3c020c8639c9d2ebacf5 Mon Sep 17 00:00:00 2001 From: Osayi-ANL Date: Thu, 24 Sep 2026 15:51:55 -0500 Subject: [PATCH 1/2] =?UTF-8?q?add:=20machine-local=20window=20layout=20pe?= =?UTF-8?q?rsistence=20to=20BaseWindow=20via=20QSettings=20=E2=80=94=20a?= =?UTF-8?q?=20window=20reopens=20at=20the=20size,=20position=20and=20dock?= =?UTF-8?q?=20arrangement=20it=20was=20closed=20at,=20per=20viewer=20type,?= =?UTF-8?q?=20with=20a=20dock-state=20version=20each=20viewer=20raises=20o?= =?UTF-8?q?n=20its=20own?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/dashpva/settings.py | 6 ++ .../viewer/area_det/area_det_viewer.py | 5 ++ src/dashpva/viewer/core/base_window.py | 84 ++++++++++++++++++- 3 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/dashpva/settings.py b/src/dashpva/settings.py index b85a3461..68ed69f6 100644 --- a/src/dashpva/settings.py +++ b/src/dashpva/settings.py @@ -364,6 +364,12 @@ def preview_settings(config): LAST_CIF_DIR: str = _DEFAULT_BROWSE_DIR LAST_TOML_DIR: str = str(PROJECT_ROOT / 'pv_configs') +# UI persistence (QSettings) — see AGENTS.md "UI persistence — QSettings". +# Default dock-state version. A viewer overrides BaseWindow.dock_state_version +# to raise its own when its dock set changes, so a stale restore cannot misplace +# that viewer's docks without retiring every other viewer's layout too. +DOCK_STATE_VERSION: int = 1 + # Internal state _locator_internal: Optional[Union[int, str]] = None _STATE_FILE: Path = PROJECT_ROOT / '.dashpva_locator' diff --git a/src/dashpva/viewer/area_det/area_det_viewer.py b/src/dashpva/viewer/area_det/area_det_viewer.py index a6390f61..4dcaf944 100644 --- a/src/dashpva/viewer/area_det/area_det_viewer.py +++ b/src/dashpva/viewer/area_det/area_det_viewer.py @@ -194,6 +194,11 @@ def closeEvent(self, event): class DiffractionImageWindow(BaseWindow): + #: Opt out of BaseWindow's store while this viewer still restores its own + #: area_det_* keys in __init__ -- both running would restore twice, the + #: second from an empty key. Dropped when it migrates onto the shared store. + persist_state = False + hkl_data_updated = pyqtSignal(bool) # Emitted from the ROI/Stats connection thread so update_status can run on # the main (GUI) thread. Args: message, level ('info' | 'warning' | 'error'). diff --git a/src/dashpva/viewer/core/base_window.py b/src/dashpva/viewer/core/base_window.py index d38cce86..67915b45 100644 --- a/src/dashpva/viewer/core/base_window.py +++ b/src/dashpva/viewer/core/base_window.py @@ -32,7 +32,7 @@ from pathlib import Path from PyQt5 import uic -from PyQt5.QtCore import Qt, QTimer, pyqtSignal +from PyQt5.QtCore import QSettings, Qt, QTimer, pyqtSignal from PyQt5.QtWidgets import ( QAction, QFileDialog, @@ -65,6 +65,17 @@ class BaseWindow(QMainWindow): file_opened = pyqtSignal(str) # Emitted when a file is opened file_saved = pyqtSignal(str) # Emitted when a file is saved + #: Persist geometry and dock layout to QSettings. Geometry and dock layout + #: are physical machine state, so they live in Qt's per-user store rather + #: than a portable TOML/JSON config. False opts out (the area-detector + #: viewer keeps its own older area_det_* keys). + persist_state = True + + #: Dock-state version for this viewer. Raise it on the one viewer whose dock + #: set changed -- Qt drops a saved state whose version does not match, and a + #: number shared across viewers would retire every other layout as well. + dock_state_version: int = app_settings.DOCK_STATE_VERSION + def __init__(self, ui_file_name=None, viewer_name=None, log_manager: LogManager = None, visible_actions=_SHOW_ALL, size_policy: dict = None): """ @@ -115,6 +126,9 @@ def __init__(self, ui_file_name=None, viewer_name=None, log_manager: LogManager # CPU, GPU, runtime in status bar self.init_perf_statusbar() + if self.persist_state: + QTimer.singleShot(0, self._restore_on_start) + def load_ui(self): """Load the UI file for this window.""" if not self.ui_file_name: @@ -633,6 +647,73 @@ def closeEvent(self, event): self._close_confirmed = True return True + # ---- Machine-local UI persistence (QSettings, per viewer type) ---- + + def _qsettings(self) -> QSettings: + """QSettings scoped to this viewer type (org "DashPVA").""" + return QSettings("DashPVA", type(self).__name__) + + def save_layout(self) -> None: + """Persist window geometry and dock layout.""" + if not self.persist_state: + return + s = self._qsettings() + s.setValue("geometry", self.saveGeometry()) + s.setValue("dock_state", self.saveState(self.dock_state_version)) + + def restore_geometry(self) -> None: + """Restore window geometry, clamped to the current screen. + + Safe to call from ``__init__``. Geometry saved on a larger monitor + would otherwise reopen off the edge of a smaller one. + """ + if not self.persist_state: + return + geom = self._qsettings().value("geometry") + if not geom: + return + try: + self.restoreGeometry(geom) + avail = self.screen().availableGeometry() + if self.width() > avail.width() or self.height() > avail.height(): + self.resize(min(self.width(), avail.width()), + min(self.height(), avail.height())) + except Exception: + pass + + def restore_dock_state(self) -> None: + """Restore dock layout. Call only once this window's docks exist. + + Qt drops a saved state whose version does not match, so bumping this + viewer's ``dock_state_version`` retires its stale layouts instead of + restoring docks into places that no longer exist. + """ + if not self.persist_state: + return + state = self._qsettings().value("dock_state") + if state: + try: + self.restoreState(state, self.dock_state_version) + except Exception: + pass + + def _restore_on_start(self) -> None: + """Restore once __init__ has finished, on the next event-loop turn. + + Deferred so every subclass __init__ body is done: its docks exist (dock + state cannot be restored before them) and its hardcoded defaults are + already set, which would otherwise overwrite restored values. Doing it + here rather than in each subclass also means no window can forget to. + """ + if not self.persist_state: + return + self.restore_layout() + + def restore_layout(self) -> None: + """Geometry and docks together, for windows whose docks already exist.""" + self.restore_geometry() + self.restore_dock_state() + def closeEvent(self, event): """Close only once any unsaved edits have been saved or discarded. @@ -641,6 +722,7 @@ def closeEvent(self, event): """ if not self.confirm_close(event): return + self.save_layout() event.accept() def save_checkable_state(self, settings, widget, key: str) -> None: From b812bcdc3d55bcb7403361ce0046355c74700195 Mon Sep 17 00:00:00 2001 From: Osayi-ANL Date: Thu, 24 Sep 2026 15:51:55 -0500 Subject: [PATCH 2/2] add: save/restore/version tests for window layout persistence --- tests/unit/test_window_layout_persistence.py | 109 +++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 tests/unit/test_window_layout_persistence.py diff --git a/tests/unit/test_window_layout_persistence.py b/tests/unit/test_window_layout_persistence.py new file mode 100644 index 00000000..751f4280 --- /dev/null +++ b/tests/unit/test_window_layout_persistence.py @@ -0,0 +1,109 @@ +# Copyright © 2026, UChicago Argonne, LLC +# All Rights Reserved +# Software Name: DashPVA +# By: Argonne National Laboratory +# +# BSD OPEN SOURCE LICENSE +# +# Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. +# 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission. +# +# ****************************************************************************************************** +# DISCLAIMER +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# ****************************************************************************************************** + + +"""Machine-local window layout persistence. + +A window reopens at the size, position and dock arrangement it was closed at. +State is written to Qt's per-user store, scoped per viewer type, so these tests +redirect that store into tmp_path rather than touching the developer's own. +""" +from __future__ import annotations + +import pytest +from PyQt5.QtCore import QSettings +from PyQt5.QtWidgets import QApplication, QDockWidget + +from dashpva.viewer.core.base_window import BaseWindow + + +@pytest.fixture(scope="module") +def qapp(): + return QApplication.instance() or QApplication([]) + + +@pytest.fixture(autouse=True) +def isolated_settings(tmp_path): + """Point QSettings at tmp_path so a test can never read or write the real store.""" + QSettings.setPath(QSettings.NativeFormat, QSettings.UserScope, str(tmp_path)) + yield + + +class _Docked(BaseWindow): + """A window with one dock, so saveState has something to record.""" + + def __init__(self): + super().__init__(viewer_name='LayoutTest') + self.dock = QDockWidget("d", self) + self.dock.setObjectName("d") + self.addDockWidget(0x1, self.dock) # Qt.LeftDockWidgetArea + + +def test_close_saves_geometry_and_dock_state(qapp): + w = _Docked() + w.resize(640, 480) + w.close() + + s = QSettings("DashPVA", "_Docked") + assert sorted(s.allKeys()) == ["dock_state", "geometry"] + + +def test_a_window_reopens_at_the_size_it_was_closed_at(qapp): + w = _Docked() + w.resize(645, 485) + w.move(90, 110) + w.close() + + reopened = _Docked() + reopened.restore_layout() + assert (reopened.width(), reopened.height()) == (645, 485) + assert (reopened.x(), reopened.y()) == (90, 110) + + +def test_a_dock_layout_saved_under_another_version_is_not_restored(qapp): + """Qt drops a mismatched state, which is what makes a version bump retire it.""" + w = _Docked() + w.dock.setFloating(True) + w.close() + + reopened = _Docked() + reopened.dock_state_version = _Docked.dock_state_version + 1 + state = QSettings("DashPVA", "_Docked").value("dock_state") + assert state, "precondition: a layout was saved" + assert reopened.restoreState(state, reopened.dock_state_version) is False + + +def test_the_dock_state_version_is_per_viewer(): + """One viewer raising its version must not retire another viewer's layout.""" + class _Other(BaseWindow): + dock_state_version = BaseWindow.dock_state_version + 5 + + assert _Other.dock_state_version != BaseWindow.dock_state_version + assert _Docked.dock_state_version == BaseWindow.dock_state_version + + +def test_opting_out_writes_nothing(qapp): + class _NoPersist(_Docked): + persist_state = False + + w = _NoPersist() + w.resize(600, 400) + w.close() + + assert QSettings("DashPVA", "_NoPersist").allKeys() == []