[fix] Frame-agnostic geometry save/restore for floating windows#2060
[fix] Frame-agnostic geometry save/restore for floating windows#2060chibondking wants to merge 1 commit into
Conversation
QWidget::saveGeometry() bakes in platform frame-extent data. When the decoration mode changes between save and restore (frameless ↔ decorated), the wrong frame size is applied and the window shifts by the title-bar height (~30 px on most Linux/Windows themes). Replace the opaque base64 blob with a plain "x,y,w,h" string read from geometry() (the client-area rect, always frame-agnostic) and restore with setGeometry() directly. No frame arithmetic needed; works identically in both modes. Legacy fallback: if the stored value is not four integers (i.e. it is a base64 blob from a build before this fix), the old restoreGeometry() path is used so existing saves survive the upgrade. Screen clamping preserved: if the saved top-left is not on any currently-attached screen the window is recentred on the primary screen. Affected: PanFloatingWindow, FloatingContainerWindow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Clean, well-scoped fix. The approach is sound — geometry() returns the client-area rect which is frame-agnostic by definition, so this eliminates the title-bar-height drift when toggling between frameless and decorated modes. The legacy base64 fallback is a nice touch for upgrade compatibility.
A few observations:
1. Duplicate parsing logic — consider a shared helper
The "x,y,w,h" parse → validate → screen-clamp pattern is repeated nearly identically in both files. A small free function (e.g. in a GeometryUtils.h or even just a file-local lambda) would reduce the duplication and make it easier to keep both paths in sync. Not blocking — just a suggestion for maintainability.
2. Minor inconsistency: toLatin1() vs toUtf8() for legacy decode
PanFloatingWindow uses QByteArray::fromBase64(val.toLatin1()) while FloatingContainerWindow uses QByteArray::fromBase64(val.toUtf8()). Since base64 is pure ASCII both are equivalent, but worth picking one for consistency.
3. Screen clamping difference between the two classes
PanFloatingWindow::restoreWindowGeometry() does its own screen clamping inline (moves the rect center to primary screen). FloatingContainerWindow::restoreAndEnsureVisible() relies on the existing post-restore clamp which checks topLeft() containment and re-centers. Both work, but the behavior differs slightly — one clamps by contains(topLeft), the other by contains(topLeft) then moveCenter. This is fine in practice, just noting it for awareness.
No bugs found. Files are within stated scope (PanFloatingWindow + FloatingContainerWindow only), uses AppSettings correctly, no null pointer risks, no resource leaks.
Thanks @chibondking — this is a solid fix, especially given the history around #2042 / #2008. The frame-agnostic serialization approach is the right long-term solution here.
|
Claude here on Jeremy's behalf — closing this one without merging. Pulled and tested on Linux (Wayland session). Saved geometry came back as Root cause: Wayland compositors don't expose absolute window coordinates to clients —
Tradeoff: keeping the original Your work on #2057, #2058, and #2059 was excellent — three of four sub-fixes from the original #2008 are landed and working. This last one is just incompatible with Wayland's design. If we ever need to address the frameless-toggle position jump, the path would be a hybrid: keep the binary blob for cross-WM compatibility, optionally store a separate frameless-state tag, and only fall back to plain coords when the toggle creates a known-stale blob. That's more complex than this PR and probably not worth the effort. Thanks for splitting #2008 carefully — the diagnosis here was straightforward only because each fix was reviewable in isolation. 73, Jeremy KK7GWY & Claude (AI dev partner) |
QWidget::saveGeometry() bakes in platform frame-extent data. When the decoration mode changes between save and restore (frameless ↔ decorated), the wrong frame size is applied and the window shifts by the title-bar height (~30 px on most Linux/Windows themes).
Replace the opaque base64 blob with a plain "x,y,w,h" string read from geometry() (the client-area rect, always frame-agnostic) and restore with setGeometry() directly. No frame arithmetic needed; works identically in both modes.
Legacy fallback: if the stored value is not four integers (i.e. it is a base64 blob from a build before this fix), the old restoreGeometry() path is used so existing saves survive the upgrade.
Screen clamping preserved: if the saved top-left is not on any currently-attached screen the window is recentred on the primary screen.
Affected: PanFloatingWindow, FloatingContainerWindow.