Skip to content

Fix external brightness delays with multiple displays - #71

Open
DEOWL-kan wants to merge 10 commits into
didriksg:mainfrom
DEOWL-kan:fix/ddc-brightness-latency
Open

Fix external brightness delays with multiple displays#71
DEOWL-kan wants to merge 10 commits into
didriksg:mainfrom
DEOWL-kan:fix/ddc-brightness-latency

Conversation

@DEOWL-kan

@DEOWL-kan DEOWL-kan commented Aug 21, 2026

Copy link
Copy Markdown

Summary

  • Match Apple Silicon DDC channels by stable CoreDisplay/IORegistry location before falling back to display identity or traversal order.
  • Run DDC operations on a serial queue per display, so one slow I2C channel cannot block every external monitor.
  • Coalesce rapid brightness changes to the latest target and apply an immediate software preview while DDC capability is unknown or slow.
  • Reject stale DDC reads, writes, and callbacks after newer user input or a display topology change.

Root cause

Crisp previously routed every DDC transaction through one global serial queue. On the reproduced setup, one external display could block an I2C read for about 12 seconds, which delayed brightness changes for the other display as well. Identical monitors could also be paired by unstable traversal order, while reconnect callbacks could update state belonging to a newer display topology.

Verification

  • make check: 75 tests passed, strict SwiftLint passed, and localization keys passed.
  • arm64 release compilation passed.
  • x86_64 Xcode build passed with warnings treated as errors.
  • Hardware-tested with two AOC U32N10 displays on Apple Silicon. While the slow channel was blocked, its slider responded in 2.26 ms with immediate gamma preview; the healthy display responded in 1.57 ms and its DDC brightness read back as 67/100 in 51 ms.
  • Restored the test displays to their original 50/77 brightness values after verification.

Closes #72.

@didriksg didriksg left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right fix for the right cause. One global DDC queue was always going to let a slow monitor hold the others hostage, and per-display queues plus location-based matching are what I'd have reached for too. The timing numbers from the two AOCs are appreciated; that's the kind of evidence I can't get here.

Two regressions I found reading the diff, both in the multi-display path this PR is about:

  1. DisplayManager.refreshDisplays now calls invalidateDDCTopology(for:) with every online external display on every add, remove or move event, not just the display that changed. invalidateDDCTopology cancels the animator, drops pendingDDCTarget, bumps the topology generation and clears ddcAvailable for each of them. So: drag display A's slider to 80, and while the write waits out the 50 ms pacing floor, display B replugs (or you rearrange displays in System Settings, which is a .movedFlag with no ID change). A's pending target is gone, pumpDDCWrite sees a stale topology and returns, and the software fallback never runs either because it checks the same generation. The slider says 80, the monitor never got it. Same thing truncates a running setBrightnessSmooth fade from the brightness keys or a preset. Before this PR only the removed display's state was cleared. Could you scope the invalidation to the IDs that actually disappeared or changed identity or location?

  2. ddcFailStreak is deleted, so a single failed coalesced write now sets ddcAvailable = false until reconnect. docs/ddc-notes.md documents the three-failure latch on purpose: the AOC Q27G3XMN drops about half of its DDC commands at random, and with this change it would fall to gamma mode on the first drag. DDCService.writeAsync retrying three times within 100 ms doesn't cover that, since the monitor ignores the retries too. Either keep a multi-cycle grace before latching, or make the case for the change in the PR so the notes can follow.

Two smaller things: please drop docs/superpowers/plans/... and docs/superpowers/specs/... (they're your planning notes, and already out of step with the code, for example the removeQueue that never got implemented) and the .worktrees/ line in .gitignore.

With those in, I'll run it on my displays and merge. Thanks for digging into this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

External brightness changes can be delayed by a slow DDC display

2 participants