Repository navigation
Add a per-display software brightness override - #251
temporaryna wants to merge 18 commits into
Conversation
A monitor can ack DDC brightness writes without ever applying them and answer every capability read with garbage, so the three-failed-writes latch never trips and the top of the slider dies with nothing to detect (seen on a LEN Y27q-20: NULL-frame DDC replies, 20-100% dead, two full link drops under write traffic). The new Software Brightness row on a display's detail routes that display's whole 0-100 range through the gamma path, like an HDR-dimmed display, persisted by UUID alongside the volume override from issue didriksg#57. The route flip runs its gamma and DDC writes off the main thread and invalidates in-flight coalesced writes; the input-switch row keeps gating on DDC write capability rather than the brightness route; the debug control-mode caption reports the forced route honestly.
The forced route exists because the monitor holds its backlight wherever it is stuck (usually full); a 5% gamma floor on top of that is a glow the slider can never get past. Every other software route (HDR, latch, blend zone) keeps the floor, so nothing else changes.
The forced-route methods and the gamma fallback move into a same-file extension (the idiom the Apple-displays extension already uses), and the override row's help string splits across two concatenated literals that keep the localization key identical.
…ightness An identity reset would clobber GammaService's table while it still owns the display, and the boost overlay above 100% was left to self-heal; the off-branch now reapplies the active adjustment (the check setSoftwareBrightness already makes) and syncs the overlay like setBrightness does.
…ection The display card had grown to a dozen rows in an order that mixed concerns: Input sat among the color rows and HDR was far from them. The rows are now grouped the way the Battery menu groups its own, a divider and a small header before each group. A group with no row on a display gets no header. The host helper moves out of the panel builder to keep it under the length limit.
# Conflicts: # Crisp/Views/DisplayDetailView.swift
rebuildBlocksIfNeeded was 3 lines under the lint limit for a function body, so the next row on the display card would not fit. The detail blocks of one display now come from detailBlocks(for:), in an extension. No change in what the panel shows.
The Software Brightness switch becomes a row in the card's Brightness group with two choices, Hardware and Software, as a checkmark list. The row also shows Software when Crisp fell back by itself after three failed writes, or while HDR is on, which the card did not show before. A click on Hardware after a fallback clears the latch, so the next write tries the monitor again. A monitor whose DDC never worked gets no row. The 5% minimum is back on this route, so no choice gives a fully black picture. What the row shows is decided in BrightnessControlRoute, with tests.
The headers used the secondary label color, the same grey as the values at the right of a row. Measured in dark mode, the Battery menu's headers are about 40% white where that grey is 55%. Light mode is not changed.
…keep a fallback HDR, a fallback after three failed writes and the first good write change what the row shows, but none of them publishes on the display, so the row kept the old value until the slider moved. The service now posts a notification for them. A fallback showed Software as checked, and a checked row takes no click, so the choice could not be saved before the monitor reconnected and went back to Hardware. A click on Software during a fallback now saves it, and the caption says so.
The debug caption above the slider read the DDC latch only, so it said DDC while HDR sent brightness through gamma. It now reads the same route as the row and redraws on the same notification.
…ss-control # Conflicts: # Crisp/App/AppDelegate.swift # Crisp/Views/DisplayDetailView.swift
|
Your route is the base of this: the saved choice per display and the path that sends the whole slider through gamma are in as you wrote them. I pushed a few changes on top, so you can see them before I merge.
I tested both choices and HDR on and off on a monitor here. I can't test your case, a monitor that acks brightness writes and ignores them. Could you run this build on the Y27q-20 and check that Software still does what your version did? |
A forced-software external runs its whole range through the gamma table, so its luminance follows the factor square while the combined math treated its percent as DDC-linear — the display sat far darker than the handle asked for at every position. The forced route now maps through the square root, keeping write and readback inverse. The combined view asks the Brightness Control row which route is shown, so fallback and HDR displays read through the same square.
The gamma square belongs to the software route, not to the EDID luminance data: without nits the mapping now falls back to a 1:1 square-root scale instead of the DDC-linear percent (and the handle readback no longer reports the display as off). View routes every forced display through the forced pair.
A fallback latch, HDR, and a click in the Brightness Control row change which curve the combined readback uses, but publish nothing on the display, so the probe-driven sync kept the old handle position. The combined handle now re-aims on the same notification the Brightness Control row redraws on.
|
Pushed three follow-ups on top of your branch. The combined slider treated a software-routed display as DDC-linear, so it sat visibly darker than the handle asked at every position — that is the thing the forced route exists for on my Y27q-20: the monitor never matches what the slider promises. Write and readback now both go through the gamma square, and without nits metadata the curve stays on the display's own scale. I also switched the combined-view check to One question I hit while testing: a native-brightness external (Pro Display XDR class) that enters HDR lands in Your 5% floor restore makes sense — the reason I had gone to true black is that this monitor's backlight parks at full, so even 5% of gamma on top is a glow you can't get past. I keep hitting that floor with the MacBook's own screen right next to it, so the lockout risk the floor prevents can't happen in my setup. If a guard like "0% only while another display is on" ever fits your plans, I'd use it. Unrelated, from the same testing: through the combined slider the built-in panel parks at about 5% at zero while its own slider reaches true black. The combined path writes through One heads-up: upstream picked up #270/#271/#273 after your merge, so the branch now conflicts in AppDelegate.swift and project.yml. Want me to merge main in and resolve, or is that your side? Tested on the Y27q-20: Software still dims through the whole range with your Brightness Control row, 0% parks at the floor, and the combined handle matches what the monitor actually shows. |
What
A new Software Brightness row on a display's detail routes that display's whole 0-100 range through the software gamma path, persisted by UUID alongside the volume override (#57). On the forced route the 5% never-fully-black floor is lifted, so 0% dims the picture to a true black (every other route — HDR, latch, blend zone — keeps the floor).
Why
Some monitors ack DDC brightness writes without ever applying them and answer every capability read with garbage, so the three-failed-writes latch never trips and the top of the slider just dies with nothing to detect. Seen on a LEN Y27q-20: every DDC read comes back a NULL frame (e.g.
6E 80 BEx4, header/checksum invalid) while writes still ack — a firmware lie no probe can catch, so the escape has to be manual and discoverable (the row shows for every DDC external, before anything looks wrong).Notes for review
isDDCAvailable) instead of the brightness route, so an HDR display with working DDC gets its Input row back — input-switch writes still work when brightness writes are software-routed.setBrightness).isSoftwareForcedresolves the display UUID on each coalesced write;saveSoftwareBrightnessalready paid that per software write, so I kept it simple rather than add a cache.make checkpasses (lint, 201 tests, x86_64 typecheck, localization keys).