Fix whole-segment blackout frame at the start of on/off transitions with non-fade blending styles#5726
Fix whole-segment blackout frame at the start of on/off transitions with non-fade blending styles#5726npabisz wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughModifies the pixel-blanking condition inside WS2812FX::blendSegment() so that pixels are only set to BLACK during On/Off transitions once the segment transition has actually started (isInTransition() is true), applied identically to both the 2D matrix and 1D slow-path blending loops. ChangesTransition Blanking Fix
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
If the root cause is correct, would it be sufficient to change the order? Like |
Unfortunately a reorder alone can't fully close the race, for two reasons:
The blanking workaround in That said, moving |
|
good catch, strange this went unnoticed for so long. I checked and the bug was introduced with the fix for #5206 before that commit it worked (but brightness change suffered from the same issue) I confirmed the fix is correct and working, thanks! |
Symptom
With a transition time set and any blending style other than Fade, toggling power off causes a visible flick: the whole segment goes black for a single frame, jumps back to full brightness, and only then the transition animation plays. Toggling on has the same defect, but it is invisible in practice (the LEDs are still dark at that moment).
Reproducible on v16.0.1 and current
main(ESP32, 313× WS2814, styles tested: Inside-out, Outside-in; transition 1.5–5 s). Not reproducible with Fade.Root cause
A race between the request context and the render loop.
stateUpdated()(led.cpp) setsbri = 0first and only callsstrip.setTransitionMode(true)further down - afternotify()/sendSysInfoUDP(), which can take several milliseconds of WiFi I/O. On ESP32 the render loop runs on a different task, so a frame can be serviced inside that window.In that window the on/off workaround in
blendSegment():already sees
bri != briT(power change pending), but no segment transition exists yet, soisPixelClipped()returnsfalsefor every pixel. As a result the!clipped && !briarm blanks every pixel of the segment for that frame. One frame later the transition has started and the animation begins from full brightness - hence the flick.Fix
Gate the blackout workaround on the segment transition actually having started (
topSegment.isInTransition()), in both the 1D and 2D paths. Outside the race window the condition is unchanged: in steady statebri == briT(andisPixelClipped()needs an active transition anyway), and at the end of the transitionapplyFinalBri()makesbri == briT, so behavior there is identical.Testing
Summary by CodeRabbit