Skip to content

feature: Color balance - #212

Merged
ewowi merged 4 commits into
MoonModules:mainfrom
Gohnnyman:feat/light-balance
Nov 2, 2026
Merged

feature: Color balance #212
ewowi merged 4 commits into
MoonModules:mainfrom
Gohnnyman:feat/light-balance

Conversation

@Gohnnyman

@Gohnnyman Gohnnyman commented Oct 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

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.

  • The gains live on the fixture profile. Every driver card stays as it is.
  • gainRed, gainGreen, gainBlue pull one die down to meet the others, and
    gainWhite reaches the white die
  • Amber and UV read untouched

Almost 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

@Gohnnyman @claude
Trim each LED die to correct a color cast
e890f9d 
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>
@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@ewowi

ewowi commented Oct 30, 2026

Copy link
Copy Markdown
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 profileHasRole walk shared by both role questions, and tests that read as what the trims do.

Two design points before it can go in, plus one thing missing from the description:

  1. The gains belong in the fixture profile, not on the driver. A cast is a property of the LED type, and the profile is where MoonLight describes the LED hardware: channel order, white. So a profile carries optional red, green, blue and white gains, and a strip from an off batch gets a profile of its own (profiles are editable rows). Every driver card then stays as it is. Right now LED, Art-Net, PanelCard, HUB75 and DMX each get three more sliders, and a fourth where the profile has white.
  2. Fold the gains into the tables, so the per-light path stays exactly as it is. Build one table per color channel when the correction is rebuilt, with the gain applied after the curve, since white balance is a gain on emitted light. A profile without gains, which is every built-in one, keeps its one table, and all three channels point at it. So the per-light work stays one lookup per channel, with no branch and no multiply. Only a profile with gains allocates the two extra tables, the way the 16-bit table is allocated only for wide drivers. That also settles the inline-budget concern. Applying the gain after the curve has two more benefits:
    • The value means what it says: green at 128 gives half the green light. With the gain before the curve, it gives about 22% at gamma 2.2, and the ratio shifts with the level under CIE.
    • The 16-bit outputs keep their full resolution, where the trimmed 8-bit index now leaves about 129 levels.
  3. The description: please add what an agent wrote and what you tested on hardware (which strip?), as Contribute a pull request asks.

The tests carry over well: with the gain after the curve, "the curve and the trim compose" can check the exact ratio instead of out[2] < out[0]. Happy to help if anything in the table rebuild is unclear.

@ewowi ewowi mentioned this pull request Oct 30, 2026
Closed
11 of 13 tasks
@Gohnnyman @claude
Move the die gains onto the fixture profile, applied after the curve
b877683 
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>
@Gohnnyman

Gohnnyman commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

@ewowi hi! Review points addressed and description changed

ewowi and others added 2 commits November 2, 2026 14:50
@ewowi @claude
Apply a profile's first gain at once, resolve the gained tables at re…
7082587 
…build, help per gain field, a Color balance section

On an 8-bit wiring the gained tables were allocated after the rebuild that fills them, and refilled only when the 16-bit table was new, so a first gain showed only at the next brightness change; resolveGainTables now reports a new table and the rebuild refills on it.

Each light asked for its tables per call, testing every channel's row for null to fall back on the curve, ungained profiles included. The tables now resolve once per rebuild, so a light reads its channel's table with no test, as before the gains.

- Correction.h: the copied state is CorrectionData; Correction adds the resolved 8-bit and 16-bit tables, and its copy and assignment resolve them again, since the ungained rows point into the copy's own curve.
- DriverBase.h: resolveGainTables returns whether a table is new, its sizing one fitGainTable per table; rebuildCorrection refills on a new gain table as on a new 16-bit one; release puts the tables back on the curve, so no row points into a freed table.
- FixtureProfilesModule.h: a help line per gain field, since the UI matches a row field's help by its exact name; a Color balance section in the guide.
- unit_FixtureProfilesModule: a driver running on an ungained profile picks a gained one and applies the gain from that one rebuild (fails before the fix: green 255).
- The card image, captured from the desktop with a gained GRBW profile open; the old one still showed LightPresets.
- help.json regenerated; the prose count falls by two.

Tested: desktop 2858 cases pass on a fresh test build, the copied-correction case among them; check_code at its baseline, check_docgen 0 errors with its warnings at the baseline, check_prose at its committed counts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ewowi @claude
Warn when a profile's gains find no memory for their table
83eec1e 
A board without PSRAM can run out of heap; the gained table then stayed unallocated and the strip ran ungained with nothing on the card to say so.

- DriverBase.h: a profile's gains without their table show the warning "out of memory for the color balance, gains off", which the driving line yields to; the strip runs ungained, and the next rebuild retries the allocation and clears the warning.
- unit_FixtureProfilesModule: with allocations of 1 KB refused, the driver runs ungained and warns; with memory back, the next pick applies the gain and the warning goes (fails before the fix: no status). The fixture pick five tests repeated is one helper.

Tested: desktop 2859 cases pass on a fresh test build; check_code, check_docgen and check_prose at their baselines.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ewowi
ewowi merged commit 7958cdd into MoonModules:main Nov 2, 2026
11 checks passed
@ewowi

ewowi commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks @Gohnnyman! Both design points from round one landed: the gains sit on the fixture profile, every driver card is untouched, and the gain is applied after the curve, with tests that pin the ratio.

I pushed two maintainer commits on your branch:

  1. A profile's first gain now applies at once. On an 8-bit wiring the gained tables were allocated after the rebuild that fills them, so a new gain only showed at the next brightness change (the soft start and a slider's second event hid it, which is why your rig looked right). There is a test that fails without the fix.
  2. The gained tables are resolved once per rebuild instead of per light, so the hot path costs the same as before the gains, ungained strips included. A copied correction resolves its own tables.
  3. When the gained table finds no memory (a board without PSRAM), the driver shows "out of memory for the color balance, gains off" instead of silently running ungained, and retries on the next rebuild.
  4. A help line per gain field (the UI matches field names exactly, so the single gain line never showed), a Color balance section in the fixture profile card's guide, and a new card image.

One request for you: could you add the strip type to the description ("184-LED" of what, SK6812?), as Contribute a pull request asks.

Thanks again!

ewowi added a commit that referenced this pull request Nov 2, 2026
@ewowi @claude
Merge main: Gohnnyman's #212, light balance
416d7c7 
prose.json conflicted, both sides having moved the count; regenerated by check_prose over the merged tree (2012). help.json merged cleanly and regenerates identical.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Gohnnyman @ewowi