-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Add pre-applied gamma to c3g & (some) fastled palettes to match original look #4711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes update color palette data in Changes
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wled00/palettes.h (1)
5-11: Clarify gamma correction rationale in header comment
The updated block accurately describes pre-applied (1.182,1.0,1.136) and inverse (2.2) gammas. Consider explicitly naming which palettes fall under each category for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wled00/palettes.h(6 hunks)wled00/wled.h(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: wled_build / Build Environments (usermods)
- GitHub Check: wled_build / Build Environments (esp32_wrover)
- GitHub Check: wled_build / Build Environments (lolin_s2_mini)
- GitHub Check: wled_build / Build Environments (esp32_eth)
- GitHub Check: wled_build / Build Environments (esp32dev_V4)
- GitHub Check: wled_build / Build Environments (esp32dev)
- GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
- GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
- GitHub Check: wled_build / Build Environments (esp32c3dev)
- GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
- GitHub Check: wled_build / Build Environments (esp01_1m_full)
- GitHub Check: wled_build / Build Environments (esp8266_2m_160)
- GitHub Check: wled_build / Build Environments (nodemcuv2)
- GitHub Check: wled_build / Build Environments (esp8266_2m)
- GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
- GitHub Check: wled_build / Build Environments (nodemcuv2_160)
- GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
- GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
- GitHub Check: wled_build / Test cdata.js
🔇 Additional comments (4)
wled00/wled.h (1)
421-424:Details
✅ Verification successful
Default gamma correction updated from 2.8f to 2.2f
This change aligns the global gamma with the new palette adjustments. Ensure there are no residual references to the old default (2.8f) elsewhere.Run:
🏁 Script executed:
rg -n "2\.8f" .Length of output: 17
No residual ‘2.8f’ references found
I ranrg -n "2\.8f" .across the entire repo and confirmed zero matches. The gamma default update from 2.8f to 2.2f in wled00/wled.h is fully applied with no leftover occurrences.wled00/palettes.h (3)
15-671: Bulk gamma‐corrected gradient palettes updated
All c3g‐imported gradients have been data‐only adjusted to match the new default gamma. PROGMEM usage and formatting remain correct.
672-694: New FastLED palettes added with inverse gamma correction
ThePartyColors_gc22,RainbowColors_gc22, andRainbowStripeColors_gc22definitions correctly contain 16 entries each. Values appear consistent with an inverse gamma of 2.2.
696-704:Details
❓ Verification inconclusive
fastledPalettes array updated to reference new palettes
Indices for Party, Rainbow, and Rainbow Bands now point to the_gc22variants. Verify external code or UI mappings relying on these indices still align.Run:
🏁 Script executed:
rg -n "fastledPalettes" -n .Length of output: 274
🏁 Script executed:
#!/bin/bash # Inspect JSON palette handling around line 982 sed -n '950,1020p' wled00/json.cpp # Inspect FX function palette assignment around line 251 sed -n '240,310p' wled00/FX_fcn.cppLength of output: 5041
Verify updated fastledPalettes indices against UI mappings
Indices for Party (6), Rainbow (11), and Rainbow Bands (12) now point to their_gc22variants. Confirm that any external code or UI reading palettes by index (6–12) still matches these changes, particularly in:
- wled00/json.cpp – default branch handling
i < 13→setPaletteColors(curPalette, *fastledPalettes[i-6])- wled00/FX_fcn.cpp – progmem palettes branch
pal < 13→targetPalette = *fastledPalettes[pal-6]
willmmiles
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
|
I'm fine with that though I'm also fine with palettes prior to update. |
With the new and proper implementation of gamma correction for colors, the palettes imported from http://seaviewsensing.com/pub/cpt-city without any correction by @blazoncek have a slightly different, washed-out look because all three color channels are corrected with the same gamma.
Originally, the palettes were pre-corrected with gammas of 2.6 for red, 2.2 for green and 2.5 for blue.
In order to match the originals after applying the new default gamma of 2.2 the red and blue channels need a slight pre-correction of 2.6/2.2=1.182 for red and 2.5/2.2=1.136 for blue.
The palettes used from FastLed require an inverse-correction of 2.2 to get a close match to the original looks. I also tried with 1.8 and 1.5 to get something "in between" which also looks quite ok but slightly different. Without any inverse-correction, they do not look good, especially party-colors and rainbow colors.
FastLed palettes that use HTML colors (clouds, lava, ocean, forest) I left untouched as I think they do look slightly better than the originals, up for discussion.
Also: changed the default value for gamma correction to 2.2.
Summary by CodeRabbit
New Features
Improvements
Documentation