Skip to content

Make twinkle FX look more like it was in the past#5635

Open
DedeHai wants to merge 1 commit into
wled:mainfrom
DedeHai:twinkleFX_fixes
Open

Make twinkle FX look more like it was in the past#5635
DedeHai wants to merge 1 commit into
wled:mainfrom
DedeHai:twinkleFX_fixes

Conversation

@DedeHai
Copy link
Copy Markdown
Collaborator

@DedeHai DedeHai commented May 20, 2026

fixes #5622

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced color brightness calculation logic in the TwinkleFOX effect for improved visual accuracy.
    • Refined RGB color averaging method in lighting calculations for better color rendering precision.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

Walkthrough

Refactored TwinkleFOX brightness calculations to use RGB-only averaging and gamma8-inverted color scaling. Replaced the CRGBW getAverageLight() four-channel average with a new getRGBaverage() method, and updated the background brightness scaling logic and per-pixel brightness delta computation accordingly.

Changes

TwinkleFOX Brightness Scaling

Layer / File(s) Summary
RGB-only averaging method
wled00/colors.h
CRGBW struct adds getRGBaverage() to compute integer average of R, G, B channels only, replacing the prior four-channel getAverageLight().
TwinkleFOX brightness scaling refactor
wled00/FX.cpp
TwinkleFOX background brightness calculation switches to getRGBaverage() with direct gamma8inv()-based color_fade() scaling, then refreshes bglight. Per-pixel brightness delta updated to use the new RGB averaging against the recalculated bglight.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • wled/WLED#4728: Modifies TwinkleFOX brightness behavior in FX.cpp, specifically the twinklefox_one_twinkle function's brightness inversion logic.
  • wled/WLED#4722: Introduces gamma32inv() inverse gamma correction, which the background brightness scaling path relies on.
  • wled/WLED#4710: Adds inverse gamma lookup support (gamma8inv) used in the new color_fade scaling approach.

Suggested labels

effect, Awaiting testing

Suggested reviewers

  • blazoncek
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main objective of the PR - fixing the Twinklefox effect appearance to match prior versions.
Linked Issues check ✅ Passed The PR changes address the core issue #5622 by applying gamma correction compensation through getRGBaverage() and color_fade() adjustments to twinkle colors and background brightness calculations.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the Twinklefox brightness issue in the twinklefox_base() function and supporting color averaging methods.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Member

@softhack007 softhack007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did a quick test, looks good to merge.
The PR seems to make the twinkling a bit brighter on average, compared to 16.0.0. But this is really a "nuance" on my LEDs, nothing like "did not look right previously." But then, i'm also not an expert on the twinkle* effects 🤷

Technicially:

  • make sure the auto brightness limiter is not set up too strictly (per-output ABL is sometimes stonger than previously)
  • Make sure you have gamma set to 2.2, and "use gamma correction for color" checked
Image Image

@DedeHai
Copy link
Copy Markdown
Collaborator Author

DedeHai commented May 21, 2026

@softhack007 I ran some experiments on different gamma gorrection curve approaches in general (less compression at the low end) but they all resulted in some color artefacts in one way or another. Then I took a more in depth look at the twinkling effects and now think your initial suggestion of using inversegamma on brightness for colorpalette should be used, but not in general, just for twinkle base: reason is that not doing it changes the original look of the effect which I have no issue with but then I also do not really use it much so lets give people what they are used to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Twinklefox/Twinklecat dimmer than expected (16.0.0)

2 participants