Repository navigation
fix(gui): the Windows main window keeps its size and position across Frameless toggles and restarts (#6303). Principle VIII. - #6310
Conversation
…ss Window on (aethersdr#6303). Principle VIII. On Windows with Qt's expanded client area (Frameless Window on), Qt 6.12 doesn't take its frame margins from the real non-client area. It subtracts a fixed border from the system margins (qwindowswindow.cpp:3069), and stops recomputing them once the window rect and the client rect are the same size (:3061). restoreGeometry() rebuilds the window from the blob's frame rect using those margins, so every launch came back a different size. Measured on Windows 10 at 85% UI scale: the saved blob had a normal rect of 1407x400 but a frame rect of 1389x391, and the relaunch came back 1424 wide (17 px more). reapplyStartupGeometryAfterShow() now puts back the blob's normal geometry, which is the client rect saveGeometry() recorded, right after restoreGeometry(). It only does this on Windows, in the expanded mode, in the normal window state. The off-screen clamp after it is unchanged. WindowChrome::savedNormalGeometry() reads that rect from the blob, and unified_title_bar_test covers it with a blob whose frame and normal rects differ. The issue also reports drift on each Frameless toggle. That comes from MainWindow::setFramelessWindow() reapplying Qt's geometry() across the flag change; geometry() is wrong by the same margin error. aethersdr#6269 fixes it on Windows 10, where the window rect then equals the client rect; toggles measured exact on that build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… not Qt's frame margins (aethersdr#6303). Principle VIII. The first attempt put back the saveGeometry() blob's normal rect after startup. On the CI build (Windows 10, 85% UI scale, Frameless Window on), that held the position, but the client still grew by 6 physical px each launch: 1210, 1216, 1222, 1228. Under the expanded client area, Qt's geometry() itself reads back 7 logical px wider than the setGeometry() that produced it, so the rect being saved was already wrong. The main window now saves and restores its client rect in physical pixels, read and set through Windows: - closeEvent() stores GetClientRect/ClientToScreen with the window's device pixel ratio (MainWindowNativeClientRect), in the expanded mode and the normal window state only. - reapplyStartupGeometryAfterShow() puts it back with SetWindowPos, sizing the window from the frame it has now, when the pixel ratio matches. A UI or display scale change falls back to Qt's restore. - setFramelessWindow() keeps the same client rect across the flag change, so the toggle no longer depends on aethersdr#6269's Windows 10 frame change and works the same on Windows 11. WindowChrome::windowRectForClient() and the format/parse pair are pure and covered in unified_title_bar_test. Mutating the caption term out of the margin math, or the pixel-ratio check out of the parser, fails it. The blob parser from the first attempt is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for this — the root-cause write-up is the best part of the PR. qwindowswindow.cpp:3057-3062 short-circuiting the margin recompute once window rect == client rect, plus the qRound(scaleFactor) * 2 border subtraction, explains both halves of #6303 in a way the issue's table alone did not, and the four-launch table with the literal saved value 131,45,1234,411@0.85 is the kind of proof that makes a Windows-only fix reviewable from a Mac.
One thing I want settled before merge, and it is a one-line change either way. Everything else is a nit.
1. Issue fit
Solves it, for the main window. The issue's three symptoms map cleanly onto the three hunks: Off→On width (setFramelessWindow, MainWindow.cpp:9979-9994), On→Off position (same hunk — the client rect is captured before configure() and re-applied after show()), and restart drift (closeEvent :4099-4107 → reapplyStartupGeometryAfterShow :3876-3889). The frame is measured live from the HWND rather than assumed, so the fix does not depend on #6269's Windows 10 frame change, as the body claims. Physical pixels end to end, which is what the issue thread's item 4 asked for.
Not addressed, and worth naming in the body: the issue thread's item 5 (sibling top-levels — see nits).
2. Scope
| File | Change | Claimed? | Verdict |
|---|---|---|---|
src/gui/MainWindow.cpp |
nativeClientRect() / setNativeClientRect(), the startup re-apply, the closeEvent save, the toggle capture/apply |
Yes | In scope |
src/gui/MainWindow.h |
the two new private member declarations | Yes | In scope |
src/gui/WindowChrome.h |
windowRectForClient, formatNativeClientRect, parseNativeClientRect, 3 new includes |
Yes | In scope |
tests/unified_title_bar_test.cpp |
8 checks for the three new helpers | Yes | In scope |
4 files, +152/−0, no deletions, no - lines to read for a removed guard, no CHANGELOG entry, no settings migration, no protocol or capability surface. Everything in the diff is explained by the issue. Both commits SIGNED.
One scope item for the maintainer, not a violation: MainWindowNativeClientRect is a new loose flat AppSettings key (MainWindow.cpp:3883, :4103). docs/agents/settings.md:30-31 says "New configuration follows Constitution Principle V: one object per feature, not another loose flat key." Against that: window geometry is app-global, not radio-scoped, and its four existing siblings (MainWindowGeometry, MainWindowState, MinimalModeGeometry, FramelessWindow) are all flat keys, so folding this one into a document would leave it the odd one out. I read this as a maintainer call rather than a blocker — but the body skipped the PR template checklist, whose "No new flat-key AppSettings calls" box would have been the place this surfaced. Worth a line in the body either way.
3. Blockers
None confirmed. One item I could not settle headless, which is a blocker if it confirms and a style nit if it does not:
1. src/gui/MainWindow.cpp:3881, :4102, :9982 — the three new gates use strict equality on a flag set.
&& windowState() == Qt::WindowNoState) { const bool nativeRestorable = windowFlags().testFlag(Qt::ExpandedClientAreaHint)
&& windowState() == Qt::WindowNoState; const QRect nativeClient = wasVisible && windowState() == Qt::WindowNoState
? nativeClientRect() : QRect();QWidget::windowState() returns Qt::WindowStates, and Qt documents the result as an OR'ed combination of WindowMinimized, WindowMaximized, WindowFullScreen and Qt::WindowActive. If WindowActive (0x8) is present on a focused window, all three gates are false whenever the main window has keyboard focus — which is exactly the state it is in when a user clicks the X to quit, or clicks View → Frameless Window. closeEvent would then store the empty string, the toggle would capture QRect(), and the fix would be inert for every real user while still passing your proof: invoke "Frameless Window" trigger and a posted WM_CLOSE do not require the window to be foreground, so the bridge run may never have exercised the active case.
Supporting evidence from the tree: every pre-existing window-state test in this file and its siblings masks rather than compares — MainWindow.cpp:4119 !(windowState() & (Qt::WindowMaximized | Qt::WindowFullScreen)), :10147, :10204, MainWindow_Menus.cpp:149 and :162 windowState() & ~Qt::WindowMinimized, MainWindow.cpp:3983 windowState().testFlag(...). Your three new sites are the only strict-equality uses of windowState() in src/.
I cannot resolve this from here — no Windows, no Qt sources on this box, and the question is about Qt's runtime value, not about your code. One qDebug() << windowState() in closeEvent, quitting via the X button rather than the bridge, settles it. The masked form is correct whichever way it lands, so it is worth taking regardless — it matches :4119's shape and keeps "normal window" meaning the same thing in all four places:
const bool nativeRestorable = windowFlags().testFlag(Qt::ExpandedClientAreaHint)
&& !(windowState()
& (Qt::WindowMinimized | Qt::WindowMaximized | Qt::WindowFullScreen));4. Nits (non-blocking)
src/gui/WindowChrome.h:124-126— the parsed rect has no upper bound. Credit: security-audit (CodeGuard), confirmed against the code.const QRect client(n[0], n[1], n[2], n[3]); return client.width() > 0 && client.height() > 0 ? client : QRect();
toIntbounds each field toint, so"0,0,2147483647,2147483647@0.85"parses as valid, andwindowRectForClient'sclient.width() + left + right(:84) is then signed-overflow UB beforeSetWindowPossees it. A merely absurd value (100000×100000) does not overflow but does stick: the off-screen rescue at:3896-3913onlymove()s, never resizes. The input is the user's own settings file, so this is self-inflicted rather than a privilege crossing — hence a nit, not a gate. AqBound-style cap (32767, or the virtual-desktop bounds) inparseNativeClientRectwould close it, and a case would slot straight in beside the zero-size check attests/unified_title_bar_test.cpp:166-169.src/gui/WindowChrome.h:94-114— the DPR guard is asymmetric.formatNativeClientRectwrites the ratio with.arg(dpr, 0, 'g', 6), butparseNativeClientRectcompares the 6-significant-digit value against the live full-precision one withqFuzzyCompare, whose tolerance is relative 1e-12. Any ratio needing more than 6 significant digits — e.g. a Windows custom scale of 133% (128/96 = 1.3333333…) times an 85% UI scale — never matches its own saved value, so the restore silently never fires and the window drifts as on main. The reporter's 0.85 and the common 125/150/175% steps are all exact at 6 digits, so this is an edge, not the main path. Comparing the formatted strings, or widening to'g', 17, makes the guard symmetric by construction.- Sibling top-levels still drift.
setFramelessWindowpropagates the same chrome change tom_panStack, the container manager,m_connPanel, the trackedPersistentDialogs andm_titleBar's child dialogs (MainWindow.cpp:9996-10010), andPanFloatingWindow/FloatingContainerWindow/CanonWindow/ theStrip*andClient*editors each save their ownsaveGeometry()blob through the same wrong margins. #6303 reports only the main window and the PR title says so, so this is completeness rather than a defect — but the body reads as if the drift class is closed. Worth one sentence plus a follow-up issue. Same forFullModeGeometry: a user who quits in Minimal Mode gets the minimal client rect restored correctly, then toggling back to full mode restoresFullModeGeometrythrough Qt and lands on the drifted size. closeEventclears the key when the gate fails (:4103-4107). Quitting maximized discards a perfectly good saved normal client rect, so the first un-maximize after that launch comes back at Qt's drifted size. Keeping the previous value instead of writingQString()would be strictly better, and costs a read.unified_title_bar_testis not in anyci.yml-Rfilter (grep over.github/workflows/*.ymlreturns nothing for it), so the 8 new checks compile in PR CI but only run in the full suite. Nothing to fix — just so the green badge is not read as "the new checks passed."- Your local
"four radios fit whole in an 1800 px bar"failure is almost certainly not yours: that binary's fit checks are font-metric dependent and go red on hosts whose default sans differs from the CI image's. Your "fails with and without the change, and on main plus theme-only edits" reads correctly to me.
Socket tests: none. The added checks are pure-function calls on WindowChrome helpers — no QTcpServer/QUdpSocket/QLocalServer, no bind/listen/connectToHost, no peer process, no Fake* device class.
Test layer: right call, as far as it goes. The three helpers are pure value transforms and belong in a socket-free CTest, which is where you put them. The Win32 plumbing (GetClientRect/SetWindowPos, the two gates, the ordering against applyWindowsCaptionStyles) has no deterministic socket-free seam — it needs a real HWND on a real display — so bridge evidence is the honest ceiling and you supplied it. I am not asking for a fake-window harness.
5. What I tried to break, and could not
windowRectForClientagainst its own tests. Checks 1 and 3 both still pass if the body is replaced withreturn windowNow;— but check 2 (a different client rect,QRect(300,200,1000,300)→QRect(292,200,1016,308)) discriminates, and so does the mutation you report (dropping the caption term). The margin maths is right for negativex/ytoo, which multi-monitor setups need, and the three cases cover both the uneven expanded frame (L8/T0/R8/B8) and system decorations (31 px caption). The tests are not asserting the implementation back to itself.- Double application of the saved blob. The constructor's
restoreGeometry()(:2592) and the post-show one (:3876) both still run, which the issue thread flagged as suspect. It does not accumulate here: the blob's +7 px error is a constant function of the saved bytes, not a function of the current geometry, and theSetWindowPosat:3820runs last and overwrites both. I could not construct an ordering where the Qt restore wins. - Ordering around the HWND re-creation.
WindowChrome::configurere-creates the native window, so I checked thatwinId()insetNativeClientRectcannot be the stale HWND: it is called aftershow(), andshowEvent→applyWindowsCaptionStyles()(SetWindowLongPtr+SWP_FRAMECHANGED,:3776-3795) runs synchronously inside thatshow(), so the frame is already in its final shape whenGetWindowRectmeasures it. The capture-before / apply-after split is the right way round. - The off-screen rescue.
setNativeClientRectruns before the frame-center screen test (:3891-3913), so a saved rect on a now-disconnected monitor is still caught and re-centred — it loses the size correction on that path, but it does not leave the window invisible. - Degradation paths. DPR mismatch, empty key, malformed text, zero-size rect,
GetClientRectfailing,winId()null, maximized or minimized at close — every one of them falls back to Qt's existing restore rather than to a worse state. No new silent-failure mode that leaves the window somewhere it was not before. - Platform containment. Every behaviour change is inside
#ifdef Q_OS_WIN; the three newWindowChrome.hhelpers are unguarded inline functions, which is what lets the test compile on Linux and macOS.check-windowsis the only lane that compiles theQ_OS_WINarm and it is green onc12b1ec, along withbuild,check-macos,Static checksand the sanitizer configure. - CodeGuard's 63 findings. Walked them with the security-audit subagent. The
CS-SEC-PASSWORDhit isQrzClient.cpp:38m_password = password;— parameter plumbing, no literal. TheCS-INJ-role_overridehit isMainWindow.cpp:11523, a comment reading// CNN override: if carrierScore > 0.70, …about the QRM classifier; the homoglyph signal is the≥/≤in the adjacent comment block. EveryCMDI-AMPis the C++&&operator and everyCMDI-BACKTICKis a backtick inside a//comment; the only process launch in the file is list-formQProcess::startDetachedwith no shell. All pre-existing and outside this diff. Only the unbounded-rect lead above survived, and it is the one item that touches PR-introduced code.
All of the above is reasoned from the code in the head checkout. I have no Windows machine, no build and no way to drive the GUI, so I did not reproduce any of it at runtime — the empirical half of this PR rests on your bridge tables, which I read as internally consistent.
6. Recommendation
Request changes — pending one verification. The approach is right (physical pixels, frame measured live, graceful fallback, Windows-only), the evidence is real, and the scope is tight. The only thing standing between this and an approval is the windowState() question in blocker candidate 1: if Qt::WindowActive is in that flag set, the fix is inert for exactly the user-driven close and user-driven toggle the issue reports, and your proof would not have caught it. Take the masked form at all three sites — it is correct either way and matches :4119 — and confirm with one user-initiated close. If it comes back clean, this is approve-with-nits, and the nits can all follow later. The flat-key question is a maintainer call, not mine.
🤖 aethersdr-agent · cost: $10.8472 · model: claude-opus-5
|
Cisco CodeGuard — static analysis of this PR (68 finding(s))
Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them. 🤖 aethersdr-agent · cost: $11.0309 · model: claude-opus-5 |
ten9876
left a comment
ten9876
left a comment
There was a problem hiding this comment.
Review — PR #6310 (head c12b1ec6)
Issue fit: Solves #6303 for the main window. All three symptoms in the issue map to a hunk. Off→On width and On→Off position are handled by setFramelessWindow() (src/gui/MainWindow.cpp:9979-9994), which captures the native client rect before configure() and puts it back after show(). Restart drift is handled by the closeEvent() save (:4099-4107) and the re-apply in reapplyStartupGeometryAfterShow() (:3876-3889). The frame is measured live from the HWND every time, not assumed, so the fix doesn't depend on #6269's Windows 10 frame change. Every behaviour change sits inside #ifdef Q_OS_WIN, so Linux and macOS run exactly the code they ran before. The pure helpers in WindowChrome.h compile everywhere and are called only from Windows code and the test. I found no blockers.
Scope
| File | Change | Claimed in body? | Verdict |
|---|---|---|---|
src/gui/MainWindow.cpp |
nativeClientRect() / setNativeClientRect(), the startup re-apply, the closeEvent save, the toggle capture/apply |
Yes | In scope |
src/gui/MainWindow.h |
Two private members, inside the existing #ifdef Q_OS_WIN block |
Yes | In scope |
src/gui/WindowChrome.h |
windowRectForClient, formatNativeClientRect, parseNativeClientRect, plus 3 Qt includes |
Yes | In scope |
tests/unified_title_bar_test.cpp |
8 checks on the three helpers | Yes | In scope |
The issue explains everything in the diff. There are no - lines, no CHANGELOG entry, and no protocol, capability or migration surface. Both commits are on-topic and dated from this PR.
One item for the maintainer (@ten9876), not a violation: MainWindowNativeClientRect is a new flat AppSettings key (src/gui/MainWindow.cpp:3883, :4103). docs/agents/settings.md:30-31 says: "New configuration follows Constitution Principle V: one object per feature, not another loose flat key." On the other side, this is app-global window state rather than configuration, and its siblings (MainWindowGeometry, MainWindowState, MinimalModeGeometry) are all flat. Folding only this one into a document would make it the odd one out. Please rule on it rather than let it pass silently.
Blockers
None.
Nits (non-blocking)
-
src/gui/WindowChrome.h:97-99— the saved scale is written at 6 significant digits but compared at full precision.formatNativeClientRectwrites.arg(dpr, 0, 'g', 6).parseNativeClientRect(:109) then compares that rounded value with the live ratio usingqFuzzyCompare, whose relative tolerance is about 1e-12. Any ratio that needs more than 6 digits never matches its own saved value, so the restore silently does nothing and the window drifts as it does on main. One example is a Windows custom scale whose DPI isn't a multiple of 24, such as 110 DPI: 110/96 × 0.85 = 0.97395833…, saved as0.973958. The standard 25%-step scales times the two-decimalQT_SCALE_FACTOR(src/main.cpp:355) have at most 5 significant digits, so the reporter's setup and the common setups are fine. This is an edge case, not the main path. Writing 17 digits makes the round trip exact, because close and launch compute the ratio the same way:return QStringLiteral("%1,%2,%3,%4@%5") .arg(client.x()).arg(client.y()).arg(client.width()).arg(client.height()) .arg(dpr, 0, 'g', 17);
The bot review raised the same point; I agree with it.
-
src/gui/WindowChrome.h:124-125— the parsed rect has no upper bound."0,0,2147483647,2147483647@0.85"parses as valid.windowRectForClient'sclient.width() + left + right(:87) then overflows a signed int. A merely huge value doesn't overflow, but it does stick, because the off-screen rescue atMainWindow.cpp:3896-3913only moves the window and never resizes it. The value comes from the user's own settings, so this is self-inflicted. A cap of 32767 per dimension, the Win32 coordinate limit, would close it. -
tests/unified_title_bar_test.cpp:140-169— two of the new checks can't fail for the reason they name. I mutation-tested the helpers on Linux; details are below.- With
rightreplaced byleftinwindowRectForClient, every check still passed. All three fixtures have left == right (8/8), so "keeps the expanded frame's uneven borders" only covers top ≠ bottom. A fixture with different left and right borders would pin it. - The
width() > 0 && height() > 0guard at:125is redundant:QRect(1,2,0,4).isValid()is already false. The "an empty rect gives no rect" check therefore passes with the guard removed. Either drop the guard or leave it as belt and braces; the test just doesn't prove it either way.
- With
-
src/gui/MainWindow.cpp:4103-4107— quitting maximized, minimized or non-expanded overwrites a good saved rect withQString(). After a maximized quit, the first un-maximize on the next launch comes from Qt's drifted normal geometry. This is no worse than main. Leaving the previous value in place would be better, but it would also need a guard against that value going stale once Frameless is turned off. -
Sibling top-levels still use Qt's margins.
FullModeGeometry(the Minimal→Full toggle), the floating pan and container windows, and thePersistentDialogs all still round-tripsaveGeometry()through the same wrong expanded-frame margins. #6303 and the title only claim the main window, so this is about completeness, not a defect. A sentence in the body and a follow-up issue would keep "drift fixed" from reading as broader than it is. -
Merge order with #6269. Both PRs add code right after
applyWindowsCaptionStyles()(MainWindow.cpp:~3796) and atMainWindow.h:352.git merge-treeof the two heads givesCONFLICT (content)in both files. The conflict is textual and trivial, but whichever PR lands second needs a rebase. The body says this PR works with or without #6269, which matches the code because the frame is measured live.
Correcting the bot review
windowState() == Qt::WindowNoStateandQt::WindowActive: in Qt 6.12, a widget's state only carriesWindowActivewhen someone passes it toQWidget::setWindowState().qwidgetwindow.cpp:1135says "QWindow does currently not know 'active'", andhandleWindowStateChangedEventonly carries over aWindowActivethat was already there (:1147). Nothing insrc/setsQt::WindowActive;grep -rn WindowActive srcfinds only an unrelated automation member. The author's saved value131,45,1234,411@0.85also proves the close-time gate was open. The masked form!(windowState() & (Min|Max|FullScreen))would match:4119and is nicer for consistency, but it isn't a blocker.- "
unified_title_bar_testis not in anyci.yml -Rfilter": that is how the project runs tests. New tests register intests/tests.cmake(done,:7410-7436) and run in the sanitizer lanes, not inci.yml. It is not a coverage gap.
What I verified vs. read
-
Built and ran
unified_title_bar_testatc12b1ec6on Linux (Qt 6.12, offscreen): passes, including the "four radios fit whole in an 1800 px bar" check that fails in the author's local Windows run. That supports the author's reading that their failure comes from local font metrics. -
Mutation-tested the helpers:
- Removing the caption (
top) term fails "the caption height goes above the client rect". - Removing the DPR comparison fails "a different scale falls back to Qt's restore".
right→leftand removing the zero-size guard both survive (nit 3).- The tree was restored and the test passed again afterwards.
- Removing the caption (
-
Engine boundary:
check_engine_boundary.pyover the three touched sources gives identical findings on the merge base and the head (12 tracked legacy, 0 new). The only includes added areQRect/QString/QStringList. -
Socket preflight: no socket-owning or fake-peer test was added or modified. The new checks are pure function calls.
-
CI freshness: the checks are green, but they ran on a merge created 03:12Z.
mainhas moved since then (4f16919, 04:34Z). None of the three new commits touch these files, andgit merge-treeagainst currentmainis clean. -
Reasoned from code only (not testable here): all the Win32 behaviour. I tried two attacks against the code:
- Is the frame settled when
setNativeClientRectmeasures it after the toggle? Yes.show()→showEvent→applyWindowsCaptionStyles()runsSWP_FRAMECHANGEDsynchronously first. - Does a missing saved monitor strand the window? No. The re-apply runs before the existing off-screen centre check, so a rect restored onto a disconnected monitor at a matching DPR is still pulled back on-screen.
Not covered by any review here: Windows 11, mixed-DPI and multi-monitor setups, and the maximized/full-screen paths. The body discloses all of these.
- Is the frame settled when
-
Did not drive the app through the automation bridge. Every change is
#ifdef Q_OS_WINand this reviewer is on Linux, so a demo session would only exercise unchanged code. The author's Windows bridge tables are the runtime evidence, and they are consistent with the code.
…ometry document (aethersdr#6303). Principle V. The native client rect is saved as one versioned JSON object under MainWindowNativeGeometry, one entry per window role, rather than a loose flat key. Besides the launch rect ("main") it now covers the Minimal Mode round trip: FullModeGeometry and MinimalModeGeometry go through restoreGeometry() and drift by the same expanded-frame margin error, so each is paired with its native client rect ("fullMode", "minimalMode"). Only MainWindow takes ExpandedClientAreaHint, so no other window drifts. - The scale is stored as a JSON double, which round-trips at full precision; a 110 DPI display under a 0.85 UI scale now matches. - Saved values outside the 16-bit Win32 coordinate range, fractional pixels, a non-numeric field or another schema version give no rect. - Closing maximized keeps the last normal rect instead of erasing it. - "Normal window" masks the minimized/maximized/full-screen bits, as the Minimal Mode geometry checks beside it do, rather than comparing the whole state for equality. - Tests: an asymmetric left/right frame, out-of-range, fractional, newer-schema and full-precision cases, each failing when its guard is removed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
46defc2 to
7004082
Compare
ten9876
left a comment
ten9876
left a comment
There was a problem hiding this comment.
Approving at 700408230, after a landing pass on top of the two author commits. Maintainer decisions: fold the native rect into a feature document, and fold in the remaining drift surfaces.
Fixed in 700408230
- Flat key → one feature document (Principle V;
docs/agents/settings.md:30-31).MainWindowNativeClientRectis replaced by one versioned JSON object,MainWindowNativeGeometry, with entriesmain/fullMode/minimalMode(src/gui/WindowChrome.hwithNativeClientRect()/savedNativeClientRect(),kNativeGeometrySchemaVersion = 1). The key never shipped, so no migration is needed. - The Minimal Mode round trip drifted the same way.
FullModeGeometryandMinimalModeGeometryboth go throughrestoreGeometry()and carry the same margin error, so each is now paired with its native rect, saved and re-applied intoggleMinimalMode()andcloseEvent()(MainWindow.cpp, thesaveNativeClientRect()/restoreNativeClientRect()call sites). - Scale precision (my review's nit, and the bot's): the ratio is now a JSON double, so it round-trips at full precision by construction. Test: "the scale survives storage at full precision" (110/96 × 0.85).
- No upper bound (the bot's / CodeGuard's nit): values outside ±32767 (the 16-bit Win32 coordinate range), fractional pixels, non-numeric fields and other schema versions give no rect. This also rules out the signed overflow in
windowRectForClient. - Tests that could not fail. Added an asymmetric frame (left 3 px, right 11 px). The redundant zero-size guard is gone because
QRectvalidity covers it; the new range, fractional and schema checks are the ones that discriminate. - A maximized close no longer erases a good saved rect (
closeEvent: savesmainonly when restorable). windowState()strict equality (the bot's potential blocker): replaced with the masked minimized/maximized/full-screen test that the neighbouring Minimal Mode checks use. On Qt 6.12 it was not live:QWidget's state only gainsWindowActivethrough an explicitsetWindowState()(qwidgetwindow.cpp:1135), andsrc/never makes that call. The masked form is still the house shape.
Refuted / no change
- Sibling top-levels "still drift" (the bot's nit 3): only
MainWindowtakesExpandedClientAreaHint(grep -rn ExpandedClientAreaHint src/guifindsMainWindow.cpp,WindowChrome.h,TitleBar.cpp,mac/).PanFloatingWindow::setFramelessMode,PersistentDialog::setFramelessMode,CanonWindowand the container windows switchQt::FramelessWindowHint. Under that hint Qt's frame margins are real, and the #6303 mechanism (qwindowswindow.cpp:3057-3081) does not apply to them. - "Not in a
ci.ymlfilter": the sanitizer sweep runs the unfiltered tree, so this is not a coverage gap. - CodeGuard's 68 findings: everything outside the unbounded-rect lead (item 4) is pre-existing and outside this diff (
&&operators, backticks in comments, list-formQProcess), as the bot's own triage also concluded.
Verified
unified_title_bar_testpasses on Linux (Qt 6.12, offscreen, lean build at700408230).- Mutations, each turning the test red, file restored afterwards: swapping right for left, dropping the range bound, storing the ratio as a float, dropping the schema check, dropping the integer check.
- Static gates: the engine-boundary findings for
MainWindow/WindowChromeare identical on the merge base and the head.gen_touchpoint_manifest --check,check_capability_records,check_command_planeandcheck_qsettingsare clean. check-windowsis the only compile proof of theQ_OS_WINarms, and it is green on this head.
Not verified
- The Win32 paths were not run on Windows after the document change: the Minimal Mode round trip, the document storage, and Windows 11 / mixed DPI. The author's Windows 10 bridge tables cover the main-window launch and toggle logic, which the document change only re-routes.
- I did not drive the app through the bridge. All behaviour is inside
#ifdef Q_OS_WIN, so a Linux demo session exercises none of it. - The automated
/code-reviewpass was not run in this landing pass; the equivalent pass was done by hand.
…nded chrome (#6266). Principle VIII. (#6269) ## Summary Fixes #6266. On Windows 10 with Frameless Window on, a light 1–2 px strip ran along the left, right and bottom edges of the main window. It started with #6198. **Cause:** under `Qt::ExpandedClientAreaHint`, Qt 6.12's `QWindowsGeometryHint::handleCalculateSize` takes over the top caption, but keeps a resize-border inset on the left, right and bottom as non-client. Windows 11 draws that area invisibly. Windows 10 paints it as a flat light strip, whatever the OS colour scheme or accent settings are. **Changes:** - **`MainWindow::nativeEvent` (Windows 10 only):** answers `WM_NCCALCSIZE` with the whole window as client area when `WindowChrome::claimsWholeWindowAsClient()` holds — `ExpandedClientAreaHint` set and the OS older than Windows 11 (build 22000). While maximized (not full screen), it insets the sides by `SM_CXSIZEFRAME + SM_CXPADDEDBORDER` and the top and bottom by `SM_CYSIZEFRAME + SM_CXPADDEDBORDER`, at the window's DPI, so nothing sits past the screen edge. No `WM_NCHITTEST` code is added: Qt's `handleNonClientHitTest` already returns `HTLEFT`/`HTRIGHT`/`HTBOTTOM`/corners from the client edges, which now are the window edges. - **`MainWindow::applyWindowsFrameColor` (Windows 11 only):** sets `DWMWA_BORDER_COLOR` to `color.background.app` on show and on every `themeChanged`, with Frameless Window on **or off**, so the DWM border follows the theme. `dwmapi` is linked on Windows only. - **Window geometry:** the native client-rect save/restore from #6303 (#6310) runs after this handler has taken effect. `applyWindowsCaptionStyles()`'s `SWP_FRAMECHANGED` in `showEvent` routes `WM_NCCALCSIZE` through it before `setFramelessWindow()` and the startup / minimal-mode re-apply measure the frame, so on Windows 10 the measured frame is zero and a saved client rect is restored as the window rect exactly. A comment at that call names the dependency. - `docs/architecture/window-chrome.md` describes both, and that edge resize on this path rests on Qt 6.12's hit-test alone. macOS and Linux are untouched. On Windows 10, Frameless Window off is untouched. Snap Layouts (#6224) is not addressed. ## Constitution principle honored Principle VIII: the window edge is continuous with the app's chrome on all four sides. ## Test plan - [x] Windows 10 22H2 (19045), FLEX-6600, dark theme, Frameless Window on: the strip is gone on the left, right and bottom (bridge luminance readback in the PR thread). - [x] Windows 10: resize from all four edges and all four corners; maximize and restore; Win+Up and Aero Snap (checked by hand). - [x] Windows 10: Frameless Window off is unchanged (checked by hand). - [x] `unified_title_bar_test` (any host): the engage predicate is on for Windows 10 under the expanded chrome, off for Windows 11 build 22000 and off for Frameless off; a zero frame restores a client rect as the window rect. Both predicate checks fail when the predicate is mutated. - [ ] Windows 10, after the rebase onto #6310: window position and size across Frameless off → on → off, three relaunch cycles, a maximized relaunch then un-maximize, and View → Minimal Mode twice. - [ ] Windows 11: border follows `color.background.app` and updates when the theme changes. - [ ] CI (`check-windows` is the compile proof for the Windows-only lines). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: CJ Johnson <chibondking@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Jeremy [KK7GWY] <kk7gwy@aethersdr.com>
Fixes #6303.
What
On Windows with Frameless Window on, the main window's size and position drift on every Frameless toggle and on every restart. Each change is saved, so the drift builds up. Measured on main (Windows 10, 85% UI scale): every restart makes the client area 6 physical px wider and moves it 3–6 px left and 3 px down.
Root cause
With
Qt::ExpandedClientAreaHint, Qt 6.12's Windows plugin doesn't take its frame margins from the real non-client area.qwindowswindow.cpp:3069-3081subtracts a fixedqRound(scaleFactor) * 2border from the system margins and guesses the top margin from what's left over. At:3057-3062it stops recomputing them once the window rect and client rect are the same size. The real expanded frame is L8 T0 R8 B8, so Qt's bookkeeping is off:geometry()reads back 7 logical px wider than the client Windows reports, even right after asetGeometry()produced that client.restoreGeometry()rebuilds the size from the saved blob's frame rect. One saved blob had a normal rect of 1407×400 but a frame rect of 1389×391, and it relaunched 1424 wide.MainWindow::setFramelessWindow()(savegeometry(), re-create,setGeometry()) and the startup restore both round-trip through those numbers.How
On Windows only, every behaviour change is inside
#ifdef Q_OS_WIN. The main window keeps its client rect in physical pixels, read and set through Windows, instead of through Qt's frame margins. The rects live in one versioned feature document (Principle V),MainWindowNativeGeometry, with one entry per window role:{"schemaVersion":1, "main": {"x":131,"y":45,"width":1234,"height":411,"dpr":0.85}, "fullMode": {...}, "minimalMode": {...}}closeEvent()storesGetClientRect/ClientToScreenand the window's device pixel ratio asmain, and asminimalModewhen quitting in Minimal Mode. Only in the expanded mode and a normal (not minimized/maximized/full-screen) window; a maximized close keeps the last normal rect.reapplyStartupGeometryAfterShow(), after the existingrestoreGeometry(), putsmainback withSetWindowPos. The window is sized from the frame it has now, and only when the pixel ratio matches. A UI or display scale change falls back to Qt's restore, as today. The off-screen clamp after it is unchanged.FullModeGeometryandMinimalModeGeometrygo throughrestoreGeometry()and drift by the same margin error, so each is paired with its native rect (fullMode,minimalMode), saved and re-applied beside it intoggleMinimalMode().setFramelessWindow()keeps the same client rect across the flag change. The toggle no longer depends on fix(gui): no light strip on the Windows 10 frame edges under the expanded chrome (#6266). Principle VIII. #6269's Windows 10 frame change, and it works the same on Windows 11, which fix(gui): no light strip on the Windows 10 frame edges under the expanded chrome (#6266). Principle VIII. #6269 doesn't touch.A saved entry is ignored — and Qt's restore runs — when it is missing, from another schema version, at another pixel ratio, fractional, non-numeric, or outside the 16-bit Win32 coordinate range. The ratio is a JSON double, so it round-trips at full precision.
Only the main window takes
ExpandedClientAreaHint. Pop-out pans, floating containers and dialogs switchQt::FramelessWindowHint, whose frame margins are real, so they don't have this drift and are unchanged.WindowChrome::windowRectForClient(),withNativeClientRect()andsavedNativeClientRect()are pure helpers; the HWND calls and the document read/write are smallMainWindowmembers.Testing
Proof with the agent automation bridge (Windows 10 22H2, Intel HD 520, 85% UI scale, FLEX-6600 connected)
Builds came from the fork's Windows Installer workflow (Qt 6.12). Frameless was switched with the bridge (
invoke "Frameless Window" trigger). Restarts were real closes:WM_CLOSEto the main window, socloseEvent()saves as it does for a user. Every rect is read from Windows (GetWindowRect,GetClientRect+ClientToScreen, DWM extended frame bounds), never from Qt.Restart with Frameless on (client rect, physical px):
65b72a65(installed 26.10.1)cb8712dac12b1ec6On this PR's build the saved value was
131,45,1234,411@0.85, which matches what came back.Frameless toggle (client rect, physical px):
geometry()now 7 px wider)Unit tests (local, Qt 6.12.0 pinned, MSVC 2022, offscreen)
unified_title_bar_testchecks forwindowRectForClient(expanded frame, uneven borders, caption, asymmetric left/right) and the document helpers (round trip through compact JSON, other role, scale mismatch, full-precision scale, nothing saved, out of range, malformed, fractional, newer schema, empty) all pass on Linux (Qt 6.12, offscreen).main(888ac58b, run 37865594009) is green, so this looks like a local font or metrics difference, not this PR.Not verified
check-windowsand were not re-driven on Windows after the document change.Other platforms
macOS and Linux compile the same code as before. The pure helpers in
WindowChrome.hcompile everywhere but are only called from Windows code and the test.Related
🤖 Generated with Claude Code