From 664d37ea262e7ec70efcaca04a227fd7bf03b0d8 Mon Sep 17 00:00:00 2001 From: Ryder Selikow <50812202+Rydersel@users.noreply.github.com> Date: Thu, 10 Sep 2026 10:23:21 -0500 Subject: [PATCH 1/2] Recover a crash-interrupted dim, fade dim entry, and name dimming on the Protection pane A crash or force-quit while a lock dim was engaged left a display's DDC register dimmed with the slider showing full brightness, and on a write-only display nothing could tell. A per-display marker is now written before the dim's first submit and cleared after the restoring one; at the next launch a surviving marker runs the restore pass's own memo reset and reassert, gated on Safe Mode directly. The launch readback holds off while a marker stands, because on a display that answers reads it would otherwise import the dim as the user's value. The startup caption names the one exception it makes. Idle dim and blackout entries now fade in over about 400 milliseconds on the overlay's content view; every decrease and every removal still lands in one write, so restore timing is unchanged. A nudge from the care loop's verify pass declines while a fade toward the same state is in flight, bounded in time so a completion that never arrives cannot park the loop, and the declined nudge counts as settling rather than a mismatch. The Protection pane opens with a Dimming row that reveals OLED Care and says how many displays are enrolled, or Paused while Safe Mode holds the care loop, with one sentence on what the other half of the pillar does. --- Candela/App/AppModel.swift | 46 +++++++ Candela/App/StatusItemController.swift | 10 ++ Candela/OledCare/OledCareCoordinator.swift | 49 ++++++-- Candela/OledCare/OledOverlay.swift | 118 ++++++++++++++++-- Candela/Settings/ProtectionPane.swift | 76 ++++++++++- CandelaAppTests/OverlayWindowTests.swift | 52 ++++++++ CandelaAppTests/ProtectionPaneTests.swift | 48 ++++++- .../Brightness/BrightnessController.swift | 32 ++++- .../Brightness/InterruptedDimRecovery.swift | 36 ++++++ .../CandelaKit/OledCare/OledDimming.swift | 4 +- .../CandelaKit/OledCare/OverlayFade.swift | 25 ++++ .../CandelaKit/Support/DisplayPrefs.swift | 17 +++ .../CandelaKit/Support/SafeModeCopy.swift | 4 + .../CandelaKitTests/DisplayPrefsTests.swift | 14 +++ .../InterruptedDimRecoveryTests.swift | 54 ++++++++ .../Tests/CandelaKitTests/LockDimTests.swift | 118 +++++++++++++++++- .../CandelaKitTests/OverlayFadeTests.swift | 26 ++++ .../PrefPropagationTests.swift | 4 + docs/ADVANCED-SETTINGS.md | 3 + tools/hardware-pass/pixeltime.swift | 26 +++- 20 files changed, 730 insertions(+), 32 deletions(-) create mode 100644 CandelaKit/Sources/CandelaKit/Brightness/InterruptedDimRecovery.swift create mode 100644 CandelaKit/Sources/CandelaKit/OledCare/OverlayFade.swift create mode 100644 CandelaKit/Tests/CandelaKitTests/InterruptedDimRecoveryTests.swift create mode 100644 CandelaKit/Tests/CandelaKitTests/OverlayFadeTests.swift diff --git a/Candela/App/AppModel.swift b/Candela/App/AppModel.swift index c3c37e8a..682fa387 100644 --- a/Candela/App/AppModel.swift +++ b/Candela/App/AppModel.swift @@ -1290,6 +1290,52 @@ final class AppModel { } } + @ObservationIgnored private let restoreLog = Logger( + subsystem: "com.rydersel.Candela", category: "restore" + ) + + /// Puts back the brightness of a display a previous process left dimmed. + /// Externals only (the built-in has no DDC register to strand) and brightness + /// only: the dim never touched contrast or volume. + func recoverInterruptedDims() { + let evaluated = displays.count + var reasserted = 0 + for state in displays { + let key = state.display.persistenceKey + let prefs = DisplayPrefs(persistenceKey: key, safeMode: safeMode) + switch InterruptedDimRecovery.action( + markerSurvived: prefs.temporaryDimEngaged, + dimIsLive: state.controller.temporaryDimFactor != nil, + hasStoredValue: state.controller.hasStoredValue, + isSafeMode: safeMode + ) { + case .leave: + continue + case .clearOnly: + prefs.temporaryDimEngaged = false + case .reassert: + // Without the memo reset the re-assert is duplicate-skipped and never + // reaches the wire. + state.controller.resetWriteMemo() + state.controller.reassertHardware() + prefs.temporaryDimEngaged = false + reasserted += 1 + // The tag, never the persistence key: a key without an EDID UUID embeds + // the panel's serial number. + restoreLog.info(""" + interrupted dim recovered on display \ + \(DisplayLogging.tag(for: key), privacy: .public) + """) + } + } + // `.info`, not `.debug`: macOS does not persist debug records. This line is + // what separates "nothing needed recovering" from "the pass never ran". + restoreLog.info(""" + interrupted dim pass: \(evaluated, privacy: .public) evaluated, \ + \(reasserted, privacy: .public) reasserted + """) + } + /// One write-restore pass: every duplicate memo reset FIRST, then re-write /// (brightness DDC leg, contrast, volume, plus the mute companion inside /// `restoreToHardware`). All three legs restore only ever-touched commands: a diff --git a/Candela/App/StatusItemController.swift b/Candela/App/StatusItemController.swift index cd9d4fbf..9dba6998 100644 --- a/Candela/App/StatusItemController.swift +++ b/Candela/App/StatusItemController.swift @@ -654,6 +654,16 @@ final class StatusItemController: NSObject, NSApplicationDelegate, NSMenuDelegat refreshTapConfig() updateStatusItemVisibility() wireInterferenceHooks() + // Crash-while-dimmed recovery, the wire companion to the gamma reset + // above. Ahead of the restore pass so the two never queue writes for one + // display out of order. Safe mode sends no unattended DDC and must not + // consume the marker: the next normal launch still needs it. + // + // LAUNCH ONLY, never on reconfigure. The topology loop above calls + // `oledCare.displaysReconfigured()` first, which drops the lock dim for at + // least one care tick; a recovery in that window would see no live dim, + // write the undimmed value to a locked screen, and spend the marker. + if !isSafeMode { model.recoverInterruptedDims() } restoreCoordinator.noteLaunchOrReconfigure() // Before the first open, for the same reason the display list is warmed // here: nothing the panel starts can be relied on to run while the menu diff --git a/Candela/OledCare/OledCareCoordinator.swift b/Candela/OledCare/OledCareCoordinator.swift index f65dddec..ed038fa6 100644 --- a/Candela/OledCare/OledCareCoordinator.swift +++ b/Candela/OledCare/OledCareCoordinator.swift @@ -1283,8 +1283,9 @@ final class OledCareCoordinator: CheckupCareHolding { switch decision { case let .dim(factor): if !state.lockDimEngaged { - // The lock edge: fade in over ~1.2 s rather than stepping, which is the - // only place a ramp is wanted. Everything else here jumps. + // The lock edge: ramp over ~1.2 s rather than stepping, because an OLED + // bands visibly when the register jumps. The only DDC ramp wanted here, + // and no lift ever fades. state.lockDimRamp = controller.rampTemporaryDim(to: factor) state.lockDimEngaged = true } else if controller.temporaryDimFactor == nil { @@ -1417,7 +1418,11 @@ final class OledCareCoordinator: CheckupCareHolding { } } - guard overlay.apply(alpha: alpha, mask: mask, blackout: blackout, on: id) else { + guard + overlay.apply( + alpha: alpha, mask: mask, blackout: blackout, + mayFadeIn: OverlayFade.fadesInOnEntry(to: dimState), on: id) + else { // No NSScreen matched: nothing reached the screen, so there is nothing to // verify and no state to memoise. The next tick retries, and the overlay // rate-limits its own warning. @@ -1469,11 +1474,13 @@ final class OledCareCoordinator: CheckupCareHolding { adaptiveProtection[key] = protection } - private enum VerifyOutcome { + enum VerifyOutcome: Equatable { case agreed case mismatched - /// A close the server has not finished reporting; neither an attempt nor a - /// mismatch. Bounded by `OledOverlay.closeGrace`. + /// In flight, so neither an attempt nor a mismatch: a close the server has + /// not reported yet (bounded by `OledOverlay.closeGrace`), or a nudge + /// declined while an entry fade toward this same state arrives (bounded by + /// `OledOverlay.fadeDeclineWindow`). case settling } @@ -1482,13 +1489,35 @@ final class OledCareCoordinator: CheckupCareHolding { /// is `reassert(on:)` (NEVER a repeat apply, which is a no-op by construction /// against the overlay's memo) and re-verification waits for the NEXT tick: /// one nudge per detected mismatch, log, don't loop. + /// + /// The nudge declines while an entry fade toward the same state is in flight, + /// so a reconcile cannot snap it. private func verifyLastRender(of state: PerDisplay, on id: CGDirectDisplayID) -> VerifyOutcome { let wanted = state.lastAppliedAlpha != nil - switch (wanted, overlay.verifyPresence(on: id)) { + let presence = overlay.verifyPresence(on: id) + var reasserted = false + if wanted, presence == .absent { + // Silent on a decline: an entry fade spans four fast ticks, so logging it + // would put four errors in the log for every dim that arrives normally. + reasserted = overlay.reassert(on: id) + if reasserted { + log.error("OLED care overlay for display \(id, privacy: .public) not on screen after apply; reasserting") + } + } + return Self.verifyOutcome(wanted: wanted, presence: presence, reasserted: reasserted) + } + + /// Split out of `verifyLastRender` so the one row that can regress silently + /// is pinnable from the app suite. + static func verifyOutcome( + wanted: Bool, presence: OledOverlay.Presence, reasserted: Bool + ) -> VerifyOutcome { + switch (wanted, presence) { case (true, .absent): - log.error("OLED care overlay for display \(id, privacy: .public) not on screen after apply; reasserting") - overlay.reassert(on: id) - return .mismatched + // A declined nudge is our own entry fade, not a structural mismatch. + // Counting it would burn four of the five attempts on a fade nothing is + // failing at; `.settling` retries later with the budget intact. + return reasserted ? .mismatched : .settling case (false, .present): // A removal the server has not honoured: verifyPresence already // re-closed the strand and logged. Check again next tick. diff --git a/Candela/OledCare/OledOverlay.swift b/Candela/OledCare/OledOverlay.swift index 38413fed..1c467ec3 100644 --- a/Candela/OledCare/OledOverlay.swift +++ b/Candela/OledCare/OledOverlay.swift @@ -75,6 +75,27 @@ final class OledOverlay { private var lastApplied: [CGDirectDisplayID: AppliedState] = [:] + /// An entry fade still running on a display's overlay. Kept out of + /// `AppliedState` on purpose: folding it in would make an unchanged state + /// compare unequal while the fade arrives, putting the 10 Hz loop straight + /// back on the window server. + private struct InFlightFade { + let target: AppliedState + let startedAt: ContinuousClock.Instant + } + + private var fadingTo: [CGDirectDisplayID: InFlightFade] = [:] + + /// Supersession counter: a superseded fade's completion handler arrives with + /// a stale generation and must not clear a newer fade's entry. + private var fadeGeneration: [CGDirectDisplayID: UInt64] = [:] + + /// How long a declined nudge stays declined: twice the fade. Past the bound + /// the entry is dropped and the nudge proceeds, because the completion handler + /// can fail to arrive at all (window torn down mid-animation, display departs + /// while it fades), and a reconcile parked forever fails silently. + static let fadeDeclineWindow: Duration = .seconds(2 * OverlayFade.entrySeconds) + /// Displays already warned about for a missing `NSScreen`. The overlay is /// re-driven on every state tick, so an unrated warning floods the log for as /// long as the display stays gone. @@ -96,9 +117,12 @@ final class OledOverlay { /// what keeps the steady-state cadence off the window server. /// /// `mask` is already in DISPLAY orientation; nil keeps the scalar behaviour. + /// + /// `mayFadeIn` only permits a fade; `fadeSeconds` still requires a darkening + /// transition, so a lift stays instant even in a state that fades in. @discardableResult func apply( - alpha: Double?, mask: OverlayMask.Oriented? = nil, blackout: Bool, + alpha: Double?, mask: OverlayMask.Oriented? = nil, blackout: Bool, mayFadeIn: Bool, on displayID: CGDirectDisplayID ) -> Bool { guard let alpha else { @@ -129,11 +153,27 @@ final class OledOverlay { guard !existed || self.lastApplied[displayID] != state else { return true } + // Read before `lastApplied` moves: the gate compares the current alpha with + // the one this state asks for. + let fade = Self.fadeSeconds( + mayFadeIn: mayFadeIn, from: self.lastApplied[displayID]?.alpha, to: state.alpha) self.lastApplied[displayID] = state - self.write(state, to: window) + self.write(state, to: window, on: displayID, fadingOver: fade) return true } + /// How long this transition fades for, or nil to land immediately. A lift + /// never animates whatever state asked for it, so every restore lands in one + /// write. A nil `current` means no overlay on screen and behaves as 0. + /// + /// One transition this cannot see: with a mask the caller passes alpha 1.0 and + /// the per-cell opacity lives in the layer's contents, so a masked idle dim + /// escalating to blackout goes 1.0 to 1.0 and lands instantly. + static func fadeSeconds(mayFadeIn: Bool, from current: Double?, to target: Double) -> Double? { + guard mayFadeIn, target > (current ?? 0) else { return nil } + return OverlayFade.entrySeconds + } + /// Re-asserts the overlay's last applied state: the recovery lever for an /// verification mismatch, where the window server dropped a window we still hold (a /// space transition, another shielding window, a reconfiguration). `apply` @@ -141,23 +181,84 @@ final class OledOverlay { /// /// A no-op when the display has no overlay: creating one here would invent a /// dim level this class does not own. - func reassert(on displayID: CGDirectDisplayID) { + /// + /// Returns false only where a fade toward this exact state is in flight and + /// the nudge was declined, so the caller can tell that from a nudge that did + /// not take. A display with no overlay answers true: no fade to wait for, and + /// a decline would park the caller on one that never ends. + /// + /// `now` is a parameter so the bound can be tested without waiting it out. + @discardableResult + func reassert(on displayID: CGDirectDisplayID, at now: ContinuousClock.Instant = .now) -> Bool { guard let window = self.windows[displayID], let state = self.lastApplied[displayID] else { - return + return true + } + if let fade = self.fadingTo[displayID], fade.target == state, + now - fade.startedAt < Self.fadeDeclineWindow { + return false } - self.write(state, to: window) + // A nudge lands on target without animating, and drops an entry that + // outlived the bound so a missing completion cannot decline forever. + self.write(state, to: window, on: displayID, fadingOver: nil) + return true } - private func write(_ state: AppliedState, to window: NSPanel) { + private func write( + _ state: AppliedState, to window: NSPanel, on displayID: CGDirectDisplayID, + fadingOver seconds: Double? + ) { // Blackout swallows mouse input (at full black a click-through click // is a blind click on live UI); every other level stays click-through. + // + // The swallow leads the pixels: set while the alpha is still fading in, so + // an early click in a blackout is swallowed before the screen is black. + // Delaying it to match the pixels would let that click land on live UI. window.ignoresMouseEvents = !state.blackout // Mask first, since it decides the alpha: 1.0 over a failed mask's flat // black layer blacks out the panel. let masked = Self.writeMask(state.mask, to: window) let alpha = masked ? state.alpha : Self.fallbackAlpha(forUnrendered: state.mask) - window.contentView?.alphaValue = CGFloat(alpha) + guard let seconds else { + self.endFade(on: displayID, view: window.contentView) + window.contentView?.alphaValue = CGFloat(alpha) + window.orderFrontRegardless() + return + } + // Front first: a window that is not on screen cannot fade onto it. window.orderFrontRegardless() + let generation = (self.fadeGeneration[displayID] ?? 0) &+ 1 + self.fadeGeneration[displayID] = generation + self.fadingTo[displayID] = InFlightFade(target: state, startedAt: .now) + NSAnimationContext.runAnimationGroup { context in + context.duration = seconds + context.timingFunction = CAMediaTimingFunction(name: .easeOut) + window.contentView?.animator().alphaValue = CGFloat(alpha) + } completionHandler: { + // AppKit calls this on the main thread, so the clear lands on the turn + // the animation ends. + MainActor.assumeIsolated { self.fadeDidFinish(generation, on: displayID) } + } + } + + /// Clears the in-flight entry only for the fade the display is still waiting + /// on; a superseded handler carries a stale generation and leaves it alone. + private func fadeDidFinish(_ generation: UInt64, on displayID: CGDirectDisplayID) { + guard self.fadeGeneration[displayID] == generation else { return } + self.fadingTo.removeValue(forKey: displayID) + } + + /// Drops the fade state and stops an animation still running. Assignment + /// alone would not: the animation keeps driving the presentation layer toward + /// its own target and only then snaps, so a lift issued mid-fade would keep + /// darkening first. + /// + /// `removeAllAnimations` is safe because ours is the only animation this layer + /// carries: the mask goes straight to `contents`, and a layer-backed view runs + /// no implicit animations outside an animation context. + private func endFade(on displayID: CGDirectDisplayID, view: NSView?) { + guard self.fadingTo.removeValue(forKey: displayID) != nil else { return } + self.fadeGeneration[displayID] = (self.fadeGeneration[displayID] ?? 0) &+ 1 + view?.layer?.removeAllAnimations() } /// Alpha for a mask that did not reach the layer. The caller's 1.0 (the mask @@ -238,6 +339,9 @@ final class OledOverlay { return } self.lastApplied.removeValue(forKey: displayID) + // The window is going and its fade goes with it: the completion handler + // may never arrive to say so. + self.endFade(on: displayID, view: window.contentView) // Retained for the stranded-overlay check and its recovery; cleared once // the server confirms the window is gone, or when a new overlay supersedes // it. A window that never reached the screen has no number to watch (0 is diff --git a/Candela/Settings/ProtectionPane.swift b/Candela/Settings/ProtectionPane.swift index c6faaaeb..f30558e0 100644 --- a/Candela/Settings/ProtectionPane.swift +++ b/Candela/Settings/ProtectionPane.swift @@ -3,12 +3,14 @@ import CoreGraphics import SwiftUI /// The Protection pillar: the policies that guard a display's -/// configuration. The startup and wake restore choice, and under it a read-only -/// summary of what Remember-size promises on each display. The Remember control -/// itself stays on that display's page, so the pref keeps one write surface. +/// configuration. A link to the dimming half of the pillar, then the startup +/// and wake restore choice, and under it a read-only summary of what +/// Remember-size promises on each display. Neither the dimming controls nor the +/// Remember control lives here, so both prefs keep one write surface. /// /// Nothing unbuilt is listed: a greyed row for a feature nobody can turn -/// on is a promise the app cannot keep. +/// on is a promise the app cannot keep. The Dimming row links to a shipped +/// feature on another pane, so it is not one of those. /// /// `@MainActor`: a `View`'s non-`body` properties are nonisolated under complete /// concurrency, and these read main-actor types. @@ -32,11 +34,75 @@ struct ProtectionPane: View { "A display does not always come back the way you left it. Protection holds the rules " + "that decide what \(AppInfo.productName) puts back at startup, at wake, and on reconnect." ) + dimmingSection startupSection rememberedSizesSection } } + // MARK: - Dimming + + /// A link, not a control: every dimming setting lives on the OLED Care pane. + /// It leads the pane because `dimmingNote` names the startup rules as the ones + /// below it. + private var dimmingSection: some View { + let row = dimmingRowValues + return SettingsCardSection(title: "Dimming") { + NavigationRow( + title: "OLED Care", + value: row.value, + spokenValue: row.spokenValue, + action: { actions.reveal(.pane(.oledCare)) }) + SettingsRowNote(verbatim: Self.dimmingNote) + } + } + + /// Both values from one read, so the sighted and the spoken form cannot + /// answer different enrollment counts. + private var dimmingRowValues: (value: String, spokenValue: String) { + Self.dimmingRow(enrolledCount: enrolledExternalCount(), isSafeMode: model.isSafeMode) + } + + /// Verbatim rather than a key: the test bundle reads this string directly. + static let dimmingNote = + "Protection has two halves. The rules below put your settings back; OLED Care does the " + + "dimming, fading idle screens and static regions so they are not left lit at full " + + "brightness for hours." + + /// The count half only. Safe mode outranks it and that branch lives in + /// `dimmingRow`, so a view calling this one directly would claim dimming in a + /// session where the care loop is not running. "Off" is the Remembered Sizes + /// summary's word for the same shape of answer, so the two sections agree. + static func dimmingRowValue(enrolledCount: Int) -> String { + switch enrolledCount { + case 0: "Off" + case 1: "On for 1 display" + default: "On for \(enrolledCount) displays" + } + } + + /// What the row draws and what VoiceOver reads. Enrollment answers first, so + /// nothing enrolled reads "Off" in either kind of session. Safe mode then + /// outranks the count, since the care loop is not running: the display hub's + /// preview already says Paused for the same state, in these words. + static func dimmingRow( + enrolledCount: Int, isSafeMode: Bool + ) -> (value: String, spokenValue: String) { + let counted = dimmingRowValue(enrolledCount: enrolledCount) + guard enrolledCount > 0, isSafeMode else { return (counted, counted) } + return ("Paused", "Paused for this session, Safe Mode") + } + + /// Nothing publishes enrollment, so this is a live read per display. The + /// `prefsRevision` read at the top of `body` is what refreshes the row when + /// enrollment changes elsewhere in the window. `model.displays` is externals + /// only, which is all OLED care enrolls. + private func enrolledExternalCount() -> Int { + model.displays.filter { + DisplayPrefs(persistenceKey: $0.display.persistenceKey).oledCareEnrolled + }.count + } + // MARK: - Startup private var startupSection: some View { @@ -121,7 +187,7 @@ struct ProtectionPane: View { switch action { case .write: "Useful when a display forgets its settings while asleep." case .read: "Reads brightness, contrast and volume back from the display. Not all hardware answers." - case .doNothing: "Keeps using the values from last time, and sends them to the display the first time you change something." + case .doNothing: "Keeps using the values from last time, and sends them to the display the first time you change something. The one exception is a display a crash left dimmed, which gets its brightness back at the next launch." } } diff --git a/CandelaAppTests/OverlayWindowTests.swift b/CandelaAppTests/OverlayWindowTests.swift index 26fd4808..be509fba 100644 --- a/CandelaAppTests/OverlayWindowTests.swift +++ b/CandelaAppTests/OverlayWindowTests.swift @@ -122,8 +122,60 @@ struct OverlayWindowTests { #expect(OledOverlay.fallbackAlpha(forUnrendered: nil) == 0) } + // MARK: - The entry fade + + /// Two of these must not animate whatever the state says, and both break + /// quietly: a faded lift reads as lag on a control that exists to feel + /// instant, and a masked escalation would fade a property that is not moving. + @Test func onlyADarkeningEntryFades() { + #expect(OledOverlay.fadeSeconds(mayFadeIn: true, from: nil, to: 0.5) == OverlayFade.entrySeconds) + #expect(OledOverlay.fadeSeconds(mayFadeIn: true, from: 0.5, to: 0.2) == nil) // a lift is instant + #expect(OledOverlay.fadeSeconds(mayFadeIn: false, from: nil, to: 0.3) == nil) // unfocused dim + #expect(OledOverlay.fadeSeconds(mayFadeIn: true, from: 1.0, to: 1.0) == nil) // masked idle dim to blackout + } + + /// A fade spans four of the five verify attempts at the fast cadence, so + /// mapping a declined nudge to `.mismatched` would spend the budget on a fade + /// nothing is failing at, then log a mismatch that is nothing of the kind. + @Test func aDeclinedNudgeIsSettlingRatherThanAMismatch() { + #expect( + OledCareCoordinator.verifyOutcome(wanted: true, presence: .absent, reasserted: false) + == .settling) + #expect( + OledCareCoordinator.verifyOutcome(wanted: true, presence: .absent, reasserted: true) + == .mismatched) + #expect( + OledCareCoordinator.verifyOutcome(wanted: false, presence: .present, reasserted: false) + == .mismatched) + } + + /// The decline's bound, for the case where the completion handler never comes + /// (window torn down mid-animation, display departs). Without it that display's + /// reconcile sits in `.settling` for the rest of the session. + @Test func anAgedFadeStopsDecliningTheNudge() throws { + let screen = try #require(Self.firstScreen, "the test process sees no screens") + let displayID = try #require(screen.displayID) + let overlay = OledOverlay() + defer { overlay.removeAll() } + + #expect(overlay.apply(alpha: 0.5, blackout: false, mayFadeIn: true, on: displayID)) + // Inside the bound: a nudge now would snap a fade that is still arriving. + #expect(overlay.reassert(on: displayID) == false) + // Past twice the fade the nudge proceeds, and the second call shows the + // bound cleared the entry rather than stepping around it once. + #expect(overlay.reassert(on: displayID, at: .now + .seconds(4 * OverlayFade.entrySeconds))) + #expect(overlay.reassert(on: displayID)) + } + // MARK: - Helpers + /// Any attached display answers the question, so it comes from the live set. A + /// process that sees none fails at `try #require` rather than asserting against + /// nothing, the way `CheckupFieldWindowTests` treats the same absence. + static var firstScreen: NSScreen? { + NSScreen.screens.first { $0.displayID != nil } + } + /// A display ID no attached screen answers to. Derived from the live set /// rather than picked as a constant, so it stays absent whatever is plugged /// in when the suite runs. diff --git a/CandelaAppTests/ProtectionPaneTests.swift b/CandelaAppTests/ProtectionPaneTests.swift index cccbfd46..b4d03011 100644 --- a/CandelaAppTests/ProtectionPaneTests.swift +++ b/CandelaAppTests/ProtectionPaneTests.swift @@ -47,12 +47,53 @@ struct ProtectionPaneTests { pixelWidth: 1920, pixelHeight: 1080, refreshHz: 59.9 ) + // MARK: - The dimming row + + /// The row previews its destination's state, and the singular is not the + /// plural with an s. + @Test func theDimmingRowSaysHowManyDisplaysAreEnrolled() { + #expect(ProtectionPane.dimmingRowValue(enrolledCount: 0) == "Off") + #expect(ProtectionPane.dimmingRowValue(enrolledCount: 1) == "On for 1 display") + #expect(ProtectionPane.dimmingRowValue(enrolledCount: 3) == "On for 3 displays") + } + + /// House copy rules: no em dash, and the hardware is never called a panel. + @Test func theDimmingNoteFollowsTheCopyRules() { + #expect(!ProtectionPane.dimmingNote.isEmpty) + #expect(!ProtectionPane.dimmingNote.contains("\u{2014}")) + #expect(!ProtectionPane.dimmingNote.lowercased().contains("panel")) + } + + /// "The rules below" names the Startup section by POSITION, so the note is + /// true only while Dimming leads the pane. A reworded note comes past this pin. + @Test func theDimmingNoteNamesTheRulesBelowIt() { + #expect( + ProtectionPane.dimmingNote + == "Protection has two halves. The rules below put your settings back; OLED Care does the dimming, fading idle screens and static regions so they are not left lit at full brightness for hours.") + } + + /// Safe mode suppresses the driver loop, so the row must not report the + /// enrollment as if it were running. The words are the display hub preview's. + /// Enrollment still answers first: nothing enrolled reads "Off" either way. + @Test func theDimmingRowSaysPausedInASafeModeSession() { + let paused = ProtectionPane.dimmingRow(enrolledCount: 2, isSafeMode: true) + #expect(paused.value == "Paused") + #expect(paused.spokenValue == "Paused for this session, Safe Mode") + #expect(ProtectionPane.dimmingRow(enrolledCount: 0, isSafeMode: true).value == "Off") + + let running = ProtectionPane.dimmingRow(enrolledCount: 2, isSafeMode: false) + #expect(running.value == "On for 2 displays") + #expect(running.spokenValue == running.value) + } + // MARK: - The startup caption + /// `.doNothing` names its one exception: the launch recovery of an interrupted + /// dim does write, so a caption promising no launch write would be wrong. @Test func eachStartupChoiceKeepsItsOwnSentence() { #expect( render(ProtectionPane.startupCaption(for: .doNothing)) - == "Keeps using the values from last time, and sends them to the display the first time you change something.") + == "Keeps using the values from last time, and sends them to the display the first time you change something. The one exception is a display a crash left dimmed, which gets its brightness back at the next launch.") #expect( render(ProtectionPane.startupCaption(for: .write)) == "Useful when a display forgets its settings while asleep.") @@ -164,8 +205,9 @@ struct ProtectionPaneTests { // MARK: - The page itself /// Layer 2. The fixture model has no displays, so this covers the page - /// with the restore picker on it and the summary in its empty state, which is - /// what a Mac with nothing attached opens on. + /// with the dimming doorway at its head, the restore picker under it, and the + /// summary in its empty state, which is what a Mac with nothing attached + /// opens on. @Test func thePageRendersWithNoDisplaysAttached() { let model = TestFixtures.appModel() let pane = ProtectionPane() diff --git a/CandelaKit/Sources/CandelaKit/Brightness/BrightnessController.swift b/CandelaKit/Sources/CandelaKit/Brightness/BrightnessController.swift index 03577a1d..b4f91376 100644 --- a/CandelaKit/Sources/CandelaKit/Brightness/BrightnessController.swift +++ b/CandelaKit/Sources/CandelaKit/Brightness/BrightnessController.swift @@ -516,6 +516,12 @@ public final class BrightnessController: PendingWireDraining { /// or zeros still publish: those the wire did carry. public func refreshFromHardware(settling: Bool = false) async { guard temporaryDimFactor == nil else { return } + // A surviving marker means the register still carries a dim no process holds + // a factor for, so a readback would adopt our own multiplier and persist it + // over the value the recovery has to put back. The guard above cannot cover + // that: the launch readback runs BEFORE the recovery. The recovery clears + // the marker after its re-assert, so later passes read again. + guard !prefs.temporaryDimEngaged else { return } if role == .builtIn { // The built-in panel has no DDC wire, so the native read is the only truth // (fork: `AppleDisplay.getAppleBrightness`). Publish only; no store, because @@ -2056,8 +2062,17 @@ public final class BrightnessController: PendingWireDraining { /// reconfiguration, which a lock dim outlasts. THIS process is covered, because that /// function returns early while a dim is outstanding. What is left is a readback by /// a process that did not set the dim: the next launch after a crash or force-quit, - /// which finds the register still down with no factor recorded anywhere. The store - /// is right either way; the readback is what would overwrite it. + /// which finds the register still down. The marker closes that one, not the + /// factor: `refreshFromHardware` returns early on both, so no process adopts a + /// register that still carries a dim. `DisplayPrefs.temporaryDimEngaged` records + /// only THAT a dim was outstanding, which is enough for `InterruptedDimRecovery` + /// to put the saved value back and not enough to overwrite it. + /// + /// The native ADOPTION route is not one of those early returns. `adoptExternal` + /// and `adoptNativeForSurface` persist what they read, and under HDR the lock dim + /// rides the native leg. Within a process the factor `freshNativeRead` checks and + /// the echo slot's generation cover them; a fresh process has neither, so a poll + /// tick that beats the recovery is the remaining gap. public private(set) var temporaryDimFactor: Double? /// The ONE place the temporary dim is folded in. Everything that computes a @@ -2137,6 +2152,13 @@ public final class BrightnessController: PendingWireDraining { lastAppliedSw = nil coalescer.resetDuplicateState() applyPaths() + // After the submit, never before: clearing first widens the window where a + // crash looks clean while the register is still down. + // + // Residual: `applyPaths` submits into the coalescer, so a crash in the few ms + // before the bytes land leaves a dimmed register with no marker. An exact + // clear would have to be async, and `applicationWillTerminate` cannot await. + prefs.temporaryDimEngaged = false } /// The one place a dim factor is set. Private so the token discipline above @@ -2159,6 +2181,12 @@ public final class BrightnessController: PendingWireDraining { if starting { lastAppliedSw = nil coalescer.resetDuplicateState() + // On the nil-to-non-nil edge only, so a ramp marks on its first step and + // on none of the rest. Synchronous and ahead of the submit: a crash from + // the moment the first bytes can leave has to find this set. Safe mode + // never reaches here, since `OledCareCoordinator.start` returns before it + // builds the driver loop. + prefs.temporaryDimEngaged = true } applyPaths() } diff --git a/CandelaKit/Sources/CandelaKit/Brightness/InterruptedDimRecovery.swift b/CandelaKit/Sources/CandelaKit/Brightness/InterruptedDimRecovery.swift new file mode 100644 index 00000000..7f927489 --- /dev/null +++ b/CandelaKit/Sources/CandelaKit/Brightness/InterruptedDimRecovery.swift @@ -0,0 +1,36 @@ +/// What a launch does about a temporary-dim marker a previous process left +/// behind. A surviving marker means the brightness register may disagree with +/// the slider, and a write-only panel cannot be asked which is right. +/// +/// Pure and fully parameterized rather than reaching for prefs or a controller, +/// because four of the five answers are silences: a hardware pass cannot tell a +/// launch that correctly wrote nothing from one where the recovery never ran. +public enum InterruptedDimRecovery { + public enum Action: Equatable, Sendable { + /// Reset the write memo and re-assert, then clear the marker. + case reassert + /// Nothing safe to write; consume the marker so later launches do not + /// re-evaluate a display nothing can help. + case clearOnly + case leave + } + + /// Guards in order, each one reason not to write. Safe mode and a live dim + /// both outrank a stored value. + public static func action( + markerSurvived: Bool, dimIsLive: Bool, hasStoredValue: Bool, isSafeMode: Bool + ) -> Action { + guard markerSurvived else { return .leave } + // Safe mode sends no unattended DDC, and consuming the marker here would + // spend the evidence the next normal launch needs. + guard !isSafeMode else { return .leave } + // This process owns the dim. Re-asserting would write the undimmed value to + // a locked screen, and clearing would strand the next crash. + guard !dimIsLive else { return .leave } + // An empty store publishes the ASSUMED 1.0 default, which on an OLED is a + // blast to full output. `AppModel.performRestorePass` refuses to write it + // and so does this. + guard hasStoredValue else { return .clearOnly } + return .reassert + } +} diff --git a/CandelaKit/Sources/CandelaKit/OledCare/OledDimming.swift b/CandelaKit/Sources/CandelaKit/OledCare/OledDimming.swift index 196b94d3..a6054a52 100644 --- a/CandelaKit/Sources/CandelaKit/OledCare/OledDimming.swift +++ b/CandelaKit/Sources/CandelaKit/OledCare/OledDimming.swift @@ -1,6 +1,8 @@ import Foundation -public enum OledDimState: Equatable, Sendable { +/// `CaseIterable` so a rule written over these states can be checked over the +/// whole state space. `OverlayFade.fadesInOnEntry` is the rule that relies on it. +public enum OledDimState: Equatable, Sendable, CaseIterable { case active, idleDim, blackout, lockDim, unfocusedDim, suspended } diff --git a/CandelaKit/Sources/CandelaKit/OledCare/OverlayFade.swift b/CandelaKit/Sources/CandelaKit/OledCare/OverlayFade.swift new file mode 100644 index 00000000..42af1ebb --- /dev/null +++ b/CandelaKit/Sources/CandelaKit/OledCare/OverlayFade.swift @@ -0,0 +1,25 @@ +/// The decision half of OLED care's entry fade: which states fade in and for +/// how long. The animation itself lives in the app target, but the care loop's +/// reconcile and the restore gate both read these. +/// +/// Nothing here fades a lift. Every decrease in dimming lands in one write, and +/// the overlay's own gate keeps that true whatever a state says. +public enum OverlayFade { + /// Long enough not to read as a step in peripheral vision, short enough that + /// the fade cannot outlive the care loop's bounded reconcile budget. + public static let entrySeconds: Double = 0.4 + + /// Which target states fade in. Exhaustive so a state added later has to be + /// ruled on rather than defaulting into the set. `.lockDim` raises no overlay + /// (it is a DDC ramp on the wire); `.unfocusedDim` is already gated on ten + /// minutes of another display holding focus; a detection mask under `.active` + /// arrives while the user is working and stays instant. + public static func fadesInOnEntry(to state: OledDimState) -> Bool { + switch state { + case .idleDim, .blackout: + return true + case .active, .unfocusedDim, .lockDim, .suspended: + return false + } + } +} diff --git a/CandelaKit/Sources/CandelaKit/Support/DisplayPrefs.swift b/CandelaKit/Sources/CandelaKit/Support/DisplayPrefs.swift index bfda5428..b2e0fc21 100644 --- a/CandelaKit/Sources/CandelaKit/Support/DisplayPrefs.swift +++ b/CandelaKit/Sources/CandelaKit/Support/DisplayPrefs.swift @@ -179,6 +179,23 @@ public final class DisplayPrefs: @unchecked Sendable { set { defaults.set(clampSwitchingPoint(newValue), forKey: key("combinedSwitchingPoint")) } } + /// True while a temporary hardware dim is engaged on this display. Set before + /// the dim's first submit and cleared after the restoring one, so a marker + /// that survives a launch means the register may disagree with the slider. + /// + /// Engine state, not a setting: no `PrefName` case, no pane writes it, and a + /// per-display OLED care reset leaves it alone. Clearing it as a setting would + /// discard the one recovery signal. + /// + /// Two identical monitors share one persistence key, so a crash while one of a + /// matched pair was dimmed makes the recovery reassert on both. The twin gets + /// its own saved brightness, so the cost is a redundant write, not a wrong + /// value. + public var temporaryDimEngaged: Bool { + get { defaults.bool(forKey: key("temporaryDimEngaged")) } + set { defaults.set(newValue, forKey: key("temporaryDimEngaged")) } + } + // MARK: - OLED care // The defaults ARE the Recommended preset, so enrolling writes nothing but diff --git a/CandelaKit/Sources/CandelaKit/Support/SafeModeCopy.swift b/CandelaKit/Sources/CandelaKit/Support/SafeModeCopy.swift index e239bd87..fed5a662 100644 --- a/CandelaKit/Sources/CandelaKit/Support/SafeModeCopy.swift +++ b/CandelaKit/Sources/CandelaKit/Support/SafeModeCopy.swift @@ -28,6 +28,10 @@ public enum SafeModeCopy { /// stored, closing both `RestoreCoordinator` passes, and /// `StatusItemController.restoreUnattended()` returns early, which is where the /// stored resolution and the saved arrangement would have been reapplied. + /// + /// Neither of those closes the interrupted-dim recovery, since safe mode + /// reports the same `.doNothing` the shipped default does. It carries its own + /// guard, in `StatusItemController` and in `InterruptedDimRecovery.action`. case restore /// `AppModel` skips `refreshFromHardware` for appeared and kept displays, /// the volume and contrast passes return early because `startupAction` diff --git a/CandelaKit/Tests/CandelaKitTests/DisplayPrefsTests.swift b/CandelaKit/Tests/CandelaKitTests/DisplayPrefsTests.swift index ad3fdb4d..3d97c4e1 100644 --- a/CandelaKit/Tests/CandelaKitTests/DisplayPrefsTests.swift +++ b/CandelaKit/Tests/CandelaKitTests/DisplayPrefsTests.swift @@ -138,6 +138,20 @@ struct DisplayPrefsTests { } } + /// Per display: a launch that recovered one must not write over another. + /// Absent reads false, so an older domain needs no migration. + @Test func theInterruptedDimMarkerIsPerDisplay() { + withSuite { defaults in + let prefs = DisplayPrefs(defaults: defaults, persistenceKey: "one") + let other = DisplayPrefs(defaults: defaults, persistenceKey: "two") + #expect(prefs.temporaryDimEngaged == false) + prefs.temporaryDimEngaged = true + #expect(prefs.temporaryDimEngaged) + #expect(other.temporaryDimEngaged == false) + #expect(defaults.object(forKey: "temporaryDimEngaged.one") as? Bool == true) + } + } + @Test func unknownStoredHDRModeFallsBackToOff() { withSuite { defaults in defaults.set(42, forKey: "hdrMode.pk") diff --git a/CandelaKit/Tests/CandelaKitTests/InterruptedDimRecoveryTests.swift b/CandelaKit/Tests/CandelaKitTests/InterruptedDimRecoveryTests.swift new file mode 100644 index 00000000..6bf88661 --- /dev/null +++ b/CandelaKit/Tests/CandelaKitTests/InterruptedDimRecoveryTests.swift @@ -0,0 +1,54 @@ +import Testing +@testable import CandelaKit + +/// The decision behind the one DDC write Candela makes unattended at launch +/// without the user asking for a restore. Four of the five cases are silences, +/// and a hardware pass cannot tell a silence that held from a pass that never +/// ran, so they are pinned here. +@Suite("Interrupted dim recovery") +struct InterruptedDimRecoveryTests { + @Test func aSurvivingMarkerWithAStoredValueIsReasserted() { + #expect( + InterruptedDimRecovery.action( + markerSurvived: true, dimIsLive: false, hasStoredValue: true, isSafeMode: false + ) == .reassert) + } + + /// Safe mode sends no unattended DDC, and consuming the marker would spend the + /// evidence the next normal launch needs. + @Test func safeModeLeavesTheMarkerForTheNextNormalLaunch() { + #expect( + InterruptedDimRecovery.action( + markerSurvived: true, dimIsLive: false, hasStoredValue: true, isSafeMode: true + ) == .leave) + } + + /// This process owns the dim, so re-asserting would flash a locked screen to + /// full brightness. + @Test func aLiveDimIsNotAnInterruptedOne() { + #expect( + InterruptedDimRecovery.action( + markerSurvived: true, dimIsLive: true, hasStoredValue: true, isSafeMode: false + ) == .leave) + } + + /// The restore pass never writes the assumed 1.0 default over an empty store, + /// and a marker nobody can act on must not be re-evaluated every launch. + @Test func aDisplayWithNothingStoredIsConsumedWithoutAWrite() { + #expect( + InterruptedDimRecovery.action( + markerSurvived: true, dimIsLive: false, hasStoredValue: false, isSafeMode: false + ) == .clearOnly) + } + + /// Over the whole state space rather than one case: "no spurious write" is a + /// silence, and a single case cannot prove it. + @Test func noMarkerIsNoWorkInEveryOtherCombination() { + let flags = [false, true] + #expect(flags.allSatisfy { live in flags.allSatisfy { stored in flags.allSatisfy { safe in + InterruptedDimRecovery.action( + markerSurvived: false, dimIsLive: live, hasStoredValue: stored, isSafeMode: safe + ) == .leave + } } }) + } +} diff --git a/CandelaKit/Tests/CandelaKitTests/LockDimTests.swift b/CandelaKit/Tests/CandelaKitTests/LockDimTests.swift index 05065b83..93c5e3a0 100644 --- a/CandelaKit/Tests/CandelaKitTests/LockDimTests.swift +++ b/CandelaKit/Tests/CandelaKitTests/LockDimTests.swift @@ -19,6 +19,7 @@ struct LockDimTests { let ddc: FakeDDC let native: FakeNativeApplier let store: PathMemoryStore + let prefs: DisplayPrefs let controller: BrightnessController } @@ -47,7 +48,7 @@ struct LockDimTests { storageKey: Self.storageKey, wireSiblings: [] ) - return Rig(ddc: ddc, native: native, store: store, controller: controller) + return Rig(ddc: ddc, native: native, store: store, prefs: prefs, controller: controller) } /// Pure DDC: `disableCombinedBrightness` app-wide, so the whole range is on @@ -431,4 +432,119 @@ struct LockDimTests { #expect(abs(engine.alpha(for: .idleDim)! - 0.8) < 1e-9) #expect(abs(engine.lockDimFactor - 0.2) < 1e-9) } + + // MARK: - The interrupted-dim marker + + /// Set synchronously on the dim's own turn, so a crash any time after the + /// first submit finds it, and gone once the restoring submit is made. No + /// `await` before the first expectation on purpose: a marker written after the + /// wire settled would leave the dim window unrecorded. + @Test func aDimMarksTheDisplayBeforeAnyWriteCanLandAndUnmarksAfterTheRestore() async { + let rig = makeHardwareRig() + rig.controller.setBrightness(0.8) + await rig.controller.waitForPendingWrites() + #expect(!rig.prefs.temporaryDimEngaged) + rig.controller.beginTemporaryDim(factor: 0.5) + #expect(rig.prefs.temporaryDimEngaged) + await rig.controller.waitForPendingWrites() + rig.controller.endTemporaryDim() + #expect(!rig.prefs.temporaryDimEngaged) + } + + /// The token discipline, at the marker. A step still in the air when the user + /// unlocked must not re-mark a display whose register was already handed back, + /// or the next launch reasserts over a session that ended clean. + @Test func aStepFromASupersededRampCannotReMarkAfterTheRestore() async { + let rig = makeHardwareRig() + rig.controller.lockDimRampInterval = .milliseconds(20) + rig.controller.setBrightness(0.8) + await rig.controller.waitForPendingWrites() + let ramp = rig.controller.rampTemporaryDim(to: 0.1) + try? await Task.sleep(for: .milliseconds(50)) // a few steps in, not finished + // That the ramp marks at all, asserted first: without it the rest passes + // just as happily if a ramp stopped marking. + #expect(rig.prefs.temporaryDimEngaged) + rig.controller.endTemporaryDim() + await rig.controller.waitForPendingWrites() + #expect(!rig.prefs.temporaryDimEngaged) + + // NOT cancelled: let the remaining steps run out and confirm none re-marked. + _ = await ramp.value + try? await Task.sleep(for: .milliseconds(60)) + await rig.controller.waitForPendingWrites() + #expect(!rig.prefs.temporaryDimEngaged) + } + + /// The marker records THAT a dim was outstanding and nothing about its value. + /// Sibling of `theDimNeverTouchesThePublishedValueOrTheStore`. + @Test func aMarkedDisplayIsStillTheUsersValueEverywhereElse() async { + let rig = makeHardwareRig() + rig.controller.setBrightness(0.8) + rig.controller.beginTemporaryDim(factor: 0.5) + await rig.controller.waitForPendingWrites() + #expect(rig.prefs.temporaryDimEngaged) + #expect(rig.controller.brightness == 0.8 && rig.store.values[Self.storageKey] == 0.8) + } + + // MARK: - The readback guard the marker carries + + /// A marked register holds a dim nothing has undone, so the panel would answer + /// our own multiplier and adopting it persists the dim over the saved value. + /// The live-dim guard cannot cover it: the launch readback runs before the + /// recovery, so the store is clobbered first and then "recovered" to the + /// clobbered number. + @Test func aSurvivingMarkerStopsTheReadbackFromAdoptingOurOwnDim() async { + let rig = makeHardwareRig() + rig.controller.setBrightness(0.8) + await rig.controller.waitForPendingWrites() + // Marked with no dim outstanding: the shape a crash leaves behind. + rig.prefs.temporaryDimEngaged = true + await rig.ddc.setReadResult((current: 40, max: 100)) + await rig.controller.refreshFromHardware() + #expect(await rig.ddc.recordedReadCount() == 0) + #expect(rig.controller.brightness == 0.8) + #expect(rig.store.values[Self.storageKey] == 0.8) + } + + /// The control: with the marker clear the same call reads and adopts, so the + /// guard above cannot be a permanent stop with every assertion still holding. + @Test func anUnmarkedDisplayStillReadsAndAdopts() async { + let rig = makeHardwareRig() + rig.controller.setBrightness(0.8) + await rig.controller.waitForPendingWrites() + await rig.ddc.setReadResult((current: 40, max: 100)) + await rig.controller.refreshFromHardware() + #expect(await rig.ddc.recordedReadCount() == 1) + #expect(abs(rig.controller.brightness - 0.4) < 1e-9) + #expect(abs((rig.store.values[Self.storageKey] ?? 0) - 0.4) < 1e-9) + } + + /// The stop is not permanent: the recovery clears the marker after it + /// re-asserts, so the next pass reads again. The reassert arm runs by hand + /// because `AppModel`'s walk builds prefs over the standard defaults domain and + /// needs a host; asserting the decision first keeps this from drifting. + @Test func theReadbackResumesOnceTheRecoveryHasClearedTheMarker() async { + let rig = makeHardwareRig() + rig.controller.setBrightness(0.8) + await rig.controller.waitForPendingWrites() + rig.prefs.temporaryDimEngaged = true + #expect( + InterruptedDimRecovery.action( + markerSurvived: rig.prefs.temporaryDimEngaged, + dimIsLive: rig.controller.temporaryDimFactor != nil, + hasStoredValue: rig.controller.hasStoredValue, + isSafeMode: false + ) == .reassert + ) + rig.controller.resetWriteMemo() + rig.controller.reassertHardware() + rig.prefs.temporaryDimEngaged = false + await rig.controller.waitForPendingWrites() + #expect(await rig.ddc.recordedWrites().last?.value == 80) + + await rig.ddc.setReadResult((current: 80, max: 100)) + await rig.controller.refreshFromHardware() + #expect(await rig.ddc.recordedReadCount() == 1) + #expect(abs(rig.controller.brightness - 0.8) < 1e-9) + } } diff --git a/CandelaKit/Tests/CandelaKitTests/OverlayFadeTests.swift b/CandelaKit/Tests/CandelaKitTests/OverlayFadeTests.swift new file mode 100644 index 00000000..c9f63600 --- /dev/null +++ b/CandelaKit/Tests/CandelaKitTests/OverlayFadeTests.swift @@ -0,0 +1,26 @@ +import Foundation +import Testing +@testable import CandelaKit + +/// The entry fade's two decisions that live in the Kit: which states fade in, +/// and for how long. Neither can be checked by looking at a panel: too short is +/// indistinguishable from a step, too long from a working fade with a reconcile +/// stalled behind it. +@Suite("Overlay entry fade") +struct OverlayFadeTests { + /// Asserted over the whole state space so a state added later has to be ruled + /// on rather than defaulting into the set. `.lockDim` raises no overlay at all, + /// since it is delivered on the wire. + @Test func onlyIdleDimAndBlackoutFadeOnEntry() { + #expect(OledDimState.allCases.filter(OverlayFade.fadesInOnEntry) == [.idleDim, .blackout]) + } + + /// Below one fast care tick the fade is indistinguishable from a step; at or + /// beyond the lock ramp it outlives the reconcile budget the declined-nudge + /// rule protects. + @Test func theEntryFadeOutlastsATickAndUndercutsTheLockRamp() { + #expect(OverlayFade.entrySeconds == 0.4) + #expect(Duration.seconds(OverlayFade.entrySeconds) > OledCareCadence.fast) + #expect(Duration.seconds(OverlayFade.entrySeconds) < LockDimRamp.duration) + } +} diff --git a/CandelaKit/Tests/CandelaKitTests/PrefPropagationTests.swift b/CandelaKit/Tests/CandelaKitTests/PrefPropagationTests.swift index f3574ecb..5f3f3b7b 100644 --- a/CandelaKit/Tests/CandelaKitTests/PrefPropagationTests.swift +++ b/CandelaKit/Tests/CandelaKitTests/PrefPropagationTests.swift @@ -38,6 +38,10 @@ struct PrefPropagationTests { // reason (`pollingMode` is read at use and IS a case); having no pane to // write it through is, so nothing can route a change. #expect(PrefName(rawValue: "wireTimingGuard") == nil) + // The controller writes the interrupted-dim marker on the dim's own edges. + // No pane writes it, and a reset that cleared it as a setting would discard + // the recovery signal. + #expect(PrefName(rawValue: "temporaryDimEngaged") == nil) } @Test func oledEngineStateIsNotAPrefName() { diff --git a/docs/ADVANCED-SETTINGS.md b/docs/ADVANCED-SETTINGS.md index 88105295..a8de0d86 100644 --- a/docs/ADVANCED-SETTINGS.md +++ b/docs/ADVANCED-SETTINGS.md @@ -130,6 +130,9 @@ page only — Candela has never written them and has no accessor for them. **only** record of the display's state, because nothing can be read back. Deleting them is a real loss, not a cache invalidation. - `muted.` is engine state, not a preference. +- `temporaryDimEngaged.` records that a temporary dim was engaged on that + display, so a launch after a crash can put the brightness back. It is engine + state, not a preference. ## Resetting diff --git a/tools/hardware-pass/pixeltime.swift b/tools/hardware-pass/pixeltime.swift index 20053686..dce3b6fd 100644 --- a/tools/hardware-pass/pixeltime.swift +++ b/tools/hardware-pass/pixeltime.swift @@ -1,6 +1,8 @@ // Restore latency at the pixel, not the window list: ScreenCaptureKit samples -// of one rect around a synthetic mouse move. The first reading is the dimmed -// control, so a run with no overlay up reports "no dim to measure", not a latency. +// of one rect around a synthetic mouse move. The control is two readings 0.6 s +// apart rather than one, so a dim that is still fading in cannot be measured as +// though it had settled; a control that is still moving aborts, and a run with +// no overlay up reports "no dim to measure" rather than a latency. // Usage: pixeltime import CoreGraphics import Foundation @@ -32,8 +34,26 @@ func run() async throws { } return n == 0 ? -1 : Double(sum) / Double(n) } + // Two samples: an idle dim or blackout fades in over OverlayFade.entrySeconds + // (0.4 s), so a run started mid-fade would report a latency that is partly the + // entry's. 0.6 is that fade plus margin, a literal because a standalone script + // cannot import CandelaKit. + let firstControl = await mean() + try await Task.sleep(for: .seconds(0.6)) let dimmed = await mean() - print(String(format: "dimmed control: %.1f", dimmed)) + // A failed capture returns -1, which looks exactly like a control that is still + // moving. Exit 3 is the hardware pass's proof that a settle can fail, so a + // capture failure must not wear it. + guard firstControl >= 0, dimmed >= 0 else { print("capture failed"); exit(2) } + // Mean channel value on 0...255. The sampled region is static while a dim is + // up, so a move larger than this is the fade, not noise. + let controlTolerance = 1.0 + if abs(dimmed - firstControl) > controlTolerance { + print(String(format: "entry fade still in flight: control moved %.1f to %.1f. Rerun once the dim has settled", + firstControl, dimmed)) + exit(3) + } + print(String(format: "dimmed control: %.1f (steady over 0.6 s)", dimmed)) let f = DateFormatter(); f.dateFormat = "HH:mm:ss.SSS" let loc = CGEvent(source: nil)?.location ?? CGPoint(x: 100, y: 100) let ev = CGEvent(mouseEventSource: nil, mouseType: .mouseMoved, From 5a60c68a971444b8bc96fc3504ae3c699e245611 Mon Sep 17 00:00:00 2001 From: Ryder Selikow <50812202+Rydersel@users.noreply.github.com> Date: Thu, 10 Sep 2026 20:42:34 -0500 Subject: [PATCH 2/2] Recover every display sharing an interrupted-dim marker --- Candela/App/AppModel.swift | 20 ++++- .../InterruptedDimRecoveryTests.swift | 86 +++++++++++++++++++ 2 files changed, 102 insertions(+), 4 deletions(-) create mode 100644 CandelaAppTests/InterruptedDimRecoveryTests.swift diff --git a/Candela/App/AppModel.swift b/Candela/App/AppModel.swift index 682fa387..8c0b64ee 100644 --- a/Candela/App/AppModel.swift +++ b/Candela/App/AppModel.swift @@ -1300,11 +1300,16 @@ final class AppModel { func recoverInterruptedDims() { let evaluated = displays.count var reasserted = 0 + // Twins share a marker, but each has its own wire. Snapshot before any + // clearing so discovery order cannot decide which one gets recovered. + let markedKeys = Set(displays.map(\.display.persistenceKey)).filter { + DisplayPrefs(persistenceKey: $0, safeMode: safeMode).temporaryDimEngaged + } + var keysToClear: Set = [] for state in displays { let key = state.display.persistenceKey - let prefs = DisplayPrefs(persistenceKey: key, safeMode: safeMode) switch InterruptedDimRecovery.action( - markerSurvived: prefs.temporaryDimEngaged, + markerSurvived: markedKeys.contains(key), dimIsLive: state.controller.temporaryDimFactor != nil, hasStoredValue: state.controller.hasStoredValue, isSafeMode: safeMode @@ -1312,13 +1317,13 @@ final class AppModel { case .leave: continue case .clearOnly: - prefs.temporaryDimEngaged = false + keysToClear.insert(key) case .reassert: // Without the memo reset the re-assert is duplicate-skipped and never // reaches the wire. state.controller.resetWriteMemo() state.controller.reassertHardware() - prefs.temporaryDimEngaged = false + keysToClear.insert(key) reasserted += 1 // The tag, never the persistence key: a key without an EDID UUID embeds // the panel's serial number. @@ -1328,6 +1333,13 @@ final class AppModel { """) } } + // A recovered twin cannot consume the signal a still-live dim needs if + // this process crashes next. The whole pass is synchronous on the main actor. + let liveDimKeys = Set(displays.filter { $0.controller.temporaryDimFactor != nil } + .map(\.display.persistenceKey)) + for key in keysToClear.subtracting(liveDimKeys) { + DisplayPrefs(persistenceKey: key, safeMode: safeMode).temporaryDimEngaged = false + } // `.info`, not `.debug`: macOS does not persist debug records. This line is // what separates "nothing needed recovering" from "the pass never ran". restoreLog.info(""" diff --git a/CandelaAppTests/InterruptedDimRecoveryTests.swift b/CandelaAppTests/InterruptedDimRecoveryTests.swift new file mode 100644 index 00000000..172ecb6e --- /dev/null +++ b/CandelaAppTests/InterruptedDimRecoveryTests.swift @@ -0,0 +1,86 @@ +import CandelaKit +import CoreGraphics +import Foundation +import Testing + +@Suite("Interrupted dim recovery across shared identities") +@MainActor +struct SharedDimRecoveryTests { + // A marker belongs to an identity, but every attached controller has its own + // wire. Clearing it after the first controller strands the second twin. + @Test(arguments: [false, true]) + func bothTwinsRecoverRegardlessOfDiscoveryOrder(reversed: Bool) async throws { + let rig = await makeRig(reversed: reversed) + defer { rig.clearPrefs() } + #expect(rig.model.displays.count == 2) + #expect(rig.writers.allSatisfy { $0.writes.isEmpty }) + + rig.model.recoverInterruptedDims() + for state in rig.model.displays { await state.controller.waitForPendingWrites() } + + for writer in rig.writers { + #expect(writer.writes.filter { $0.command == 0x10 }.map(\.value) == [100]) + } + #expect(!rig.prefs.temporaryDimEngaged) + } + + // A live twin must retain its marker even when another twin recovers first. + // Otherwise a second crash loses the only signal that its register is dimmed. + @Test(arguments: [false, true]) + func aLiveTwinKeepsTheSharedMarker(reversed: Bool) async throws { + let rig = await makeRig(reversed: reversed) + defer { rig.clearPrefs() } + let live = try #require(rig.model.controller(for: 901)) + live.beginTemporaryDim(factor: 0.2) + await live.waitForPendingWrites() + #expect(rig.writers[0].writes.filter { $0.command == 0x10 }.map(\.value) == [20]) + + rig.model.recoverInterruptedDims() + for state in rig.model.displays { await state.controller.waitForPendingWrites() } + + #expect(live.temporaryDimFactor == 0.2) + #expect(rig.writers[0].writes.filter { $0.command == 0x10 }.map(\.value) == [20]) + #expect(rig.writers[1].writes.filter { $0.command == 0x10 }.map(\.value) == [100]) + #expect(rig.prefs.temporaryDimEngaged) + } + + private struct Rig { + let model: AppModel + let key: String + let prefs: DisplayPrefs + let writers: [FakeDDCWriter] + + func clearPrefs() { + for name in UserDefaults.standard.dictionaryRepresentation().keys where name.hasSuffix(".\(key)") { + UserDefaults.standard.removeObject(forKey: name) + } + } + } + + private func makeRig(reversed: Bool) async -> Rig { + let key = "app-tests-shared-dim-\(UUID().uuidString)" + let prefs = DisplayPrefs(persistenceKey: key) + prefs.combinedSwitchingPoint = -8 + prefs.temporaryDimEngaged = true + UserDefaults.standard.set(1.0, forKey: "combinedBrightness.\(key)") + let writers = [FakeDDCWriter(), FakeDDCWriter()] + var entries: AppModel.DiscoveredDisplays = writers.enumerated().map { index, writer in + ( + display: ExternalDisplay(id: CGDirectDisplayID(901 + index), name: "Twin", persistenceKey: key), + writer: writer, + facts: DisplayHardwareFacts( + transportUpstream: nil, transportDownstream: nil, manufacturerID: nil, + alphanumericSerialNumber: nil, numericSerialNumber: nil, + physicalWidthCm: nil, physicalHeightCm: nil, ioDisplayLocation: nil, + ioregMatchScore: 0) + ) + } + if reversed { entries.reverse() } + let model = AppModel( + shade: FakeShade(), gamma: FakeGamma(), hdrToggling: FakeHDR(), audioDevices: FakeAudio(), + discoverDisplays: { _ in entries }) + await model.refresh() + for state in model.displays { await state.controller.waitForPendingWrites() } + return Rig(model: model, key: key, prefs: prefs, writers: writers) + } +}