Skip to content

brightness: match monitors by DRM connector, per-monitor setProc#190

Open
kanikamaxxing wants to merge 1 commit into
Axenide:mainfrom
kanikamaxxing:fix/brightness-slider-monitor-mismatch
Open

brightness: match monitors by DRM connector, per-monitor setProc#190
kanikamaxxing wants to merge 1 commit into
Axenide:mainfrom
kanikamaxxing:fix/brightness-slider-monitor-mismatch

Conversation

@kanikamaxxing
Copy link
Copy Markdown

Summary

Two related bugs that combined to make per-monitor brightness sliders unusable on multi-monitor setups where Quickshell.screens and ddcutil detect enumerate monitors in a different order.

Bug 1 — shared setProc race

setProc was a single Process declared at the Brightness Singleton root, shared by every BrightnessMonitor. When two sliders fired setBrightness near-simultaneously, the second setProc.command = … overwrote the first before startDetached() had executed, so the first monitor's brightness change never reached ddcutil.

Bug 2 — ddcEntry matching cross-wires sliders

The ddcEntry lookup tried screen.model first, then fell back to first-unused-bus order. Quickshell's screen.model can be the bare model name while ddcutil includes a manufacturer prefix (e.g. H27T6 vs SKG H27T6), so the model match often fails. When Quickshell.screens enumerates monitors in a different order than ddcutil detect, the first-unused-bus fallback then assigns each BrightnessMonitor to the wrong DDC bus — slider for screen A controls bus B and vice versa.

Changes

  • Per-monitor setProc — each BrightnessMonitor now owns its own Process so concurrent slider changes don't overwrite each other's command.
  • DRM connector matchddcutil detect --brief already emits DRM connector: card0-DP-1; we now parse that into entry.drmConnector and match entry.drmConnector.endsWith(screen.name) as the primary lookup. This is the most reliable cross-reference back to a ShellScreen and avoids both the model-prefix mismatch and the screen-order fallback.
  • Model match is kept as a secondary fallback, and the original first-unused-bus heuristic is kept as a last resort.

Test plan

  • Dual-monitor setup (DP-1, DP-2) where Quickshell enumerates them in the opposite order to ddcutil. Each monitor's slider now controls its own monitor's brightness deterministically.
  • Drag both sliders simultaneously — both monitors receive their respective brightness updates (no more last-write-wins drop).

— robin

Two related bugs that combined to make per-monitor brightness sliders unusable
on multi-monitor setups where Quickshell.screens and ddcutil's detect order
disagree:

1. The shared setProc was a single Process instance reused across all
   BrightnessMonitor instances. Concurrent setBrightness calls from different
   sliders would overwrite each other's command before startDetached fired,
   so the last-written command wins and the other slider's change is lost.
   Each BrightnessMonitor now owns its own setProc.

2. ddcEntry matching only used screen.model (which can be stripped of the
   manufacturer prefix that ddcutil includes) and otherwise fell back to
   first-unused-bus order. When Quickshell.screens enumerates monitors in a
   different order than ddcutil, the fallback cross-wires the sliders: the
   slider for screen A controls ddc-bus B and vice versa. Now matches the
   DRM connector first (entry.drmConnector.endsWith(screen.name)), which is
   the most reliable cross-reference, then falls back to model match, then
   to the existing first-unused-bus heuristic.

Together: the slider on each monitor now controls its own monitor's brightness
deterministically regardless of screen enumeration order or simultaneous use.
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.

1 participant