Sync XDR state after wake and restore native brightness keys below 100% - #6
Conversation
Two wake-related XDR fixes: - After a plain sleep/wake the XDR boost is now resumed before the refresh loop restarts (the loop's first pass used to overwrite the stored >1.0 brightness with the un-boosted panel reading, silently dropping the setting), the loop itself now restarts on every architecture, and a delayed reconciliation snaps the stored value, slider and menus back to what the panel really shows whenever the boost fails to come back at all. - The brightness keys are only held back from macOS while the built-in XDR panel is at the top of the standard range or boosting; below that macOS gets the keys back, so its native brightness OSD and sliders return. The one-second refresh loop retakes the keys as the panel reaches 100% again, keeping the XDR opt-in reachable from the keys. The native OSD is also shown again for Apple displays everywhere except macOS 27, where the OSDManager overlay stays permanently drawn.
Greptile SummaryThis PR synchronizes stored XDR brightness with the panel after wake and restores native brightness-key handling below 100%.
Confidence Score: 4/5The PR is not yet safe to merge because the delayed wake callback can still reconcile state during a newer wake cycle. The previous wake-cycle finding remains unresolved: the delayed callback still checks only whether Files Needing Attention: MonitorControl/Support/AppDelegate.swift, MonitorControlTests/XDRBrightnessTests.swift
|
| Filename | Overview |
|---|---|
| MonitorControl/Support/AppDelegate.swift | Reorders wake recovery and schedules delayed XDR reconciliation; the previously reported stale-callback race remains outstanding. |
| MonitorControl/Model/AppleDisplay.swift | Adds failed-resume reconciliation and enables native brightness OSD on supported macOS versions. |
| MonitorControl/Support/MediaKeyTapManager.swift | Dynamically returns brightness keys to macOS below the XDR boundary and retakes them at 100%. |
| MonitorControlTests/XDRBrightnessTests.swift | Adds XDR wake and media-key tests; the previously reported dependence on process-wide smooth-brightness preferences remains outstanding. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Mac wakes] --> B{Display reconfiguration needed?}
B -->|Yes| C[configure displays and resume XDR]
B -->|No| D[update AVServices]
D --> E[resume XDR]
E --> F[restart refresh loop]
C --> G[Schedule reconciliation after 10 seconds]
F --> G
G --> H{Same wake remains settled?}
H -->|No| I[Skip reconciliation]
H -->|Yes| J{Stored brightness above 100% and boost inactive?}
J -->|No| K[Keep current state]
J -->|Yes| L[Read panel brightness]
L --> M[Correct preference and slider]
Reviews (3): Last reviewed commit: "Drop the menu rebuild from wake reconcil..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| DispatchQueue.main.asyncAfter(deadline: .now() + 10.0) { [weak self] in | ||
| guard let self = self, self.sleepID == 0, self.reconfigureID == 0 else { | ||
| return | ||
| } | ||
| self.reconcileXDRStateForAllDisplays() | ||
| } |
There was a problem hiding this comment.
If another sleep/wake cycle completes during this ten-second delay, both lifecycle IDs can return to zero, so the callback from the earlier wake passes this guard while the newer XDR resume is still settling. Because the boost may not yet be reported as active, reconciliation can replace the valid stored extended brightness with the panel's SDR value. Tie the callback to the wake cycle that scheduled it or cancel it when another sleep begins.
Prompt To Fix With AI
This is a comment left during a code review.
Path: MonitorControl/Support/AppDelegate.swift
Line: 315-320
Comment:
**Stale wake callback reconciles**
If another sleep/wake cycle completes during this ten-second delay, both lifecycle IDs can return to zero, so the callback from the earlier wake passes this guard while the newer XDR resume is still settling. Because the boost may not yet be reported as active, reconciliation can replace the valid stored extended brightness with the panel's SDR value. Tie the callback to the wake cycle that scheduled it or cancel it when another sleep begins.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
The dummy AppleDisplay.getBrightness() short-circuits to 1, so the reconciliation assertions were reading the constant instead of the stored value: the leave-alone cases failed outright and the snap case passed vacuously. The stub reads the preference, the same read the real non-dummy path performs.
| display.savePref(Float(1.5), for: .brightness) | ||
|
|
||
| // The engine never resumed after the wake, so the panel is really at its SDR maximum. | ||
| // The app must stop claiming 150% and say what the panel shows. |
There was a problem hiding this comment.
If the process-wide disableSmoothBrightness preference is true, setBrightness takes the direct-write path, which rejects writes to this dummy display. The stored brightness therefore remains at 1.5 and this test fails because of external user-default state rather than the behavior under test. Set and restore the smooth-brightness preference in the test, or use a stub that records the reconciliation write independently of global preferences.
Prompt To Fix With AI
This is a comment left during a code review.
Path: MonitorControlTests/XDRBrightnessTests.swift
Line: 307
Comment:
**Test Depends on Global Preference**
If the process-wide `disableSmoothBrightness` preference is true, `setBrightness` takes the direct-write path, which rejects writes to this dummy display. The stored brightness therefore remains at 1.5 and this test fails because of external user-default state rather than the behavior under test. Set and restore the smooth-brightness preference in the test, or use a stub that records the reconciliation write independently of global preferences.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.The test host never builds the menu (applicationDidFinishLaunching returns before setMenu() under XCTest), so the deferred app.updateMenusAndKeys() force-unwrapped a nil menu and crashed the next test. The rebuild was not doing anything anyway: reconciliation leaves xdrEnabled alone, so the menu items and the slider's extended range are unchanged, the live slider is updated directly, and the menu-bar icon follows the engine's boost notifications.
1 participant
Problem 1: stale XDR state after sleep/wake
Sometimes, after the Mac sleeps with XDR extended brightness on, waking leaves the boost disabled (fine — the panel is back at SDR) but the app still reports the panel as boosted: the slider stays at 150% and the stored preference says 1.5.
Three contributing causes, all fixed here:
soberNow()restarted the one-second refresh loop before resuming the boost. The loop's first pass read the un-boosted panel and dragged the stored >1.0 brightness back toward the SDR maximum before the boost had been re-applied — so the boost silently never came back. The resume now runs first.soberNow()only restarted the loop in the arm64 branch; the loop is what keeps sliders and stored values in step with panels, so it now restarts on every architecture (updateArm64AVServices()is internally a no-op where it does not apply).AppleDisplay.reconcileXDRStateAfterWake()). XDR stays enabled, so pushing past 100% starts the boost again.Problem 2: the Mac's own brightness UI vanished while the app runs
With only the built-in display attached, the app kept the brightness keys away from macOS at every brightness (so the XDR opt-in stayed reachable from the keys), and the native brightness OSD was suppressed for Apple displays everywhere (macOS 27's
OSDManagerleaves the overlay permanently drawn). The result: pressing F1/F2 changed the panel with no OSD, no slider — no feedback at all.MediaKeyTapManager.shouldHoldBrightnessKeysForXDR()holds the decision.Tests
Added to
XDRBrightnessTests(no hardware touched — dummy and stub displays):