From 317b7a09624649fc1e93c4a3b3eea905045db294 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 21 Aug 2026 23:06:50 -0400 Subject: [PATCH] Give a reconnected display the arrangement it had hypr/monitors.lua applies the stored per-output entries when the compositor reads its config, and never again. A monitor plugged in an hour later got the compositor's automatic placement instead of the position, scale and rotation this machine was told to use, and the only way back was to open Settings and apply it again. Docking should not cost you your desk. Deliberately not a confirmed transaction. applyLayout arms a fifteen second countdown because it is about to show you something you might not be able to undo; this restores a layout you already confirmed, on hardware you already had, and a countdown would be asking you to re-approve your own decision every time you sat down. It refuses rather than guesses when the stored mode is one the connected panel does not offer -- DP-1 on one dock is not DP-1 on another -- and when the surviving layout would name no primary. Both land on the compositor's automatic placement plus a toast that opens the Displays page, which is recoverable; silence would not be. That toast needed a new open-settings verb in StatusEvents, whose page name goes through ShellState's existing allow-list. The decision is split from the action as plannedRestore so it can be tested without driving a real compositor, and the harness sets topology and stored arrangement in one call because a real query landing between two would replace the fixture. Both fixtures travel base64: qs ipc call splits a JSON array of several objects into one argument per object, so a two-monitor fixture was arriving as an extra argument. --- config/dot/quickshell/displays-harness.qml | 26 ++++ config/dot/quickshell/services/Displays.qml | 125 +++++++++++++++++- .../dot/quickshell/services/StatusEvents.qml | 5 + tests/quickshell/displays-contract | 60 +++++++++ 4 files changed, 215 insertions(+), 1 deletion(-) diff --git a/config/dot/quickshell/displays-harness.qml b/config/dot/quickshell/displays-harness.qml index 9095478..37008bc 100644 --- a/config/dot/quickshell/displays-harness.qml +++ b/config/dot/quickshell/displays-harness.qml @@ -79,6 +79,32 @@ ShellRoot { Displays.parse(text, generation); } + // The restore-on-reconnect decision, without applying anything. The + // caller injects a topology with injectReadback first, sets the stored + // arrangement here, and reads back what Panama would do about it. + // Both arguments are base64. `qs ipc call` splits an argument that + // looks like a JSON array of several objects into one argument per + // object, so a two-monitor fixture arrives as two arguments and the + // call is rejected for arity. Encoding sidesteps the parsing entirely. + // + // Topology and stored arrangement are set in one call on purpose: a + // real compositor query landing between two calls would replace the + // injected topology, and the answer would be about this machine's + // actual monitor instead of the fixture. + function restorePlan(readbackB64: string, storedB64: string): string { + Displays.parse(Qt.atob(readbackB64), 0); + DesktopPreferences.set("displays", JSON.parse(Qt.atob(storedB64))); + const plan = Displays.plannedRestore(); + return JSON.stringify({ + action: plan.action, + layout: (plan.layout ?? []).map(record => ({ + name: record.name, mode: record.mode, scale: record.scale, + transform: record.transform, x: record.x, y: record.y, + primary: record.primary + })) + }); + } + function expireApplyVerification(): void { Displays.verificationTimedOut(); } diff --git a/config/dot/quickshell/services/Displays.qml b/config/dot/quickshell/services/Displays.qml index ef2fd2f..72b4eec 100644 --- a/config/dot/quickshell/services/Displays.qml +++ b/config/dot/quickshell/services/Displays.qml @@ -136,8 +136,131 @@ Singleton { // rollback then filters out any output that has disappeared. function reconcileTopology(): void { root.refresh(); - if (root.awaitingConfirmation) + if (root.awaitingConfirmation) { root.revertWithMessage("A display was connected or disconnected, so Panama restored the previous setting."); + return; + } + // Nothing in flight, so a display genuinely arrived or left. Give it + // back the arrangement it was last confirmed with -- see restoreStored. + restoreDebounce.restart(); + } + + // Docking and undocking should not cost you your arrangement. + // + // hypr/monitors.lua applies the stored per-output entries, but only when + // the compositor reads its config. A monitor plugged in an hour later gets + // the compositor's automatic placement instead of the position, scale and + // rotation this machine was told to use, and until now the only way to get + // them back was to open Settings and apply them again. + // + // This is deliberately NOT a confirmed transaction. applyLayout arms a + // fifteen-second countdown because it is about to show you something you + // might not be able to undo; this is restoring a layout you already + // confirmed, on hardware you already had, so a countdown would be asking + // you to re-approve your own decision every time you sat down at a desk. + // + // It refuses rather than guesses in two cases, because a wrong answer here + // is a screen you cannot see to fix: + // + // * A stored entry whose mode the connected panel does not offer. This + // is the same monitor name on different hardware, which happens with + // DP-1 on one dock and DP-1 on another. + // * A stored arrangement that leaves any output with no on-screen + // position at all. + // + // In both cases the compositor's automatic placement stands and a toast + // says so, which is recoverable. Silence would not be. + // The decision, with no side effects, so it can be tested without driving + // a real compositor. Returns one of: + // { action: "none" } nothing stored, or already correct + // { action: "apply", layout } restore this + // { action: "refuse" } stored arrangement does not fit + function plannedRestore(): var { + if (root.busy || root.awaitingConfirmation || root.monitors.length === 0) + return { action: "none" }; + + const stored = DesktopPreferences.get("displays"); + const persisted = stored && typeof stored === "object" ? stored : {}; + + // Start from what is on screen and overlay each stored entry, so an + // output with nothing saved keeps the compositor's own placement. + let changed = false; + const layout = root.currentLayout(); + for (const record of layout) { + const entry = persisted[record.name]; + if (!root.isPersistedLayoutEntry(entry)) + continue; + const parts = root.modeParts(entry.mode); + if (!parts) + continue; + if (entry.mode !== record.mode + || Math.abs(entry.scale - record.scale) >= 0.001 + || entry.transform !== record.transform + || (entry.x !== undefined && entry.x !== record.x) + || (entry.y !== undefined && entry.y !== record.y)) + changed = true; + record.mode = entry.mode; + record.width = parts.width; + record.height = parts.height; + record.refreshRate = parts.refresh; + record.scale = entry.scale; + record.transform = entry.transform; + if (entry.x !== undefined) record.x = entry.x; + if (entry.y !== undefined) record.y = entry.y; + record.primary = entry.primary === true; + } + + if (!changed) + return { action: "none" }; + + // Exactly one primary, on a display that is actually here. Undocking + // takes the primary away, and a layout with none is one + // DisplayLayout.validate refuses outright. + if (layout.filter(record => record.primary).length !== 1) { + layout.forEach(record => record.primary = false); + layout[0].primary = true; + } + + // The same validator every user-initiated change goes through: it + // checks the mode is one this panel offers, the scale is whole-pixel, + // and the names match what is connected. A stored entry for hardware + // that is no longer on this connector fails here, which is the point. + const normalized = DisplayLayout.normalize(layout); + if (!root.validRequestedLayout(normalized)) + return { action: "refuse" }; + + return { action: "apply", layout: normalized }; + } + + function restoreStored(): void { + const plan = root.plannedRestore(); + if (plan.action === "none") + return; + + if (plan.action === "refuse") { + StatusEvents.publish({ + key: "display-restore", + icon: "video-display-symbolic", + title: "Kept the automatic display arrangement", + detail: "The saved arrangement does not fit the displays connected now", + tone: "warn", + priority: StatusEvents.importantPriority, + actionId: "open-settings", + actionData: "displays" + }); + return; + } + + root.pushLayout(plan.layout, applyRun); + } + + // Displays announce themselves one at a time: plugging in a dock produces + // several signature changes in quick succession, and applying a layout to + // each intermediate topology would fight the compositor as it settles. + Timer { + id: restoreDebounce + interval: 1200 + onTriggered: root.restoreStored() } function refresh(): bool { diff --git a/config/dot/quickshell/services/StatusEvents.qml b/config/dot/quickshell/services/StatusEvents.qml index 85078e2..f30d7a5 100644 --- a/config/dot/quickshell/services/StatusEvents.qml +++ b/config/dot/quickshell/services/StatusEvents.qml @@ -91,6 +91,11 @@ Singleton { ShellState.open("activity"); } else if (action === "open-path" && data) { Quickshell.execDetached(["xdg-open", data]); + } else if (action === "open-settings" && data) { + // The page name is checked by ShellState's own allow-list, so an + // event naming a page that does not exist lands on Home rather + // than nowhere. + ShellState.openSettings(data); } } diff --git a/tests/quickshell/displays-contract b/tests/quickshell/displays-contract index 135c180..793531e 100755 --- a/tests/quickshell/displays-contract +++ b/tests/quickshell/displays-contract @@ -374,7 +374,67 @@ run ipc call displays-test forget >/dev/null sleep 0.6 [[ "$(status | jq -r .overridden)" == "false" ]] || fail 'forget did not clear the stored display setting' +# ── Restoring an arrangement when a display comes back ─────────────────────── +# +# hypr/monitors.lua applies stored per-output entries when the compositor reads +# its config, and never again. A monitor plugged in an hour later used to get +# the compositor's automatic placement instead of the arrangement this machine +# was told to use, and the only way back was to open Settings and apply it +# again. Docking should not cost you your desk. +# +# Driven through plannedRestore, which is the decision with no side effects -- +# applying a real layout here would drive the live compositor. +# +# Three cases, and the last two matter most: a wrong answer is a screen you +# cannot see well enough to fix, so it refuses rather than guesses. + +# Single-line and space-free: the IPC call splits its arguments on whitespace, +# so a pretty-printed fixture arrives as several arguments instead of one. +two_screens='[{"name":"DP-2","width":1920,"height":1080,"refreshRate":60.0,"scale":1.0,"transform":0,"x":0,"y":0,"availableModes":["1920x1080@60.00Hz","2560x1440@60.00Hz"]},{"name":"HDMI-A-1","width":1920,"height":1080,"refreshRate":60.0,"scale":1.0,"transform":0,"x":1920,"y":0,"availableModes":["1920x1080@60.00Hz"]}]' + +# base64 because `qs ipc call` splits a JSON array of several objects into one +# argument per object, which makes a two-monitor fixture look like an extra +# argument and the call is refused for arity. +b64() { printf '%s' "$1" | base64 -w0; } +plan() { run ipc call displays-test restorePlan "$(b64 "$1")" "$(b64 "$2")"; } + +# Nothing stored: the compositor's placement stands. +result="$(plan "$two_screens" '{}')" +jq -e '.action == "none"' <<<"$result" >/dev/null \ + || fail "with nothing stored, Panama wanted to change the arrangement: $result" + +# Stored and already correct: still nothing to do, so a reconnect does not +# push a layout the compositor is already showing. +result="$(plan "$two_screens" '{"DP-2":{"mode":"1920x1080@60.00","scale":1,"transform":0,"x":0,"y":0,"primary":true}}')" +jq -e '.action == "none"' <<<"$result" >/dev/null \ + || fail "an arrangement that already matches was re-applied anyway: $result" + +# Stored and different: restore it, with the stored scale and position. +result="$(plan "$two_screens" '{"DP-2":{"mode":"2560x1440@60.00","scale":1,"transform":0,"x":0,"y":0,"primary":true}}')" +jq -e '.action == "apply" and (.layout[] | select(.name == "DP-2") | .mode == "2560x1440@60.00")' \ + <<<"$result" >/dev/null \ + || fail "a stored arrangement was not restored when the display reconnected: $result" + +# A mode this panel does not offer. Same connector, different hardware -- DP-1 +# on one dock is not DP-1 on another. It must refuse rather than ask the +# compositor for a mode that does not exist. +result="$(plan "$two_screens" '{"DP-2":{"mode":"3840x2160@120.00","scale":1,"transform":0,"x":0,"y":0,"primary":true}}')" +jq -e '.action == "refuse"' <<<"$result" >/dev/null \ + || fail "a stored mode the connected panel does not offer was requested anyway: $result" + +# Undocked: the stored primary is gone. The layout that survives must still +# name exactly one primary, or DisplayLayout.validate refuses it outright and +# the machine keeps whatever it happens to have. +one_screen='[{"name":"DP-2","width":1920,"height":1080,"refreshRate":60.0,"scale":1.0,"transform":0,"x":0,"y":0,"availableModes":["1920x1080@60.00Hz","2560x1440@60.00Hz"]}]' +result="$(plan "$one_screen" '{"DP-2":{"mode":"2560x1440@60.00","scale":1,"transform":0,"x":0,"y":0,"primary":false},"HDMI-A-1":{"mode":"1920x1080@60.00","scale":1,"transform":0,"x":1920,"y":0,"primary":true}}')" +jq -e '.action == "apply" and ([.layout[] | select(.primary)] | length == 1)' <<<"$result" >/dev/null \ + || fail "after undocking, the restored layout did not name exactly one primary: $result" +jq -e '[.layout[] | .name] == ["DP-2"]' <<<"$result" >/dev/null \ + || fail "the restored layout mentions a display that is not connected: $result" + + restore_display || fail 'the final cleanup could not restore and verify the original display' + original_mode="" stop_harness trap - EXIT