diff --git a/Crisp/Models/DDCOperationQueuePool.swift b/Crisp/Models/DDCOperationQueuePool.swift index 6659ae4..aa28662 100644 --- a/Crisp/Models/DDCOperationQueuePool.swift +++ b/Crisp/Models/DDCOperationQueuePool.swift @@ -44,7 +44,7 @@ final class DDCOperationQueuePool: @unchecked Sendable { /// Parks every per-display queue, present and future, until the returned closure /// runs; `onIdle` fires once queued work drains, so the caller knows I2C is free. /// Each queue gives up after `timeout` so a lost release cannot wedge DDC. - func hold(timeout: TimeInterval, onIdle: @escaping () -> Void) -> () -> Void { + func hold(timeout: TimeInterval, onIdle: @escaping () -> Void) -> @Sendable () -> Void { let hold = Hold(timeout: timeout) lock.lock() activeHolds.append(hold) diff --git a/Crisp/Models/DisplayConfigurationGate.swift b/Crisp/Models/DisplayConfigurationGate.swift new file mode 100644 index 0000000..e53de47 --- /dev/null +++ b/Crisp/Models/DisplayConfigurationGate.swift @@ -0,0 +1,21 @@ +import Foundation + +/// Unlike the UI timeout, this lease ends only when the blocking transaction finishes. +final class DisplayConfigurationGate: @unchecked Sendable { + private let lock = NSLock() + private var pending = false + + var isPending: Bool { lock.withLock { pending } } + + func begin() -> Bool { + lock.withLock { + guard !pending else { return false } + pending = true + return true + } + } + + func finish() { + lock.withLock { pending = false } + } +} diff --git a/Crisp/Models/DisplayConnectionQueue.swift b/Crisp/Models/DisplayConnectionQueue.swift new file mode 100644 index 0000000..7453979 --- /dev/null +++ b/Crisp/Models/DisplayConnectionQueue.swift @@ -0,0 +1,19 @@ +@MainActor +final class DisplayConnectionQueue { + private var tail: Task? + private var generation: UInt64 = 0 + + func run(_ operation: @escaping @MainActor () async -> Output) async -> Output { + let previous = tail + let task = Task { @MainActor in + await previous?.value + return await operation() + } + generation &+= 1 + let requestGeneration = generation + tail = Task { @MainActor in _ = await task.value } + let result = await task.value + if generation == requestGeneration { tail = nil } + return result + } +} diff --git a/Crisp/Resources/Localizable.xcstrings b/Crisp/Resources/Localizable.xcstrings index 8d7c3ab..d83c83c 100644 --- a/Crisp/Resources/Localizable.xcstrings +++ b/Crisp/Resources/Localizable.xcstrings @@ -1640,6 +1640,22 @@ } } }, + "Display configuration is still in progress. Wait for it to finish." : { + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "显示器配置仍在进行，请等待完成。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "顯示器設定仍在進行，請等待完成。" + } + } + } + }, "Display configuration timed out." : { "localizations" : { "zh-Hans" : { @@ -1706,6 +1722,22 @@ } } }, + "Display Connection" : { + "localizations" : { + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "显示器连接" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "顯示器連線" + } + } + } + }, "Display: %@" : { "localizations" : { "zh-Hans" : { @@ -4303,4 +4335,4 @@ } }, "version" : "1.0" -} \ No newline at end of file +} diff --git a/Crisp/Services/CGHelpers.swift b/Crisp/Services/CGHelpers.swift index bb2b063..b05bd6e 100644 --- a/Crisp/Services/CGHelpers.swift +++ b/Crisp/Services/CGHelpers.swift @@ -8,6 +8,7 @@ enum CGHelpers { static func runWithTimeout( seconds: Double, fallback: T, + onOperationFinished: (@Sendable (T, Bool) -> Void)? = nil, operation: @escaping @Sendable () -> T ) async -> T { await withCheckedContinuation { cont in @@ -15,7 +16,9 @@ enum CGHelpers { DispatchQueue.global(qos: .userInitiated).async { let result = operation() - if once.claim() { cont.resume(returning: result) } + let completedInTime = once.claim() + onOperationFinished?(result, !completedInTime) + if completedInTime { cont.resume(returning: result) } } DispatchQueue.global().asyncAfter(deadline: .now() + seconds) { diff --git a/Crisp/Services/DDCService.swift b/Crisp/Services/DDCService.swift index d630bfe..314620a 100644 --- a/Crisp/Services/DDCService.swift +++ b/Crisp/Services/DDCService.swift @@ -728,7 +728,7 @@ final class DDCService: ObservableObject, @unchecked Sendable { /// cap). Do not skip this around a display enable/disable: WindowServer's enable waits /// behind an in-flight I2C transaction and freezes the whole Mac with it. See /// docs/ddc-notes.md (the hold around enable and disable). - func hold() async -> () -> Void { + func hold() async -> @Sendable () -> Void { let idle = DispatchSemaphore(value: 0) let release = operationQueues.hold(timeout: 15) { idle.signal() } await withCheckedContinuation { (continuation: CheckedContinuation) in diff --git a/Crisp/Services/InputSwitchService.swift b/Crisp/Services/InputSwitchService.swift index dc30a68..aef6093 100644 --- a/Crisp/Services/InputSwitchService.swift +++ b/Crisp/Services/InputSwitchService.swift @@ -150,13 +150,23 @@ final class InputSwitchService: ObservableObject { let returnInput = toggle.disconnected.first { $0.uuid == uuid }?.returnInput let result = await toggle.reconnect(uuid: uuid) guard case .success = result, let returnInput else { return result } - // One write: the monitor re-links to this Mac (a 1.5 s hot plug drop) when it lands. - guard let displayID = Self.onlineDisplayID(uuid: uuid) else { return result } - let written = await DDCService.shared.writeInputAfterReconnect(displayID: displayID, value: returnInput) - Self.log.notice("display \(uuid, privacy: .public) reconnected, input \(returnInput, privacy: .public) written: \(written, privacy: .public)") + await restoreInputAfterReconnect(uuid: uuid, input: returnInput) return result } + func restoreInputAfterReconnect(uuid: String, input: UInt16) async { + var target: CGDirectDisplayID? + for _ in 0..<20 { + target = Self.onlineDisplayID(uuid: uuid) + if target != nil { break } + try? await Task.sleep(nanoseconds: 100_000_000) + } + // One write: the monitor re-links to this Mac (a 1.5 s hot plug drop) when it lands. + guard let displayID = target else { return } + let written = await DDCService.shared.writeInputAfterReconnect(displayID: displayID, value: input) + Self.log.notice("display \(uuid, privacy: .public) reconnected, input \(input, privacy: .public) written: \(written, privacy: .public)") + } + // MARK: - Private private static func onlineDisplayID(uuid: String) -> CGDirectDisplayID? { diff --git a/Crisp/Services/PhysicalDisplayToggleService.swift b/Crisp/Services/PhysicalDisplayToggleService.swift index d32a107..e27a7a1 100644 --- a/Crisp/Services/PhysicalDisplayToggleService.swift +++ b/Crisp/Services/PhysicalDisplayToggleService.swift @@ -21,6 +21,10 @@ final class PhysicalDisplayToggleService: ObservableObject { /// Set by DisplayManager at launch. The restore below needs a DisplayInfo to read and put /// back a display's HDR switch, and this service otherwise works from CGDirectDisplayIDs. weak var displayManager: DisplayManager? + private let connectionQueue = DisplayConnectionQueue() + private let configurationGate = DisplayConfigurationGate() + @Published private(set) var configurationInProgress = false + private var reconcileWhenSettled = false /// Snapshot of a display we disconnected, kept because a disconnected display no longer /// appears in DisplayManager.displays, so we need its metadata to render a Reconnect row. @@ -50,6 +54,7 @@ final class PhysicalDisplayToggleService: ObservableObject { /// The 10s wrapper only stops waiting; it can't cancel CGCompleteDisplayConfiguration, /// so this is not proof the change didn't take. case timedOut + case configurationInProgress var description: String { switch self { @@ -63,6 +68,8 @@ final class PhysicalDisplayToggleService: ObservableObject { return String(localized: "Display not found.") case .timedOut: return String(localized: "Display configuration timed out.") + case .configurationInProgress: + return String(localized: "Display configuration is still in progress. Wait for it to finish.") } } } @@ -138,7 +145,8 @@ final class PhysicalDisplayToggleService: ObservableObject { /// True if disconnecting `display` now would leave no *viewable* screen. Virtual displays /// don't count: a headless virtual left alone still blacks out the physical machine. func wouldLeaveNoActiveDisplay(_ displayID: CGDirectDisplayID) -> Bool { - CGDisplayIsActive(displayID) != 0 && physicalActiveDisplayCount() <= 1 + let active = viewableActiveDisplays().filter { !isDisconnected(uuid: uuid(for: $0)) } + return active.isEmpty || (CGDisplayIsActive(displayID) != 0 && active.count <= 1) } /// All display IDs known to the window server, INCLUDING ones disabled via @@ -240,20 +248,30 @@ final class PhysicalDisplayToggleService: ObservableObject { /// would leave zero active displays, so the user can never black out their only screen. @discardableResult func disconnect(_ display: DisplayInfo, returnInput: UInt16? = nil) async -> Result { + let requestedUUID = display.displayUUID + return await connectionQueue.run { await self.performDisconnect(display, uuid: requestedUUID, returnInput: returnInput) } + } + + private func performDisconnect(_ display: DisplayInfo, uuid requestedUUID: String, + returnInput: UInt16?) async -> Result { guard isSupported else { return .failure(.unsupportedPlatform) } - let displayID = display.displayID + guard !configurationGate.isPending else { return .failure(.configurationInProgress) } + guard let displayID = onlineDisplayIDs().first(where: { uuid(for: $0) == requestedUUID }), + display.isBuiltin == (CGDisplayIsBuiltin(displayID) != 0) else { + return .failure(.displayNotFound) + } if wouldLeaveNoActiveDisplay(displayID) { return .failure(.wouldLeaveNoActiveDisplay) } // A new click adds this dock's externals to the ones a parked record already knew. var companions: [String]? if display.isBuiltin { let externals = viewableActiveDisplays().filter { $0 != displayID }.map { uuid(for: $0) } - let known = parked?.uuid == display.displayUUID ? parked?.companions ?? [] : [] + let known = parked?.uuid == requestedUUID ? parked?.companions ?? [] : [] companions = Array(Set(externals + known)) } // Snapshot BEFORE disabling, afterwards the display is gone from the normal APIs. let snapshot = DisconnectedDisplay( - uuid: display.displayUUID, + uuid: requestedUUID, displayID: displayID, name: display.name, width: display.pixelWidth, @@ -263,19 +281,27 @@ final class PhysicalDisplayToggleService: ObservableObject { companions: companions ) - Self.log.notice("disconnect requested: \(display.displayUUID, privacy: .public) id \(displayID, privacy: .public)") + Self.log.notice("disconnect requested: \(requestedUUID, privacy: .public) id \(displayID, privacy: .public)") let otherStates = currentStates(excluding: [displayID]) - let result = await setEnabled(false, displayID: displayID) + let result = await setEnabled(false, displayID: displayID, expectedUUID: requestedUUID) { [weak self] lateResult in + guard let self, case .success = lateResult else { return } + self.rememberDisconnected(snapshot) + Task { await self.restoreStates(otherStates) } + } if case .success = result { - disconnected.removeAll { $0.uuid == snapshot.uuid } - disconnected.append(snapshot) - saveDesired() - if parked?.uuid == snapshot.uuid { setParked(nil) } + rememberDisconnected(snapshot) Task { [weak self] in await self?.restoreStates(otherStates) } } return result } + private func rememberDisconnected(_ snapshot: DisconnectedDisplay) { + disconnected.removeAll { $0.uuid == snapshot.uuid } + disconnected.append(snapshot) + saveDesired() + if parked?.uuid == snapshot.uuid { setParked(nil) } + } + /// What an arrangement decides for one display: its mode, its rotation, and whether it is /// in HDR. `hdr` is nil for a display that has no HDR switch to put back. struct DisplayState { @@ -351,7 +377,12 @@ final class PhysicalDisplayToggleService: ObservableObject { /// the Tools switch off until the next undock. @discardableResult func reconnect(uuid: String, byUser: Bool = true) async -> Result { + await connectionQueue.run { await self.performReconnect(uuid: uuid, byUser: byUser) } + } + + private func performReconnect(uuid: String, byUser: Bool) async -> Result { guard isSupported else { return .failure(.unsupportedPlatform) } + guard !configurationGate.isPending else { return .failure(.configurationInProgress) } guard let record = disconnected.first(where: { $0.uuid == uuid }) else { return .failure(.displayNotFound) } @@ -363,7 +394,14 @@ final class PhysicalDisplayToggleService: ObservableObject { if byUser, record.isBuiltin == true, SettingsService.shared.disconnectBuiltinWhenDocked { standDownUntilUndock = true } - let result = await setEnabled(true, displayID: targetID) + let result = await setEnabled(true, displayID: targetID) { [weak self] lateResult in + guard let self, case .success = lateResult else { return } + self.disconnected.removeAll { $0.uuid == uuid } + self.saveDesired() + if byUser, let returnInput = record.returnInput { + Task { await InputSwitchService.shared.restoreInputAfterReconnect(uuid: uuid, input: returnInput) } + } + } if case .success = result { // Not proof of recovery (see verifyBackOnline); record drops either way, since // keeping it would have reconcile switch the display back off the moment it appears. @@ -388,6 +426,16 @@ final class PhysicalDisplayToggleService: ObservableObject { allDisplaysIncludingDisabled().first { uuid(for: $0) == record.uuid } } + private func preserveBlinkRecoveryIfUnresolved(_ result: Result, uuid: String, + sleepGuard: CGVirtualDisplay?) { + switch result { + case .failure(.timedOut), .failure(.configurationInProgress): + lingeringSleepGuard = sleepGuard + default: + removePendingSoftReconnect(uuid) + } + } + /// Disables then re-enables a display's framebuffer to force macOS to re-read a freshly /// written HiDPI override and re-enumerate modes, without a physical unplug. Leaves /// `disconnected` untouched (a re-enumeration blip, not a user disconnect); blinks even @@ -395,7 +443,7 @@ final class PhysicalDisplayToggleService: ObservableObject { /// portables. See docs/display-notes.md (softReconnect). @discardableResult func softReconnect(_ display: DisplayInfo) async -> Bool { - guard isSupported else { return false } + guard isSupported, !configurationGate.isPending else { return false } let blinkUUID = display.displayUUID // Two callers can race the same display; a fixed virtual-display identity can't be // created twice, so the first blink wins and the rest adopt its result. @@ -425,8 +473,9 @@ final class PhysicalDisplayToggleService: ObservableObject { // Persisted before disabling: if the app dies mid-toggle, recoverStrandedSoftReconnect // finds this at the next launch and finishes the job. addPendingSoftReconnect(blinkUUID) - guard case .success = await setEnabled(false, displayID: startID) else { - removePendingSoftReconnect(blinkUUID) + let disableResult = await setEnabled(false, displayID: startID) + guard case .success = disableResult else { + preserveBlinkRecoveryIfUnresolved(disableResult, uuid: blinkUUID, sleepGuard: sleepGuard) return false } // Wait for the framebuffer to actually drop before re-enabling (0.9s ceiling if the event never comes). @@ -482,61 +531,6 @@ final class PhysicalDisplayToggleService: ObservableObject { return true } - /// Runs SLSConfigureDisplayEnabled inside a CG transaction with `.permanently` (matching - /// Lunar BlackOut, screen_tune, BetterDisplay), which is what makes the disconnect stick. - private func setEnabled(_ enabled: Bool, displayID: CGDirectDisplayID) async -> Result { - let action = enabled ? "enable" : "disable" - let waited = DispatchTime.now() - // No DDC traffic while this transaction runs: WindowServer's enable can wait behind an - // in-flight I2C read and freeze the whole machine with it (issue #33). - // See docs/display-notes.md (PhysicalDisplayToggleService). - let releaseDDC = await DDCService.shared.hold() - defer { releaseDDC() } - let heldMs = Self.millisSince(waited) - if heldMs > Self.slowOpThresholdMs { - Self.log.notice("\(action, privacy: .public) \(displayID, privacy: .public): waited \(Int(heldMs), privacy: .public) ms for DDC to go idle") - } - let result: Result = await CGHelpers.runWithTimeout( - seconds: 10, fallback: .failure(.timedOut) - ) { - var config: CGDisplayConfigRef? - guard CGBeginDisplayConfiguration(&config) == .success, let cfg = config else { - Self.log.error("\(action, privacy: .public) \(displayID, privacy: .public): CGBeginDisplayConfiguration failed") - return .failure(.configurationFailed(.failure)) - } - let setErr = SLSConfigureDisplayEnabled(cfg, displayID, enabled) - guard setErr == .success else { - CGCancelDisplayConfiguration(cfg) - Self.log.error("\(action, privacy: .public) \(displayID, privacy: .public): SLSConfigureDisplayEnabled failed \(setErr.rawValue, privacy: .public)") - return .failure(.configurationFailed(setErr)) - } - // The call that can block: keeps running after the 10s wrapper gives up (it can - // only stop waiting, not cancel), and WindowServer holding it can stall the whole - // machine, not just Crisp (issue #33). Timed unconditionally for captures. - let committing = DispatchTime.now() - let complete = CGCompleteDisplayConfiguration(cfg, .permanently) - let commitMs = Self.millisSince(committing) - guard complete == .success else { - CGCancelDisplayConfiguration(cfg) - Self.log.error("\(action, privacy: .public) \(displayID, privacy: .public): commit failed \(complete.rawValue, privacy: .public) after \(Int(commitMs), privacy: .public) ms") - return .failure(.configurationFailed(complete)) - } - if commitMs > Self.slowOpThresholdMs { - Self.log.notice("slow \(action, privacy: .public) \(displayID, privacy: .public): commit took \(Int(commitMs), privacy: .public) ms") - } - return .success(()) - } - // Not the same as what WindowServer will actually do: on a wrapper timeout this - // reports failure at ~10000ms while the commit keeps running. - let waitedMs = Self.millisSince(waited) - if case .failure = result { - Self.log.notice("\(action, privacy: .public) \(displayID, privacy: .public): reported failure after \(Int(waitedMs), privacy: .public) ms") - } else { - Self.log.notice("\(action, privacy: .public) \(displayID, privacy: .public): reported success after \(Int(waitedMs), privacy: .public) ms") - } - return result - } - /// Safety net for softReconnect's re-enable retries all failing: sweeps every SLS-disabled /// display Crisp didn't disconnect on purpose, so a transient failure can't leave a screen /// stuck black. Leaves other apps' intentional disables alone. @@ -632,6 +626,11 @@ final class PhysicalDisplayToggleService: ObservableObject { /// drops the record if it can't take, so the list never claims a lit display is /// disconnected. See docs/display-notes.md (reconcile). func reconcile() { + if configurationGate.isPending { + reconcileWhenSettled = true + return + } + guard reapplyInFlight.isEmpty else { reconcileWhenSettled = true; return } guard !disconnected.isEmpty else { return } let onlineIDs = onlineDisplayIDs() let onlineUUIDs = Set(onlineIDs.map { uuid(for: $0) }) @@ -666,6 +665,10 @@ final class PhysicalDisplayToggleService: ObservableObject { guard let self else { return } await self.reapplyRemembered(record.uuid) self.reapplyInFlight.remove(record.uuid) + if self.reapplyInFlight.isEmpty, self.reconcileWhenSettled, !self.configurationGate.isPending { + self.reconcileWhenSettled = false + self.displayManager?.refreshDisplays() + } } } } @@ -697,16 +700,24 @@ final class PhysicalDisplayToggleService: ObservableObject { guard !reconnectInFlight.contains(recordUUID) else { return } guard let liveID = onlineDisplayIDs().first(where: { uuid(for: $0) == recordUUID }) else { return } // gone again by itself; the record still stands for next time - let refused = wouldLeaveNoActiveDisplay(liveID) + var refused = wouldLeaveNoActiveDisplay(liveID) Self.log.notice("remembered disconnect for \(recordUUID, privacy: .public) id \(liveID, privacy: .public): \(refused ? "refused, it is the last active display" : "re-applying", privacy: .public)") var stillOnline = true var timedOut = false if !refused { // Same arrangement-move risk as disconnect() (#108); restore targets the modes // from before the display resurfaced (baselineModes), not the ones its own enable produced. - if case .failure(.timedOut) = await setEnabled(false, displayID: liveID) { + let result = await setEnabled(false, displayID: liveID, expectedUUID: recordUUID) + if case .failure(.configurationInProgress) = result { + reconcileWhenSettled = true + return + } + if case .failure(.timedOut) = result { timedOut = true } + if case .failure(.wouldLeaveNoActiveDisplay) = result { + refused = true + } // Started before the verify below (which can hold for seconds) so the visible flip // stays short; a no-op if nothing actually moved. startPassRestoreIfNeeded() @@ -965,3 +976,92 @@ final class PhysicalDisplayToggleService: ObservableObject { // refresh after launch re-applies it through reconcile(), where the safety rails are. } } + +// MARK: - Blocking Configuration Boundary + +extension PhysicalDisplayToggleService { + /// Runs SLSConfigureDisplayEnabled inside a CG transaction with `.permanently` (matching + /// Lunar BlackOut, screen_tune, BetterDisplay), which is what makes the disconnect stick. + private func setEnabled(_ enabled: Bool, displayID: CGDirectDisplayID, expectedUUID: String? = nil, + onLateCompletion: (@MainActor @Sendable (Result) -> Void)? = nil) + async -> Result { + guard configurationGate.begin() else { return .failure(.configurationInProgress) } + configurationInProgress = true + let gate = configurationGate + let requestedUUID = expectedUUID ?? uuid(for: displayID) + let action = enabled ? "enable" : "disable" + let waited = DispatchTime.now() + // No DDC traffic while this transaction runs: WindowServer's enable can wait behind an + // in-flight I2C read and freeze the whole machine with it (issue #33). + // See docs/display-notes.md (PhysicalDisplayToggleService). + let releaseDDC = await DDCService.shared.hold() + let targetID: CGDirectDisplayID + if enabled || expectedUUID == nil { + targetID = displayID + } else if let currentID = onlineDisplayIDs().first(where: { uuid(for: $0) == requestedUUID }), + !wouldLeaveNoActiveDisplay(currentID) { + targetID = currentID + } else { + releaseDDC() + gate.finish() + configurationInProgress = false + return .failure(.wouldLeaveNoActiveDisplay) + } + let heldMs = Self.millisSince(waited) + if heldMs > Self.slowOpThresholdMs { + Self.log.notice("\(action, privacy: .public) \(displayID, privacy: .public): waited \(Int(heldMs), privacy: .public) ms for DDC to go idle") + } + let result: Result = await CGHelpers.runWithTimeout( + seconds: 10, fallback: .failure(.timedOut), + onOperationFinished: { [weak self] result, late in + releaseDDC() + if !late { gate.finish() } + Task { @MainActor in + if late { onLateCompletion?(result); gate.finish() } + guard let self else { return } + self.configurationInProgress = gate.isPending + if late || self.reconcileWhenSettled { + self.reconcileWhenSettled = false + self.displayManager?.refreshDisplays() + } + } + } + ) { + var config: CGDisplayConfigRef? + guard CGBeginDisplayConfiguration(&config) == .success, let cfg = config else { + Self.log.error("\(action, privacy: .public) \(displayID, privacy: .public): CGBeginDisplayConfiguration failed") + return .failure(.configurationFailed(.failure)) + } + let setErr = SLSConfigureDisplayEnabled(cfg, targetID, enabled) + guard setErr == .success else { + CGCancelDisplayConfiguration(cfg) + Self.log.error("\(action, privacy: .public) \(displayID, privacy: .public): SLSConfigureDisplayEnabled failed \(setErr.rawValue, privacy: .public)") + return .failure(.configurationFailed(setErr)) + } + // The call that can block: keeps running after the 10s wrapper gives up (it can + // only stop waiting, not cancel), and WindowServer holding it can stall the whole + // machine, not just Crisp (issue #33). Timed unconditionally for captures. + let committing = DispatchTime.now() + let complete = CGCompleteDisplayConfiguration(cfg, .permanently) + let commitMs = Self.millisSince(committing) + guard complete == .success else { + CGCancelDisplayConfiguration(cfg) + Self.log.error("\(action, privacy: .public) \(displayID, privacy: .public): commit failed \(complete.rawValue, privacy: .public) after \(Int(commitMs), privacy: .public) ms") + return .failure(.configurationFailed(complete)) + } + if commitMs > Self.slowOpThresholdMs { + Self.log.notice("slow \(action, privacy: .public) \(displayID, privacy: .public): commit took \(Int(commitMs), privacy: .public) ms") + } + return .success(()) + } + // Not the same as what WindowServer will actually do: on a wrapper timeout this + // reports failure at ~10000ms while the commit keeps running. + let waitedMs = Self.millisSince(waited) + if case .failure = result { + Self.log.notice("\(action, privacy: .public) \(displayID, privacy: .public): reported failure after \(Int(waitedMs), privacy: .public) ms") + } else { + Self.log.notice("\(action, privacy: .public) \(displayID, privacy: .public): reported success after \(Int(waitedMs), privacy: .public) ms") + } + return result + } +} diff --git a/Crisp/Views/PhysicalDisplayToggleView.swift b/Crisp/Views/PhysicalDisplayToggleView.swift index 8285993..f36211e 100644 --- a/Crisp/Views/PhysicalDisplayToggleView.swift +++ b/Crisp/Views/PhysicalDisplayToggleView.swift @@ -1,9 +1,6 @@ import SwiftUI import CoreGraphics -/// Per-display "Disconnect Display" control (Apple Silicon only), hidden when -/// disconnecting would leave no active screen. Removes the display from the -/// layout via SkyLight; it then reappears in ReconnectDisplaysSection. struct DisconnectDisplayRow: View { @ObservedObject var display: DisplayInfo @EnvironmentObject var displayManager: DisplayManager @@ -13,58 +10,72 @@ struct DisconnectDisplayRow: View { @State private var errorMessage: String? var body: some View { - if service.isSupported, !service.wouldLeaveNoActiveDisplay(display.displayID) { + if service.isSupported, !VirtualDisplayService.shared.isVirtualDisplay(display.displayID) { VStack(alignment: .leading, spacing: 0) { - HStack { - MenuItemIcon(systemName: "rectangle.slash", color: .orange, active: false) - Text("Disconnect Display") - .font(.body) - Spacer() - if busy { + Toggle(isOn: Binding( + get: { !service.isDisconnected(uuid: display.displayUUID) }, + set: { setConnected($0) } + )) { + HStack(spacing: 8) { + MenuItemIcon(systemName: "display", color: .blue, + active: !service.isDisconnected(uuid: display.displayUUID)) + .accessibilityHidden(true) + Text("Display Connection").font(.body) + Spacer() + } + } + .toggleStyle(.switch) + .controlSize(.small) + .disabled(busy || service.configurationInProgress || service.wouldLeaveNoActiveDisplay(display.displayID)) + .help(service.wouldLeaveNoActiveDisplay(display.displayID) + ? String(localized: "Refusing to disconnect: it would leave no active display.") + : String(localized: "Display Connection")) + .accessibilityLabel("Display Connection") + .overlay(alignment: .trailing) { + if busy || service.configurationInProgress { ProgressView().scaleEffect(0.6).frame(width: 16, height: 16) + .frame(width: 32, height: 20) } } .padding(.horizontal, 12) .padding(.vertical, 3) .menuRowHover(isHovered) .onHover { isHovered = $0 } - .contentShape(Rectangle()) - .onTapGesture { - guard !busy else { return } - busy = true - errorMessage = nil - Task { @MainActor in - let result = await service.disconnect(display) - displayManager.refreshDisplays() - if case .failure(let err) = result { - errorMessage = err.description - Task { @MainActor in - try? await Task.sleep(nanoseconds: 3_000_000_000) - errorMessage = nil - } - } - busy = false - } - } - // Outlives a replug and reboot (PhysicalDisplayToggleService.reconcile); - // nothing outside this menu shows the display's state, so it's said here. Text("Stays disconnected until you reconnect it here.") .font(.caption) .foregroundColor(.secondary) .padding(.horizontal, 12) .padding(.bottom, 4) - if let msg = errorMessage { - Text(msg) + if let errorMessage { + Text(errorMessage) .font(.caption) .foregroundColor(.red) + .fixedSize(horizontal: false, vertical: true) .padding(.horizontal, 12) .padding(.bottom, 4) } } } } + + private func setConnected(_ connected: Bool) { + guard PanelOpenGuard.allowsActivation, !busy else { return } + busy = true + errorMessage = nil + Task { @MainActor in + let result: Result + if connected { + result = await InputSwitchService.shared.reconnect(uuid: display.displayUUID) + } else { + result = await service.disconnect(display) + } + displayManager.refreshDisplays() + if case .failure(let error) = result { errorMessage = error.description } + busy = false + } + } } /// Inline "Disconnected" section for the main display list: lists displays the @@ -73,6 +84,7 @@ struct ReconnectDisplaysSection: View { @EnvironmentObject var displayManager: DisplayManager @ObservedObject private var service = PhysicalDisplayToggleService.shared @State private var busyUUIDs: Set = [] + @State private var errors: [String: String] = [:] var body: some View { if !service.disconnected.isEmpty { @@ -87,9 +99,16 @@ struct ReconnectDisplaysSection: View { ForEach(service.disconnected) { record in DisconnectedDisplayRow( record: record, - busy: busyUUIDs.contains(record.uuid), + busy: busyUUIDs.contains(record.uuid) || service.configurationInProgress, onReconnect: { reconnect(record) } ) + if let error = errors[record.uuid] { + Text(error) + .font(.caption) + .foregroundColor(.red) + .fixedSize(horizontal: false, vertical: true) + .padding(.horizontal, 12) + } } } } @@ -98,16 +117,16 @@ struct ReconnectDisplaysSection: View { private func reconnect(_ record: PhysicalDisplayToggleService.DisconnectedDisplay) { guard !busyUUIDs.contains(record.uuid) else { return } busyUUIDs.insert(record.uuid) + errors[record.uuid] = nil Task { @MainActor in - _ = await InputSwitchService.shared.reconnect(uuid: record.uuid) + let result = await InputSwitchService.shared.reconnect(uuid: record.uuid) + if case .failure(let error) = result { errors[record.uuid] = error.description } displayManager.refreshDisplays() busyUUIDs.remove(record.uuid) } } } -/// One disconnected-display row: the whole row is tappable to reconnect (like -/// a network in the native Wi-Fi menu), with a "Reconnect" hint always visible. private struct DisconnectedDisplayRow: View { let record: PhysicalDisplayToggleService.DisconnectedDisplay let busy: Bool @@ -115,33 +134,35 @@ private struct DisconnectedDisplayRow: View { @State private var isHovered = false var body: some View { - HStack(spacing: 8) { - // Dim while inactive; brightens on hover to cue that a click reconnects it. + Toggle(isOn: Binding( + get: { false }, + set: { connected in + guard connected, !busy, PanelOpenGuard.allowsActivation else { return } + onReconnect() + } + )) { HStack(spacing: 8) { - MenuItemIcon(systemName: "rectangle.slash", color: .secondary, active: false) - VStack(alignment: .leading, spacing: 1) { - Text(record.name).font(.body).lineLimit(1) - Text(verbatim: "\(record.width)×\(record.height)") - .font(.caption2).foregroundColor(.secondary) + HStack(spacing: 8) { + MenuItemIcon(systemName: "rectangle.slash", color: .secondary, active: false) + .accessibilityHidden(true) + VStack(alignment: .leading, spacing: 1) { + Text(record.name).font(.body).lineLimit(1) + Text(verbatim: "\(record.width)×\(record.height)") + .font(.caption2).foregroundColor(.secondary) + } } + .opacity(isHovered ? 1 : 0.6) + Spacer() } - .opacity(isHovered ? 1 : 0.6) - - Spacer() - + } + .toggleStyle(.switch) + .controlSize(.small) + .disabled(busy) + .help("Reconnect this display") + .overlay(alignment: .trailing) { if busy { ProgressView().scaleEffect(0.6).frame(width: 16, height: 16) - } else { - // Liquid-Glass accent capsule; reads as the affordance since the - // whole row is the tap target. - Text("Reconnect") - .font(.caption).fontWeight(.medium) - .foregroundColor(.accentColor) - .padding(.horizontal, 10) - .padding(.vertical, 4) - .background( - Capsule().fill(Color.accentColor.opacity(isHovered ? 0.22 : 0.12)) - ) + .frame(width: 32, height: 20) } } .padding(.horizontal, 12) @@ -149,13 +170,7 @@ private struct DisconnectedDisplayRow: View { .menuRowHover(isHovered) .contentShape(Rectangle()) .onHover { isHovered = $0 } - .onTapGesture { - guard !busy else { return } - onReconnect() - } - .accessibilityElement(children: .combine) .accessibilityLabel("\(record.name), disconnected") .accessibilityHint("Reconnect this display") - .accessibilityAddTraits(.isButton) } } diff --git a/CrispTests/DisplayConfigurationGateTests.swift b/CrispTests/DisplayConfigurationGateTests.swift new file mode 100644 index 0000000..e7a054b --- /dev/null +++ b/CrispTests/DisplayConfigurationGateTests.swift @@ -0,0 +1,93 @@ +import XCTest +import Foundation + +private final class SimulatedConfiguration: @unchecked Sendable { + private let lock = NSLock() + private var count = 2 + private var peak = 0 + private var activeTransactions = 0 + private var completedLate = false + let started = DispatchSemaphore(value: 0) + let release = DispatchSemaphore(value: 0) + + var activeScreens: Int { lock.withLock { count } } + var peakTransactions: Int { lock.withLock { peak } } + var wasLate: Bool { lock.withLock { completedLate } } + + func commit() -> Bool { + lock.withLock { activeTransactions += 1; peak = max(peak, activeTransactions) } + started.signal() + _ = release.wait(timeout: .now() + 2) + lock.withLock { count -= 1; activeTransactions -= 1 } + return true + } + + func record(late: Bool) { lock.withLock { completedLate = late } } +} + +final class DisplayConfigurationGateTests: XCTestCase { + func testOnlyOneLeaseCanBeActive() { + let gate = DisplayConfigurationGate() + XCTAssertTrue(gate.begin()) + XCTAssertTrue(gate.isPending) + XCTAssertFalse(gate.begin()) + gate.finish() + XCTAssertFalse(gate.isPending) + XCTAssertTrue(gate.begin()) + gate.finish() + } + + @MainActor + func testTimeoutCannotReleaseTheGuardForAnotherDisconnect() async { + let queue = DisplayConnectionQueue() + let gate = DisplayConfigurationGate() + let state = SimulatedConfiguration() + let disconnect: @MainActor @Sendable () async -> Bool = { + guard state.activeScreens > 1, gate.begin() else { return false } + return await CGHelpers.runWithTimeout(seconds: 0.02, fallback: false, onOperationFinished: { _, late in + Task { @MainActor in state.record(late: late); gate.finish() } + }) { state.commit() } + } + let first = await queue.run(disconnect) + XCTAssertFalse(first) + XCTAssertTrue(state.started.wait(timeout: .now() + 1) == .success) + XCTAssertTrue(gate.isPending) + let second = await queue.run(disconnect) + XCTAssertFalse(second) + XCTAssertEqual(state.peakTransactions, 1) + state.release.signal() + for _ in 0..<100 where gate.isPending { try? await Task.sleep(nanoseconds: 10_000_000) } + XCTAssertFalse(gate.isPending) + XCTAssertTrue(state.wasLate) + XCTAssertEqual(state.activeScreens, 1) + XCTAssertTrue(gate.begin(), "Real completion must unblock later requests") + gate.finish() + } + + @MainActor + func testSuccessfulOperationCompletesBeforeItsReturn() async { + let gate = DisplayConfigurationGate() + XCTAssertTrue(gate.begin()) + let result = await CGHelpers.runWithTimeout(seconds: 1, fallback: false, + onOperationFinished: { _, late in XCTAssertFalse(late); gate.finish() }) { true } + XCTAssertTrue(result) + XCTAssertFalse(gate.isPending) + } + + @MainActor + func testQueuedOperationPinsIdentityAtSubmission() async { + let queue = DisplayConnectionQueue() + var liveIdentity = "requested-display" + let blocker = Task { @MainActor in + await queue.run { try? await Task.sleep(nanoseconds: 20_000_000) } + } + await Task.yield() + let requestedIdentity = liveIdentity + let request = Task { @MainActor in await queue.run { requestedIdentity } } + liveIdentity = "reassigned-display" + await blocker.value + let identity = await request.value + XCTAssertEqual(identity, "requested-display") + XCTAssertEqual(liveIdentity, "reassigned-display") + } +} diff --git a/CrispTests/DisplayConnectionQueueTests.swift b/CrispTests/DisplayConnectionQueueTests.swift new file mode 100644 index 0000000..6ec2149 --- /dev/null +++ b/CrispTests/DisplayConnectionQueueTests.swift @@ -0,0 +1,35 @@ +import XCTest + +final class DisplayConnectionQueueTests: XCTestCase { + @MainActor + func testOverlappingDisconnectsRecheckTheLastScreenAfterPreviousChange() async { + let queue = DisplayConnectionQueue() + var activeScreens = 2 + var inFlight = 0 + var peakInFlight = 0 + let disconnect: @MainActor () async -> Bool = { + guard activeScreens > 1 else { return false } + inFlight += 1 + peakInFlight = max(peakInFlight, inFlight) + try? await Task.sleep(nanoseconds: 10_000_000) + activeScreens -= 1 + inFlight -= 1 + return true + } + async let first = queue.run(disconnect) + async let second = queue.run(disconnect) + let results = await [first, second] + XCTAssertEqual(results.filter { $0 }.count, 1) + XCTAssertEqual(activeScreens, 1) + XCTAssertEqual(peakInFlight, 1) + } + + @MainActor + func testQueueContinuesAfterARefusedOperation() async { + let queue = DisplayConnectionQueue() + let refused = await queue.run { false } + let reconnected = await queue.run { true } + XCTAssertFalse(refused) + XCTAssertTrue(reconnected) + } +} diff --git a/project.yml b/project.yml index 94b36b4..5f55db3 100644 --- a/project.yml +++ b/project.yml @@ -86,6 +86,9 @@ targets: - path: Crisp/Models/DDCVolumeMax.swift - path: Crisp/Models/EdgeCrossing.swift - path: Crisp/Models/DDCInputSource.swift + - path: Crisp/Models/DisplayConnectionQueue.swift + - path: Crisp/Models/DisplayConfigurationGate.swift + - path: Crisp/Services/CGHelpers.swift settings: base: PRODUCT_BUNDLE_IDENTIFIER: com.crisp.tests