From c9848d8fa66fced271b8ceb815fb7750d275d258 Mon Sep 17 00:00:00 2001 From: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> Date: Sat, 29 Aug 2026 08:37:20 +1000 Subject: [PATCH] [light] Fix gamma table dead zone collapsing to 0 (#18845) --- .../components/light/esp_color_correction.cpp | 6 +- .../light/test_gamma_correction.cpp | 92 +++++++++++++++++++ .../components/light/test_gamma_table.py | 17 +++- 3 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 tests/components/light/test_gamma_correction.cpp diff --git a/esphome/components/light/esp_color_correction.cpp b/esphome/components/light/esp_color_correction.cpp index e793226bb1..12eb6a3008 100644 --- a/esphome/components/light/esp_color_correction.cpp +++ b/esphome/components/light/esp_color_correction.cpp @@ -5,7 +5,11 @@ namespace esphome::light { uint8_t ESPColorCorrection::gamma_correct_(uint8_t value) const { if (this->gamma_table_ == nullptr) return value; - return static_cast((progmem_read_uint16(&this->gamma_table_[value]) + 128) / 257); + uint16_t table_value = progmem_read_uint16(&this->gamma_table_[value]); + uint8_t result = (table_value + 128) / 257; + if (result == 0 && table_value != 0) + return 1; + return result; } uint8_t ESPColorCorrection::gamma_uncorrect_(uint8_t value) const { diff --git a/tests/components/light/test_gamma_correction.cpp b/tests/components/light/test_gamma_correction.cpp new file mode 100644 index 0000000000..4b8d83c544 --- /dev/null +++ b/tests/components/light/test_gamma_correction.cpp @@ -0,0 +1,92 @@ +#include + +#include +#include +#include +#include + +#include "esphome/components/light/esp_color_correction.h" + +namespace esphome::light::testing { + +namespace { + +// A representative fixture for ESPColorCorrection/gamma_table_reverse_search tests below -- +// not a spec for generate_gamma_table() itself, which the Python tests own. +std::array build_gamma_table(double gamma) { + std::array table{}; + table[0] = 0; + for (int i = 1; i < 256; i++) { + double raw = std::round(std::pow(i / 255.0, gamma) * 65535.0); + table[i] = static_cast(std::max(1.0, std::min(65535.0, raw))); + } + return table; +} + +// Bundles a table with an ESPColorCorrection pointing at it, since the correction only holds +// a raw pointer into the table and doesn't own it. +struct GammaFixture { + explicit GammaFixture(double gamma) : table(build_gamma_table(gamma)) { correction.set_gamma_table(table.data()); } + std::array table; + ESPColorCorrection correction; +}; + +} // namespace + +// Regression test for esphome/esphome#18842: ESPColorCorrection's own 16-bit -> 8-bit +// conversion must never round a non-zero table entry down to a zero 8-bit output. +TEST(GammaCorrection, NonZeroInputsSurviveConversion) { + for (double gamma : {1.0, 1.8, 2.0, 2.2, 2.8, 3.0, 4.0}) { + GammaFixture fixture(gamma); + for (int i = 1; i < 256; i++) { + EXPECT_GE(fixture.correction.color_correct_red(i), 1) << "gamma=" << gamma << " index=" << i; + } + } +} + +TEST(GammaCorrection, ZeroInputStaysZero) { + for (double gamma : {1.0, 2.2, 2.8, 4.0}) { + GammaFixture fixture(gamma); + EXPECT_EQ(fixture.correction.color_correct_red(0), 0) << "gamma=" << gamma; + } +} + +TEST(GammaCorrection, FullBrightnessStaysFull) { + for (double gamma : {1.0, 2.2, 2.8, 4.0}) { + GammaFixture fixture(gamma); + EXPECT_EQ(fixture.correction.color_correct_red(255), 255) << "gamma=" << gamma; + } +} + +// Reproduces the reporter's own numbers from esphome/esphome#18842 at gamma=2.8: codes +// 1-27 previously collapsed to an 8-bit output of 0 and must now be non-zero. +TEST(GammaCorrection, DeadZoneFixedAtGamma28) { + GammaFixture fixture(2.8); + for (int i = 1; i < 28; i++) { + EXPECT_GE(fixture.correction.color_correct_red(i), 1) << "index=" << i << " still collapses to 0"; + } +} + +TEST(GammaCorrection, ReverseSearchFindsLargestIndexLessEqualTarget) { + auto table = build_gamma_table(2.8); + for (uint16_t target : {0, 128, 129, 135, 1000, 32768, 65535}) { + uint8_t lo = gamma_table_reverse_search(table.data(), target); + EXPECT_LE(table[lo], target) << "target=" << target; + if (lo < 255) { + EXPECT_GT(table[lo + 1], target) << "target=" << target; + } + } +} + +// color_uncorrect_* binary-searches the table via gamma_table_reverse_search(). +TEST(GammaCorrection, UncorrectStaysMonotonic) { + GammaFixture fixture(2.8); + uint8_t prev = 0; + for (int i = 1; i < 256; i++) { + uint8_t result = fixture.correction.color_uncorrect_red(i); + EXPECT_GE(result, prev) << "index=" << i; + prev = result; + } +} + +} // namespace esphome::light::testing diff --git a/tests/unit_tests/components/light/test_gamma_table.py b/tests/unit_tests/components/light/test_gamma_table.py index a302a355dc..75c3f18e42 100644 --- a/tests/unit_tests/components/light/test_gamma_table.py +++ b/tests/unit_tests/components/light/test_gamma_table.py @@ -53,9 +53,12 @@ def test_nonzero_indices_are_nonzero(gamma: float) -> None: assert table[i] >= 1, f"gamma={gamma}, index {i}: got {table[i]}" -@pytest.mark.parametrize("gamma", [1.0, 2.0, 2.2, 2.8, 3.0]) +@pytest.mark.parametrize("gamma", [1.0, 1.8, 2.0, 2.2, 2.8, 3.0, 4.0]) def test_table_monotonically_nondecreasing(gamma: float) -> None: - """The gamma table must be monotonically non-decreasing.""" + """The gamma table must be monotonically non-decreasing. + + gamma_table_reverse_search()'s binary search depends on this. + """ table = generate_gamma_table(gamma) for i in range(1, 256): assert table[i] >= table[i - 1], ( @@ -115,3 +118,13 @@ def test_lut_output_monotonically_nondecreasing() -> None: result = _simulate_gamma_correct_lut(table, value) assert result >= prev, f"value={value}: result {result} < previous {prev}" prev = result + + +def test_table_matches_raw_power_curve() -> None: + """Check the gamma table against known good values for gamma=2.8.""" + table = generate_gamma_table(2.8) + golden = {1: 1, 5: 1, 15: 24, 27: 122, 28: 135, 100: 4766, 200: 33193, 254: 64818} + for i, expected in golden.items(): + assert table[i] == expected, ( + f"index {i}: table[{i}]={table[i]} expected {expected}" + )