Normalize combined brightness using estimated luminance - #83
Conversation
Normalize built-in and external displays on an estimated nit scale, use the built-in panel's linear brightness API, and account for Crisp's low-range DDC/gamma blend. Apply the same mapping to absolute auto-brightness, expose a fine-tuning ratio, and add focused math tests and localization.
didriksg
left a comment
There was a problem hiding this comment.
Thanks for taking this on. Equal slider percentages really don't look equal across panels, and matching by estimated nits is the right idea. The math holds up: the forward and inverse mappings agree, the sub-15% DDC+gamma inverse is continuous at the boundary, and you registered the test file in project.yml and added the zh-Hans strings, which most first PRs miss.
A few things before I can merge:
-
swiftlint --strictfails on_CoreDisplayCreateInfoDictionary(identifier_name), and CI lints strict. The dlsym'd names are allowlisted one by one in.swiftlint.ymlunderidentifier_name.excluded; add this one there. -
The fallback in
setBuiltinLinearBrightness(and the smooth variant) mixes domains. WhenDisplayServicesSetLinearBrightnessdoesn't resolve,linearBrightness * 100goes into the nonlinear native-percent API, so the built-in lands visibly wrong instead of degrading to the old mapping. The description says it falls back to proportional in that case; make it actually do that. Simplest is to expose "linear API available" from BrightnessService and havebuiltinLinearTargetreturn nil when it isn't. -
Please reuse the existing IORegistry matching instead of a second walk.
DDCService.buildAVServiceMapByProximity()already traverses the service plane and matches LegacyManufacturerID/ProductID/SerialNumber throughDDCServiceMatcher, and itsu32()reinterprets negative Int bit-patterns because real monitors return them. Yournumber()doesn't, so those monitors would silently never get a nits value. One shared path fixes both. -
DisplayInfo.initruns on the main actor and now does the full registry walk inline, per display, on every reconfiguration.loadDetails()in the same class offloads its expensive work for exactly this reason; the nits lookup should get the same treatment so a hot-plug doesn't hitch the panel. -
While you're in the absolute branch of AutoBrightnessService: clamp to
display.maxBrightness, not 100. The old code had the same cap, but the relative branch right above documents why it's a trap (it drags an Extra Brightness display back down on every tick). -
Small ones: the new defaults key ends in
.v2but there's no v1, so drop the suffix before it ships; and pure logic likeCombinedBrightnessMathlives inModels/in this repo.
I'll approve the CI run once these land, and I'll verify on my own hardware (the built-in panel plus an HP 27fw over DDC) before merging, since brightness behavior only really gets proven live here.
What & why
Combined Brightness currently treats slider percentages as comparable across every panel. They are not: Apple's built-in brightness control follows a strongly nonlinear user curve, monitors expose different peak luminance ranges, and Crisp adds a second gamma-dimming stage below 15% on external displays. Equal percentages can therefore produce visibly different luminance.
This change:
How tested
./scripts/release.sh v0.0.0-ci— built and signed a universal arm64/x86_64 app and DMG targeting macOS 14CombinedBrightnessMathTests; the full XCTest target is left to CI because this machine has Command Line Tools but not full XcodeChecklist
./dev.sh, or./scripts/release.sh v0.0.0-cifor the full release build)Crisp/Resources/Localizable.xcstringsfor any new user-facing strings