Fixes issue in Dissolve when a palette has same color pixel as the current background color when Complete is enabled#5453
Conversation
…rrent background color when Complete mode is enabled
WalkthroughAdded an early bailout in dissolve() when segment length is <1 and adjusted dissolve-to-primary/palette spawning so the computed spawn color is stored in a temporary Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wled00/FX.cpp`:
- Around line 706-708: The workaround that flips the low bit of c (c ^=
0x00000001) should only run in Complete mode; change the condition to only
modify c when it equals SEGCOLOR(1) AND the segment is in Complete mode (use
SEGMENT.check2 as the flag referenced in this logic). In other words, leave the
palette/fill selection using SEGCOLOR(0) and SEGMENT.color_from_palette(...)
unchanged, but replace the unconditional "if (c == SEGCOLOR(1)) c ^=
0x00000001;" with a guarded check "if (c == SEGCOLOR(1) && SEGMENT.check2) ..."
so identical fg/bg cases remain a true no-op when not in Complete mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
good find, I overlooked this situation when making the update. |
|
@coderabbitai Please add the Effect label to this PR. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 269
🧠 Learnings used |
|
@coderabbitai I do not have permissions to add labels, so I do not have the Labels gear icon on this PR page. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainNo problem! Let me try to add the label directly for you. 🏁 Script executed: Length of output: 266
Since you don't have label permissions and I'm restricted as well, the best path is to ask a repository maintainer to add the "effect" label. I can see that @DedeHai has already commented on your PR — you could ask them (or another WLED maintainer) to apply the label for you. 🧠 Learnings used |
When you select the Dissolve effect, select a palette, and enable the Complete checkbox, the effect should fill in the LEDs with the colors from the palette. Then, after all of them have been filled in ("completed"), it will wait the normal delay time and then dissolve all of them. But if the selected palette contains the same color as you have selected as the background color (SEGCOLOR(1)), then the Complete process will never be able to complete, so it looks like the effect is hung up or paused. Example palettes that cause this issue are the Fire and Ice Fire palettes if you have black selected for your background color since those palettes contain black, CRGB(0,0,0).
Summary by CodeRabbit