[PR #1906] Preserve the brightness offset between displays while syncing #1329

Open
opened 2026-10-02 22:09:50 -06:00 by gitea-mirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/MonitorControl/MonitorControl/pull/1906
Author: @anandghegde
Created: 9/20/2026
Status: 🔄 Open

Base: main ← Head: feat/preserve-brightness-sync-offsets


📝 Commits (1)

  • a4e1f7d Preserve the brightness offset between displays while syncing (#1781)

📊 Changes

5 files changed (+71 additions, -3 deletions)

View changed files

📝 MonitorControl.xcodeproj/project.pbxproj (+4 -0)
📝 MonitorControl/Model/Display.swift (+2 -0)
📝 MonitorControl/Support/AppDelegate.swift (+8 -3)
➕ MonitorControl/Support/BrightnessSync.swift (+52 -0)
📝 MonitorControl/View Controllers/Preferences/MainPrefsViewController.swift (+5 -0)

📄 Description

Closes #1781.

The problem

AppDelegate.job() synced by adding the reported delta to each display and clamping the result:

let newValue = max(0, min(1, targetDisplay.getBrightness() + delta))

Whatever part of the change did not fit was dropped. A display parked near the top of its range loses ground on every ambient light swing up, but gets the whole change back on the way down — so the displays creep together until the sliders are aligned, which is what the issue describes ("especially when the external monitor reaches its maximum brightness and then scales back — the brightness sliders often end up aligned").

The change

BrightnessSync keeps an unclamped anchor per display: where the display would be if it had no ends. The delta is applied to the anchor, the display is driven to the clamped anchor. The part that did not fit is therefore given back on the way down instead of being lost, and the offset survives.

The anchor re-anchors itself whenever the display is no longer where the last sync left it — so a brightness the user sets by hand simply becomes the new relationship to keep, and no other code path has to know the anchor exists. The anchors are also dropped when the sync checkbox is toggled, since the user was free to move the displays apart while it was off.

Two small side effects, both intentional:

  • The anchor is bounded to -1...2. Two displays can never be further apart than the whole range, so this only matters if several displays report changes of their own at the same time.
  • While a display is pinned at the end of its range, sync no longer writes to it every tick. That is one less DDC write per 100 ms for anyone who runs an external display at full brightness.

Verification

  • xcodebuild -scheme MonitorControl -configuration Debug -destination 'platform=macOS' build → BUILD SUCCEEDED
  • swiftformat --lint MonitorControl → 0/30 files require formatting
  • swiftlint → no new violations (the existing ones in KeyboardShortcuts.swift, DisplayManager.swift, MenuslidersPrefsViewController.swift and AppDelegate.swift:89 are untouched)

BrightnessSync.swift only imports Foundation, so the arithmetic can be driven directly. Replaying a day of ambient light against the real AppleDisplay.refreshBrightness() easing, starting from a built-in at 0.50 and an external parked 0.40 higher:

built-in   external before   external after   offset before   offset after
  0.80            1.00             1.00           +0.20           +0.20
  0.50            0.70             0.90           +0.20           +0.40
  1.00            1.00             1.00           +0.00           +0.00
  0.35            0.35             0.75           +0.00           +0.40
  0.90            0.90             1.00           +0.00           +0.10
  0.45            0.45             0.85           +0.00           +0.40
  0.75            0.75             1.00           +0.00           +0.25
  0.50            0.50             0.90           +0.00           +0.40

Then the user moves the external display to 0.60 by hand and the ambient light swings once more:
built-in 0.50, external 0.60, offset +0.10 - the new relationship is the one that is kept

Before, the offset decays to zero the first time the built-in reaches the top and never comes back. After, the external is still compressed while it sits at its own maximum (+0.10, +0.25 above) — nothing can be done about that, it has nowhere higher to go — but the full +0.40 is restored as soon as the built-in is back in range.

The harness, if you want to re-run it
// swiftc MonitorControl/Support/BrightnessSync.swift main.swift -o mc-sim && ./mc-sim
import Foundation

// The easing AppleDisplay.refreshBrightness() applies: each tick it moves a third of
// the remaining way towards the real brightness and reports that much as the delta.
func deltas(from: Float, to: Float) -> [Float] {
  var value = from
  var deltas: [Float] = []
  while value != to {
    let next: Float
    if abs(to - value) < 0.01 {
      next = to
    } else if to > value {
      next = value + max((to - value) / 3, 0.005)
    } else {
      next = value + min((to - value) / 3, -0.005)
    }
    deltas.append(next - value)
    value = next
  }
  return deltas
}

// What AppDelegate.job() did before: add the delta and clamp to the usable range.
func oldSync(current: Float, delta: Float) -> Float {
  max(0, min(1, current + delta))
}

func f(_ value: Float) -> String { String(format: "%.2f", value) }
func s(_ value: Float) -> String { String(format: "%+.2f", value) }

let ambient: [Float] = [0.80, 0.50, 1.00, 0.35, 0.90, 0.45, 0.75, 0.50]
var source: Float = 0.50
var old: Float = 0.90
var new: Float = 0.90
var anchor: Float?

print("built-in \(f(source)), external \(f(new)), offset \(s(new - source))")
print("")
print("built-in   external before   external after   offset before   offset after")

for level in ambient {
  for delta in deltas(from: source, to: level) {
    old = oldSync(current: old, delta: delta)
    let step = BrightnessSync.step(current: new, anchor: anchor, delta: delta)
    anchor = step.anchor
    new = step.value
  }
  source = level
  print("  \(f(source))            \(f(old))             \(f(new))           \(s(old - source))           \(s(new - source))")
}

print("")
print("Then the user moves the external display to 0.60 by hand and the ambient light swings once more:")
old = 0.60
new = 0.60
for level in [Float(0.90), Float(0.50)] {
  for delta in deltas(from: source, to: level) {
    old = oldSync(current: old, delta: delta)
    let step = BrightnessSync.step(current: new, anchor: anchor, delta: delta)
    anchor = step.anchor
    new = step.value
  }
  source = level
}
print("built-in \(f(source)), external \(f(new)), offset \(s(new - source)) - the new relationship is the one that is kept")

What I could not verify

No run on real hardware. This machine has no external display and no Apple Developer signing identity, so the build above needed CODE_SIGNING_ALLOWED=NO and I could not exercise the actual loop. So these are reasoned, not observed:

  1. that the anchor's re-anchor tolerance of 0.01 is loose enough not to trip on DDC read-back jitter on a real external display — if a panel reports back a brightness that drifts by more than 0.01 from what was written, it would re-anchor on its own and behave like today. OtherDisplay.getBrightness() reads the saved pref rather than the panel, so I do not believe it can, but a real monitor is the only way to be sure;
  2. the feel of it over a day of real ambient light changes;
  3. that skipping the write while a display is pinned at its maximum does not upset any monitor that expects the periodic DDC traffic.

Worth a run on your setup before merging. Happy to adjust the tolerance, or to put the old behaviour back behind the checkbox if you would rather have it opt-in.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/MonitorControl/MonitorControl/pull/1906 **Author:** [@anandghegde](https://github.com/anandghegde) **Created:** 9/20/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `feat/preserve-brightness-sync-offsets` --- ### 📝 Commits (1) - [`a4e1f7d`](https://github.com/MonitorControl/MonitorControl/commit/a4e1f7df644524fd4d9456b930f520f4ce9ebb4a) Preserve the brightness offset between displays while syncing (#1781) ### 📊 Changes **5 files changed** (+71 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `MonitorControl.xcodeproj/project.pbxproj` (+4 -0) 📝 `MonitorControl/Model/Display.swift` (+2 -0) 📝 `MonitorControl/Support/AppDelegate.swift` (+8 -3) ➕ `MonitorControl/Support/BrightnessSync.swift` (+52 -0) 📝 `MonitorControl/View Controllers/Preferences/MainPrefsViewController.swift` (+5 -0) </details> ### 📄 Description Closes #1781. ## The problem `AppDelegate.job()` synced by adding the reported delta to each display and clamping the result: ```swift let newValue = max(0, min(1, targetDisplay.getBrightness() + delta)) ``` Whatever part of the change did not fit was dropped. A display parked near the top of its range loses ground on every ambient light swing **up**, but gets the whole change back on the way **down** — so the displays creep together until the sliders are aligned, which is what the issue describes ("especially when the external monitor reaches its maximum brightness and then scales back — the brightness sliders often end up aligned"). ## The change `BrightnessSync` keeps an unclamped anchor per display: where the display would be if it had no ends. The delta is applied to the anchor, the display is driven to the clamped anchor. The part that did not fit is therefore given back on the way down instead of being lost, and the offset survives. The anchor re-anchors itself whenever the display is no longer where the last sync left it — so a brightness the user sets by hand simply becomes the new relationship to keep, and no other code path has to know the anchor exists. The anchors are also dropped when the sync checkbox is toggled, since the user was free to move the displays apart while it was off. Two small side effects, both intentional: - The anchor is bounded to `-1...2`. Two displays can never be further apart than the whole range, so this only matters if several displays report changes of their own at the same time. - While a display is pinned at the end of its range, sync no longer writes to it every tick. That is one less DDC write per 100 ms for anyone who runs an external display at full brightness. ## Verification - `xcodebuild -scheme MonitorControl -configuration Debug -destination 'platform=macOS' build` → **BUILD SUCCEEDED** - `swiftformat --lint MonitorControl` → `0/30 files require formatting` - `swiftlint` → no new violations (the existing ones in `KeyboardShortcuts.swift`, `DisplayManager.swift`, `MenuslidersPrefsViewController.swift` and `AppDelegate.swift:89` are untouched) `BrightnessSync.swift` only imports Foundation, so the arithmetic can be driven directly. Replaying a day of ambient light against the real `AppleDisplay.refreshBrightness()` easing, starting from a built-in at 0.50 and an external parked 0.40 higher: ``` built-in external before external after offset before offset after 0.80 1.00 1.00 +0.20 +0.20 0.50 0.70 0.90 +0.20 +0.40 1.00 1.00 1.00 +0.00 +0.00 0.35 0.35 0.75 +0.00 +0.40 0.90 0.90 1.00 +0.00 +0.10 0.45 0.45 0.85 +0.00 +0.40 0.75 0.75 1.00 +0.00 +0.25 0.50 0.50 0.90 +0.00 +0.40 Then the user moves the external display to 0.60 by hand and the ambient light swings once more: built-in 0.50, external 0.60, offset +0.10 - the new relationship is the one that is kept ``` Before, the offset decays to zero the first time the built-in reaches the top and never comes back. After, the external is still compressed while it sits at its own maximum (+0.10, +0.25 above) — nothing can be done about that, it has nowhere higher to go — but the full +0.40 is restored as soon as the built-in is back in range. <details> <summary>The harness, if you want to re-run it</summary> ```swift // swiftc MonitorControl/Support/BrightnessSync.swift main.swift -o mc-sim && ./mc-sim import Foundation // The easing AppleDisplay.refreshBrightness() applies: each tick it moves a third of // the remaining way towards the real brightness and reports that much as the delta. func deltas(from: Float, to: Float) -> [Float] { var value = from var deltas: [Float] = [] while value != to { let next: Float if abs(to - value) < 0.01 { next = to } else if to > value { next = value + max((to - value) / 3, 0.005) } else { next = value + min((to - value) / 3, -0.005) } deltas.append(next - value) value = next } return deltas } // What AppDelegate.job() did before: add the delta and clamp to the usable range. func oldSync(current: Float, delta: Float) -> Float { max(0, min(1, current + delta)) } func f(_ value: Float) -> String { String(format: "%.2f", value) } func s(_ value: Float) -> String { String(format: "%+.2f", value) } let ambient: [Float] = [0.80, 0.50, 1.00, 0.35, 0.90, 0.45, 0.75, 0.50] var source: Float = 0.50 var old: Float = 0.90 var new: Float = 0.90 var anchor: Float? print("built-in \(f(source)), external \(f(new)), offset \(s(new - source))") print("") print("built-in external before external after offset before offset after") for level in ambient { for delta in deltas(from: source, to: level) { old = oldSync(current: old, delta: delta) let step = BrightnessSync.step(current: new, anchor: anchor, delta: delta) anchor = step.anchor new = step.value } source = level print(" \(f(source)) \(f(old)) \(f(new)) \(s(old - source)) \(s(new - source))") } print("") print("Then the user moves the external display to 0.60 by hand and the ambient light swings once more:") old = 0.60 new = 0.60 for level in [Float(0.90), Float(0.50)] { for delta in deltas(from: source, to: level) { old = oldSync(current: old, delta: delta) let step = BrightnessSync.step(current: new, anchor: anchor, delta: delta) anchor = step.anchor new = step.value } source = level } print("built-in \(f(source)), external \(f(new)), offset \(s(new - source)) - the new relationship is the one that is kept") ``` </details> ### What I could not verify **No run on real hardware.** This machine has no external display and no Apple Developer signing identity, so the build above needed `CODE_SIGNING_ALLOWED=NO` and I could not exercise the actual loop. So these are reasoned, not observed: 1. that the anchor's re-anchor tolerance of 0.01 is loose enough not to trip on DDC read-back jitter on a real external display — if a panel reports back a brightness that drifts by more than 0.01 from what was written, it would re-anchor on its own and behave like today. `OtherDisplay.getBrightness()` reads the saved pref rather than the panel, so I do not believe it can, but a real monitor is the only way to be sure; 2. the feel of it over a day of real ambient light changes; 3. that skipping the write while a display is pinned at its maximum does not upset any monitor that expects the periodic DDC traffic. Worth a run on your setup before merging. Happy to adjust the tolerance, or to put the old behaviour back behind the checkbox if you would rather have it opt-in. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
github-starred/MonitorControl#1329
No description provided.