Keep the offsets when placing monitors from the system layout - #531
Merged
Conversation
Two monitors side by side with the second hanging 219px higher came out stacked a whole screen height apart. Two separate faults, and between them they are why the first layout a user is shown never matched what "Place from windows config" gave them a moment later. The offset was never computed. The placement rules only ever recognised exact equalities — edge against edge, or tops/bottoms matching to the pixel. A monitor adjacent on one axis but offset by an arbitrary amount on the other matched the adjacency rule and none of the alignment ones, so it kept whatever coordinate it happened to have and the compact pass shoved it somewhere. Measured on a real desktop: 219px up became 340mm up, when 219 * 340 / 1440 = 51.7mm was the answer. ForceCompact ran inside the walk. Every monitor not yet reached is still stacked at the origin, so compacting mid-walk resolves overlaps against screens that have no position yet, and pushes the ones just placed correctly out of the way. That is the whole of why running the placement twice gave a better answer than running it once — the second pass started from a layout already spread out, so the same compacts found nothing to do. The button was never better than the startup path; it just ran second. One compact at the end, which is what catches monitors with no adjacency at all. The new rule is the exact inverse of PixelLocationSolver: the physical midpoint of the shared span projects to the same pixel on both monitors. So "apply to system" and "place from system" now round-trip instead of drifting a little further apart on each pass, which is asserted on the real desktop — (0,0), (2560,-219), (5120,0) out and back unchanged. Taking that midpoint in pixel space is closed-form and tempting, and it round-trips perfectly until two monitors have very different pitches, which is the case this file exists for: a 1280x720 TV drifted 115px. In millimetre space, as the inverse takes it, the shared span depends on the position being solved for; it is settled by iterating from the pixel-space estimate, which is already the answer whenever the two spans cover each other. One existing assertion changed. SystemPlacement_SideBySidePixels wanted three monitors bottom-aligned while spanning identical pixel rows — their tops match exactly as much as their bottoms do, and a 920mm panel cannot have both. That arrangement did not survive a round trip either. The TV is now centred on what it shares with its neighbour.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two monitors side by side with the second hanging 219 px higher came out stacked a whole screen height apart. Two separate faults, and between them they are why the first layout a user is shown never matched what Place from windows config gave them a moment later.
Measured on a real desktop — two 1440p panels, the second 219 px up, and a 1280×720 TV:
219 × 340 / 1440 = 51.7is the answer.The offset was never computed
The placement rules only ever recognised exact equalities — edge against edge, or tops/bottoms matching to the pixel. A monitor adjacent on one axis but offset by an arbitrary amount on the other matched the adjacency rule and none of the alignment ones, so it kept whatever coordinate it happened to have and the compact pass shoved it somewhere.
ForceCompact ran inside the walk
Every monitor not yet reached is still stacked at the origin, so compacting mid-walk resolves overlaps against screens that have no position yet — and pushes the ones just placed correctly out of the way.
That is the whole of why running the placement twice gave a better answer than running it once: the second pass started from a layout already spread out, so the same compacts found nothing to do. The button was never better than the startup path; it just ran second. One compact at the end, which is what catches monitors with no adjacency at all.
The rule, and a wrong turn worth recording
The new perpendicular rule is the exact inverse of
PixelLocationSolver: the physical midpoint of the shared span projects to the same pixel on both monitors. So apply to system and place from system round-trip instead of drifting further apart on each pass — asserted on the real desktop:Taking that midpoint in pixel space is closed-form and tempting, and it round-trips perfectly until two monitors have very different pitches — which is the case this file exists for. The TV drifted 115 px. In millimetre space, as the inverse takes it, the shared span depends on the position being solved for; it is settled by iterating from the pixel-space estimate, which is already the answer whenever the two spans cover each other.
One existing assertion changed
SystemPlacement_SideBySidePixels_StaysSideBySideInMmwanted three monitors bottom-aligned while spanning identical pixel rows. Their tops match exactly as much as their bottoms do, and a 920 mm panel cannot have both — the choice was arbitrary, and that arrangement did not survive a round trip. The TV is now centred on what it shares with its neighbour.Tests
Six new (
PlaceFromSystemTests): the offset itself, placing twice landing where placing once did — on a simple row and on the real desktop — pixel order, contact, and the round trip. 111 in the DisplayLayout suite; 129 + 60 + 5 elsewhere, 56 Rust.