diff --git a/Furious/Backends/Xray/RoutingWindow.py b/Furious/Backends/Xray/RoutingWindow.py index 5564844..6b192df 100644 --- a/Furious/Backends/Xray/RoutingWindow.py +++ b/Furious/Backends/Xray/RoutingWindow.py @@ -1262,6 +1262,10 @@ class XrayRoutingWindow(AppQMainWindow): restored = False if not restored: + logger.warning( + 'saved routing-window geometry was invalid and was ignored' + ) + self.resize(self.DEFAULT_WINDOW_SIZE) savedState = AppSettings.get('UserRoutingWindowState') @@ -1283,5 +1287,11 @@ class XrayRoutingWindow(AppQMainWindow): def cleanup(self): """Release resources owned by the user routing window.""" + # Settings eagerly owns this reusable editor even when Edit Routing was + # never opened. Saving then would replace the user's geometry with Qt's + # tiny pre-show placeholder, so leave the persisted state untouched. + if not self.hasPreparedInitialGeometry(): + return + AppSettings.set('UserRoutingWindowGeometry', self.saveGeometry()) AppSettings.set('UserRoutingWindowState', self.saveState()) diff --git a/Furious/Qt/AGENTS.md b/Furious/Qt/AGENTS.md index b03394f..79790d6 100644 --- a/Furious/Qt/AGENTS.md +++ b/Furious/Qt/AGENTS.md @@ -32,6 +32,8 @@ Use the `manage-qt-pyside6-lifetimes` skill for Qt ownership or lifecycle work. - Top-level subclasses use the canonical first-show geometry hooks and centering/retention behavior; do not call overridable geometry hooks from constructors or alter private first-show state. +- Persistent top-level subclasses save geometry/state only after `hasPreparedInitialGeometry()` confirms first-show + preparation; never replace saved user geometry with a never-shown widget's native default. - Use `exec()` only when synchronous control flow is required; otherwise connect completion before managed `open()`. - Run focused behavior tests plus repeated native lifecycle cycles. For compiled-signal or transient-dialog changes, also run the Nuitka probe and verify destroyed signals, weak references, registries, callbacks, timers, replies, and diff --git a/Furious/Qt/QtWidgets.py b/Furious/Qt/QtWidgets.py index 7fe9d31..a9255ad 100644 --- a/Furious/Qt/QtWidgets.py +++ b/Furious/Qt/QtWidgets.py @@ -732,6 +732,12 @@ class AppQMainWindow( super().__init__(*args, **kwargs) self._lifetimeKey = object() + # Some reusable windows are composed eagerly but may never reach show(). + # Qt exposes only native placeholder geometry in that state (a parented + # Windows top-level can be as small as 100x30). Keep preparation as an + # explicit first-show fact so persistence code can preserve existing + # user settings instead of serializing that placeholder at shutdown. + self._initialGeometryPrepared = False self._initialPositionAuthoritative = False release = functools.partial( @@ -751,10 +757,26 @@ class AppQMainWindow( self.resize(self.DEFAULT_WINDOW_SIZE) def restoreInitialGeometry(self, geometry) -> bool: - """Restore saved geometry and mark its position as authoritative.""" + """Restore usable saved geometry and mark its position as authoritative.""" restored = bool(self.restoreGeometry(geometry)) if restored: + # restoreGeometry() validates the Qt payload, not whether the restored + # size is usable for this fully composed window. Geometry captured + # before first show can therefore be structurally valid while still + # describing only a native placeholder. Current Qt minimum constraints + # reject that state without imposing an arbitrary application size on + # intentionally compact, but usable, user geometry. + minimumSize = self.minimumSize().expandedTo(self.minimumSizeHint()) + + if self.size().expandedTo(minimumSize) == self.size(): + restored = True + else: + restored = False + + if restored: + # Only usable restored geometry owns the initial position. A rejected + # placeholder must remain eligible for normal first-show centering. self._initialPositionAuthoritative = True return restored @@ -763,10 +785,15 @@ class AppQMainWindow( """Return whether the first show should center this window.""" return self.CENTER_ON_INITIAL_SHOW and not self._initialPositionAuthoritative + def hasPreparedInitialGeometry(self) -> bool: + """Return whether first-show geometry preparation completed.""" + return self._initialGeometryPrepared + @callOnceOnly def _prepareInitialGeometry(self): """Prepare initial geometry once after subclass construction.""" self.prepareInitialGeometry() + self._initialGeometryPrepared = True @callOnceOnly def _centerOnInitialShow(self): diff --git a/Furious/Window/MainWindow.py b/Furious/Window/MainWindow.py index 7f7948d..42b285e 100644 --- a/Furious/Window/MainWindow.py +++ b/Furious/Window/MainWindow.py @@ -310,5 +310,11 @@ class MainWindow(AppQMainWindow): def cleanup(self): """Persist application-level window state.""" + # Tray startup can construct MainWindow without ever presenting it. In + # that case saveGeometry() describes Qt's native pre-show placeholder, + # not a user decision, so preserve the last persisted window state. + if not self.hasPreparedInitialGeometry(): + return + AppSettings.set('AppMainWindowGeometry', self.saveGeometry()) AppSettings.set('AppMainWindowState', self.saveState()) diff --git a/tests/test_main_window_geometry.py b/tests/test_main_window_geometry.py index da9767f..687322e 100644 --- a/tests/test_main_window_geometry.py +++ b/tests/test_main_window_geometry.py @@ -35,7 +35,7 @@ from Furious.Window.QRCodeWindow import QRCodeWindow from Furious.Window.TextEditorWindow import TextEditorWindow from PySide6 import QtCore -from PySide6.QtWidgets import QWidget +from PySide6.QtWidgets import QMainWindow, QWidget from tests.support import ( application, @@ -180,11 +180,13 @@ class AppQMainWindowLifecycleTest(unittest.TestCase): window = _LifecycleWindow() self.assertEqual(window.prepareCalls, 0) + self.assertFalse(window.hasPreparedInitialGeometry()) with patch('Furious.Qt.QtWidgets.moveToCenter') as moveToCenter: window.show() self.assertEqual(window.prepareCalls, 1) + self.assertTrue(window.hasPreparedInitialGeometry()) self.assertTrue(window.preparedAfterComposition) self.assertEqual(window.size(), window.DEFAULT_WINDOW_SIZE) moveToCenter.assert_called_once_with(window) @@ -693,6 +695,148 @@ class MainWindowGeometryTest(unittest.TestCase): window.close() window.deleteLater() + def testRoutingWindowUsesDefaultForMissingAndInvalidGeometry(self): + """Center the routing default only when no valid position was restored.""" + savedValues = ( + None, + QtCore.QByteArray(), + QtCore.QByteArray(b'broken'), + self._saveGeometry(QtCore.QRect(1, 22, 100, 30)), + ) + + for savedGeometry in savedValues: + with self.subTest(savedGeometry=savedGeometry), isolatedSettings(): + if savedGeometry is not None: + AppSettings.set('UserRoutingWindowGeometry', savedGeometry) + + window = XrayRoutingWindow() + + with patch('Furious.Qt.QtWidgets.moveToCenter') as moveToCenter: + window.show() + + self.assertEqual(window.size(), window.DEFAULT_WINDOW_SIZE) + moveToCenter.assert_called_once_with(window) + + window.close() + window.deleteLater() + + def testRoutingWindowKeepsValidSmallGeometryAndIgnoresInvalidState(self): + """Preserve intentional compact geometry independently from layout state.""" + with isolatedSettings(): + expected = QtCore.QRect(35, 45, 420, 260) + AppSettings.set( + 'UserRoutingWindowGeometry', + self._saveGeometry(expected), + ) + AppSettings.set( + 'UserRoutingWindowState', + QtCore.QByteArray(b'broken'), + ) + + window = XrayRoutingWindow() + + with patch('Furious.Qt.QtWidgets.moveToCenter') as moveToCenter: + window.show() + + self.assertEqual(window.size(), expected.size()) + moveToCenter.assert_not_called() + + window.close() + window.deleteLater() + + def testRoutingWindowCloseAndReopenPreservesLiveGeometry(self): + """Reuse one routing editor without rerunning first-show preparation.""" + with isolatedSettings(): + window = XrayRoutingWindow() + + with patch.object( + window, + 'prepareInitialGeometry', + wraps=window.prepareInitialGeometry, + ) as prepareInitialGeometry: + window.show() + window.setGeometry(73, 91, 760, 510) + expectedGeometry = QtCore.QRect(window.geometry()) + window.close() + window.show() + + self.assertEqual(window.geometry(), expectedGeometry) + prepareInitialGeometry.assert_called_once_with() + + window.close() + window.deleteLater() + + def testRoutingWindowFallbackSavesAndRestoresOnNextInstance(self): + """Replace failed restoration with a stable default for the next launch.""" + with isolatedSettings(): + AppSettings.set( + 'UserRoutingWindowGeometry', + QtCore.QByteArray(b'broken'), + ) + + firstWindow = XrayRoutingWindow() + firstWindow.show() + firstWindow.cleanup() + + restoredGeometry = AppSettings.get('UserRoutingWindowGeometry') + restoreProbe = QMainWindow() + self.assertTrue(restoreProbe.restoreGeometry(restoredGeometry)) + expectedSize = QtCore.QSize(restoreProbe.size()) + + secondWindow = XrayRoutingWindow() + + with patch('Furious.Qt.QtWidgets.moveToCenter') as moveToCenter: + secondWindow.show() + + self.assertEqual(secondWindow.size(), expectedSize) + moveToCenter.assert_not_called() + + restoreProbe.deleteLater() + firstWindow.close() + firstWindow.deleteLater() + secondWindow.close() + secondWindow.deleteLater() + + def testNeverShownPersistentWindowsDoNotOverwriteSavedGeometry(self): + """Keep eager hidden windows from persisting native pre-show defaults.""" + cases = ( + ( + _GeometryWindow, + 'AppMainWindowGeometry', + 'AppMainWindowState', + ), + ( + XrayRoutingWindow, + 'UserRoutingWindowGeometry', + 'UserRoutingWindowState', + ), + ) + + for windowType, geometryKey, stateKey in cases: + with self.subTest(windowType=windowType.__name__), isolatedSettings(): + geometry = self._saveGeometry(QtCore.QRect(50, 60, 820, 540)) + state = QtCore.QByteArray(b'preserve-existing-state') + AppSettings.set(geometryKey, geometry) + AppSettings.set(stateKey, state) + + parent = QWidget() + window = ( + windowType(parent=parent) + if windowType is XrayRoutingWindow + else windowType() + ) + + self.assertFalse(window.hasPreparedInitialGeometry()) + + window.cleanup() + + self.assertEqual(AppSettings.get(geometryKey), geometry) + self.assertEqual(AppSettings.get(stateKey), state) + + window.close() + window.deleteLater() + parent.deleteLater() + if __name__ == '__main__': unittest.main()