From ba2b4a4d5d795270cfddb2eb44c060691a999010 Mon Sep 17 00:00:00 2001 From: Connor Welsh Date: Wed, 9 Sep 2026 13:02:21 -0400 Subject: [PATCH] outputs: only re-enable displays the cycle itself turned off (#3377) The cycle fallback fired from a never-cycled state. With one display enabled and another marked off in settings, a DPMS standby that drops the DisplayPort link looked like an unplug, and DMS sent an On to the disabled display over niri IPC. It stayed on until the next config reload and the workspaces round tripped through it. Track each output's enabled state the first time a cycle touches it, drop the entry once it is back at that state, and only rescue outputs that were on before the cycle disabled them. Outputs that were already off are never enabled automatically. --- docs/IPC.md | 6 +-- quickshell/Services/OutputCycleState.js | 47 +++++++++++++++----- quickshell/tests/tst_NiriOutputCycle.qml | 9 ++-- quickshell/tests/tst_OutputCycleState.qml | 52 +++++++++++++++++++---- 4 files changed, 87 insertions(+), 27 deletions(-) diff --git a/docs/IPC.md b/docs/IPC.md index de03b2dc3..5070e8aa7 100644 --- a/docs/IPC.md +++ b/docs/IPC.md @@ -118,9 +118,9 @@ The command returns immediately with one of these statuses: - `OUTPUT_CYCLE_NOOP` — fewer than two outputs are connected, or DMS could not start switching outputs. - `OUTPUT_CYCLE_UNSUPPORTED` — the active compositor is not Niri, its IPC socket is unavailable, or DMS output-state notifications are unavailable. -If the selected output disconnects and no connected output is enabled, DMS enables the previous connected output in the cycle. -This fallback also works before the first cycle command and leaves any already-enabled outputs unchanged. -DMS keeps this cycle order in memory until it restarts. +If the selected output disconnects and no connected output is enabled, DMS re-enables the previous output in the cycle, but only if a cycle command disabled it. +Outputs that were already off, such as those disabled in display settings, are never enabled by this fallback. +DMS keeps this cycle order and the list of outputs it disabled in memory until it restarts. ### Niri keybinding example diff --git a/quickshell/Services/OutputCycleState.js b/quickshell/Services/OutputCycleState.js index 938c1feb7..c948cd053 100644 --- a/quickshell/Services/OutputCycleState.js +++ b/quickshell/Services/OutputCycleState.js @@ -4,7 +4,8 @@ function emptyState() { return { ring: [], selected: "", - pending: "" + pending: "", + original: {} }; } @@ -19,10 +20,13 @@ function requestCycle(state, outputs, currentOutput) { const current = findOutput(outputs, currentOutput) ? currentOutput : (firstEnabled(outputs) || ring[0]); const target = ring[(ring.indexOf(current) + 1) % ring.length]; const targetOutput = findOutput(outputs, target); - if (targetOutput.enabled) - return result(withState(state, { selected: target }), "accepted", disableOthers(outputs, target)); + if (targetOutput.enabled) { + const intents = disableOthers(outputs, target); + return result(withState(state, { selected: target, original: recordOriginal(state.original, outputs, intents) }), "accepted", intents); + } - return result(withState(state, { pending: target }), "accepted", [{ id: target, enabled: true }]); + const intents = [{ id: target, enabled: true }]; + return result(withState(state, { pending: target, original: recordOriginal(state.original, outputs, intents) }), "accepted", intents); } function handleOutputsChanged(state, outputs) { @@ -30,23 +34,24 @@ function handleOutputsChanged(state, outputs) { if (ring.length === 0) return result(state, "", []); - let nextState = state; + let nextState = withState(state, { original: pruneRestored(state.original, outputs) }); let intents = []; if (state.pending) { const pendingOutput = findOutput(outputs, state.pending); if (!pendingOutput) { nextState = withState(nextState, { pending: "" }); } else if (pendingOutput.enabled) { + intents = disableOthers(outputs, pendingOutput.id); nextState = withState(nextState, { pending: "", - selected: pendingOutput.id + selected: pendingOutput.id, + original: recordOriginal(nextState.original, outputs, intents) }); - intents = disableOthers(outputs, pendingOutput.id); } } if (!nextState.pending && nextState.selected && !findOutput(outputs, nextState.selected) && !firstEnabled(outputs)) { - const fallback = previousConnectedOutput(state.ring, nextState.selected, outputs); + const fallback = previousConnectedOutput(state.ring, nextState.selected, outputs, nextState.original); if (fallback) { nextState = withState(nextState, { selected: fallback.id }); intents = intents.concat([{ id: fallback.id, enabled: true }]); @@ -86,23 +91,43 @@ function disableOthers(outputs, selected) { .map(output => ({ id: output.id, enabled: false })); } -function previousConnectedOutput(ring, selected, outputs) { +function previousConnectedOutput(ring, selected, outputs, original) { const selectedIndex = ring.indexOf(selected); if (selectedIndex < 0) return null; for (let offset = 1; offset < ring.length; offset++) { const candidate = findOutput(outputs, ring[(selectedIndex - offset + ring.length) % ring.length]); - if (candidate) + if (candidate && original[candidate.id] === true) return candidate; } return null; } +function recordOriginal(original, outputs, intents) { + const next = Object.assign({}, original); + for (const intent of intents) { + if (next[intent.id] === undefined) + next[intent.id] = findOutput(outputs, intent.id).enabled; + } + return next; +} + +function pruneRestored(original, outputs) { + const next = {}; + for (const id in original) { + const output = findOutput(outputs, id); + if (!output || output.enabled !== original[id]) + next[id] = original[id]; + } + return next; +} + function withState(state, changes) { return { ring: changes.ring !== undefined ? changes.ring : state.ring, selected: changes.selected !== undefined ? changes.selected : state.selected, - pending: changes.pending !== undefined ? changes.pending : state.pending + pending: changes.pending !== undefined ? changes.pending : state.pending, + original: changes.original !== undefined ? changes.original : (state.original || {}) }; } diff --git a/quickshell/tests/tst_NiriOutputCycle.qml b/quickshell/tests/tst_NiriOutputCycle.qml index ffe5630b1..82e36dd85 100644 --- a/quickshell/tests/tst_NiriOutputCycle.qml +++ b/quickshell/tests/tst_NiriOutputCycle.qml @@ -133,16 +133,15 @@ TestCase { compareRequests([["DP-1", "On"], ["eDP-1", "Off"]]); } - function test_initialSelectionRecoversDisabledHeadAfterUnplug() { + function test_initialSelectionLeavesDisabledHeadAloneAfterUnplug() { start([head("DP-1", true), head("eDP-1", false)], "DP-1"); compareRequests([]); wlr.publish([head("eDP-1", false)]); - compareRequests([["eDP-1", "On"]]); + compareRequests([]); - wlr.publish([head("eDP-1", true)]); - compareRequests([["eDP-1", "On"]]); - compare(adapter.cycleSingleOutput(), "OUTPUT_CYCLE_NOOP"); + wlr.publish([head("DP-1", true), head("eDP-1", false)]); + compareRequests([]); } function test_unpluggedPendingTargetCancelsAndNewHeadJoinsCycle() { diff --git a/quickshell/tests/tst_OutputCycleState.qml b/quickshell/tests/tst_OutputCycleState.qml index c8a53d008..1a57fc410 100644 --- a/quickshell/tests/tst_OutputCycleState.qml +++ b/quickshell/tests/tst_OutputCycleState.qml @@ -95,7 +95,7 @@ TestCase { } function test_disabledFallbackOnlyEnablesPreviousOutput() { - const state = { ring: ["DP-1", "HDMI-A-1", "eDP-1"], selected: "HDMI-A-1", pending: "" }; + const state = { ring: ["DP-1", "HDMI-A-1", "eDP-1"], selected: "HDMI-A-1", pending: "", original: { "DP-1": true, "eDP-1": true } }; const result = OutputCycleState.handleOutputsChanged(state, [ output("DP-1", false), output("eDP-1", false) @@ -127,18 +127,54 @@ TestCase { } function test_fallbackConfirmationDoesNotDisableNewlyEnabledPeer() { - const state = OutputCycleState.handleOutputsChanged(OutputCycleState.emptyState(), [ - output("DP-1", true), output("DP-2", false), output("eDP-1", false) - ]).state; - const unplugged = OutputCycleState.handleOutputsChanged(state, [ - output("DP-2", false), output("eDP-1", false) + const heads = [output("DP-1", true), output("DP-2", false), output("eDP-1", false)]; + const initial = OutputCycleState.handleOutputsChanged(OutputCycleState.emptyState(), heads).state; + + const cycle = OutputCycleState.requestCycle(initial, heads, "DP-1"); + compare(cycle.intents, [{ id: "DP-2", enabled: true }]); + const cycled = OutputCycleState.handleOutputsChanged(cycle.state, [ + output("DP-1", true), output("DP-2", true), output("eDP-1", false) ]); - compare(unplugged.intents, [{ id: "eDP-1", enabled: true }]); + compare(cycled.intents, [{ id: "DP-1", enabled: false }]); + + const unplugged = OutputCycleState.handleOutputsChanged(cycled.state, [ + output("DP-1", false), output("eDP-1", false) + ]); + compare(unplugged.intents, [{ id: "DP-1", enabled: true }]); const confirmed = OutputCycleState.handleOutputsChanged(unplugged.state, [ - output("DP-2", true), output("eDP-1", true) + output("DP-1", true), output("eDP-1", false) ]); compare(confirmed.intents, []); compare(confirmed.state.pending, ""); + compare(confirmed.state.original, { "DP-2": false }); + } + + function test_unplugWithoutCycleLeavesDisabledOutputAlone() { + const state = OutputCycleState.handleOutputsChanged(OutputCycleState.emptyState(), [ + output("DP-2", true), output("HDMI-A-2", false) + ]).state; + const asleep = OutputCycleState.handleOutputsChanged(state, [output("HDMI-A-2", false)]); + compare(asleep.intents, []); + const awake = OutputCycleState.handleOutputsChanged(asleep.state, [ + output("DP-2", true), output("HDMI-A-2", false) + ]); + compare(awake.intents, []); + } + + function test_cycleRoundTripForgetsOutputDmsEnabled() { + let state = OutputCycleState.handleOutputsChanged(OutputCycleState.emptyState(), [ + output("DP-2", true), output("HDMI-A-2", false) + ]).state; + state = OutputCycleState.requestCycle(state, [output("DP-2", true), output("HDMI-A-2", false)], "DP-2").state; + state = OutputCycleState.handleOutputsChanged(state, [output("DP-2", true), output("HDMI-A-2", true)]).state; + state = OutputCycleState.handleOutputsChanged(state, [output("DP-2", false), output("HDMI-A-2", true)]).state; + state = OutputCycleState.requestCycle(state, [output("DP-2", false), output("HDMI-A-2", true)], "HDMI-A-2").state; + state = OutputCycleState.handleOutputsChanged(state, [output("DP-2", true), output("HDMI-A-2", true)]).state; + state = OutputCycleState.handleOutputsChanged(state, [output("DP-2", true), output("HDMI-A-2", false)]).state; + compare(state.original, {}); + + const asleep = OutputCycleState.handleOutputsChanged(state, [output("HDMI-A-2", false)]); + compare(asleep.intents, []); } function test_missingPendingTargetCancelsWithoutIntent() {