From 615c0ed3b5df78d2f22d9cf31c0a9bf403391c6a Mon Sep 17 00:00:00 2001 From: Strycher Date: Mon, 20 Jul 2026 21:57:33 -0400 Subject: [PATCH] fix(#349): address Gemini review of window geometry Three findings from the adversarial review (standards#145), all real, all fixed: - Move-then-close inside the 400ms debounce lost the final position. The service now setPreventClose(true) and saves on onWindowClose before destroying the window, so the closing frame is always persisted. - _isReachable mixed a display's visible ORIGIN with its total SIZE when visibleSize was null, producing a rect offset from the real area. It now uses the visible pair when both are known, else the total pair. Negative visiblePosition (a monitor left of/above primary) is handled by intersect(). - The window listener and debounce timer are now released in onWindowClose. flutter analyze clean, dart format clean, 509 tests pass. --- lib/services/window_geometry_service.dart | 32 ++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/lib/services/window_geometry_service.dart b/lib/services/window_geometry_service.dart index a9023dd..1d76c78 100644 --- a/lib/services/window_geometry_service.dart +++ b/lib/services/window_geometry_service.dart @@ -45,6 +45,10 @@ class WindowGeometryService with WindowListener { await windowManager.focus(); }); windowManager.addListener(this); + // Intercept close so the final geometry is saved even if the user moves + // then closes inside the debounce window; onWindowClose does the save and + // then destroys the window (Gemini review). + await windowManager.setPreventClose(true); } Future _restore() async { @@ -88,10 +92,19 @@ class WindowGeometryService with WindowListener { Future _isReachable(Rect saved) async { final displays = await screenRetriever.getAllDisplays(); for (final d in displays) { - final origin = d.visiblePosition ?? Offset.zero; - final size = d.visibleSize ?? d.size; - final display = origin & size; // Rect from an Offset and a Size. + // Use the visible pair when BOTH are known, else the total pair. Mixing a + // visible origin with a total size makes a rect offset from the real + // area (Gemini review). visiblePosition can be negative for a monitor + // left of / above the primary, which intersect handles correctly. + final Rect display; + if (d.visiblePosition != null && d.visibleSize != null) { + display = d.visiblePosition! & d.visibleSize!; + } else { + display = Offset.zero & d.size; + } final overlap = saved.intersect(display); + // intersect() can return negative width/height when the rects miss; only + // a genuine overlap on both axes counts. if (overlap.width >= _minVisible && overlap.height >= _minVisible) { return true; } @@ -121,4 +134,17 @@ class WindowGeometryService with WindowListener { @override void onWindowResized() => _scheduleSave(); + + /// Fires because setPreventClose(true) intercepted the close. Save the final + /// geometry, release resources, then actually close the window. + @override + Future onWindowClose() async { + _saveDebounce?.cancel(); + try { + await _save(); + } finally { + windowManager.removeListener(this); + await windowManager.destroy(); + } + } }