Repository navigation
Give the gamma correction 0 for 0, a test and its source - #81
Merged
Merged
Conversation
The curve is the one from the "Sparsamere Implementierung" section of https://www.mikrocontroller.net/articles/Diskussion:LED-Fading - 16 stored values for the top doubling, every lower one a shift. Its author notes that the exponential never reaches zero and recommends answering a setting of 0 with 0; that part did not come over, so a setting of 0 became 1. It is answered apart now rather than by subtracting 1 from every result, which would have taken 255 down to 254 as well. What reaches the strip does not change today: DisplayColor keeps every channel at 1 at the bottom on purpose, with and without the correction. The correction now hands on the same 0 an uncorrected setting does and leaves that decision to the stage that makes it. The header now says what the curve is and where it comes from, with both links; why 128 steps, 16 values and 8 bits have to agree, which a static_assert holds instead of a numberOfBits() that only happened to be 8; and what to know before changing it - the coarse bottom, which is 8 bits and not the code, and the dark middle, which is the base of 256 the shift trick fixes. testGammaCorrectionCurve() checks both ends, that the curve never falls, and a value from each end of the shift range. Run against the old header, exactly the 0 check fails and the rest pass - the curve itself is unchanged. AVR flash 48116 -> 48140 bytes for the check at its two call sites. Built: simulator with ctest, AVR, ESP32, RP2350, both host suites; clang-tidy clean on all four builds.
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.
The gamma correction is the exponential curve from the "Sparsamere Implementierung" section of Diskussion:LED-Fading. It stores 16 values for the top doubling and gets every lower doubling by shifting. The background is the article itself, LED-Fading.
What changed
static_assertnow enforces; before,numberOfBits()happened to return 8. It also notes what to know before changing the curve:exponentholds the mantissa), and the empty sections are gone.memcpy_Pstays, because that is how the firmware core reads flash everywhere.What does not change
What reaches the strip stays the same today.
DisplayColor::dimmColordeliberately keeps every channel at 1 or above, and a test enforces that, with or without the correction. A setting of 0 therefore still gives the faintest light. After this change, the correction passes on the same 0 that an uncorrected setting does, and the stage after it decides what that means.Measured
testGammaCorrectionCurve()checks both ends, that the curve never falls, and one value from each end of the shift range. Made to fail on purpose: against the old header, exactly the 0 check fails and every other check passes. The curve itself is unchanged.ctest, AVR, ESP32, RP2350, and both host-test suites.tools/documented-sizes.pypasses for all three firmwares, and clang-tidy is clean on all four builds.Whether the middle of the range should be brighter (a flatter base) can only be judged on the real strip, so this PR leaves it alone.