diff --git a/config/dot/quickshell/modules/settings/README.md b/config/dot/quickshell/modules/settings/README.md index 34c48bc..ea50bfe 100644 --- a/config/dot/quickshell/modules/settings/README.md +++ b/config/dot/quickshell/modules/settings/README.md @@ -83,6 +83,12 @@ clock, date, user name, and password-field presentation. **Power** owns when the session locks, while **Privacy** keeps only the established timing mirrors above. Visual controls must not be copied onto either page. +**Displays** is the sole owner of mode, scale, rotation, arrangement, and primary role. +Those values form one safety transaction: every connected output +is applied, verified, confirmed, or restored together. Other pages may link to +Displays, but must never expose a second geometry control or persist a partial +layout. + Mirrors must remain the same schema-backed control, never a second preference or a copied default. Additions to this table require a concrete discoverability reason and an update to `tests/quickshell/settings-ownership-contract.sh`. diff --git a/config/dot/quickshell/services/Displays.qml b/config/dot/quickshell/services/Displays.qml index 0a95b7a..1a862da 100644 --- a/config/dot/quickshell/services/Displays.qml +++ b/config/dot/quickshell/services/Displays.qml @@ -365,8 +365,8 @@ Singleton { // Applies immediately and starts the countdown. Nothing is stored yet: the // complete connected layout is only written by confirm(). - function applyLayout(layout: var): bool { - if (root.externalChangeBlocked) { + function applyLayout(layout: var, protectedOperation: bool): bool { + if (root.externalChangeBlocked && protectedOperation !== true) { root.lastError = "Wait for Settings to finish restoring before changing a display."; return false; } @@ -397,6 +397,13 @@ Singleton { return true; } + // Settings restore holds the external-change lock while it proves a + // snapshot. This narrow entry point authorizes that one transaction while + // keeping every user-facing control blocked until restore settles. + function applyProtectedLayout(layout: var): bool { + return root.applyLayout(layout, true); + } + function makePrimary(output: string): bool { const layout = root.currentLayout(); if (!layout.some(record => record.name === output)) { diff --git a/config/dot/quickshell/services/SettingsBackup.qml b/config/dot/quickshell/services/SettingsBackup.qml index 59a4719..e798831 100644 --- a/config/dot/quickshell/services/SettingsBackup.qml +++ b/config/dot/quickshell/services/SettingsBackup.qml @@ -41,6 +41,10 @@ Singleton { property var readDisplays: function() { return DesktopPreferences.get("displays"); } property var protectDisplays: function(value) { return DesktopPreferences.set("displays", value); } property var displayBusy: function() { return Displays.busy || Displays.awaitingConfirmation; } + property var readLiveDisplayLayout: function() { return Displays.currentLayout(); } + property var applyDisplayLayout: function(layout) { return Displays.applyProtectedLayout(layout); } + property var displayCanConfirm: function() { return Displays.canConfirm; } + property var confirmDisplayLayout: function() { return Displays.confirm(); } property var setDisplayBlocked: function(blocked) { Displays.externalChangeBlocked = blocked; } property var applyCompositor: function() { SystemSettings.applyPersistedDisplayPolicy(); } property var reloadKeybinds: function() { Keybinds.applyReload(); } @@ -52,9 +56,11 @@ Singleton { property var lockBusy: function() { return LockScreen.busy; } property var reloadShell: function() { Quickshell.reload(false); } property var protectedDisplays: ({}) + property var protectedDisplayLayout: [] + property var pendingRestoredLayout: null readonly property bool busy: listQuery.running || actionRun.running - || applyRestoredState.running || settleReload.running + || settleDisplayRestore.running || applyRestoredState.running || settleReload.running Process { id: listQuery @@ -89,18 +95,21 @@ Singleton { if (actionRun.restoring) { root.setDisplayBlocked(false); root.protectedDisplays = ({}); + root.protectedDisplayLayout = []; } return; } root.lastAction = actionRun.restoring ? "restored" : "saved"; if (actionRun.restoring) { - const homeReloaded = root.handleRestoreOutput(actionRun.outputText); - root.lastError = homeReloaded - ? "" - : "Desktop settings were restored, but Home favourites could not be reloaded."; - if (!homeReloaded) { + const restoreAccepted = root.handleRestoreOutput(actionRun.outputText); + if (restoreAccepted) + root.lastError = ""; + else if (root.lastError === "") + root.lastError = "Desktop settings were restored, but Home favourites could not be reloaded."; + if (!restoreAccepted) { root.setDisplayBlocked(false); root.protectedDisplays = ({}); + root.protectedDisplayLayout = []; } } else root.lastError = ""; @@ -108,6 +117,28 @@ Singleton { } } + Timer { + id: settleDisplayRestore + property int attempts: 0 + interval: 100 + repeat: true + onTriggered: { + attempts++; + if (root.displayCanConfirm()) { + stop(); + if (!root.confirmDisplayLayout()) { + root.failDisplayRestore("The restored display layout could not be confirmed."); + return; + } + root.pendingRestoredLayout = null; + root.beginRestoredStateReplay(); + } else if (!root.displayBusy() || attempts >= 180) { + stop(); + root.failDisplayRestore("The restored display layout could not be verified."); + } + } + } + Timer { id: applyRestoredState interval: 80 @@ -141,6 +172,7 @@ Singleton { stop(); root.setDisplayBlocked(false); root.protectedDisplays = ({}); + root.protectedDisplayLayout = []; root.reloadShell(); } } @@ -179,12 +211,77 @@ Singleton { if (!root.reloadHomeState(text)) return false; root.reloadDesktop(); - if (!root.protectDisplays(root.protectedDisplays)) - return false; - applyRestoredState.restart(); + const restoredLayout = root.layoutFromStoredDisplays(root.readDisplays()); + if (restoredLayout === null || root.layoutsEqual( + restoredLayout, root.protectedDisplayLayout)) { + root.beginRestoredStateReplay(); + return true; + } + root.pendingRestoredLayout = restoredLayout; + if (!root.applyDisplayLayout(restoredLayout)) + return root.failDisplayRestore("The restored display layout was rejected."); + settleDisplayRestore.attempts = 0; + settleDisplayRestore.restart(); return true; } + function beginRestoredStateReplay(): void { + applyRestoredState.restart(); + } + + function layoutFromStoredDisplays(stored: var): var { + if (!stored || typeof stored !== "object") + return null; + const current = root.readLiveDisplayLayout(); + if (!Array.isArray(current) || current.length === 0) + return null; + const layout = []; + for (const live of current) { + const entry = stored[live.name]; + const match = String(entry?.mode ?? "").match( + /^(\d+)x(\d+)@(\d+(?:\.\d+)?)$/); + if (!entry || !match || !Number.isFinite(entry.scale) || entry.scale <= 0 + || !Number.isInteger(entry.transform) + || entry.transform < 0 || entry.transform > 3 + || !Number.isInteger(entry.x) || !Number.isInteger(entry.y) + || typeof entry.primary !== "boolean") + return null; + layout.push(Object.assign({}, live, { + width: Number(match[1]), + height: Number(match[2]), + refreshRate: Number(match[3]), + mode: entry.mode, + scale: entry.scale, + transform: entry.transform, + x: entry.x, + y: entry.y, + primary: entry.primary + })); + } + return layout.filter(record => record.primary).length === 1 ? layout : null; + } + + function layoutsEqual(left: var, right: var): bool { + if (!Array.isArray(left) || !Array.isArray(right) || left.length !== right.length) + return false; + const fields = ["name", "mode", "scale", "transform", "x", "y", "primary"]; + const a = Array.from(left).sort((x, y) => x.name.localeCompare(y.name)); + const b = Array.from(right).sort((x, y) => x.name.localeCompare(y.name)); + return a.every((record, index) => fields.every( + field => record[field] === b[index][field])); + } + + function failDisplayRestore(message: string): bool { + root.pendingRestoredLayout = null; + if (!root.protectDisplays(root.protectedDisplays)) + message += " The original display preference also could not be restored."; + root.setDisplayBlocked(false); + root.protectedDisplays = ({}); + root.protectedDisplayLayout = []; + root.lastError = message; + return false; + } + // Restore output carries the canonical Home state. Reconstructing through // these methods keeps validation and persistence inside HomePreferences; // this service never mutates its aliases or private FileView directly. @@ -252,6 +349,8 @@ Singleton { const currentDisplays = root.readDisplays(); root.protectedDisplays = JSON.parse(JSON.stringify( currentDisplays && typeof currentDisplays === "object" ? currentDisplays : {})); + root.protectedDisplayLayout = JSON.parse(JSON.stringify( + root.readLiveDisplayLayout() ?? [])); root.setDisplayBlocked(true); actionRun.restoring = true; actionRun.exec([root.helperPath, "restore", name]); diff --git a/config/dot/quickshell/services/SettingsSearch.qml b/config/dot/quickshell/services/SettingsSearch.qml index ccfb428..d95a683 100644 --- a/config/dot/quickshell/services/SettingsSearch.qml +++ b/config/dot/quickshell/services/SettingsSearch.qml @@ -69,7 +69,10 @@ Singleton { { label: "Copy health report", detail: "Copy a redacted Panama doctor report", page: "services" }, { label: "Lock screen background", detail: "Choose a blurred desktop, wallpaper, or solid colour", page: "appearance" }, { label: "Password field", detail: "Choose whether the empty lock-screen field stays visible", page: "appearance" }, - { label: "Per-display wallpaper", detail: "Assign a different image to each connected display", page: "appearance" } + { label: "Per-display wallpaper", detail: "Assign a different image to each connected display", page: "appearance" }, + { label: "Arrange displays", detail: "Drag connected displays into their physical positions", page: "displays" }, + { label: "Monitor position", detail: "Set where each display sits in the desktop", page: "displays" }, + { label: "Primary display", detail: "Choose the display that anchors the desktop", page: "displays" } ] function pageFor(group: string): string { diff --git a/config/dot/quickshell/services/SystemSettings.qml b/config/dot/quickshell/services/SystemSettings.qml index 4520c40..84af036 100644 --- a/config/dot/quickshell/services/SystemSettings.qml +++ b/config/dot/quickshell/services/SystemSettings.qml @@ -509,16 +509,13 @@ Singleton { return false; } - const currentDisplays = root.readDisplays(); - const protectedDisplays = JSON.parse(JSON.stringify( - currentDisplays && typeof currentDisplays === "object" ? currentDisplays : {})); root.setDisplayBlocked(true); DesktopPreferences.resetDesktopDefaults(); - if (!root.protectDisplays(protectedDisplays)) { - root.setDisplayBlocked(false); - root.lastError = "The current display setting could not be protected during reset."; - return false; - } + + // Do not apply geometry during a reset: doing so would need the same + // visible confirmation transaction as the Displays page. Clearing the + // stored records is still important, though, so the next session uses + // Panama's shipped DP-2 placement and automatic placement elsewhere. // Home accessories keep their own store (panama-home.json), so a reset // that only cleared the schema store would silently leave a customised diff --git a/config/dot/quickshell/settings-backup-harness.qml b/config/dot/quickshell/settings-backup-harness.qml index 2b06453..73c404b 100644 --- a/config/dot/quickshell/settings-backup-harness.qml +++ b/config/dot/quickshell/settings-backup-harness.qml @@ -15,7 +15,21 @@ ShellRoot { property var homeFavorites: [] property bool displayOperationBusy: false property bool displayBlocked: false - property var displayGeneration: ({ "DP-2": { mode: "4500x3000@60", scale: 1.5, transform: 0 } }) + property bool displayApplyAccepted: true + property bool displayConfirmationReady: false + property var originalDisplays: ({ + "DP-2": { mode: "4500x3000@60", scale: 1.5, transform: 0, x: 0, y: 0, primary: true }, + "HDMI-A-1": { mode: "2560x1440@60", scale: 1, transform: 0, x: 3000, y: 0, primary: false } + }) + property var restoredDisplays: ({ + "DP-2": { mode: "4500x3000@60", scale: 1.5, transform: 0, x: -2560, y: 0, primary: false }, + "HDMI-A-1": { mode: "2560x1440@60", scale: 1, transform: 0, x: 0, y: 0, primary: true } + }) + property var displayGeneration: originalDisplays + property var liveLayout: [ + { name: "DP-2", width: 4500, height: 3000, refreshRate: 60, mode: "4500x3000@60", scale: 1.5, transform: 0, x: 0, y: 0, primary: true }, + { name: "HDMI-A-1", width: 2560, height: 1440, refreshRate: 60, mode: "2560x1440@60", scale: 1, transform: 0, x: 3000, y: 0, primary: false } + ] function record(name: string): void { const next = root.calls.slice(); @@ -45,7 +59,10 @@ ShellRoot { root.homeFavorites = root.homeFavorites.map(favorite => favorite.id === id ? { id: id, alias: alias } : favorite); }; - SettingsBackup.reloadDesktop = function() { root.record("desktop.reload"); }; + SettingsBackup.reloadDesktop = function() { + root.record("desktop.reload"); + root.displayGeneration = JSON.parse(JSON.stringify(root.restoredDisplays)); + }; SettingsBackup.readDisplays = function() { return root.displayGeneration; }; SettingsBackup.protectDisplays = function(value) { root.record("display.protect:" + JSON.stringify(value)); @@ -53,6 +70,25 @@ ShellRoot { return true; }; SettingsBackup.displayBusy = function() { return root.displayOperationBusy; }; + SettingsBackup.readLiveDisplayLayout = function() { + return root.liveLayout.map(record => Object.assign({}, record)); + }; + SettingsBackup.applyDisplayLayout = function(layout) { + root.record("display.apply:" + JSON.stringify(layout)); + if (!root.displayApplyAccepted) + return false; + root.liveLayout = layout.map(record => Object.assign({}, record)); + root.displayOperationBusy = true; + root.displayConfirmationReady = true; + return true; + }; + SettingsBackup.displayCanConfirm = function() { return root.displayConfirmationReady; }; + SettingsBackup.confirmDisplayLayout = function() { + root.record("display.confirm"); + root.displayOperationBusy = false; + root.displayConfirmationReady = false; + return true; + }; SettingsBackup.setDisplayBlocked = function(blocked) { root.record("display.block:" + blocked); root.displayBlocked = blocked; @@ -77,7 +113,15 @@ ShellRoot { root.homeFavorites = []; root.displayOperationBusy = false; root.displayBlocked = false; + root.displayApplyAccepted = true; + root.displayConfirmationReady = false; + root.displayGeneration = JSON.parse(JSON.stringify(root.originalDisplays)); + root.liveLayout = [ + { name: "DP-2", width: 4500, height: 3000, refreshRate: 60, mode: "4500x3000@60", scale: 1.5, transform: 0, x: 0, y: 0, primary: true }, + { name: "HDMI-A-1", width: 2560, height: 1440, refreshRate: 60, mode: "2560x1440@60", scale: 1, transform: 0, x: 3000, y: 0, primary: false } + ]; SettingsBackup.protectedDisplays = root.displayGeneration; + SettingsBackup.protectedDisplayLayout = root.liveLayout; } function apply(output: string): bool { @@ -90,6 +134,11 @@ ShellRoot { return SettingsBackup.restore("settings-20260818-010203004.json"); } + function applyDisplayFailure(output: string): bool { + root.displayApplyAccepted = false; + return SettingsBackup.handleRestoreOutput(output); + } + function status(): string { return JSON.stringify({ calls: root.calls, diff --git a/tests/quickshell/settings-backup-live-contract.sh b/tests/quickshell/settings-backup-live-contract.sh index adee5e9..39526f2 100755 --- a/tests/quickshell/settings-backup-live-contract.sh +++ b/tests/quickshell/settings-backup-live-contract.sh @@ -44,6 +44,10 @@ for mapping in \ 'HomePreferences.setAlias(id, alias);' \ 'DesktopPreferences.reload();' \ 'DesktopPreferences.set("displays", value);' \ + 'Displays.currentLayout();' \ + 'Displays.applyProtectedLayout(layout);' \ + 'Displays.canConfirm;' \ + 'Displays.confirm();' \ 'Displays.externalChangeBlocked = blocked;' \ 'SystemSettings.applyPersistedDisplayPolicy();' \ 'Keybinds.applyReload();' \ @@ -85,7 +89,8 @@ jq -e ' "home.alias:light.desk=Desk", "home.alias:light.office=Office", "desktop.reload", - "display.protect:{\"DP-2\":{\"mode\":\"4500x3000@60\",\"scale\":1.5,\"transform\":0}}", + "display.apply:[{\"name\":\"DP-2\",\"width\":4500,\"height\":3000,\"refreshRate\":60,\"mode\":\"4500x3000@60\",\"scale\":1.5,\"transform\":0,\"x\":-2560,\"y\":0,\"primary\":false},{\"name\":\"HDMI-A-1\",\"width\":2560,\"height\":1440,\"refreshRate\":60,\"mode\":\"2560x1440@60\",\"scale\":1,\"transform\":0,\"x\":0,\"y\":0,\"primary\":true}]", + "display.confirm", "system.apply", "keybinds.reload", "wallpaper.apply-policy", @@ -124,7 +129,8 @@ jq -e ' .calls == [ "home.reset", "desktop.reload", - "display.protect:{\"DP-2\":{\"mode\":\"4500x3000@60\",\"scale\":1.5,\"transform\":0}}", + "display.apply:[{\"name\":\"DP-2\",\"width\":4500,\"height\":3000,\"refreshRate\":60,\"mode\":\"4500x3000@60\",\"scale\":1.5,\"transform\":0,\"x\":-2560,\"y\":0,\"primary\":false},{\"name\":\"HDMI-A-1\",\"width\":2560,\"height\":1440,\"refreshRate\":60,\"mode\":\"2560x1440@60\",\"scale\":1,\"transform\":0,\"x\":0,\"y\":0,\"primary\":true}]", + "display.confirm", "system.apply", "keybinds.reload", "wallpaper.apply-policy", @@ -136,6 +142,25 @@ jq -e ' and .favorites == [] ' <<<"$status" >/dev/null || fail "absent Home handoff was wrong: $status" +# If the restored complete layout is rejected before it can be verified, the +# original persisted layout is put back and the shell is not reloaded over an +# unproven display state. +qs_test ipc call settings-backup-behavior reset >/dev/null +[[ "$(qs_test ipc call settings-backup-behavior applyDisplayFailure "$payload")" == "false" ]] \ + || fail 'a rejected restored display layout was reported as successful' +status="$(qs_test ipc call settings-backup-behavior status)" +jq -e '.calls == [ + "home.reset", + "home.initialize:light.desk,light.office", + "home.alias:light.desk=Desk", + "home.alias:light.office=Office", + "desktop.reload", + "display.apply:[{\"name\":\"DP-2\",\"width\":4500,\"height\":3000,\"refreshRate\":60,\"mode\":\"4500x3000@60\",\"scale\":1.5,\"transform\":0,\"x\":-2560,\"y\":0,\"primary\":false},{\"name\":\"HDMI-A-1\",\"width\":2560,\"height\":1440,\"refreshRate\":60,\"mode\":\"2560x1440@60\",\"scale\":1,\"transform\":0,\"x\":0,\"y\":0,\"primary\":true}]", + "display.protect:{\"DP-2\":{\"mode\":\"4500x3000@60\",\"scale\":1.5,\"transform\":0,\"x\":0,\"y\":0,\"primary\":true},\"HDMI-A-1\":{\"mode\":\"2560x1440@60\",\"scale\":1,\"transform\":0,\"x\":3000,\"y\":0,\"primary\":false}}", + "display.block:false" +] and (.lastError | contains("display layout"))' <<<"$status" >/dev/null \ + || fail "rejected display restore did not retain the original layout: $status" + # Restore refuses before launching the helper while a display apply/recovery is # active, so no snapshot can race the confirmation boundary. qs_test ipc call settings-backup-behavior reset >/dev/null diff --git a/tests/quickshell/settings-commit-reset-contract.sh b/tests/quickshell/settings-commit-reset-contract.sh index 4dde3fd..93bec4b 100755 --- a/tests/quickshell/settings-commit-reset-contract.sh +++ b/tests/quickshell/settings-commit-reset-contract.sh @@ -39,8 +39,8 @@ rg -Fq 'HomePreferences.resetHomeDefaults();' "$system_settings" \ if rg -q 'HomePreferences\.(favorites|initialized)\s*=' "$system_settings"; then fail 'restoreDefaults mutates Home aliases instead of using resetHomeDefaults' fi -rg -Fq 'root.protectDisplays(protectedDisplays)' "$system_settings" \ - || fail 'restoreDefaults can apply unconfirmed display geometry during reload' +rg -Fq 'DesktopPreferences.resetDesktopDefaults();' "$system_settings" \ + || fail 'restoreDefaults does not clear confirmed display layout fields' rg -Fq 'Keybinds.applyReload();' "$system_settings" \ || fail 'restoreDefaults does not replay shipped keybindings' rg -Fq 'root.applyWallpaper(String(DesktopPreferences.get("wallpaperPath") ?? ""));' "$system_settings" \ @@ -120,7 +120,7 @@ before="$(qs_for_harness ipc call settings-system-test stored windowRounding)" # ── Reset spans every store, not just the schema one ───────────────────────── qs_for_harness ipc call settings-system-test seedHome >/dev/null qs_for_harness ipc call settings-system-test commit dockHideDelayMs 900 >/dev/null -display_fixture='{"DP-2":{"mode":"4500x3000@60","scale":1.5,"transform":0}}' +display_fixture='{"DP-2":{"mode":"4500x3000@60","scale":1.5,"transform":0,"x":0,"y":0,"primary":true},"HDMI-A-1":{"mode":"2560x1440@60","scale":1,"transform":0,"x":3000,"y":0,"primary":false}}' [[ "$(qs_for_harness ipc call settings-system-test commit displays "$display_fixture")" == "true" ]] \ || fail 'the protected display fixture did not apply' sleep 0.4 @@ -138,7 +138,6 @@ sleep 0.6 reset_state="$(qs_for_harness ipc call settings-system-test resetState)" jq -e '.calls == [ "display.block:true", - "display.protect", "keybinds.reload", "wallpaper.set:", "lock.regenerate", @@ -164,8 +163,8 @@ for default_case in \ [[ "$(qs_for_harness ipc call settings-system-test stored "$key")" == "$expected" ]] \ || fail "reset did not restore $key to $expected" done -[[ "$(qs_for_harness ipc call settings-system-test stored displays | jq -cS .)" == "$(jq -cS . <<<"$display_fixture")" ]] \ - || fail 'reset replaced confirmed display geometry without confirmation' +[[ "$(qs_for_harness ipc call settings-system-test stored displays | jq -cS .)" == '{}' ]] \ + || fail 'reset retained confirmed arrangement fields instead of returning startup to shipped placement' home_after="$(qs_for_harness ipc call settings-system-test homeState)" jq -e '.count == 0 and .initialized == false' <<<"$home_after" >/dev/null \ diff --git a/tests/quickshell/settings-ownership-contract.sh b/tests/quickshell/settings-ownership-contract.sh index 68fc365..e4c2152 100755 --- a/tests/quickshell/settings-ownership-contract.sh +++ b/tests/quickshell/settings-ownership-contract.sh @@ -102,7 +102,8 @@ for needle in \ '`inactiveOpacity`' \ '`lockMinutes`' \ '`lockOnSleep`' \ - 'scheme-relative role'; do + 'scheme-relative role' \ + 'mode, scale, rotation, arrangement, and primary role'; do rg -Fq "$needle" "$readme" || fail "README is missing $needle" done diff --git a/tests/quickshell/settings-search-contract.sh b/tests/quickshell/settings-search-contract.sh index 3051b81..b2d08be 100755 --- a/tests/quickshell/settings-search-contract.sh +++ b/tests/quickshell/settings-search-contract.sh @@ -65,6 +65,9 @@ password field|Password field|appearance slideshow|Wallpaper mode|appearance shuffle|Shuffle|appearance per-display wallpaper|Per-display wallpaper|appearance +arrange displays|Arrange displays|displays +monitor position|Monitor position|displays +primary display|Primary display|displays CASES ! rg -Fq 'Startup & Services' "$repo_dir/config/dot/quickshell/services/SettingsSearch.qml" \