From e890f9d622b47edad64cfb59141d7d086ba443a8 Mon Sep 17 00:00:00 2001 From: Shura Fedorovskyi Date: Mon, 26 Oct 2026 17:59:24 +0400 Subject: [PATCH 1/2] Trim each LED die to correct a color cast 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) --- docs/help/help.json | 108 ++++++++++++++++++++++ src/light/drivers/Correction.h | 29 ++++-- src/light/drivers/DriverBase.h | 19 +++- src/light/drivers/FixtureProfilesModule.h | 31 ++++--- test/unit/light/unit_Correction.cpp | 73 +++++++++++++++ 5 files changed, 238 insertions(+), 22 deletions(-) diff --git a/docs/help/help.json b/docs/help/help.json index 6bf19017..3bbe49b3 100644 --- a/docs/help/help.json +++ b/docs/help/help.json @@ -3182,9 +3182,21 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, + "balanceRed": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceGreen": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceBlue": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, + "whiteLevel": { + "line": "The white die's own trim, which the three above cannot reach." + }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3317,9 +3329,21 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, + "balanceRed": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceGreen": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceBlue": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, + "whiteLevel": { + "line": "The white die's own trim, which the three above cannot reach." + }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3363,9 +3387,21 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, + "balanceRed": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceGreen": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceBlue": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, + "whiteLevel": { + "line": "The white die's own trim, which the three above cannot reach." + }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3422,9 +3458,21 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, + "balanceRed": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceGreen": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceBlue": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, + "whiteLevel": { + "line": "The white die's own trim, which the three above cannot reach." + }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3467,9 +3515,21 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, + "balanceRed": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceGreen": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceBlue": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, + "whiteLevel": { + "line": "The white die's own trim, which the three above cannot reach." + }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3523,9 +3583,21 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, + "balanceRed": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceGreen": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceBlue": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, + "whiteLevel": { + "line": "The white die's own trim, which the three above cannot reach." + }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3573,9 +3645,21 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, + "balanceRed": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceGreen": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceBlue": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, + "whiteLevel": { + "line": "The white die's own trim, which the three above cannot reach." + }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3637,9 +3721,21 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, + "balanceRed": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceGreen": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceBlue": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, + "whiteLevel": { + "line": "The white die's own trim, which the three above cannot reach." + }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3689,9 +3785,21 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, + "balanceRed": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceGreen": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, + "balanceBlue": { + "line": "One die pulled down to meet the others, which corrects a strip's color cast." + }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, + "whiteLevel": { + "line": "The white die's own trim, which the three above cannot reach." + }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, diff --git a/src/light/drivers/Correction.h b/src/light/drivers/Correction.h index 821f4a1d..4fa413ba 100644 --- a/src/light/drivers/Correction.h +++ b/src/light/drivers/Correction.h @@ -47,6 +47,10 @@ struct Correction { uint16_t* lut16 = nullptr; /// Set by a driver whose output resolves more than a byte, so the 16-bit table is filled whatever the profile. bool wide = false; + /// Per-channel white balance, 255 = untouched; trimmed down only, since there is no headroom above 255 and raising clips instead. + uint8_t balRed = 255, balGreen = 255, balBlue = 255; + /// The white die's trim, 255 = untouched; a separate emitter, often brighter than the RGB trio, that the three trims above cannot reach. + uint8_t whiteLevel = 255; /// Which curve the brightness rebuild fills through; a driver's setting, not a global one. Curve curve = Curve::Cie; /// Output byte position of the red role; each color role's position is recomputed from the role array. @@ -107,6 +111,11 @@ struct Correction { return linear; } + /// One channel trimmed down by `bal`, where 255 leaves the byte alone. A pre-scale on the index, not a table per trim, which would cost every driver a kilobyte of its inline budget. + static uint8_t trim(uint8_t v, uint8_t bal) MM_NONBLOCKING { + return bal == 255 ? v : static_cast((static_cast(v) * bal + 127) / 255); + } + /// Round a curved value to an integer of at most `top`, keeping a lit input above zero so a fade-out does not snap off partway down. static int quantize(float out, int top, bool lit) MM_NONBLOCKING { int q = static_cast(out + 0.5f); @@ -155,9 +164,9 @@ struct Correction { inline void MM_RAMFUNC applyThrough(const T* lut, const uint8_t* src, T* out, uint8_t srcChannels) const { // A plain strip: the curve on each color and nothing optional, the drivers' commonest light. if (plainRgb) { - if (offRed != kAbsent) out[offRed] = lut[src[0]]; - if (offGreen != kAbsent) out[offGreen] = lut[src[1]]; - if (offBlue != kAbsent) out[offBlue] = lut[src[2]]; + if (offRed != kAbsent) out[offRed] = lut[trim(src[0], balRed)]; + if (offGreen != kAbsent) out[offGreen] = lut[trim(src[1], balGreen)]; + if (offBlue != kAbsent) out[offBlue] = lut[trim(src[2], balBlue)]; return; } constexpr T kFull = static_cast(~T{0}); // 255 or 65535 @@ -190,7 +199,7 @@ struct Correction { } else { w = r < g ? (r < b ? r : b) : (g < b ? g : b); // min(r,g,b): the white component // Computed off the PRE-subtraction values, which only rebalance the RGB emitters. - if (offWarmWhite != kAbsent) out[offWarmWhite] = lut[w]; + if (offWarmWhite != kAbsent) out[offWarmWhite] = lut[trim(w, whiteLevel)]; // yellow ≈ min(R,G) (the shared red+green component). if (offYellow != kAbsent) out[offYellow] = lut[r < g ? r : g]; // Driven from the blue with no red or green to pair with, so it stays dark on warm colors. @@ -201,20 +210,22 @@ struct Correction { // White last: it is the only emitter that rebalances RGB. if (offWhite != kAbsent) { if (whiteMode == WhiteMode::Accurate) { r -= w; g -= w; b -= w; } // pull white out of RGB - out[offWhite] = lut[w]; + out[offWhite] = lut[trim(w, whiteLevel)]; } } // The curve, applied ONCE: everything above this line is linear light. - if (offRed != kAbsent) out[offRed] = lut[r]; - if (offGreen != kAbsent) out[offGreen] = lut[g]; - if (offBlue != kAbsent) out[offBlue] = lut[b]; + if (offRed != kAbsent) out[offRed] = lut[trim(r, balRed)]; + if (offGreen != kAbsent) out[offGreen] = lut[trim(g, balGreen)]; + if (offBlue != kAbsent) out[offBlue] = lut[trim(b, balBlue)]; if constexpr (sizeof(T) == 1) { if (hasFine) applyFine(r, g, b, w, out); } } /// Hot path, 16-bit profiles only: each color channel with a fine role as its high and low byte from the same table, and a fine dimmer wide open. void applyFine(uint8_t r, uint8_t g, uint8_t b, uint8_t w, uint8_t* out) const { if (offDimmerFine != kAbsent) out[offDimmerFine] = 255; // wide open is 65535, not 65280 - const uint8_t value[5] = {r, g, b, w, w}; // both whites carry the white component, 0 when none is synthesized + // Trimmed here as the coarse bytes are, so a fine channel is balanced exactly as an 8-bit one. + const uint8_t value[5] = {trim(r, balRed), trim(g, balGreen), trim(b, balBlue), + trim(w, whiteLevel), trim(w, whiteLevel)}; const uint8_t coarse[5] = {offRed, offGreen, offBlue, offWhite, offWarmWhite}; const uint8_t fine[5] = {offRedFine, offGreenFine, offBlueFine, offWhiteFine, offWarmWhiteFine}; for (uint8_t k = 0; k < 5; k++) { diff --git a/src/light/drivers/DriverBase.h b/src/light/drivers/DriverBase.h index ed02ec10..494bfd07 100644 --- a/src/light/drivers/DriverBase.h +++ b/src/light/drivers/DriverBase.h @@ -72,6 +72,10 @@ class DriverBase : public MoonModule { // Applied unconditionally, so brightness works even before the fixture-profile library is up. const uint8_t tableBrightness = adoptBrightness(globalBrightness); correction_.whiteMode = static_cast(whiteMode_); + correction_.balRed = balanceRed_; + correction_.balGreen = balanceGreen_; + correction_.balBlue = balanceBlue_; + correction_.whiteLevel = whiteLevel_; correction_.curve = static_cast(curveSel_); // A missing id falls back to the default, so a driver degrades rather than crashing. if (auto* lib = FixtureProfilesModule::active()) { @@ -181,6 +185,10 @@ class DriverBase : public MoonModule { uint8_t fixtureSel_ = 0; /// How the white channel is derived, shown when the profile carries one. uint8_t whiteMode_ = static_cast(WhiteMode::Min); + /// One die pulled down to meet the others, which corrects a strip's color cast. + uint8_t balanceRed_ = 255, balanceGreen_ = 255, balanceBlue_ = 255; + /// The white die's own trim, which the three above cannot reach. + uint8_t whiteLevel_ = 255; /// This driver's dim, multiplied with the global brightness. uint8_t localBrightness_ = 255; /// How brightness maps to output: CIE 1931, a gamma, or linear. @@ -207,10 +215,17 @@ class DriverBase : public MoonModule { buildFixtureOptions(); // fill fixtureOptions_ from the library, sync id and row controls_.addSelect("fixture", fixtureSel_, fixtureOptions_, fixtureOptionCount_); controls_.setPersistLabel(controls_.count() - 1); // saved by name, so a longer or reordered library never moves it + // Trim DOWN only: there is no headroom above 255, so raising clips rather than balancing. A strip whose green die runs hot is corrected here rather than in every effect. + controls_.addControl("balanceRed", balanceRed_, 0, 255); + controls_.addControl("balanceGreen", balanceGreen_, 0, 255); + controls_.addControl("balanceBlue", balanceBlue_, 0, 255); controls_.addSelect("whiteMode", whiteMode_, kWhiteModeOptions, kWhiteModeCount); // Hidden unless the referenced profile carries a channel there is something to synthesize for. auto* lib = FixtureProfilesModule::active(); controls_.setHidden(controls_.count() - 1, !(lib && lib->profileHasSynthChannel(profileId_))); + // A separate emitter the three RGB trims cannot reach, so it carries its own. + controls_.addControl("whiteLevel", whiteLevel_, 0, 255); + controls_.setHidden(controls_.count() - 1, !(lib && lib->profileHasWhiteDie(profileId_))); // Hidden, and only read: the name an older config saved. controls_.addText("fixtureRef", fixtureRef_, sizeof(fixtureRef_)); controls_.setHidden(controls_.count() - 1, true); @@ -222,7 +237,9 @@ class DriverBase : public MoonModule { /// Whether `name` is one of the correction controls, for a driver's own prepare test. static bool isCorrectionControl(const char* name) { return std::strcmp(name, "fixture") == 0 || std::strcmp(name, "fixtureRef") == 0 || std::strcmp(name, "localBrightness") == 0 - || std::strcmp(name, "whiteMode") == 0 || std::strcmp(name, "curve") == 0; + || std::strcmp(name, "whiteMode") == 0 || std::strcmp(name, "curve") == 0 + || std::strcmp(name, "balanceRed") == 0 || std::strcmp(name, "balanceGreen") == 0 + || std::strcmp(name, "balanceBlue") == 0 || std::strcmp(name, "whiteLevel") == 0; } private: diff --git a/src/light/drivers/FixtureProfilesModule.h b/src/light/drivers/FixtureProfilesModule.h index 904efbb0..19a12cb1 100644 --- a/src/light/drivers/FixtureProfilesModule.h +++ b/src/light/drivers/FixtureProfilesModule.h @@ -74,23 +74,30 @@ class FixtureProfilesModule : public MoonModule, public ListSource { return 0; // a dangling id renders at the first profile; rebuildCorrection falls back to it too } + /// Whether the profile carries a white die, cold or warm, at either precision; narrower than profileHasSynthChannel, since a profile with amber but no white has nothing the white trim reaches. + bool profileHasWhiteDie(uint32_t id) const { + return profileHasRole(id, [](ChannelRole r) { + return r == ChannelRole::White || r == ChannelRole::WarmWhite + || r == ChannelRole::WhiteFine || r == ChannelRole::WarmWhiteFine; + }); + } + /// Whether the profile carries a channel the white mode would synthesize; any channel apply() synthesizes from RGB counts, since one control governs them all. bool profileHasSynthChannel(uint32_t id) const { + return profileHasRole(id, [](ChannelRole r) { + return r == ChannelRole::White || r == ChannelRole::WarmWhite + || r == ChannelRole::WhiteFine || r == ChannelRole::WarmWhiteFine + || r == ChannelRole::Yellow || r == ChannelRole::UV; + }); + } + + /// Whether any of the profile's channels carries a role `wanted` accepts, the walk both role questions share. + bool profileHasRole(uint32_t id, bool (*wanted)(ChannelRole)) const { const Profile* p = find(id); if (!p) return false; const uint8_t* r = roleAt(*p); - for (uint8_t c = 0; c < p->channelCount; c++) { - switch (static_cast(r[c])) { - case ChannelRole::White: - case ChannelRole::WarmWhite: - case ChannelRole::WhiteFine: - case ChannelRole::WarmWhiteFine: - case ChannelRole::Yellow: - case ChannelRole::UV: - return true; - default: break; - } - } + for (uint8_t c = 0; c < p->channelCount; c++) + if (wanted(static_cast(r[c]))) return true; return false; } diff --git a/test/unit/light/unit_Correction.cpp b/test/unit/light/unit_Correction.cpp index 8d2533b7..3c687a2f 100644 --- a/test/unit/light/unit_Correction.cpp +++ b/test/unit/light/unit_Correction.cpp @@ -642,3 +642,76 @@ TEST_CASE("Dimmer fine is driven wide open with its coarse byte") { c.apply(src, out, 3); CHECK((out[0] == 255 && out[1] == 255)); } + +// An LED's three emitters are not equally efficient, so a neutral triple can render with a cast. The trim pulls one die down to meet the others, at the output rather than in every effect. +TEST_CASE("Correction balance: trimming one channel leaves the others untouched") { + Correction c; + c.balGreen = 128; + mm::test::rebuildFromPreset(c, 255, mm::test::PresetOrder::RGB); + const uint8_t src[3] = {200, 200, 200}; // the triple a green-cast strip renders wrong + uint8_t out[3] = {}; + c.apply(src, out, 3); + CHECK(out[0] == 200); + CHECK(out[1] == 100); // pulled down to match the weaker channels + CHECK(out[2] == 200); +} + +// A white phosphor is separate hardware, so an RGB trim must say nothing about it. +TEST_CASE("Correction balance: an RGB trim does not reach the white die") { + Correction c; + c.balBlue = 128; + mm::test::rebuildFromPreset(c, 255, mm::test::PresetOrder::RGBW); + const uint8_t src[3] = {200, 200, 200}; + uint8_t out[4] = {}; + c.apply(src, out, 3); + CHECK(out[0] == 200); + CHECK(out[1] == 200); + CHECK(out[2] == 100); // blue trimmed + CHECK(out[3] == 200); // the white component, from the untrimmed minimum +} + +// The trim pre-scales the index the curve is read at, so the curve still lands last. +TEST_CASE("Correction balance: the curve and the trim compose") { + Correction c; + c.balBlue = 128; + mm::test::rebuildFromPreset(c, 255, mm::test::PresetOrder::RGB); + c.curve = Correction::Curve::Gamma22; + c.rebuildBrightness(255); // the same offsets, the table refilled through the curve + const uint8_t src[3] = {200, 200, 200}; + uint8_t out[3] = {}; + c.apply(src, out, 3); + CHECK(out[0] == 149); // red: the curve alone, (200/255)^2.2 * 255 + CHECK(out[2] < out[0]); // blue: trimmed first, so it reads the curve lower down + CHECK(out[2] > 0); +} + +// The white die carries its own trim, since it is often brighter than the trio beside it. +TEST_CASE("Correction whiteLevel: trims the white die without touching RGB") { + const uint8_t src[3] = {200, 160, 120}; // the white component is the minimum, 120 + + Correction full; + mm::test::rebuildFromPreset(full, 255, mm::test::PresetOrder::RGBW); + uint8_t a[4] = {}; + full.apply(src, a, 3); + + Correction half; + half.whiteLevel = 128; + mm::test::rebuildFromPreset(half, 255, mm::test::PresetOrder::RGBW); + uint8_t b[4] = {}; + half.apply(src, b, 3); + + CHECK(b[3] < a[3]); // the white die alone moves + CHECK((b[0] == a[0] && b[1] == a[1] && b[2] == a[2])); +} + +// A die asked for nothing goes out, rather than resting on the floor that keeps a fade from snapping off. +TEST_CASE("Correction whiteLevel: zero puts the white die out") { + Correction c; + c.whiteLevel = 0; + mm::test::rebuildFromPreset(c, 255, mm::test::PresetOrder::RGBW); + const uint8_t src[3] = {200, 160, 120}; + uint8_t out[4] = {}; + c.apply(src, out, 3); + CHECK(out[3] == 0); + CHECK(out[0] == 200); // and the trio is untouched +} From b877683c11b82c71c03745ceb72952c441639d75 Mon Sep 17 00:00:00 2001 From: Shura Fedorovskyi Date: Fri, 30 Oct 2026 16:49:24 +0400 Subject: [PATCH 2/2] Move the die gains onto the fixture profile, applied after the curve 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 #212: gains belong in the fixture profile, not the driver: done here. - ewowi on #212: fold them into the tables, gain after the curve: done here. - ewowi on #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) --- docs/help/help.json | 111 +---------------- src/light/drivers/Correction.h | 112 ++++++++++++++---- src/light/drivers/DriverBase.h | 36 +++--- src/light/drivers/FixtureProfilesModule.h | 50 ++++++++ test/unit/light/unit_Correction.cpp | 98 +++++++++++---- .../unit/light/unit_FixtureProfilesModule.cpp | 23 ++++ 6 files changed, 257 insertions(+), 173 deletions(-) diff --git a/docs/help/help.json b/docs/help/help.json index 3bbe49b3..23413c65 100644 --- a/docs/help/help.json +++ b/docs/help/help.json @@ -173,6 +173,9 @@ }, "ch": { "line": "What one channel carries, such as red, dimmer or pan; none for a spacer or a channel set elsewhere." + }, + "gain": { + "line": "One die pulled down to meet the others, 255 for a type that needs none; the gain lands on emitted light, so 128 is half of it." } } } @@ -3182,21 +3185,9 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, - "balanceRed": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceGreen": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceBlue": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, - "whiteLevel": { - "line": "The white die's own trim, which the three above cannot reach." - }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3329,21 +3320,9 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, - "balanceRed": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceGreen": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceBlue": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, - "whiteLevel": { - "line": "The white die's own trim, which the three above cannot reach." - }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3387,21 +3366,9 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, - "balanceRed": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceGreen": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceBlue": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, - "whiteLevel": { - "line": "The white die's own trim, which the three above cannot reach." - }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3458,21 +3425,9 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, - "balanceRed": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceGreen": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceBlue": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, - "whiteLevel": { - "line": "The white die's own trim, which the three above cannot reach." - }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3515,21 +3470,9 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, - "balanceRed": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceGreen": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceBlue": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, - "whiteLevel": { - "line": "The white die's own trim, which the three above cannot reach." - }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3583,21 +3526,9 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, - "balanceRed": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceGreen": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceBlue": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, - "whiteLevel": { - "line": "The white die's own trim, which the three above cannot reach." - }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3645,21 +3576,9 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, - "balanceRed": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceGreen": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceBlue": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, - "whiteLevel": { - "line": "The white die's own trim, which the three above cannot reach." - }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3721,21 +3640,9 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, - "balanceRed": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceGreen": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceBlue": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, - "whiteLevel": { - "line": "The white die's own trim, which the three above cannot reach." - }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, @@ -3785,21 +3692,9 @@ "line": "The fixture profile applied per light, for channel order and white.", "more": "A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default." }, - "balanceRed": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceGreen": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, - "balanceBlue": { - "line": "One die pulled down to meet the others, which corrects a strip's color cast." - }, "whiteMode": { "line": "How the white channel is derived, shown when the profile carries one." }, - "whiteLevel": { - "line": "The white die's own trim, which the three above cannot reach." - }, "fixtureRef": { "line": "The profile name an older config saved, read once and then emptied." }, diff --git a/src/light/drivers/Correction.h b/src/light/drivers/Correction.h index 4fa413ba..fdcf0715 100644 --- a/src/light/drivers/Correction.h +++ b/src/light/drivers/Correction.h @@ -31,6 +31,19 @@ struct Correction { /// Marks a role this light does not carry. static constexpr uint8_t kAbsent = 255; // color role not carried by this light + /// How many gained rows the tables carry: red, green, blue and the white dies. + static constexpr uint8_t kGainRows = 4; + + /// The tables one light is read through, so a channel's gain costs a pointer at rebuild and nothing per light. + template + struct Tables { + const T* base = nullptr; ///< the ungained curve, which amber, UV and every channel at gain 255 read + const T* red = nullptr; ///< the red die's table, gained or the curve itself + const T* green = nullptr; ///< the green die's + const T* blue = nullptr; ///< the blue die's + const T* white = nullptr; ///< both white dies, which the RGB gains do not reach + }; + /// The perceptual curve the output LUT is filled through; Linear is a requirement, not a fallback, since correcting twice darkens as the square. enum class Curve : uint8_t { Cie = 0, Gamma22, Gamma28, Linear }; @@ -47,10 +60,12 @@ struct Correction { uint16_t* lut16 = nullptr; /// Set by a driver whose output resolves more than a byte, so the 16-bit table is filled whatever the profile. bool wide = false; - /// Per-channel white balance, 255 = untouched; trimmed down only, since there is no headroom above 255 and raising clips instead. - uint8_t balRed = 255, balGreen = 255, balBlue = 255; - /// The white die's trim, 255 = untouched; a separate emitter, often brighter than the RGB trio, that the three trims above cannot reach. - uint8_t whiteLevel = 255; + /// The fixture profile's per-channel gain, 255 where the LED type needs none. Applied after the curve, so the value means emitted light: green at 128 is half the green. + uint8_t gainRed = 255, gainGreen = 255, gainBlue = 255, gainWhite = 255; + /// The gained tables, `kGainRows` of 256, which a driver allocates only for a profile that carries a gain; null leaves every channel on the ungained curve. + uint8_t* gainLut = nullptr; + /// The same to 16 bits, so a wide output keeps its full resolution rather than the 8-bit table's levels. + uint16_t* gainLut16 = nullptr; /// Which curve the brightness rebuild fills through; a driver's setting, not a global one. Curve curve = Curve::Cie; /// Output byte position of the red role; each color role's position is recomputed from the role array. @@ -85,16 +100,64 @@ struct Correction { /// Whether the fixture carries red, green and blue alone, resolved at rebuild so a plain strip skips every optional role per light. bool plainRgb = true; uint8_t outChannels = 3; // bytes emitted per light (= channelsPerLight of the wiring) + /// Each color channel's gained row, or null for the ungained curve. Into the driver's `gainLut`, never this object's own table, so a Correction stays copyable. + const uint8_t* rowRed = nullptr; ///< red's gained row, or null for the curve itself + const uint8_t* rowGreen = nullptr; ///< green's + const uint8_t* rowBlue = nullptr; ///< blue's + const uint8_t* rowWhite = nullptr; ///< the white dies' + /// Red's gained row to 16 bits, or null where no wide table is filled, which leaves the channel 8-bit. + const uint16_t* rowRed16 = nullptr; + const uint16_t* rowGreen16 = nullptr; ///< green's, likewise + const uint16_t* rowBlue16 = nullptr; ///< blue's + const uint16_t* rowWhite16 = nullptr; ///< the white dies' WhiteMode whiteMode = WhiteMode::Min; // how white is synthesized from RGB (white lights only) - /// Refresh the brightness LUT alone, leaving the channel offsets untouched; order is the design: brightness is a linear pre-scale and the curve is applied last. + /// Whether any channel carries a gain, so a driver allocates the gained tables only for a profile that needs them. + bool wantsGains() const MM_NONBLOCKING { + return gainRed != 255 || gainGreen != 255 || gainBlue != 255 || gainWhite != 255; + } + + /// Refresh the brightness LUT alone, leaving the channel offsets untouched; order is the design: brightness is a linear pre-scale, the curve lands next, and the gain last, on emitted light. void rebuildBrightness(uint8_t brightness) MM_NONBLOCKING { + const uint8_t gain[kGainRows] = {gainRed, gainGreen, gainBlue, gainWhite}; for (int v = 0; v < 256; v++) { const float out = shape(static_cast(v) * brightness / 255.0f); // scale first, then curve const bool lit = v > 0 && brightness > 0; briLut[v] = static_cast(quantize(out, 255, lit)); if (wantsTable() && lut16) lut16[v] = static_cast(quantize(out * 257.0f, 65535, lit)); // 255 * 257 = 65535 + for (uint8_t g = 0; g < kGainRows; g++) { + // From the curve's own float, so the gain does not quantize a value the curve already rounded. + const float scaled = out * gain[g] / 255.0f; + const bool onAtAll = lit && gain[g] > 0; + if (gainLut) gainLut[g * 256 + v] = static_cast(quantize(scaled, 255, onAtAll)); + if (gainLut16) gainLut16[g * 256 + v] = static_cast(quantize(scaled * 257.0f, 65535, onAtAll)); + } } + resolveTables(); + } + + /// Point each channel at its gained row, or at null where its gain is 255 or the gained tables are absent. + void resolveTables() MM_NONBLOCKING { + const uint8_t gain[kGainRows] = {gainRed, gainGreen, gainBlue, gainWhite}; + const uint8_t** row8[kGainRows] = {&rowRed, &rowGreen, &rowBlue, &rowWhite}; + const uint16_t** row16[kGainRows] = {&rowRed16, &rowGreen16, &rowBlue16, &rowWhite16}; + for (uint8_t g = 0; g < kGainRows; g++) { + const bool gained = gain[g] != 255; + *row8[g] = gained && gainLut ? gainLut + g * 256 : nullptr; + *row16[g] = gained && gainLut16 ? gainLut16 + g * 256 : nullptr; + } + } + + /// The 8-bit tables one light is read through, the gained rows over the curve this object carries. + Tables tables8() const MM_NONBLOCKING { + return Tables{briLut, rowRed ? rowRed : briLut, rowGreen ? rowGreen : briLut, + rowBlue ? rowBlue : briLut, rowWhite ? rowWhite : briLut}; + } + + /// The same to 16 bits, each row null where no table is filled, which leaves that channel 8-bit. + Tables tables16() const MM_NONBLOCKING { + return Tables{lut16, rowRed16 ? rowRed16 : lut16, rowGreen16 ? rowGreen16 : lut16, + rowBlue16 ? rowBlue16 : lut16, rowWhite16 ? rowWhite16 : lut16}; } /// Whether the 16-bit table is filled: a profile with fine roles, or a wide driver. @@ -111,11 +174,6 @@ struct Correction { return linear; } - /// One channel trimmed down by `bal`, where 255 leaves the byte alone. A pre-scale on the index, not a table per trim, which would cost every driver a kilobyte of its inline budget. - static uint8_t trim(uint8_t v, uint8_t bal) MM_NONBLOCKING { - return bal == 255 ? v : static_cast((static_cast(v) * bal + 127) / 255); - } - /// Round a curved value to an integer of at most `top`, keeping a lit input above zero so a fade-out does not snap off partway down. static int quantize(float out, int top, bool lit) MM_NONBLOCKING { int q = static_cast(out + 0.5f); @@ -154,19 +212,20 @@ struct Correction { } /// Hot path: transform one source light into its output bytes, integer-only and allocation-free. - inline void apply(const uint8_t* src, uint8_t* out, uint8_t srcChannels) const { applyThrough(briLut, src, out, srcChannels); } + inline void apply(const uint8_t* src, uint8_t* out, uint8_t srcChannels) const { applyThrough(tables8(), src, out, srcChannels); } /// Hot path, wide drivers only: the same transform through the 16-bit table, one value per channel; needs `lut16`. - inline void applyWide(const uint8_t* src, uint16_t* out, uint8_t srcChannels) const { applyThrough(lut16, src, out, srcChannels); } + inline void applyWide(const uint8_t* src, uint16_t* out, uint8_t srcChannels) const { applyThrough(tables16(), src, out, srcChannels); } /// The transform through a curve table of either width, so the 8-bit and 16-bit outputs share every line. A remap, not a copy: the layer's packed slots become the fixture's own offsets. template - inline void MM_RAMFUNC applyThrough(const T* lut, const uint8_t* src, T* out, uint8_t srcChannels) const { + inline void MM_RAMFUNC applyThrough(const Tables& t, const uint8_t* src, T* out, uint8_t srcChannels) const { + const T* lut = t.base; // A plain strip: the curve on each color and nothing optional, the drivers' commonest light. if (plainRgb) { - if (offRed != kAbsent) out[offRed] = lut[trim(src[0], balRed)]; - if (offGreen != kAbsent) out[offGreen] = lut[trim(src[1], balGreen)]; - if (offBlue != kAbsent) out[offBlue] = lut[trim(src[2], balBlue)]; + if (offRed != kAbsent) out[offRed] = t.red[src[0]]; + if (offGreen != kAbsent) out[offGreen] = t.green[src[1]]; + if (offBlue != kAbsent) out[offBlue] = t.blue[src[2]]; return; } constexpr T kFull = static_cast(~T{0}); // 255 or 65535 @@ -199,7 +258,7 @@ struct Correction { } else { w = r < g ? (r < b ? r : b) : (g < b ? g : b); // min(r,g,b): the white component // Computed off the PRE-subtraction values, which only rebalance the RGB emitters. - if (offWarmWhite != kAbsent) out[offWarmWhite] = lut[trim(w, whiteLevel)]; + if (offWarmWhite != kAbsent) out[offWarmWhite] = t.white[w]; // yellow ≈ min(R,G) (the shared red+green component). if (offYellow != kAbsent) out[offYellow] = lut[r < g ? r : g]; // Driven from the blue with no red or green to pair with, so it stays dark on warm colors. @@ -210,28 +269,31 @@ struct Correction { // White last: it is the only emitter that rebalances RGB. if (offWhite != kAbsent) { if (whiteMode == WhiteMode::Accurate) { r -= w; g -= w; b -= w; } // pull white out of RGB - out[offWhite] = lut[trim(w, whiteLevel)]; + out[offWhite] = t.white[w]; } } // The curve, applied ONCE: everything above this line is linear light. - if (offRed != kAbsent) out[offRed] = lut[trim(r, balRed)]; - if (offGreen != kAbsent) out[offGreen] = lut[trim(g, balGreen)]; - if (offBlue != kAbsent) out[offBlue] = lut[trim(b, balBlue)]; + if (offRed != kAbsent) out[offRed] = t.red[r]; + if (offGreen != kAbsent) out[offGreen] = t.green[g]; + if (offBlue != kAbsent) out[offBlue] = t.blue[b]; if constexpr (sizeof(T) == 1) { if (hasFine) applyFine(r, g, b, w, out); } } /// Hot path, 16-bit profiles only: each color channel with a fine role as its high and low byte from the same table, and a fine dimmer wide open. void applyFine(uint8_t r, uint8_t g, uint8_t b, uint8_t w, uint8_t* out) const { if (offDimmerFine != kAbsent) out[offDimmerFine] = 255; // wide open is 65535, not 65280 - // Trimmed here as the coarse bytes are, so a fine channel is balanced exactly as an 8-bit one. - const uint8_t value[5] = {trim(r, balRed), trim(g, balGreen), trim(b, balBlue), - trim(w, whiteLevel), trim(w, whiteLevel)}; + const uint8_t value[5] = {r, g, b, w, w}; // both whites carry the white component, 0 when none is synthesized + // Each channel's own gained table, so a fine channel is gained exactly as an 8-bit one and at full resolution. + const Tables w16 = tables16(); + const Tables n8 = tables8(); + const uint16_t* wide16[5] = {w16.red, w16.green, w16.blue, w16.white, w16.white}; + const uint8_t* narrow8[5] = {n8.red, n8.green, n8.blue, n8.white, n8.white}; const uint8_t coarse[5] = {offRed, offGreen, offBlue, offWhite, offWarmWhite}; const uint8_t fine[5] = {offRedFine, offGreenFine, offBlueFine, offWhiteFine, offWarmWhiteFine}; for (uint8_t k = 0; k < 5; k++) { if (fine[k] == kAbsent) continue; // Without its table (an allocation that failed) the channel stays 8-bit: the coarse byte as written, the fine one 0. - const uint16_t q = lut16 ? lut16[value[k]] : static_cast(briLut[value[k]] << 8); + const uint16_t q = wide16[k] ? wide16[k][value[k]] : static_cast(narrow8[k][value[k]] << 8); if (coarse[k] != kAbsent) out[coarse[k]] = static_cast(q >> 8); out[fine[k]] = static_cast(q & 0xFF); } diff --git a/src/light/drivers/DriverBase.h b/src/light/drivers/DriverBase.h index 494bfd07..5caff835 100644 --- a/src/light/drivers/DriverBase.h +++ b/src/light/drivers/DriverBase.h @@ -72,10 +72,6 @@ class DriverBase : public MoonModule { // Applied unconditionally, so brightness works even before the fixture-profile library is up. const uint8_t tableBrightness = adoptBrightness(globalBrightness); correction_.whiteMode = static_cast(whiteMode_); - correction_.balRed = balanceRed_; - correction_.balGreen = balanceGreen_; - correction_.balBlue = balanceBlue_; - correction_.whiteLevel = whiteLevel_; correction_.curve = static_cast(curveSel_); // A missing id falls back to the default, so a driver degrades rather than crashing. if (auto* lib = FixtureProfilesModule::active()) { @@ -93,6 +89,7 @@ class DriverBase : public MoonModule { const bool firstTable = correction_.wantsTable() && !lut16_; if (firstTable) lut16_.resize(256); correction_.lut16 = lut16_.data(); + resolveGainTables(); if (firstTable) correction_.rebuildBrightness(tableBrightness); // a table that existed was filled by the rebuild above onCorrectionChanged(); // let a driver resize its correction-applied buffer } @@ -134,6 +131,8 @@ class DriverBase : public MoonModule { void release() override { freeWire(); // the shared correction scratch: owned here, so no driver re-frees it correction_.lut16 = nullptr; // the table goes with the module's scratch buffers below + correction_.gainLut = nullptr; + correction_.gainLut16 = nullptr; clearFailBuf(); clearConfigErr(); setConfigWarn(nullptr); @@ -179,16 +178,14 @@ class DriverBase : public MoonModule { /// The resolved correction: the wiring comes from a named profile by stable id, and the render loop never reads the library. Correction correction_; ScratchBuffer lut16_{*this}; // the correction's 16-bit table, allocated the first time it is wanted + ScratchBuffer gainLut_{*this}; // the gained tables, only for a profile whose LED type carries a gain + ScratchBuffer gainLut16_{*this}; // the same to 16 bits, only where a channel is read wide uint32_t profileId_ = 0; // stable id into the FixtureProfiles library (0 → resolve to default) /// The fixture profile applied per light, for channel order and white. /// @guide A reorder or a reboot keeps it. After renaming the profile, re-pick it: the next reboot falls back to the default. uint8_t fixtureSel_ = 0; /// How the white channel is derived, shown when the profile carries one. uint8_t whiteMode_ = static_cast(WhiteMode::Min); - /// One die pulled down to meet the others, which corrects a strip's color cast. - uint8_t balanceRed_ = 255, balanceGreen_ = 255, balanceBlue_ = 255; - /// The white die's own trim, which the three above cannot reach. - uint8_t whiteLevel_ = 255; /// This driver's dim, multiplied with the global brightness. uint8_t localBrightness_ = 255; /// How brightness maps to output: CIE 1931, a gamma, or linear. @@ -215,17 +212,10 @@ class DriverBase : public MoonModule { buildFixtureOptions(); // fill fixtureOptions_ from the library, sync id and row controls_.addSelect("fixture", fixtureSel_, fixtureOptions_, fixtureOptionCount_); controls_.setPersistLabel(controls_.count() - 1); // saved by name, so a longer or reordered library never moves it - // Trim DOWN only: there is no headroom above 255, so raising clips rather than balancing. A strip whose green die runs hot is corrected here rather than in every effect. - controls_.addControl("balanceRed", balanceRed_, 0, 255); - controls_.addControl("balanceGreen", balanceGreen_, 0, 255); - controls_.addControl("balanceBlue", balanceBlue_, 0, 255); controls_.addSelect("whiteMode", whiteMode_, kWhiteModeOptions, kWhiteModeCount); // Hidden unless the referenced profile carries a channel there is something to synthesize for. auto* lib = FixtureProfilesModule::active(); controls_.setHidden(controls_.count() - 1, !(lib && lib->profileHasSynthChannel(profileId_))); - // A separate emitter the three RGB trims cannot reach, so it carries its own. - controls_.addControl("whiteLevel", whiteLevel_, 0, 255); - controls_.setHidden(controls_.count() - 1, !(lib && lib->profileHasWhiteDie(profileId_))); // Hidden, and only read: the name an older config saved. controls_.addText("fixtureRef", fixtureRef_, sizeof(fixtureRef_)); controls_.setHidden(controls_.count() - 1, true); @@ -234,12 +224,22 @@ class DriverBase : public MoonModule { /// Set this driver's default referenced profile by name, from its constructor. void setDefaultFixtureName(const char* name) { defaultFixture_ = name; } + /// Give the correction the gained tables its profile asks for, and take them back when it asks for none. Only a profile whose LED type carries a gain pays for them, the way the 16-bit table is allocated only where it is read. + void resolveGainTables() { + const bool wants = correction_.wantsGains(); + if (wants && !gainLut_) gainLut_.resize(Correction::kGainRows * 256); + if (!wants) gainLut_.resize(0); + correction_.gainLut = wants ? gainLut_.data() : nullptr; + const bool wide = wants && correction_.wantsTable(); + if (wide && !gainLut16_) gainLut16_.resize(Correction::kGainRows * 256); + if (!wide) gainLut16_.resize(0); + correction_.gainLut16 = gainLut16_ ? gainLut16_.data() : nullptr; + } + /// Whether `name` is one of the correction controls, for a driver's own prepare test. static bool isCorrectionControl(const char* name) { return std::strcmp(name, "fixture") == 0 || std::strcmp(name, "fixtureRef") == 0 || std::strcmp(name, "localBrightness") == 0 - || std::strcmp(name, "whiteMode") == 0 || std::strcmp(name, "curve") == 0 - || std::strcmp(name, "balanceRed") == 0 || std::strcmp(name, "balanceGreen") == 0 - || std::strcmp(name, "balanceBlue") == 0 || std::strcmp(name, "whiteLevel") == 0; + || std::strcmp(name, "whiteMode") == 0 || std::strcmp(name, "curve") == 0; } private: diff --git a/src/light/drivers/FixtureProfilesModule.h b/src/light/drivers/FixtureProfilesModule.h index 19a12cb1..b0c9c0b6 100644 --- a/src/light/drivers/FixtureProfilesModule.h +++ b/src/light/drivers/FixtureProfilesModule.h @@ -106,6 +106,10 @@ class FixtureProfilesModule : public MoonModule, public ListSource { const Profile* p = find(id); if (!p) return false; // The option indices are aligned with the enum, so a role byte IS a ChannelRole value. + out.gainRed = p->gainRed; + out.gainGreen = p->gainGreen; + out.gainBlue = p->gainBlue; + out.gainWhite = p->gainWhite; out.rebuild(brightness, reinterpret_cast(roleAt(*p)), p->channelCount); return true; } @@ -136,6 +140,7 @@ class FixtureProfilesModule : public MoonModule, public ListSource { /// @field name The profile's name, as a driver's fixture picker lists it. /// @field channels How many DMX channels the fixture takes. /// @field ch What one channel carries, such as red, dimmer or pan; none for a spacer or a channel set elsewhere. + /// @field gain One die pulled down to meet the others, 255 for a type that needs none; the gain lands on emitted light, so 128 is half of it. /// @guide Built-in rows are read-only, and custom rows persist across a reboot. controls_.addList("profiles", *this); // this module is the (editable) ListSource } @@ -156,6 +161,11 @@ class FixtureProfilesModule : public MoonModule, public ListSource { for (uint8_t c = 0; c < p.channelCount; c++) sink.appendf("%s%u", c ? "," : "", static_cast(roles[c])); sink.append("]"); + // Only a type that needs one, so every built-in row stays the shape it was. + if (hasGain(p)) + sink.appendf(",\"gains\":[%u,%u,%u,%u]", static_cast(p.gainRed), + static_cast(p.gainGreen), static_cast(p.gainBlue), + static_cast(p.gainWhite)); if (p.locked) sink.append(",\"locked\":true"); sink.append("}"); } @@ -183,6 +193,13 @@ class FixtureProfilesModule : public MoonModule, public ListSource { sink.appendf(",{\"name\":\"ch%u\",\"type\":\"select\",\"value\":%u,\"optionsRef\":\"channelRole\"}", static_cast(c), static_cast(roles[c])); } + const char* const kGainFields[] = {"gainRed", "gainGreen", "gainBlue", "gainWhite"}; + const uint8_t gains[] = {p.gainRed, p.gainGreen, p.gainBlue, p.gainWhite}; + // The white gain reaches the white dies alone, so a profile carrying none is not offered a field that changes nothing. + const uint8_t offered = profileHasWhiteDie(p.id) ? 4u : 3u; + for (uint8_t gi = 0; gi < offered; gi++) + sink.appendf(",{\"name\":\"%s\",\"type\":\"uint8\",\"value\":%u,\"min\":0,\"max\":255}", + kGainFields[gi], static_cast(gains[gi])); sink.append("]}"); } @@ -257,6 +274,7 @@ class FixtureProfilesModule : public MoonModule, public ListSource { setChannelCount(p, static_cast(v)); return true; } + if (setGain(p, field, valueJson)) return true; return setChannelRole(p, field, valueJson); } @@ -288,6 +306,8 @@ class FixtureProfilesModule : public MoonModule, public ListSource { uint16_t poolOffset = 0; // start of this profile's roles in pool_ (set by rebuildPool); 48 x 255 fits uint16_t poolLen = 0; // the OLD slice size, distinct from channelCount mid-change bool locked = false; + /// Per-channel gain, 255 where this LED type needs none: an off batch gets a profile of its own. + uint8_t gainRed = 255, gainGreen = 255, gainBlue = 255, gainWhite = 255; }; static_assert(kMaxProfiles * 255 <= 0xFFFF, "a full library's role pool must fit the 16-bit pool offsets"); @@ -329,6 +349,25 @@ class FixtureProfilesModule : public MoonModule, public ListSource { return field[0] == 'c' && field[1] == 'h' && field[2] >= '0' && field[2] <= '9'; } + /// Whether this profile carries a gain, which is what decides whether a driver allocates the gained tables. + static bool hasGain(const Profile& p) { + return p.gainRed != 255 || p.gainGreen != 255 || p.gainBlue != 255 || p.gainWhite != 255; + } + + /// Set the gain a `gainRed`-style field names; false for any other field or an out-of-range value. + bool setGain(Profile& p, const char* field, const char* valueJson) { + uint8_t* slot = std::strcmp(field, "gainRed") == 0 ? &p.gainRed + : std::strcmp(field, "gainGreen") == 0 ? &p.gainGreen + : std::strcmp(field, "gainBlue") == 0 ? &p.gainBlue + : std::strcmp(field, "gainWhite") == 0 ? &p.gainWhite + : nullptr; + if (!slot) return false; + const int v = mm::json::parseInt(valueJson, "value"); + if (v < 0 || v > 255) return false; + *slot = static_cast(v); + return true; + } + /// Set the role of the channel a `chN` field names; false for any other field or an out-of-range value. bool setChannelRole(Profile& p, const char* field, const char* valueJson) { if (!isChannelField(field)) return false; @@ -357,9 +396,20 @@ class FixtureProfilesModule : public MoonModule, public ListSource { const int ch = mm::json::readInt(mm::json::member(doc, row, "channels"), 3); p.channelCount = static_cast(ch < 1 ? 1 : ch > 255 ? 255 : ch); p.locked = mm::json::readBool(mm::json::member(doc, row, "locked"), false); + restoreGains(doc, row, p); if (p.id >= nextId_) nextId_ = p.id + 1; // never reissue a persisted id } + /// Read one persisted row's gains, an absent array leaving the type ungained. Clamped, since a corrupt file can carry a value outside the range the UI offers. + void restoreGains(const mm::json::JsonDoc& doc, const mm::json::JsonNode* row, Profile& p) { + const mm::json::JsonNode* gains = mm::json::member(doc, row, "gains"); + uint8_t* slot[] = {&p.gainRed, &p.gainGreen, &p.gainBlue, &p.gainWhite}; + for (uint8_t gi = 0; gi < 4; gi++) { + const int v = mm::json::readInt(mm::json::element(doc, gains, gi), 255); + *slot[gi] = static_cast(v < 0 ? 255 : v > 255 ? 255 : v); + } + } + /// Read one persisted row's roles into its slice of the pool, an absent or invalid one as None. It is clamped, since a corrupt file can carry a byte the UI mis-renders and the Correction drops. void restoreRoles(const mm::json::JsonDoc& doc, const mm::json::JsonNode* row, const Profile& p) { const mm::json::JsonNode* roles = mm::json::member(doc, row, "roles"); diff --git a/test/unit/light/unit_Correction.cpp b/test/unit/light/unit_Correction.cpp index 3c687a2f..f0243071 100644 --- a/test/unit/light/unit_Correction.cpp +++ b/test/unit/light/unit_Correction.cpp @@ -643,50 +643,82 @@ TEST_CASE("Dimmer fine is driven wide open with its coarse byte") { CHECK((out[0] == 255 && out[1] == 255)); } -// An LED's three emitters are not equally efficient, so a neutral triple can render with a cast. The trim pulls one die down to meet the others, at the output rather than in every effect. -TEST_CASE("Correction balance: trimming one channel leaves the others untouched") { +// The gain lives on the fixture profile, since a cast is a property of the LED type; a driver allocates these tables only for a profile that carries one. +namespace { +struct Gained { + uint8_t lut[mm::Correction::kGainRows * 256] = {}; + uint16_t lut16[mm::Correction::kGainRows * 256] = {}; + /// Wire the tables a driver would own, then fill them through the production rebuild. + void wire(Correction& c) { + c.gainLut = lut; + c.gainLut16 = lut16; + } +}; +} // namespace + +// An LED's three emitters are not equally efficient, so a neutral triple can render with a cast. The profile's gain pulls one die down to meet the others, at the output rather than in every effect. +TEST_CASE("Correction gain: one channel's gain leaves the others untouched") { Correction c; - c.balGreen = 128; + Gained g; + g.wire(c); + c.gainGreen = 128; mm::test::rebuildFromPreset(c, 255, mm::test::PresetOrder::RGB); const uint8_t src[3] = {200, 200, 200}; // the triple a green-cast strip renders wrong uint8_t out[3] = {}; c.apply(src, out, 3); CHECK(out[0] == 200); - CHECK(out[1] == 100); // pulled down to match the weaker channels + CHECK(out[1] == 100); // half the green light, which is what 128 says CHECK(out[2] == 200); } -// A white phosphor is separate hardware, so an RGB trim must say nothing about it. -TEST_CASE("Correction balance: an RGB trim does not reach the white die") { +// A white phosphor is separate hardware, so an RGB gain must say nothing about it. +TEST_CASE("Correction gain: an RGB gain does not reach the white die") { Correction c; - c.balBlue = 128; + Gained g; + g.wire(c); + c.gainBlue = 128; mm::test::rebuildFromPreset(c, 255, mm::test::PresetOrder::RGBW); const uint8_t src[3] = {200, 200, 200}; uint8_t out[4] = {}; c.apply(src, out, 3); CHECK(out[0] == 200); CHECK(out[1] == 200); - CHECK(out[2] == 100); // blue trimmed - CHECK(out[3] == 200); // the white component, from the untrimmed minimum + CHECK(out[2] == 100); // blue gained + CHECK(out[3] == 200); // the white component, ungained } -// The trim pre-scales the index the curve is read at, so the curve still lands last. -TEST_CASE("Correction balance: the curve and the trim compose") { +// The gain lands on emitted light, after the curve, so its value means what it says at any curve. +TEST_CASE("Correction gain: the curve and the gain compose to the gain's own ratio") { Correction c; - c.balBlue = 128; + Gained g; + g.wire(c); + c.gainBlue = 128; mm::test::rebuildFromPreset(c, 255, mm::test::PresetOrder::RGB); c.curve = Correction::Curve::Gamma22; - c.rebuildBrightness(255); // the same offsets, the table refilled through the curve + c.rebuildBrightness(255); // the same offsets, the tables refilled through the curve const uint8_t src[3] = {200, 200, 200}; uint8_t out[3] = {}; c.apply(src, out, 3); - CHECK(out[0] == 149); // red: the curve alone, (200/255)^2.2 * 255 - CHECK(out[2] < out[0]); // blue: trimmed first, so it reads the curve lower down - CHECK(out[2] > 0); + CHECK(out[0] == 149); // red: the curve alone, (200/255)^2.2 * 255 + CHECK(out[2] == 75); // blue: that light at 128/255 of it, not an index pulled down before the curve } -// The white die carries its own trim, since it is often brighter than the trio beside it. -TEST_CASE("Correction whiteLevel: trims the white die without touching RGB") { +// Every built-in profile carries no gain, so it allocates nothing and every channel reads the one curve. +TEST_CASE("Correction gain: a profile with no gain wants no tables") { + Correction c; + mm::test::rebuildFromPreset(c, 255, mm::test::PresetOrder::RGBW); + CHECK_FALSE(c.wantsGains()); + const uint8_t src[3] = {200, 160, 120}; + uint8_t out[4] = {}; + c.apply(src, out, 3); + CHECK(out[0] == 200); + CHECK(out[1] == 160); + CHECK(out[2] == 120); + CHECK(out[3] == 120); +} + +// The white die carries its own gain, since it is often brighter than the trio beside it. +TEST_CASE("Correction gainWhite: gains the white die without touching RGB") { const uint8_t src[3] = {200, 160, 120}; // the white component is the minimum, 120 Correction full; @@ -695,19 +727,24 @@ TEST_CASE("Correction whiteLevel: trims the white die without touching RGB") { full.apply(src, a, 3); Correction half; - half.whiteLevel = 128; + Gained g; + g.wire(half); + half.gainWhite = 128; mm::test::rebuildFromPreset(half, 255, mm::test::PresetOrder::RGBW); uint8_t b[4] = {}; half.apply(src, b, 3); - CHECK(b[3] < a[3]); // the white die alone moves + CHECK(a[3] == 120); + CHECK(b[3] == 60); // the white die alone moves CHECK((b[0] == a[0] && b[1] == a[1] && b[2] == a[2])); } // A die asked for nothing goes out, rather than resting on the floor that keeps a fade from snapping off. -TEST_CASE("Correction whiteLevel: zero puts the white die out") { +TEST_CASE("Correction gainWhite: zero puts the white die out") { Correction c; - c.whiteLevel = 0; + Gained g; + g.wire(c); + c.gainWhite = 0; mm::test::rebuildFromPreset(c, 255, mm::test::PresetOrder::RGBW); const uint8_t src[3] = {200, 160, 120}; uint8_t out[4] = {}; @@ -715,3 +752,20 @@ TEST_CASE("Correction whiteLevel: zero puts the white die out") { CHECK(out[3] == 0); CHECK(out[0] == 200); // and the trio is untouched } + +// A driver resolving a profile assigns a Correction whole, and a test does the same, so a gained one must carry no pointer into the object it came from. +TEST_CASE("Correction gain: a copied correction reads its own tables") { + Correction built; + Gained g; + g.wire(built); + built.gainGreen = 128; + mm::test::rebuildFromPreset(built, 255, mm::test::PresetOrder::RGB); + + Correction copy = built; // what a driver's assignment does + const uint8_t src[3] = {200, 200, 200}; + uint8_t out[3] = {}; + copy.apply(src, out, 3); + CHECK(out[0] == 200); + CHECK(out[1] == 100); // the gain survives the copy, read through the copy's own curve + CHECK(out[2] == 200); +} diff --git a/test/unit/light/unit_FixtureProfilesModule.cpp b/test/unit/light/unit_FixtureProfilesModule.cpp index 3068b320..632e10cc 100644 --- a/test/unit/light/unit_FixtureProfilesModule.cpp +++ b/test/unit/light/unit_FixtureProfilesModule.cpp @@ -622,3 +622,26 @@ TEST_CASE("A saved profile without an id gets its own, and its name is not seede CHECK(grb == 1); CHECK(builtinId(m, "GRB") > 1); // not the id the next saved row holds } + +// A cast is a property of the LED type, so the gain is the profile's; an edit must survive the row being written and restored. +TEST_CASE("FixtureProfiles gain: an edited gain is kept, serialized and restored") { + FixtureProfilesModule m; + m.setup(); + uint32_t id = 0; + REQUIRE(m.addListRow(id)); + REQUIRE(m.setListRowField(id, "channels", "{\"value\":4}")); + REQUIRE(m.setListRowField(id, "ch3", "{\"value\":4}")); // a white die, so gainWhite is offered + + CHECK(m.setListRowField(id, "gainGreen", "{\"value\":180}")); + CHECK(m.setListRowField(id, "gainWhite", "{\"value\":150}")); + + // The correction the driver resolves carries them. + Correction c; + REQUIRE(m.deriveCorrection(id, 255, c)); + CHECK(c.gainGreen == 180); + CHECK(c.gainWhite == 150); + + // The row the UI re-reads carries them, rather than snapping back to 255. + const std::string rows = profilesJson(m); + CHECK(rows.find("\"gains\":[255,180,255,150]") != std::string::npos); +}