diff --git a/Candela/App/GammaReconfigurationRecovery.swift b/Candela/App/GammaReconfigurationRecovery.swift new file mode 100644 index 00000000..7301e4c0 --- /dev/null +++ b/Candela/App/GammaReconfigurationRecovery.swift @@ -0,0 +1,181 @@ +import AppKit +import CandelaKit +import os + +/// A burst of screen notifications shares one deadline and write allowance. +/// The final pass and the first subsequent topology change each start a window. +struct GammaRecoveryBudget { + private var deadline: TimeInterval? + private var writesRemaining = 8 + + mutating func begin(at now: TimeInterval) -> Bool { + if deadline == nil { deadline = now + 5 } + return now < deadline! && writesRemaining > 0 + } + + mutating func recordWrite() { writesRemaining = max(0, writesRemaining - 1) } + mutating func finish() { deadline = nil; writesRemaining = 8 } +} + +/// WindowServer can reset gamma after the screen notification but before the +/// topology debounce completes, or several seconds after the final pass. +/// Follow a bounded settling interval, restoring +/// only a cached baseline reset on the same directly drawn SDR display. +@MainActor +final class GammaReconfigurationRecovery { + private struct HDRObservation: Sendable { let enabled: Bool? } + private struct Replies: Sendable { + var generation: UInt64 = 0 + var values: [CGDirectDisplayID: HDRObservation] = [:] + } + + private let gamma: GammaController + private let targets: @MainActor () -> [CGDirectDisplayID] + private let readHDR: @Sendable (CGDirectDisplayID) async -> Bool? + private let epoch: @Sendable () -> UInt64 + private let asleep: @Sendable () -> Bool + private let now: @MainActor () -> TimeInterval + private let interval: TimeInterval + private let replies = OSAllocatedUnfairLock(initialState: Replies()) + private var hdrTask: Task? + private var timer: Timer? + private var candidates: [CGDirectDisplayID: GammaController.RecoverySnapshot] = [:] + private var generation: UInt64 = 0 + private var observedEpoch: UInt64 = 0 + private var settledEpoch: UInt64? + private var budget = GammaRecoveryBudget() + private var inFinalPass = false + private var pendingNotification = false + private var stopped = false + private static let log = Logger(subsystem: "com.rydersel.Candela", category: "gamma") + + init( + gamma: GammaController, targets: @escaping @MainActor () -> [CGDirectDisplayID], + readHDR: @escaping @Sendable (CGDirectDisplayID) async -> Bool?, + epoch: @escaping @Sendable () -> UInt64, asleep: @escaping @Sendable () -> Bool, + now: @escaping @MainActor () -> TimeInterval = { ProcessInfo.processInfo.systemUptime }, + interval: TimeInterval = 1.0 / 120.0 + ) { + self.gamma = gamma; self.targets = targets; self.readHDR = readHDR + self.epoch = epoch; self.asleep = asleep; self.now = now; self.interval = interval + // A startup screen notification can precede the first actual reconfiguration. + self.settledEpoch = epoch() + } + + @discardableResult + func begin() -> Task? { + guard !stopped else { return nil } + guard !inFinalPass else { pendingNotification = true; return nil } + cancelPending() + guard !asleep() else { budget.finish(); return nil } + let currentEpoch = epoch() + if let settledEpoch, currentEpoch != settledEpoch { + // A departure's settling tail must not consume the next arrival's + // deadline. Renew once; further events share the new burst's budget. + self.settledEpoch = nil + budget.finish() + } + guard budget.begin(at: now()) else { + Self.log.debug("Gamma recovery notification declined: deadline or write allowance exhausted") + return nil + } + observedEpoch = currentEpoch + for id in targets() { + if let snapshot = gamma.recoverySnapshot(on: id) { candidates[id] = snapshot } + } + Self.log.debug("Gamma recovery prepared \(self.candidates.count, privacy: .public) candidates at epoch \(self.observedEpoch, privacy: .public)") + guard !candidates.isEmpty else { return nil } + let ids = Array(candidates.keys) + let generation = generation + let replies = replies + let readHDR = readHDR + let log = Self.log + // No return hop to the main actor: it may be inside menu tracking. A + // common-mode timer consumes this mailbox even while the menu is open. + hdrTask = Task.detached { + for id in ids { + guard !Task.isCancelled else { return } + let enabled = await readHDR(id) + log.debug("Gamma recovery HDR reply for display \(id, privacy: .public), generation \(generation, privacy: .public): \(String(describing: enabled), privacy: .public)") + replies.withLock { state in + guard state.generation == generation else { return } + state.values[id] = HDRObservation(enabled: enabled) + } + } + } + let timer = Timer(timeInterval: interval, repeats: true) { [weak self] _ in + MainActor.assumeIsolated { self?.tick() } + } + self.timer = timer + RunLoop.main.add(timer, forMode: .common) + return hdrTask + } + + func tick() { + guard !stopped, !inFinalPass else { return } + guard !asleep() else { cancelPending(); budget.finish(); return } + guard epoch() == observedEpoch else { begin(); return } + guard budget.begin(at: now()) else { + Self.log.debug("Gamma recovery timer stopped: deadline or write allowance exhausted") + cancelPending(); return + } + let observations = replies.withLock { $0.values } + for (id, snapshot) in Array(candidates) { + guard let observation = observations[id] else { continue } + // The raw CG callback bumps this synchronously, including on HDR changes. + guard epoch() == observedEpoch else { begin(); return } + guard !asleep(), budget.begin(at: now()) else { + cancelPending(); return + } + switch gamma.recoverBaselineIfReset(snapshot, hdrEnabled: observation.enabled) { + case .unchanged: break + case .written: + budget.recordWrite() + Self.log.info("Reconfiguration gamma table reasserted for display \(id, privacy: .public)") + case .superseded: + // Capture the newer successful owner and obtain fresh HDR observations. + // begin() preserves this burst's deadline and remaining write allowance. + begin() + return + case .stopped: + gamma.cancelRecovery(snapshot) + candidates.removeValue(forKey: id) + } + } + if candidates.isEmpty { cancelPending() } + } + + func beginFinalPass() { + Self.log.debug("Gamma recovery paused for final topology pass") + inFinalPass = true + pendingNotification = false + cancelPending() + budget.finish() + } + + @discardableResult + func endFinalPass() -> Task? { + Self.log.debug("Gamma recovery final topology pass ended; pending notification: \(self.pendingNotification, privacy: .public)") + inFinalPass = false + pendingNotification = false + settledEpoch = epoch() + // WindowServer has been observed resetting gamma several seconds after + // this pass. Watch its fresh successful writes through that settling tail. + return begin() + } + + func stop() { + stopped = true + cancelPending() + budget.finish() + } + + private func cancelPending() { + generation &+= 1 + let generation = generation + replies.withLock { $0 = Replies(generation: generation) } + hdrTask?.cancel(); hdrTask = nil + timer?.invalidate(); timer = nil + candidates.removeAll() + } +} diff --git a/Candela/App/KeyActionExecutor.swift b/Candela/App/KeyActionExecutor.swift index 37a64406..f1451583 100644 --- a/Candela/App/KeyActionExecutor.swift +++ b/Candela/App/KeyActionExecutor.swift @@ -6,11 +6,36 @@ import CandelaKit final class KeyActionExecutor { private let model: AppModel private let hud: (any BrightnessHUDPresenting)? - private let feedback = VolumeFeedbackSound() + private let feedback: any VolumeFeedbackPlaying + /// Injected so the deep-link cases can run without opening System Settings on + /// the machine running the suite. + private let openURL: (URL) -> Void - init(model: AppModel, hud: (any BrightnessHUDPresenting)?) { + /// Armed by a volume step, spent by the next release, and what decides whether + /// that release blips: Option plus a volume key opens Sound settings instead of + /// stepping, and the release still routes the feedback trigger. + /// + /// ONE latch, not one per key, because `.volumeKeyUp` carries no key identity. + /// The two differ only while volume up and volume down are held together, where + /// this blips for the first release and not the second. It lives on `execute` + /// because both key-down paths reach it, the media-key tap through `KeyRouter` + /// and `ShortcutManager` directly, so `ShortcutManager` must not grow a second. + /// + /// Known gap: `ShortcutManager` re-reads the volume key mode on the release, so + /// a mode change mid-press swallows that release and the next one spends the + /// latch on a step that never happened. The step after re-arms correctly. + private var feedbackArmedByStep = false + + init( + model: AppModel, + hud: (any BrightnessHUDPresenting)?, + feedback: any VolumeFeedbackPlaying = VolumeFeedbackSound(), + openURL: @escaping (URL) -> Void = { NSWorkspace.shared.open($0) } + ) { self.model = model self.hud = hud + self.feedback = feedback + self.openURL = openURL } /// `isFresh` separates a fresh press from key-repeat: mute toggling and the @@ -80,7 +105,8 @@ final class KeyActionExecutor { } case let .stepVolume(isUp, isFine): // No feedback sound here: it plays on key RELEASE (.volumeKeyUp), fork - // parity. + // parity. Armed before targets resolve; the release re-asks availability. + feedbackArmedByStep = true var stepped: [(state: AppModel.DisplayState, value: Double)] = [] for state in resolveVolumeTargets() { guard let newValue = state.volume.step(isUp: isUp, isFine: isFine) else { continue } @@ -110,8 +136,12 @@ final class KeyActionExecutor { showVolumeHUDs(toggled) case .volumeKeyUp: // Fork rule: volume steps play feedback on key release, once per event, - // only when some affected display has volume enabled. - if resolveVolumeTargets().contains(where: { $0.volume.isAvailable }) { + // only when some affected display has volume enabled. The latch is spent + // whether or not the sound played, so an unavailable display cannot leave + // a later release armed. + let wasStep = feedbackArmedByStep + feedbackArmedByStep = false + if wasStep, resolveVolumeTargets().contains(where: { $0.volume.isAvailable }) { feedback.play() } case let .stepContrast(isUp, isFine): @@ -130,13 +160,13 @@ final class KeyActionExecutor { // by the brightness keys, and only volume and mute get their own place. showStateHUDs(stepped, position: appPrefs.hudPositionBrightness) { _ in .contrast } case .openSoundSettings: - NSWorkspace.shared.open( - URL(string: "x-apple.systempreferences:com.apple.Sound-Settings.extension")! - ) + // This key-down was a deep link, not a step. Option plus MUTE lands here + // too, harmlessly: a mute release routes nothing, so nothing spends the + // disarm and the next volume step re-arms. + feedbackArmedByStep = false + openURL(URL(string: "x-apple.systempreferences:com.apple.Sound-Settings.extension")!) case .openDisplaysSettings: - NSWorkspace.shared.open( - URL(string: "x-apple.systempreferences:com.apple.Displays-Settings.extension")! - ) + openURL(URL(string: "x-apple.systempreferences:com.apple.Displays-Settings.extension")!) case .none: break } diff --git a/Candela/App/StatusItemController.swift b/Candela/App/StatusItemController.swift index 95a37687..e8203429 100644 --- a/Candela/App/StatusItemController.swift +++ b/Candela/App/StatusItemController.swift @@ -135,6 +135,15 @@ final class StatusItemController: NSObject, NSApplicationDelegate, NSMenuDelegat /// owns a block-based notification registration, and dropping it freezes the /// store at the launch sample with nothing saying so. private lazy var mirrorSampler = MirrorTopologySampler(store: model.mirrorTopology) + private var gammaRecoveryObserver: (any NSObjectProtocol)? + private lazy var gammaRecovery: GammaReconfigurationRecovery = { + let manager = model.displayManager + let hdr = model.hdrToggling + return GammaReconfigurationRecovery( + gamma: gammaController, targets: { [weak model = model] in model?.displays.map(\.id) ?? [] }, + readHDR: { await hdr.observedHDREnabled(displayID: $0) }, + epoch: { manager.currentEpoch() }, asleep: { manager.isAsleep }) + }() private let log = Logger(subsystem: "com.rydersel.Candela", category: "keys") private let checkupLog = Logger(subsystem: "com.rydersel.Candela", category: "checkup") @@ -247,11 +256,34 @@ final class StatusItemController: NSObject, NSApplicationDelegate, NSMenuDelegat // the identity function. Launching into an already-engaged mirror set is an // ordinary way to start. mirrorSampler.start() + if !isSafeMode { + gammaRecoveryObserver = NotificationCenter.default.addObserver( + forName: NSApplication.didChangeScreenParametersNotification, object: nil, queue: nil + ) { [weak self] _ in + // AppKit delivers this on main; a queued actor hop may wait until an + // open menu stops tracking, leaving the reset visible the whole time. + MainActor.assumeIsolated { + guard let self else { return } + self.mirrorSampler.refresh() + self.gammaRecovery.begin() + } + } + } // Reconfiguration intake: synchronous registration on the main thread is // load-bearing, since CG delivers the callback on the registering thread's // run loop and only the main thread has one that lives forever. - model.displayManager.activate() + model.displayManager.activate { [weak self] flags in + guard !flags.contains(.beginConfigurationFlag) else { return } + // AppKit's screen notification can lag the reset. Queue only a signal + // here; inspect displays after the CG callback returns, even in a menu. + RunLoop.main.perform(inModes: [.common]) { [weak self] in + MainActor.assumeIsolated { + guard let self, !self.isSafeMode else { return } + self.gammaRecovery.begin() + } + } + } // Starts the OLED care driver loop (Safe Mode still builds the chrome // controller) and wires its lock and sleep/wake observers. Display membership @@ -513,43 +545,13 @@ final class StatusItemController: NSObject, NSApplicationDelegate, NSMenuDelegat // The counter zeroes on every configure so unrelated events across a long // session never add up to an offer. `suspendedForSession` survives. self.interferenceMonitor.resetCounter() - // HDR state may have changed under the 2 s cache, since a mode switch is - // itself a reconfiguration. Dropped BEFORE the per-display re-evaluation - // so fresh state is read. - await self.model.hdrToggling.displaysReconfigured() - // Gamma reset once per event, before any recapture (reset → - // recapture → re-apply; recapture must see an OS-owned table). Done - // here rather than per display so a later display's - // reset cannot wipe an earlier display's just-reapplied dim. - self.gammaController.resetAllGamma() - // Shades reset the same way, for a reason the gamma line does not have: a - // shade is keyed by the DRAWABLE id, so a topology change MOVES ITS KEY. - // - // A mirror ENGAGING is the direction that strands one. A display dimming - // under its own key becomes a slave, its controller resolves to the master - // from that instant, the key is never named again, and `repinFrames()` - // skips it (a slave has no `NSScreen`). What is left is a full-screen - // black window at `CGShieldingWindowLevel()` holding its last dim alpha - // over a display with no desktop, with no way out short of quitting. - // - // A mirror BREAKING strands nothing: the ex-master re-names its own key - // and the ex-slave gets a fresh shade. Stated so nobody narrows this call - // to that path; removing wholesale covers both, so it is unconditional. - // - // `MirroringCoordinator` performs the same teardown from the RAW - // screen-parameters notification, earlier than this debounced stream, so - // this is the backstop for a change that posts no notification. - // - // Safe wholesale because the loop below re-establishes it: - // `handleReconfigure` nils the software dedupe memo and re-runs the - // software leg, recreating the shade under the NEW drawable id. Displays - // with no shade (native path, pure DDC, the built-in slot) are - // unaffected. - self.shadeOverlay.removeAllShades() - for state in self.model.displays { - await state.controller.noteHDRStateMayHaveChanged() - await state.controller.handleReconfigure() - } + await ReconfigureDimming.run( + displays: self.model.displays, + hdrToggling: self.model.hdrToggling, + gamma: self.gammaController, + shade: self.shadeOverlay, + beforeReset: { self.gammaRecovery.beginFinalPass() }) + self.gammaRecovery.endFinalPass() #if DEBUG // Panel row model, last in the pass: the HDR state above and the // re-applied dimming are what a dump taken here can report. @@ -987,6 +989,7 @@ final class StatusItemController: NSObject, NSApplicationDelegate, NSMenuDelegat /// equivalent of the published brightness, so the monitor is not left at a /// combined-mode DDC floor. func applicationWillTerminate(_: Notification) { + gammaRecovery.stop() gammaController.resetAllGamma() // not DDC, always runs shadeOverlay.removeAllShades() // not DDC, always runs // Above the safe-mode guard: this writes a report, not DDC, and a run @@ -1626,3 +1629,80 @@ private final class PanelHostingView: NSHostingView { } } } + +/// The reconfiguration dimming pass, in the one order that does not flash. +/// Outside `StatusItemController` so the host-free suite can drive the order +/// without building an `NSStatusItem`. +@MainActor +enum ReconfigureDimming { + /// `displays` is ONE snapshot, a parameter rather than a re-read: both halves + /// of the pass must see the same set even if a display departs mid-pass. + static func run( + displays: [AppModel.DisplayState], + hdrToggling: any HDRToggling, + gamma: any GammaApplying, + shade: any ShadeRendering, + beforeReset: @MainActor () -> Void = {} + ) async { + // HDR state may have changed under the 2 s cache, since a mode switch is + // itself a reconfiguration. Dropped BEFORE the per-display re-evaluation + // so fresh state is read. + await hdrToggling.displaysReconfigured() + // The HDR re-evaluation runs HERE, above the reset and the removal, which is + // the point of the whole pass: each call awaits two `MonitorPanelService` + // reads and the actor re-enumerates MonitorPanel for both, because MPDisplay + // objects cannot be cached across a reconfiguration. Below the removal, that + // suspension is a visibly undimmed display for as long as it takes. + for state in displays { + await state.controller.noteHDRStateMayHaveChanged() + } + // Gamma reset once per event, above the recapture loop at the end (reset → + // recapture → re-apply; recapture must see an OS-owned table). Here rather + // than per display, so a later display's reset cannot wipe an earlier + // display's just-reapplied dim. + // + // The HDR pass above can capture a baseline LAZILY: a display entering HDR + // clears its software leg there, reaching `GammaController.applyGammaScale`, + // whose `defaultTable(for:)` captures the INSTALLED table whenever it holds + // none, and that table can be Candela's own scaled one. Safe only because + // `handleReconfigure` recaptures by default (this pass never asks it not to), + // above its native guard, and this reset runs first in the same pass, so a + // baseline with our curve baked in is replaced before anything scales against + // it. Moving either half breaks that. + // Early recovery stays active through the HDR awaits above. Pause only + // across the reset, baseline capture and immediate reapply below. + beforeReset() + gamma.resetAllGamma() + // Shades reset the same way, for a reason the gamma line does not have: a + // shade is keyed by the DRAWABLE id, so a topology change MOVES ITS KEY. + // + // A mirror ENGAGING is the direction that strands one. A display dimming + // under its own key becomes a slave, its controller resolves to the master + // from that instant, the key is never named again, and `repinFrames()` + // skips it (a slave has no `NSScreen`). What is left is a full-screen + // black window at `CGShieldingWindowLevel()` holding its last dim alpha + // over a display with no desktop, with no way out short of quitting. + // + // A mirror BREAKING strands nothing: the ex-master re-names its own key + // and the ex-slave gets a fresh shade. Stated so nobody narrows this call + // to that path; removing wholesale covers both, so it is unconditional. + // + // `MirroringCoordinator` performs the same teardown from the RAW + // screen-parameters notification, earlier than this debounced stream, so + // this is the backstop for a change that posts no notification. It also + // covers the delay this line carries: a shade reaching here has waited out + // every enumeration in the HDR pass above. + // + // Safe wholesale because the re-apply loop below re-establishes it: + // `handleReconfigure` nils the software dedupe memo and re-runs the + // software leg, recreating the shade under the NEW drawable id. Displays + // with no shade (native path, pure DDC, the built-in slot) are unaffected. + // Safe from a flash because that loop follows IMMEDIATELY and has no + // suspension point of its own, so nothing runs between this line and the + // dim going back on. + shade.removeAllShades() + for state in displays { + await state.controller.handleReconfigure() + } + } +} diff --git a/Candela/AppKitIslands/GammaController.swift b/Candela/AppKitIslands/GammaController.swift index 4b175cc3..fbaa8511 100644 --- a/Candela/AppKitIslands/GammaController.swift +++ b/Candela/AppKitIslands/GammaController.swift @@ -6,6 +6,7 @@ import AppKit import CandelaKit +import ColorSync import os /// One display's three transfer-table channels, however they were obtained. @@ -56,6 +57,9 @@ protocol GammaTableDriving: AnyObject { /// Park the 1×1 activity window on `displayID`. False when it has no screen. func moveEnforcer(to displayID: CGDirectDisplayID) -> Bool func enforceActivity() + /// A unique, live, unmirrored drawable display. Nil is ineligible for early + /// recovery; a CG display number alone can be reassigned during a replug. + func recoveryIdentity(on displayID: CGDirectDisplayID) -> String? } /// Software dimming by gamma-table scaling: the display's captured default @@ -89,6 +93,96 @@ final class GammaController: GammaApplying { /// preserved and repeated scales do not compound. private var defaultTables: [CGDirectDisplayID: GammaSamples] = [:] private var lastAppliedScale: [CGDirectDisplayID: Double] = [:] + private var writeGeneration: UInt64 = 0 + private var recoveryOwners: [CGDirectDisplayID: RecoveryOwner] = [:] + + private struct RecoveryOwner { + let identity: String + let generation: UInt64 + } + + struct RecoverySnapshot { + let displayID: CGDirectDisplayID + fileprivate let identity: String + fileprivate let generation: UInt64 + fileprivate let baseline: GammaSamples + fileprivate let expected: GammaSamples + } + + enum RecoveryResult { case unchanged, written, superseded, stopped } + + /// Only a previous successful, directly drawn gamma write can authorize + /// recovery. Never capture a new baseline while the display is settling. + func recoverySnapshot(on displayID: CGDirectDisplayID) -> RecoverySnapshot? { + guard let owner = recoveryOwners[displayID] else { return nil } + guard driver.recoveryIdentity(on: displayID) == owner.identity else { + Self.log.debug("Gamma recovery identity unavailable or changed for display \(displayID, privacy: .public)") + recoveryOwners[displayID] = nil + return nil + } + guard let baseline = defaultTables[displayID], + let scale = lastAppliedScale[displayID], scale < 1 + else { return nil } + return RecoverySnapshot( + displayID: displayID, identity: owner.identity, generation: owner.generation, + baseline: baseline, expected: baseline.scaled(by: CGGammaValue(scale))) + } + + /// A failed recovery must stay stopped across repeated notifications. An old + /// snapshot cannot revoke ownership established by a newer brightness write. + func cancelRecovery(_ snapshot: RecoverySnapshot) { + guard recoveryOwners[snapshot.displayID]?.generation == snapshot.generation else { return } + recoveryOwners[snapshot.displayID] = nil + } + + /// Reassert only a recognizable ColorSync reset, never an arbitrary curve + /// another app installed. The owner bounds retries to the reconfiguration. + func recoverBaselineIfReset(_ snapshot: RecoverySnapshot, hdrEnabled: Bool?) -> RecoveryResult { + let id = snapshot.displayID + guard hdrEnabled == false else { + Self.log.debug("Gamma recovery stopped: HDR or unknown HDR state, display \(id, privacy: .public)") + return .stopped + } + guard driver.recoveryIdentity(on: id) == snapshot.identity else { + Self.log.debug("Gamma recovery stopped: identity unavailable or changed, display \(id, privacy: .public)") + return .stopped + } + guard let owner = recoveryOwners[id], owner.identity == snapshot.identity else { return .stopped } + guard owner.generation == snapshot.generation else { + // The topology rebuild can write after recovery captured its snapshot. + // Report the handoff without writing from the stale snapshot or HDR read. + Self.log.debug("Gamma recovery owner superseded for display \(id, privacy: .public)") + return .superseded + } + guard case let .table(table) = driver.readTable(id, capacity: Self.sampleCapacity) else { + Self.log.debug("Gamma recovery stopped: table read failed, display \(id, privacy: .public)") + return .stopped + } + if Self.matches(table, snapshot.expected) { return .unchanged } + guard Self.matches(table, snapshot.baseline) else { + Self.log.debug("Gamma recovery stopped: unfamiliar curve, display \(id, privacy: .public)") + return .stopped + } + guard driver.moveEnforcer(to: id), driver.writeTable(id, snapshot.expected) == .success else { + Self.log.debug("Gamma recovery stopped: enforcer or table write failed, display \(id, privacy: .public)") + return .stopped + } + driver.enforceActivity() + return .written + } + + private static func matches(_ lhs: GammaSamples, _ rhs: GammaSamples) -> Bool { + // Full RGB shape, not just the peak used by the interference warning. + // Allow the transfer table's small quantization error on readback. + let tolerance: CGGammaValue = 1.0 / 1024.0 + for (a, b) in zip([lhs.red, lhs.green, lhs.blue], [rhs.red, rhs.green, rhs.blue]) { + guard !a.isEmpty, a.count == b.count else { return false } + for (x, y) in zip(a, b) { + guard x.isFinite, y.isFinite, abs(x - y) <= tolerance else { return false } + } + } + return true + } /// Displays whose baseline capture already failed and was already logged. /// @@ -130,6 +224,8 @@ final class GammaController: GammaApplying { _ scale: Double, baseline: GammaSamples, on displayID: CGDirectDisplayID, enforcerOn drawableDisplayID: CGDirectDisplayID ) -> Bool { + writeGeneration &+= 1 + recoveryOwners[displayID] = nil // Scales above 1 would push table entries out of the API's 0…1 range (and // gamma cannot brighten a panel past its own output anyway). let clamped = min(max(scale, 0), 1) @@ -164,6 +260,9 @@ final class GammaController: GammaApplying { } self.driver.enforceActivity() self.lastAppliedScale[displayID] = clamped + if displayID == drawableDisplayID, let identity = driver.recoveryIdentity(on: displayID) { + recoveryOwners[displayID] = RecoveryOwner(identity: identity, generation: writeGeneration) + } return true } @@ -189,6 +288,7 @@ final class GammaController: GammaApplying { /// is installed). Capturing while dimmed bakes the dimming into the baseline /// and the display can never get back to full brightness. func recaptureDefaultTable(on displayID: CGDirectDisplayID) { + recoveryOwners[displayID] = nil self.defaultTables.removeValue(forKey: displayID) // The previous scale was measured against the previous baseline; keeping it // would make `verifyTableIntact` compare against a stale reference. @@ -200,6 +300,9 @@ final class GammaController: GammaApplying { } func resetAllGamma() { + // Invalidate before touching the system table. A recovery tick must never + // re-dim the table the final pass is about to capture as its baseline. + recoveryOwners.removeAll() self.driver.restoreColorSyncSettings() // Baselines stay valid (they were captured from the OS-owned table), but // nothing of ours is installed anymore. @@ -253,6 +356,24 @@ final class GammaController: GammaApplying { final class CoreGraphicsGammaDriver: GammaTableDriving { private static let log = Logger(subsystem: "com.rydersel.Candela", category: "gamma") + func recoveryIdentity(on displayID: CGDirectDisplayID) -> String? { + guard CGDisplayIsOnline(displayID) != 0, CGDisplayIsInMirrorSet(displayID) == 0, + NSScreen.screens.contains(where: { $0.displayID == displayID }), + let uuid = CGDisplayCreateUUIDFromDisplayID(displayID)?.takeRetainedValue() + else { return nil } + var ids = [CGDirectDisplayID](repeating: 0, count: 32) + var count: UInt32 = 0 + guard CGGetOnlineDisplayList(UInt32(ids.count), &ids, &count) == .success, + count < ids.count + else { return nil } + let matches = ids.prefix(Int(count)).filter { id in + guard let other = CGDisplayCreateUUIDFromDisplayID(id)?.takeRetainedValue() else { return false } + return CFEqual(uuid, other) + } + guard matches == [displayID] else { return nil } + return CFUUIDCreateString(nil, uuid) as String + } + func readTable(_ displayID: CGDirectDisplayID, capacity: UInt32) -> GammaReadOutcome { var red = [CGGammaValue](repeating: 0, count: Int(capacity)) var green = red diff --git a/Candela/AppKitIslands/OverlayWindow.swift b/Candela/AppKitIslands/OverlayWindow.swift index 41d4cef2..03454e8d 100644 --- a/Candela/AppKitIslands/OverlayWindow.swift +++ b/Candela/AppKitIslands/OverlayWindow.swift @@ -97,6 +97,10 @@ enum OverlayWindow { window.level = config.level window.collectionBehavior = config.collectionBehavior window.hasShadow = config.hasShadow + // AppKit's automatic show/hide animation outlives close(), so replacing a + // shade briefly stacks two dims and restoring an OLED blackout fades out. + // Only the owner's intentional content-view entry fade may animate. + window.animationBehavior = .none window.setFrame(frame, display: true) window.contentView?.wantsLayer = true window.contentView?.alphaValue = config.initialContentAlpha diff --git a/Candela/AppKitIslands/VolumeFeedbackSound.swift b/Candela/AppKitIslands/VolumeFeedbackSound.swift index 54b16925..1a1dca7e 100644 --- a/Candela/AppKitIslands/VolumeFeedbackSound.swift +++ b/Candela/AppKitIslands/VolumeFeedbackSound.swift @@ -5,10 +5,17 @@ import AVFoundation import Foundation +/// The blip as the executor sees it, so a test can count plays instead of +/// sounding them. +@MainActor +protocol VolumeFeedbackPlaying { + func play() +} + /// Fork AppDelegate.playVolumeChangedSound: the system volume-feedback blip, /// honoring the user's "Play feedback when volume is changed" Sound setting. @MainActor -final class VolumeFeedbackSound { +final class VolumeFeedbackSound: VolumeFeedbackPlaying { private static let soundURL = URL( fileURLWithPath: "/System/Library/LoginPlugins/BezelServices.loginPlugin/Contents/Resources/volume.aiff" ) diff --git a/Candela/Panel/CandelaSlider.swift b/Candela/Panel/CandelaSlider.swift index 4a137319..584f3fb4 100644 --- a/Candela/Panel/CandelaSlider.swift +++ b/Candela/Panel/CandelaSlider.swift @@ -40,10 +40,21 @@ struct CandelaSlider: View { private let height: CGFloat = 30 private let strokeColor = Color.gray.opacity(0.5) - /// Wide enough for "100%" with monospaced digits, so the capsule never resizes - /// as the number changes width. It scales with the readout's text style, so a - /// larger accessibility size widens the column instead of truncating. - @ScaledMetric(relativeTo: .caption2) private var readoutWidth: CGFloat = 34 + /// Wide enough for "100%" in monospaced digits at `readoutFontSize`, so the + /// number cannot truncate and the capsule does not resize as it changes width. + /// + /// Fixed rather than scaled with the text style: the rest of the panel is fixed + /// point counts, so a few readouts growing at a large accessibility size while + /// the labels beside them hold still reads worse than nothing growing. That is + /// a trade against a person who needs larger text, and it stands until the + /// panel has a bounded, scrolling height: a panel that grows with the text size + /// pushes Settings and Quit off the bottom of the menu, stranding that same + /// person with no way to reach either. + static let readoutWidth: CGFloat = 34 + /// What `.caption2` resolved to at the default text size [MEASURED 2026-09-10], + /// so dropping the scaling moved nothing. It resolves MEDIUM weight as well as + /// 10 pt, which is why the use site names a weight. + static let readoutFontSize: CGFloat = 10 /// Applied to the whole control rather than per layer. The fill, knob and /// glyph are tuned against each other (a black glyph on a white fill), so @@ -64,9 +75,9 @@ struct CandelaSlider: View { .frame(height: height) if showsPercent { Text(SliderSnap.percentText(value)) - .font(.caption2.monospacedDigit()) + .font(.system(size: Self.readoutFontSize, weight: .medium).monospacedDigit()) .foregroundStyle(.secondary) - .frame(width: readoutWidth, alignment: .trailing) + .frame(width: Self.readoutWidth, alignment: .trailing) // The row is one accessibility element and already publishes this // number as its value; reading it twice is noise. .accessibilityHidden(true) diff --git a/Candela/Panel/SliderRows.swift b/Candela/Panel/SliderRows.swift index 65735dbc..a2c395ee 100644 --- a/Candela/Panel/SliderRows.swift +++ b/Candela/Panel/SliderRows.swift @@ -138,7 +138,9 @@ private struct PanelHoverReason: ViewModifier { } if let reason { Text(verbatim: reason) - .font(.caption) + // What `.caption` resolved to at the default text size [MEASURED 2026-09-10], + // fixed for the reason spelled out on `CandelaSlider.readoutWidth`. + .font(.system(size: 10)) .foregroundStyle(.secondary) .fixedSize(horizontal: false, vertical: true) .opacity(hovering ? 1 : 0) diff --git a/CandelaAppTests/Fakes/Fakes.swift b/CandelaAppTests/Fakes/Fakes.swift index 4b6b2067..65fbfc47 100644 --- a/CandelaAppTests/Fakes/Fakes.swift +++ b/CandelaAppTests/Fakes/Fakes.swift @@ -139,14 +139,17 @@ enum TestFixtures { id: CGDirectDisplayID = 7, name: String = "Test Display", persistenceKey: String = "test-display", - capabilities: String? = nil + capabilities: String? = nil, + gamma: (any GammaApplying)? = nil, + shade: (any ShadeRendering)? = nil, + hdr: (any HDRToggling)? = nil ) -> AppModel.DisplayState { let writer = FakeDDCWriter(capabilities: capabilities) let display = ExternalDisplay(id: id, name: name, persistenceKey: persistenceKey) let prefs = prefs(persistenceKey: persistenceKey) let backends = BrightnessBackends( applierNative: FakeBrightnessApplier(), - hdr: nil, shade: nil, gamma: nil) + hdr: hdr, shade: shade, gamma: gamma) let controller = BrightnessController( writer: writer, backends: backends, prefs: prefs, displayID: id, wireSiblings: []) diff --git a/CandelaAppTests/GammaControllerTests.swift b/CandelaAppTests/GammaControllerTests.swift index 10e9e7fa..b83292e8 100644 --- a/CandelaAppTests/GammaControllerTests.swift +++ b/CandelaAppTests/GammaControllerTests.swift @@ -1,6 +1,28 @@ import CandelaKit import CoreGraphics import Testing +import os + +private actor HeldRecoveryHDR { + private var count = 0 + private var pending: [Int: CheckedContinuation] = [:] + func read() async -> Bool? { + await withCheckedContinuation { continuation in + pending[count] = continuation + count += 1 + } + } + func waitForRequests(_ expected: Int) async -> Bool { + let deadline = ContinuousClock.now.advanced(by: .seconds(2)) + while count < expected && ContinuousClock.now < deadline { + try? await Task.sleep(for: .milliseconds(1)) + } + return count >= expected + } + func answer(_ request: Int, _ enabled: Bool?) { + pending.removeValue(forKey: request)?.resume(returning: enabled) + } +} /// Answers table reads from a dictionary, so a display can be made to refuse its /// own baseline. No display in the rig can be told to do that on demand, and @@ -11,6 +33,7 @@ private final class StubGammaDriver: GammaTableDriving { var tables: [CGDirectDisplayID: GammaSamples] = [:] /// Displays with a screen, i.e. ones the activity enforcer can be parked on. var screens: Set = [] + var identities: [CGDirectDisplayID: String] = [:] private(set) var writes: [(target: CGDirectDisplayID, samples: GammaSamples)] = [] private(set) var enforcedCount = 0 @@ -29,11 +52,354 @@ private final class StubGammaDriver: GammaTableDriving { func restoreColorSyncSettings() { restoreCount += 1 } func moveEnforcer(to displayID: CGDirectDisplayID) -> Bool { screens.contains(displayID) } func enforceActivity() { enforcedCount += 1 } + func recoveryIdentity(on displayID: CGDirectDisplayID) -> String? { + screens.contains(displayID) ? identities[displayID] : nil + } } @Suite("Gamma controller baselines") @MainActor struct GammaControllerTests { + @Test func recoveryFollowsANewerSuccessfulWriteWithoutRenewingItsBudget() async throws { + let driver = StubGammaDriver() + driver.screens = [2]; driver.identities[2] = "panel-A" + driver.tables[2] = Self.profileTable() + let gamma = GammaController(driver: driver) + gamma.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let hdr = HeldRecoveryHDR() + let clock = OSAllocatedUnfairLock(initialState: 0.0) + let recovery = GammaReconfigurationRecovery( + gamma: gamma, targets: { [2] }, readHDR: { _ in await hdr.read() }, + epoch: { 0 }, asleep: { false }, now: { clock.withLock { $0 } }, interval: 3600) + defer { recovery.stop() } + let first = try #require(recovery.begin()) + try #require(await hdr.waitForRequests(1)) + await hdr.answer(0, false) + await first.value + // The normal topology rebuild writes after recovery captured its owner. + clock.withLock { $0 = 4.8 } + gamma.applyGammaScale(0.6, on: 2, enforcerOn: 2) + recovery.tick() + try #require(await hdr.waitForRequests(2)) + recovery.tick() + #expect(driver.writes.count == 2) // the old HDR reply cannot authorize it + await hdr.answer(1, false) + let deadline = ContinuousClock.now.advanced(by: .seconds(2)) + while driver.writes.count == 2 && ContinuousClock.now < deadline { + recovery.tick() + try await Task.sleep(for: .milliseconds(1)) + } + #expect(driver.writes.count == 3) + #expect(driver.writes.last?.samples == Self.profileTable().scaled(by: 0.6)) + clock.withLock { $0 = 5 } + recovery.tick() + #expect(driver.writes.count == 3) + #expect(recovery.begin() == nil) + } + + @Test(arguments: [4.9, 6.0, 120.0], [false, true]) + func aNewReconfigurationHasItsOwnWindowAfterStartupOrAFinalPass(start: Double, afterFinalPass: Bool) async throws { + let driver = StubGammaDriver() + driver.screens = [2]; driver.identities[2] = "panel-A" + let baseline = Self.profileTable() + driver.tables[2] = baseline + let gamma = GammaController(driver: driver) + gamma.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let clock = OSAllocatedUnfairLock(initialState: 0.0) + let epoch = OSAllocatedUnfairLock(initialState: UInt64(0)) + let recovery = GammaReconfigurationRecovery( + gamma: gamma, targets: { [2] }, readHDR: { _ in false }, + epoch: { epoch.withLock { $0 } }, asleep: { false }, + now: { clock.withLock { $0 } }, interval: 3600) + defer { recovery.stop() } + let settling: Task + if afterFinalPass { + recovery.beginFinalPass() + settling = try #require(recovery.endFinalPass()) + } else { + settling = try #require(recovery.begin()) + } + await settling.value + // The reconnect may begin just before, just after, or long after the + // departure's post-pass watch expires. All are separate bursts. + clock.withLock { $0 = start } + epoch.withLock { $0 = 1 } + let reconnect = try #require(recovery.begin()) + await reconnect.value + clock.withLock { $0 = start + 0.2 } + recovery.tick() + #expect(driver.writes.count == 2) + // Further CG events in this burst must not renew its allowance. + clock.withLock { $0 = start + 4.9 } + epoch.withLock { $0 = 2 } + let repeated = try #require(recovery.begin()) + await repeated.value + clock.withLock { $0 = start + 5 } + recovery.tick() + #expect(driver.writes.count == 2) + #expect(recovery.begin() == nil) + } + + @Test func aLateResetAfterTheFinalPassDoesNotWaitForAnotherNotification() async throws { + let driver = StubGammaDriver() + driver.screens = [2]; driver.identities[2] = "panel-A" + let baseline = Self.profileTable() + driver.tables[2] = baseline + let gamma = GammaController(driver: driver) + gamma.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let clock = OSAllocatedUnfairLock(initialState: 0.0) + let recovery = GammaReconfigurationRecovery( + gamma: gamma, targets: { [2] }, readHDR: { _ in false }, + epoch: { 0 }, asleep: { false }, now: { clock.withLock { $0 } }, interval: 3600) + defer { recovery.stop() } + recovery.beginFinalPass() + gamma.resetAllGamma() + gamma.recaptureDefaultTable(on: 2) + gamma.applyGammaScale(0.5, on: 2, enforcerOn: 2) + driver.tables[2] = baseline.scaled(by: 0.5) + let settling = try #require(recovery.endFinalPass()) + await settling.value + recovery.tick() + #expect(driver.writes.count == 2) + // Measured on hardware: the system reset arrives 3.6 seconds after + // the completed pass, before its delayed AppKit notification. + clock.withLock { $0 = 3.6 } + driver.tables[2] = baseline + recovery.tick() + #expect(driver.writes.count == 3) + #expect(driver.writes.last?.samples == baseline.scaled(by: 0.5)) + } + + @Test func aStaleHDRReplyCannotAuthorizeRecoveryForANewerEpoch() async throws { + let driver = StubGammaDriver() + driver.screens = [2]; driver.identities[2] = "panel-A" + driver.tables[2] = Self.profileTable() + let gamma = GammaController(driver: driver) + gamma.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let epoch = OSAllocatedUnfairLock(initialState: UInt64(0)) + let hdr = HeldRecoveryHDR() + let recovery = GammaReconfigurationRecovery( + gamma: gamma, targets: { [2] }, readHDR: { _ in await hdr.read() }, + epoch: { epoch.withLock { $0 } }, asleep: { false }, now: { 0 }, interval: 3600) + defer { recovery.stop() } + let old = try #require(recovery.begin()) + try #require(await hdr.waitForRequests(1)) + epoch.withLock { $0 = 1 } + let new = try #require(recovery.begin()) + try #require(await hdr.waitForRequests(2)) + await hdr.answer(1, true) + await new.value + // The old SDR answer arrives AFTER the new HDR answer. + await hdr.answer(0, false) + await old.value + recovery.tick() + #expect(driver.writes.count == 1) + // A fresh brightness write establishes ownership after the HDR stand-down. + gamma.applyGammaScale(0.6, on: 2, enforcerOn: 2) + epoch.withLock { $0 = 2 } + let current = try #require(recovery.begin()) + try #require(await hdr.waitForRequests(3)) + await hdr.answer(2, false) + await current.value + recovery.tick() + #expect(driver.writes.count == 3) + } + + @Test func anUnfamiliarCurveStopsRecoveryAcrossRepeatedNotifications() async throws { + let driver = StubGammaDriver() + driver.screens = [2]; driver.identities[2] = "panel-A" + let baseline = Self.profileTable() + driver.tables[2] = baseline + let gamma = GammaController(driver: driver) + gamma.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let recovery = GammaReconfigurationRecovery( + gamma: gamma, targets: { [2] }, readHDR: { _ in false }, + epoch: { 0 }, asleep: { false }, now: { 0 }, interval: 3600) + defer { recovery.stop() } + let first = try #require(recovery.begin()) + await first.value + driver.tables[2] = GammaSamples.linear(count: Self.sampleCount) + recovery.tick() + driver.tables[2] = baseline + if let repeated = recovery.begin() { await repeated.value } + recovery.tick() + #expect(driver.writes.count == 1) + // A fresh brightness write deliberately establishes a new recovery owner. + gamma.applyGammaScale(0.6, on: 2, enforcerOn: 2) + let fresh = try #require(recovery.begin()) + await fresh.value + recovery.tick() + #expect(driver.writes.count == 3) + } + + @Test func aFinalPassStopsAnHDRReplyBeforeItCanTouchTheBaseline() async throws { + let driver = StubGammaDriver() + driver.screens = [2]; driver.identities[2] = "panel-A" + driver.tables[2] = Self.profileTable() + let gamma = GammaController(driver: driver) + gamma.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let hdr = HeldRecoveryHDR() + let recovery = GammaReconfigurationRecovery( + gamma: gamma, targets: { [2] }, readHDR: { _ in await hdr.read() }, + epoch: { 0 }, asleep: { false }, now: { 0 }, interval: 3600) + defer { recovery.stop() } + let preparation = try #require(recovery.begin()) + try #require(await hdr.waitForRequests(1)) + recovery.beginFinalPass() + gamma.resetAllGamma() + await hdr.answer(0, false) + await preparation.value + recovery.tick() + #expect(driver.writes.count == 1) + gamma.recaptureDefaultTable(on: 2) + recovery.endFinalPass() + recovery.tick() + #expect(driver.writes.count == 1) + } + + @Test func aDelayedSystemResetRestoresTheCapturedProfileWithoutCompounding() throws { + let driver = StubGammaDriver() + driver.screens = [2] + driver.identities[2] = "panel-A" + let baseline = Self.profileTable() + driver.tables[2] = baseline + let controller = GammaController(driver: driver) + #expect(controller.applyGammaScale(0.5, on: 2, enforcerOn: 2)) + let snapshot = try #require(controller.recoverySnapshot(on: 2)) + driver.tables[2] = baseline.scaled(by: 0.5) + #expect(controller.recoverBaselineIfReset(snapshot, hdrEnabled: false) == .unchanged) + #expect(driver.writes.count == 1) + // The table can remain intact at notification time and reset later. + driver.tables[2] = baseline + #expect(controller.recoverBaselineIfReset(snapshot, hdrEnabled: false) == .written) + #expect(driver.writes.count == 2) + #expect(driver.writes.last?.samples == baseline.scaled(by: 0.5)) + #expect(controller.recoverBaselineIfReset(snapshot, hdrEnabled: false) == .written) + #expect(driver.writes.count == 3) + #expect(driver.writes.last?.samples == baseline.scaled(by: 0.5)) + } + + @Test func recoveryDoesNotOverwriteAnUnfamiliarCurveWithTheSamePeak() throws { + let driver = StubGammaDriver() + driver.screens = [2] + driver.identities[2] = "panel-A" + driver.tables[2] = Self.profileTable() + let controller = GammaController(driver: driver) + controller.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let snapshot = try #require(controller.recoverySnapshot(on: 2)) + driver.tables[2] = GammaSamples.linear(count: Self.sampleCount) + #expect(controller.recoverBaselineIfReset(snapshot, hdrEnabled: false) == .stopped) + #expect(driver.writes.count == 1) + } + + @Test(arguments: [true, nil] as [Bool?]) + func recoveryRequiresAnAffirmativeSDRObservation(hdr: Bool?) throws { + let driver = StubGammaDriver() + driver.screens = [2] + driver.identities[2] = "panel-A" + driver.tables[2] = Self.profileTable() + let controller = GammaController(driver: driver) + controller.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let snapshot = try #require(controller.recoverySnapshot(on: 2)) + #expect(controller.recoverBaselineIfReset(snapshot, hdrEnabled: hdr) == .stopped) + #expect(driver.writes.count == 1) + } + + @Test func recoveryRejectsIDReuseAndMissingScreensButDistinguishesNewBrightness() throws { + let driver = StubGammaDriver() + driver.screens = [2] + driver.identities[2] = "panel-A" + driver.tables[2] = Self.profileTable() + let controller = GammaController(driver: driver) + controller.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let snapshot = try #require(controller.recoverySnapshot(on: 2)) + driver.identities[2] = "panel-B" + #expect(controller.recoverBaselineIfReset(snapshot, hdrEnabled: false) == .stopped) + driver.identities[2] = "panel-A" + driver.screens = [] + #expect(controller.recoverBaselineIfReset(snapshot, hdrEnabled: false) == .stopped) + driver.screens = [2] + controller.applyGammaScale(0.7, on: 2, enforcerOn: 2) + #expect(controller.recoverBaselineIfReset(snapshot, hdrEnabled: false) == .superseded) + #expect(driver.writes.count == 2) + driver.identities[2] = "panel-B" + controller.applyGammaScale(0.8, on: 2, enforcerOn: 2) + #expect(controller.recoverBaselineIfReset(snapshot, hdrEnabled: false) == .stopped) + #expect(driver.writes.count == 3) + } + + @Test func cancellingAnOldSnapshotPreservesANewerBrightnessOwner() throws { + let driver = StubGammaDriver() + driver.screens = [2]; driver.identities[2] = "panel-A" + driver.tables[2] = Self.profileTable() + let gamma = GammaController(driver: driver) + gamma.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let old = try #require(gamma.recoverySnapshot(on: 2)) + gamma.applyGammaScale(0.6, on: 2, enforcerOn: 2) + gamma.cancelRecovery(old) + #expect(gamma.recoverySnapshot(on: 2) != nil) + } + + @Test func aMissingIdentityCannotRegainOwnershipWithoutAFreshWrite() { + let driver = StubGammaDriver() + driver.screens = [2]; driver.identities[2] = "panel-A" + driver.tables[2] = Self.profileTable() + let gamma = GammaController(driver: driver) + gamma.applyGammaScale(0.5, on: 2, enforcerOn: 2) + driver.identities[2] = nil + #expect(gamma.recoverySnapshot(on: 2) == nil) + driver.identities[2] = "panel-A" + #expect(gamma.recoverySnapshot(on: 2) == nil) + gamma.applyGammaScale(0.6, on: 2, enforcerOn: 2) + #expect(gamma.recoverySnapshot(on: 2) != nil) + } + + @Test func finalResetInvalidatesRecoveryBeforeBaselineRecapture() throws { + let driver = StubGammaDriver() + driver.screens = [2] + driver.identities[2] = "panel-A" + driver.tables[2] = Self.profileTable() + let controller = GammaController(driver: driver) + controller.applyGammaScale(0.5, on: 2, enforcerOn: 2) + let snapshot = try #require(controller.recoverySnapshot(on: 2)) + controller.resetAllGamma() + #expect(controller.recoverBaselineIfReset(snapshot, hdrEnabled: false) == .stopped) + #expect(driver.writes.count == 1) + controller.recaptureDefaultTable(on: 2) + #expect(controller.recoverySnapshot(on: 2) == nil) + } + + @Test func recoveryNeverCapturesAnUnknownBaselineOrUsesACompanionTarget() { + let driver = StubGammaDriver() + driver.screens = [2, 5] + driver.identities = [2: "panel-A", 5: "virtual"] + let controller = GammaController(driver: driver) + controller.applyGammaScale(assumingLinearBaseline: 0.5, on: 2, enforcerOn: 2) + #expect(controller.recoverySnapshot(on: 2) == nil) + driver.tables[2] = Self.profileTable() + controller.applyGammaScale(0.5, on: 2, enforcerOn: 5) + #expect(controller.recoverySnapshot(on: 2) == nil) + } + + @Test func repeatedNotificationsDoNotExtendTheRecoveryDeadlineOrWriteBudget() { + var budget = GammaRecoveryBudget() + let allowed0 = budget.begin(at: 10) + #expect(allowed0) + let allowed1 = budget.begin(at: 14.99) + #expect(allowed1) + let allowed2 = budget.begin(at: 15) + #expect(!allowed2) + budget.finish() + let allowed3 = budget.begin(at: 20) + #expect(allowed3) + for _ in 0..<8 { budget.recordWrite() } + let allowed4 = budget.begin(at: 20.1) + #expect(!allowed4) + budget.finish() + let allowed5 = budget.begin(at: 30) + #expect(allowed5) + } + private static let panelID: CGDirectDisplayID = 2 private static let virtualID: CGDirectDisplayID = 5 private static let sampleCount = 256 diff --git a/CandelaAppTests/OverlayWindowTests.swift b/CandelaAppTests/OverlayWindowTests.swift index 26fd4808..c25d1ca6 100644 --- a/CandelaAppTests/OverlayWindowTests.swift +++ b/CandelaAppTests/OverlayWindowTests.swift @@ -10,6 +10,20 @@ import Testing // UI. Every assertion below is on a property whose failure is silent. @Suite("Overlay window") @MainActor struct OverlayWindowTests { + /// A closing shade must disappear immediately instead of fading over its + /// replacement. The owner animates content only when dim entry calls for it. + @Test func dimmingWindowsDoNotUseAppKitShowOrHideAnimations() { + let window = NSWindow( + contentRect: OverlayWindow.seedRect, styleMask: OverlayWindow.styleMask, + backing: .buffered, defer: false) + window.animationBehavior = .documentWindow + OverlayWindow.configure( + window, title: "Overlay animation test", + covering: NSRect(x: 120, y: 60, width: 400, height: 300)) + defer { window.close() } + #expect(window.animationBehavior == .none) + } + // MARK: - The recipe, as a value /// The recipe is checkable without opening a window, which is the reason it diff --git a/CandelaAppTests/ReconfigureDimmingOrderTests.swift b/CandelaAppTests/ReconfigureDimmingOrderTests.swift new file mode 100644 index 00000000..10e86167 --- /dev/null +++ b/CandelaAppTests/ReconfigureDimmingOrderTests.swift @@ -0,0 +1,176 @@ +import CandelaKit +import CoreGraphics +import Foundation +import Testing + +/// One ordered log across the three protocols, so calls stay comparable by +/// index. Per-protocol recorders could count calls but never answer what ran +/// between two of them, which is the whole question here. +private enum ReconfigureEvent: Equatable { + case hdrCacheDropped + case hdrRead(CGDirectDisplayID) + case recoveryPaused + case gammaReset + case shadeRemoveAll + case gammaRecapture(CGDirectDisplayID) + case gammaApply(CGDirectDisplayID) + case shadeAlpha(CGDirectDisplayID) + + var isHDRRead: Bool { + if case .hdrRead = self { return true } + return false + } + + var isRecapture: Bool { + if case .gammaRecapture = self { return true } + return false + } + + /// Anything the per-display re-apply pass emits. The recapture is its first + /// call, so this is what "the dim went back on" looks like from outside. + var isReapply: Bool { + switch self { + case .gammaRecapture, .gammaApply, .shadeAlpha: return true + case .hdrCacheDropped, .hdrRead, .recoveryPaused, .gammaReset, .shadeRemoveAll: return false + } + } +} + +/// One instance is all three backends AND the pass's own collaborators, so it +/// sees every call in the order it was made. +/// +/// `isHDREnabled` answers false deliberately: true puts every controller on the +/// native path, where `handleReconfigure` returns before the software leg and +/// there is no re-apply left to measure. +@MainActor +private final class ReconfigureRecorder: GammaApplying, ShadeRendering, HDRToggling { + private(set) var events: [ReconfigureEvent] = [] + func pauseRecovery() { events.append(.recoveryPaused) } + + // MARK: - HDRToggling + + func supportsHDR(displayID: CGDirectDisplayID) async -> Bool { + events.append(.hdrRead(displayID)) + return false + } + + func isHDREnabled(displayID: CGDirectDisplayID) async -> Bool { + events.append(.hdrRead(displayID)) + return false + } + + func measuredHDREnabled(displayID: CGDirectDisplayID) async -> Bool { + events.append(.hdrRead(displayID)) + return false + } + + @discardableResult + func setHDR(displayID _: CGDirectDisplayID, enabled _: Bool) async -> Bool { true } + + func displaysReconfigured() async { + events.append(.hdrCacheDropped) + } + + // MARK: - GammaApplying + + @discardableResult + func applyGammaScale( + _: Double, on displayID: CGDirectDisplayID, enforcerOn _: CGDirectDisplayID + ) -> Bool { + events.append(.gammaApply(displayID)) + return true + } + + func verifyTableIntact(on _: CGDirectDisplayID) -> Bool { true } + + func recaptureDefaultTable(on displayID: CGDirectDisplayID) { + events.append(.gammaRecapture(displayID)) + } + + func resetAllGamma() { + events.append(.gammaReset) + } + + // MARK: - ShadeRendering + + @discardableResult + func setShadeAlpha(_: Double, on displayID: CGDirectDisplayID) -> Bool { + events.append(.shadeAlpha(displayID)) + return true + } + + func removeShade(for _: CGDirectDisplayID) {} + + func removeAllShades() { + events.append(.shadeRemoveAll) + } + + func repinFrames() {} +} + +/// Every dimmed display is undimmed between the wholesale shade removal and the +/// per-display re-apply, so anything that runs in that gap shows as a flash. The +/// HDR re-evaluation is the one that fits: two MonitorPanel enumerations per +/// display, off the main actor and back. +/// +/// These pin the order, not the timing. A duration assertion would measure the +/// machine; what shuts the window is that nothing in the gap can suspend. +@Suite("Reconfigure dimming order") +@MainActor +struct ReconfigureDimmingOrderTests { + /// Two displays, because a single one cannot tell a hoisted loop from a + /// per-display reordering: with one display both shapes emit the same log. + private func run() async -> [ReconfigureEvent] { + let recorder = ReconfigureRecorder() + let displays = [ + TestFixtures.displayState( + id: 7, name: "Panel A", persistenceKey: "reconfigure-order-a", + gamma: recorder, shade: recorder, hdr: recorder), + TestFixtures.displayState( + id: 8, name: "Panel B", persistenceKey: "reconfigure-order-b", + gamma: recorder, shade: recorder, hdr: recorder), + ] + await ReconfigureDimming.run( + displays: displays, hdrToggling: recorder, gamma: recorder, shade: recorder, + beforeReset: { recorder.pauseRecovery() }) + return recorder.events + } + + @Test("early recovery remains active through HDR preparation") + func recoveryPausesOnlyAtTheResetBoundary() async { + let events = await run() + #expect(events.lastIndex(where: \.isHDRRead)! < events.firstIndex(of: .recoveryPaused)!) + #expect(events.firstIndex(of: .recoveryPaused)! + 1 == events.firstIndex(of: .gammaReset)!) + } + + @Test("nothing at all sits between the shade removal and the re-apply") + func theDimIsReappliedWithNothingBetweenItAndTheRemoval() async { + let events = await run() + #expect( + events.firstIndex(of: .shadeRemoveAll)! + 1 == events.firstIndex(where: \.isReapply)!) + } + + @Test("every HDR read happens before the gamma table is handed back") + func everyHDRReadHappensBeforeTheTableIsHandedBack() async { + let events = await run() + #expect(events.lastIndex(where: \.isHDRRead)! < events.firstIndex(of: .gammaReset)!) + } + + @Test("the reconfiguration notice still precedes the reads") + func theReconfigurationNoticeStillPrecedesTheReads() async { + let events = await run() + #expect(events.firstIndex(of: .hdrCacheDropped)! < events.firstIndex(where: \.isHDRRead)!) + } + + @Test("the recapture still sees an OS-owned table") + func theRecaptureStillSeesAnOSOwnedTable() async { + let events = await run() + #expect(events.firstIndex(of: .gammaReset)! < events.firstIndex(where: \.isRecapture)!) + } + + @Test("both displays are covered, from one snapshot") + func bothDisplaysAreCoveredFromOneSnapshot() async { + let events = await run() + #expect(events.filter(\.isRecapture) == [.gammaRecapture(7), .gammaRecapture(8)]) + } +} diff --git a/CandelaAppTests/RenderSmokeTests.swift b/CandelaAppTests/RenderSmokeTests.swift index b4907148..60751a67 100644 --- a/CandelaAppTests/RenderSmokeTests.swift +++ b/CandelaAppTests/RenderSmokeTests.swift @@ -1,3 +1,4 @@ +import AppKit import CandelaKit import CoreGraphics import SwiftUI @@ -139,6 +140,33 @@ struct RenderSmokeTests { "two display rows must make the panel taller than the same panel without them") } + /// The widest string the readout can hold has to fit the column it is framed + /// into. `SliderSnapTests.percentTextIsWholePercentAndClamped` is the other + /// half, pinning "100%" as that widest string. + /// + /// Medium weight because that is what the control draws; measuring a regular + /// font would report the column 0.9 pt roomier than it is. + @Test func theReadoutColumnStillFitsAHundredPercent() { + let font = NSFont.monospacedDigitSystemFont( + ofSize: CandelaSlider.readoutFontSize, weight: .medium) + let width = ("100%" as NSString).size(withAttributes: [.font: font]).width + #expect( + width <= CandelaSlider.readoutWidth, + "\"100%\" draws \(width) pt wide and the readout column is \(CandelaSlider.readoutWidth) pt") + } + + // No render test pins "the panel does not grow with the system text size", and + // none can be written from this bundle [MEASURED 2026-09-10]: `.dynamicTypeSize` + // is inert under `ImageRenderer` on macOS, so a `Text` at `.caption2` renders + // 28x13 at both `.large` and `.accessibility3` and a two-render comparison + // passes over scaling code. Only a person at a large accessibility size can + // tell you. + // + // Anyone attempting one anyway: the FIRST `PanelView` render in a process + // differs from every later one by about 144 per channel at identical inputs, so + // two compared panel renders pass or fail on which ran first. Nothing above + // compares panel renders byte for byte. + // MARK: - The settings window /// The wordmark, every static pane row and the empty-display-list message, diff --git a/CandelaAppTests/VolumeFeedbackTests.swift b/CandelaAppTests/VolumeFeedbackTests.swift new file mode 100644 index 00000000..95d0bea7 --- /dev/null +++ b/CandelaAppTests/VolumeFeedbackTests.swift @@ -0,0 +1,109 @@ +import CandelaKit +import CoreGraphics +import Foundation +import Testing + +/// WHICH releases play the volume-feedback blip. The router answers what a key +/// event is, never whether it makes a sound, so a release under Option still +/// routes the feedback trigger; only the executor knows the preceding key-down +/// opened Sound settings instead of stepping. +/// +/// The executor's two injection points exist for this suite alone: counting +/// plays instead of sounding the blip, and recording the deep link instead of +/// opening System Settings on the machine running the suite. +@Suite("Volume feedback sound", .timeLimit(.minutes(1))) +@MainActor +struct VolumeFeedbackTests { + @Test func aPlainVolumeStepPlaysTheSoundOnRelease() async { + let rig = await rig() + rig.executor.execute(.stepVolume(isUp: true, isFine: false)) + rig.executor.execute(.volumeKeyUp) + #expect(rig.sound.playCount == 1) + } + + @Test func aReleaseAfterSoundSettingsPlaysNothing() async { + let rig = await rig() + rig.executor.execute(.openSoundSettings) + rig.executor.execute(.volumeKeyUp) + // The deep link has to have run, or the release is being judged against a + // case that never fired. + #expect(rig.opened.urls.count == 1) + #expect(rig.sound.playCount == 0) + } + + /// A press at the rail is still a step, and macOS blips there too. Gating on + /// the value having moved would take that away. + @Test func theSoundStillPlaysAtTheTopOfTheRange() async { + let rig = await rig() + let volume = rig.model.displays.first?.volume + volume?.setValue(1) + // Or the step below is an ordinary mid-range one and the rail is untested. + #expect(volume?.value == 1) + rig.executor.execute(.stepVolume(isUp: true, isFine: false)) + rig.executor.execute(.volumeKeyUp) + #expect(rig.sound.playCount == 1) + } + + /// The app started with the key already held, or a mode change swallowed the + /// down: nothing armed the release, so nothing sounds. + @Test func aReleaseWithNoPrecedingDownPlaysNothing() async { + let rig = await rig() + rig.executor.execute(.volumeKeyUp) + #expect(rig.sound.playCount == 0) + } + + @Test func keyRepeatStillPlaysOncePerEvent() async { + let rig = await rig() + for _ in 0..<3 { + rig.executor.execute(.stepVolume(isUp: true, isFine: false)) + } + rig.executor.execute(.volumeKeyUp) + #expect(rig.sound.playCount == 1) + } + + @Test func aStepAfterSoundSettingsRearmsTheSound() async { + let rig = await rig() + rig.executor.execute(.openSoundSettings) + rig.executor.execute(.stepVolume(isUp: true, isFine: false)) + rig.executor.execute(.volumeKeyUp) + #expect(rig.sound.playCount == 1) + } + + // MARK: - Fixture + + private struct Rig { + let model: AppModel + let sound: CountingFeedback + let opened: RecordedURLs + let executor: KeyActionExecutor + } + + /// The fixture display must accept volume keys, or every "plays nothing" case + /// above passes for the wrong reason. Asserted here so no case can skip it. + private func rig() async -> Rig { + let discovery = ScriptedDiscovery([ + (id: 91, key: "volume-feedback-test-panel", name: "Feedback Test Panel"), + ]) + let model = TestFixtures.appModel(discovery: discovery) + await model.refresh() + #expect(model.volumeKeyEnabledStates(model.displays).contains { $0.volume.isAvailable }) + + let sound = CountingFeedback() + let opened = RecordedURLs() + return Rig( + model: model, sound: sound, opened: opened, + executor: KeyActionExecutor( + model: model, hud: nil, feedback: sound, openURL: { opened.urls.append($0) } + ) + ) + } + + @MainActor private final class CountingFeedback: VolumeFeedbackPlaying { + private(set) var playCount = 0 + func play() { playCount += 1 } + } + + @MainActor private final class RecordedURLs { + var urls: [URL] = [] + } +} diff --git a/CandelaKit/Sources/CandelaKit/Brightness/BrightnessController.swift b/CandelaKit/Sources/CandelaKit/Brightness/BrightnessController.swift index 55dc6f1f..d231c943 100644 --- a/CandelaKit/Sources/CandelaKit/Brightness/BrightnessController.swift +++ b/CandelaKit/Sources/CandelaKit/Brightness/BrightnessController.swift @@ -1693,8 +1693,14 @@ public final class BrightnessController: PendingWireDraining { /// Reconfigure re-apply: the WindowServer rebuilt display state, so re-capture the /// gamma baseline, re-pin shade frames, and re-run the software leg for the current - /// value. Skipped under the native path per the restore-latch clearing. Ordering contract: the app-side loop - /// calls `resetAllGamma()` once per event BEFORE this, so the table is OS-owned. + /// value. Skipped under the native path per the restore-latch clearing. Ordering contract: + /// `ReconfigureDimming.run` on the app side calls `resetAllGamma()` once per event BEFORE + /// this, so the table is OS-owned, and has already re-evaluated HDR for EVERY surviving + /// display. + /// + /// The half this owes back: NO suspension point anywhere on this path. That caller removes + /// every shade immediately before the loop that calls this, so an `await` here is a + /// visibly undimmed display until it resumes. public func handleReconfigure(recapture: Bool = true) async { // recapture: false is the interference-accept path: at accept time the // interfering app may own the table, and capturing that as the baseline bakes its diff --git a/CandelaKit/Sources/CandelaKit/Displays/DisplayManager.swift b/CandelaKit/Sources/CandelaKit/Displays/DisplayManager.swift index a5a9b40c..a2156ba6 100644 --- a/CandelaKit/Sources/CandelaKit/Displays/DisplayManager.swift +++ b/CandelaKit/Sources/CandelaKit/Displays/DisplayManager.swift @@ -49,14 +49,19 @@ public actor DisplayManager { /// Shared with the `@convention(c)` reconfiguration callback through the /// `userInfo` pointer. Sendable: it holds only the state lock and the raw - /// intake continuation (both Sendable), and its one method is synchronous. + /// intake continuation and signal callback (all Sendable). final class IntakeBox: Sendable { let state: OSAllocatedUnfairLock let rawEvents: AsyncStream.Continuation + let onReconfigure: @Sendable (CGDisplayChangeSummaryFlags) -> Void - init(state: OSAllocatedUnfairLock, rawEvents: AsyncStream.Continuation) { + init( + state: OSAllocatedUnfairLock, rawEvents: AsyncStream.Continuation, + onReconfigure: @escaping @Sendable (CGDisplayChangeSummaryFlags) -> Void = { _ in } + ) { self.state = state self.rawEvents = rawEvents + self.onReconfigure = onReconfigure } /// The entire intake for one raw CG event, run synchronously in the @@ -71,6 +76,7 @@ public actor DisplayManager { } topologyLog.log("reconfigure intake: display=\(displayID) flags=0x\(String(flags.rawValue, radix: 16), privacy: .public) epoch=\(epoch)") rawEvents.yield(()) + onReconfigure(flags) } } @@ -118,6 +124,10 @@ public actor DisplayManager { state.withLock { $0.epoch } } + /// Software-only recovery must stop during sleep too, while leaving the + /// stricter reconfiguration suspension on every DDC write untouched. + public nonisolated var isAsleep: Bool { state.withLock { $0.asleep } } + /// Sleep intake (NSWorkspace notifications stay app-side and forward here): /// synchronous bump and suspend, NO topology element, because sleep is a write /// gate rather than a topology change. The arm-token bump invalidates any @@ -153,8 +163,14 @@ public actor DisplayManager { /// the main thread in `applicationDidFinishLaunching`. Call once: the /// registration lives for the process, so the box handed to CG is /// intentionally immortal (`passRetained`, never balanced). - public nonisolated func activate() { - let userInfo = Unmanaged.passRetained(intake).toOpaque() + /// `onReconfigure` runs after the synchronous epoch bump. It must only + /// signal deferred work, without display inspection or writes in the callback. + public nonisolated func activate( + onReconfigure: @escaping @Sendable (CGDisplayChangeSummaryFlags) -> Void = { _ in } + ) { + let callbackIntake = IntakeBox( + state: state, rawEvents: intake.rawEvents, onReconfigure: onReconfigure) + let userInfo = Unmanaged.passRetained(callbackIntake).toOpaque() let result = CGDisplayRegisterReconfigurationCallback({ displayID, flags, userInfo in guard let userInfo else { return } Unmanaged.fromOpaque(userInfo).takeUnretainedValue() diff --git a/CandelaKit/Sources/CandelaKit/HDR/MonitorPanelService.swift b/CandelaKit/Sources/CandelaKit/HDR/MonitorPanelService.swift index 90e6cfe3..b02736bd 100644 --- a/CandelaKit/Sources/CandelaKit/HDR/MonitorPanelService.swift +++ b/CandelaKit/Sources/CandelaKit/HDR/MonitorPanelService.swift @@ -18,6 +18,9 @@ public protocol HDRToggling: Sendable { /// cache turns every achieved-state check built on this into one that cannot /// fail. Each conformance states its own read-through. func measuredHDREnabled(displayID: CGDirectDisplayID) async -> Bool + /// A fresh observation that preserves unavailable as unknown. Early gamma + /// recovery may act only on an affirmative SDR observation. + func observedHDREnabled(displayID: CGDirectDisplayID) async -> Bool? /// Reports whether the write was ISSUED, never whether the display switched. /// See the implementation's note. @discardableResult @@ -25,6 +28,12 @@ public protocol HDRToggling: Sendable { func displaysReconfigured() async } +public extension HDRToggling { + /// Existing providers that cannot distinguish unavailable from SDR are not + /// eligible for early recovery. Never forward to a false-defaulting read. + func observedHDREnabled(displayID: CGDirectDisplayID) async -> Bool? { nil } +} + /// Programmatic control of the System Settings HDR toggle through the private /// MonitorPanel framework, which is what the Displays pane itself drives. /// Actor isolation replaces the fork's serial DispatchQueue; MPDisplay and @@ -81,6 +90,10 @@ public actor MonitorPanelService: HDRToggling { return value } + public func observedHDREnabled(displayID: CGDirectDisplayID) -> Bool? { + self.mpDisplay(displayID)?.preferHDRModes + } + /// The display blanks and re-modes for ~2 s after this returns; the caller /// owns the settle delay and any deferred brightness write. /// diff --git a/CandelaKit/Tests/CandelaKitTests/DisplayManagerTests.swift b/CandelaKit/Tests/CandelaKitTests/DisplayManagerTests.swift index 8d3f236f..a350b801 100644 --- a/CandelaKit/Tests/CandelaKitTests/DisplayManagerTests.swift +++ b/CandelaKit/Tests/CandelaKitTests/DisplayManagerTests.swift @@ -124,6 +124,7 @@ private final class TopologyCounter: Sendable { manager.noteSleep() #expect(manager.currentEpoch() == 1) // synchronous bump + #expect(manager.isAsleep) #expect(manager.isEpochCurrent(1) == false) // suspended // Absence cannot be polled, so this stays a real elapsed wait. A machine slow @@ -144,6 +145,7 @@ private final class TopologyCounter: Sendable { manager.noteSleep() manager.noteWake() + #expect(manager.isAsleep) // Remains asleep until the wake quiet window. // Immediately after wake: still suspended, no bump yet. #expect(manager.currentEpoch() == 1) #expect(manager.isEpochCurrent(manager.currentEpoch()) == false) @@ -156,6 +158,7 @@ private final class TopologyCounter: Sendable { #expect(counter.elements == 1) // one element once sober #expect(manager.currentEpoch() == 2) // wake fire bumps: pre-sleep epochs stay stale #expect(manager.isEpochCurrent(2)) // suspension cleared + #expect(!manager.isAsleep) #expect(manager.isEpochCurrent(1) == false) } @@ -214,3 +217,20 @@ private actor GatedDDC: DDCWriting { let landed = await writer.recordedWrites() #expect(landed.map(\.value) == [30]) // the post-sleep-stale write never hit hardware } + +@Test func rawReconfigurationSignalSeesTheAlreadySuspendedEpoch() { + let state = OSAllocatedUnfairLock(initialState: DisplayManager.EpochState()) + let (_, events) = AsyncStream.makeStream(of: Void.self) + let observed = OSAllocatedUnfairLock(initialState: [(UInt64, Bool, UInt32)]()) + let intake = DisplayManager.IntakeBox(state: state, rawEvents: events) { flags in + let snapshot = state.withLock { ($0.epoch, $0.suspended, flags.rawValue) } + observed.withLock { $0.append(snapshot) } + } + intake.reconfigureEvent(displayID: 2, flags: .beginConfigurationFlag) + intake.reconfigureEvent(displayID: 2, flags: .addFlag) + let snapshots = observed.withLock { $0 } + #expect(snapshots.map { $0.0 } == [1, 2]) + #expect(snapshots.map { $0.1 } == [true, true]) + #expect(snapshots.map { $0.2 } == [CGDisplayChangeSummaryFlags.beginConfigurationFlag.rawValue, + CGDisplayChangeSummaryFlags.addFlag.rawValue]) +} diff --git a/CandelaKit/Tests/CandelaKitTests/KeyRouterTests.swift b/CandelaKit/Tests/CandelaKitTests/KeyRouterTests.swift index 02c88045..ddd4e854 100644 --- a/CandelaKit/Tests/CandelaKitTests/KeyRouterTests.swift +++ b/CandelaKit/Tests/CandelaKitTests/KeyRouterTests.swift @@ -144,8 +144,15 @@ struct KeyRouterTests { } // The cells a future edit would most plausibly break: key-up routes whatever - // the modifiers are (the fork plays the sound on an Option-only release too), - // only EXACT Option is the deep link, and brightness chords mean nothing to volume. + // the modifiers are, only EXACT Option is the deep link, and brightness chords + // mean nothing to volume. + // + // The release routes under any modifier on purpose: a router decides what an + // event is, never whether it makes a sound. The executor answers the blip, from + // whether the preceding key-down was a step, so an Option-only release (which + // opened Sound settings) is silent. The fork blips there; no written rule + // covers it, since Appendix A of the v1 design spec asks only that the sound + // honor the system "play feedback" setting. @Test func modifiedVolumeKeyUpStillRoutesTheRelease() { #expect(routeVol(.volumeUp, pressed: false, [.option]) == .volumeKeyUp) }