Keep a window's position when moving it to another display - #1863
anandghegde wants to merge 2 commits into
Conversation
|
Thanks! I pushed a branch, |
Moving a window to the next, previous or a specific display centered it on the destination, which loses the spot the user had put it in. DisplayTransfer works out each axis on its own from whether the window was against a screen edge: against both edges it follows both and spans the destination, against one it keeps its size and stays against that edge, against neither it keeps its size and its center keeps the same relative spot. The distance from an edge is carried over rather than flattened, so a window snapped with gaps arrives with the same gaps. This is now the default for next/previous and specific display moves. The old centering is available as centerOnDisplayChange, and attemptMatchOnNextPrevDisplay still replays the last Rectangle action where there is one. Closes rxhanson#1666
The test double overrides setFrame(_:adjustSizeFirst:), but AccessibilityElement.setFrame gained an adjustPosition parameter in "Add optional snapping animations and a blurred snap preview" (rxhanson#1849), so the override no longer matches and the test target does not compile: RectangleTests.swift:6153:23: error: method does not override any method from its superclass This is not related to the rest of the branch - it reproduces on a clean checkout - but the test target has to build before any of the tests can run.
d2d0663 to
0675d33
Compare
|
Thanks — that branch was exactly what was needed. I cherry-picked onto 1. The test target didn't compile, on a clean
2. With that out of the way, my change broke four existing tests — which I'd flagged as unverified and which turned out to be a real gap.
I rewrote the class to assert the same four #1723 geometries against
The first row is the one to look at if you're weighing defaults: under the old mapping a right-third window stays a right third, under this one it keeps its 1000pt width and becomes two thirds of a smaller display. That's the size-preserving trade-off from the PR description, made concrete. If you'd rather that case scaled, that's Full suite: 431 tests, 0 failures, including the 15 in |
Closes #1666.
Moving a window to the next, previous or a specific display centers it on the destination, which throws away the spot the user put it in. #1666 asks for that spot to be kept, and your comment on the issue sketched the behaviour:
The rule
DisplayTransferworks out each axis on its own, and both axes use the same rule: an edge that was against the source screen edge is put back against the matching destination screen edge. The three cases you called out fall out of that one rule rather than needing to be decided separately:Sizes only change where the window has to follow the screen edges. That is the part I went back and forth on, and I landed there because it is what dragging a window across displays yourself does, and because scaling everything proportionally shrinks a 400x300 utility window to 236x197 on the way from a 27" to a laptop. A window that is larger than the destination display is the one exception - it gets cut down to fit.
The distance from an edge is carried over rather than flattened to zero, which is what makes this work with gaps turned on: with a 10pt gap nothing is ever flush, so "against the edge" has to allow for the gap, and the same gap has to come out the other side.
edgeToleranceis4 + Defaults.gapSize.Defaults
This is now the default for
nextDisplay,previousDisplayand the specific display actions, and the old centering moved to a terminal command, as you asked:attemptMatchOnNextPrevDisplayis untouched in intent and still off by default: with a replayable action it replays that snap on the destination, which resizes the window to the new display instead of keeping its size. Its documentation needed rewriting though, since it can no longer be described as "preserve position instead of centering" - I retitled that section to say what it now does.A maximized window still goes through
autoMaximizerather than the transfer, so it stays recorded as maximized on the destination and can be restored from there.Relationship to #1809
#1809 added
relativePositionedRectfor #1723, the proportional mapping used whenattemptMatchOnNextPrevDisplayis on but there is no action to replay. This replaces it - the edge rule above covers the same case and handles maximized and gap-snapped windows, which proportional mapping gets wrong once it is the default rather than opt-in. Happy to put it back alongside if you would rather keep both.Tests
RectangleTests/DisplayTransferTests.swift, 15 cases covering the three branches, both directions between a 1512x945 built-in and a 2560x1415 external placed to the right and lower down (so a transfer that forgets to translate coordinates is caught), gaps, windows hanging off the edge, windows too large for the destination, identical displays, empty frames, and a sweep asserting the result always fits on the destination.What I could not verify
I could not build the app or run the test target.
maindoes not build here:AppDelegate.swift:381is insideif #available(macOS 27.0, *)and usesNSMenuItem.preferredImageVisibility, which is not in the macOS 26.4 SDK, so Xcode 26.4.1 is too old for this tree. I reproduced that on a pristinemainbefore writing anything, so it is not from this change, but it does mean the three edited files have not been through the compiler and the Xcode test target has not been run.What I did instead:
DisplayTransfer.swiftonly needs CoreGraphics plusDefaults.gapSize, so I compiled the real file in a throwaway SwiftPM package with a stub for that one symbol, and ranDisplayTransferTests.swiftagainst it verbatim apart from the@testable importline. All 15 pass. That covers the geometry, which is where the thinking is, but not the wiring inNextPrevDisplayCalculation/SpecificDisplayCalculation- those are small, but they are unverified and worth a look.Also unverified, for the same reason plus having one display here:
RectangleTests.swiftdepends on display moves centering.If it turns out you would rather this not change the default, it is a two line change to gate it the other way round.