Repository navigation
Conversation
An LED's three emitters are not equally efficient, so a neutral triple can render with a cast: on an SK6812 the green die runs hot and grey scenes come out tinted. Four trims correct it at the output, once per strip, rather than leaving every effect to guess. balanceRed, balanceGreen and balanceBlue pull one die down to meet the others, and whiteLevel reaches the white die, which the RGB trims cannot: a white phosphor is separate hardware, often brighter than the trio beside it. Trimmed down only, since there is no headroom above 255 and raising a channel clips rather than balances. The trim pre-scales the index the curve is read at, so the hot path stays one lookup per channel and the curve still lands last. A table per trim would cost every driver a kilobyte of its inline budget, which the contract holds at 2 KB. Amber and UV read untrimmed, being their own dies that an RGB trim says nothing about, and whiteLevel is offered only where the profile carries a white die. Five cases: one trim leaves the others untouched, a trim does not reach the white die, the curve and a trim compose, whiteLevel moves the white alone, and whiteLevel of 0 puts the die out rather than leaving it at the never-black floor. Those caught the plain-RGB fast path, which skipped the trim entirely on the commonest light there is. Desktop 2769/2769; esp32p4rev3-eth-wifi builds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
Collaborator
|
Thanks @Gohnnyman! Correcting a strip's color cast is a real need, and the PR is easy to follow: amber and UV left untrimmed, the Two design points before it can go in, plus one thing missing from the description:
The tests carry over well: with the gain after the curve, "the curve and the trim compose" can check the exact ratio instead of |
11 of 13 tasks
A color cast is a property of the LED type, and the profile is where MoonLight describes LED hardware, so the gains live there rather than on every driver card. The five driver cards lose the three sliders, and the fourth where a profile carries white. A strip from an off batch gets a profile of its own, which the editable list already allows. The gain lands after the curve, on emitted light, so its value means what it says: green at 128 gives half the green at any curve, where a pre-scaled index gave about 22% at gamma 2.2 and shifted with the level under CIE. It is folded into the tables at rebuild, one row per color channel, so the per-light path stays one lookup per channel with no branch and no multiply. A profile with no gain allocates nothing and every channel reads the one curve; only a profile that carries one pays for the rows, as the 16-bit table is allocated only where it is read. Each row is filled from the curve's own float, so the gain never re-quantizes a value the curve already rounded, and the wide outputs keep their full resolution. Subtracted: Correction::trim(), and profileHasWhiteDie() moved to the profile row that now uses it. Tests: a gain on one channel leaves the others alone, an RGB gain does not reach the white die, the curve and the gain compose to the gain's own ratio (149 red, 75 blue at gain 128), a profile with no gain wants no tables, the white die's own gain, a zero gain putting it out, and an edited gain surviving the row being written and restored. A Correction is assigned whole by deriveCorrection and by the driver tests, so one more pins that a copy reads its own tables: the first version stored resolved pointers and four driver tests caught a copy reading into the object it came from. Reviews - ewowi on MoonModules#212: gains belong in the fixture profile, not the driver: done here. - ewowi on MoonModules#212: fold them into the tables, gain after the curve: done here. - ewowi on MoonModules#212: the description says what an agent wrote and what was tested: the pull request text, not the code. Desktop 2857/2857; esp32p4rev3-eth-wifi builds and runs the strip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@ewowi hi! Review points addressed and description changed |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's new
An LED's three emitters are not equally efficient, so a neutral triple can
render with a cast - on an SK6812 the green die runs hot and grey scenes come
out tinted. Four gains correct it at the output.
gainRed,gainGreen,gainBluepull one die down to meet the others, andgainWhitereaches the white dieAlmost everything in the PR Written by Claude (Opus 5) under my direction (not just blind vibecoding)
Tested on an ESP32-P4 driving a 184-LED, a
custom profile with gains green 190, blue 200, white 200