Skip to content

Fix whole-segment blackout frame at the start of on/off transitions with non-fade blending styles#5726

Open
npabisz wants to merge 1 commit into
wled:mainfrom
npabisz:fix/onoff-transition-flick
Open

Fix whole-segment blackout frame at the start of on/off transitions with non-fade blending styles#5726
npabisz wants to merge 1 commit into
wled:mainfrom
npabisz:fix/onoff-transition-flick

Conversation

@npabisz

@npabisz npabisz commented Jul 9, 2026

Copy link
Copy Markdown

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) sets bri = 0 first and only calls strip.setTransitionMode(true) further down - after notify() / 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():

if ((briOld == 0 || bri == 0) && ((!clipped && (bri != briT) && !bri) || (clipped && (bri != briT) && bri))) c_a = BLACK;

already sees bri != briT (power change pending), but no segment transition exists yet, so isPixelClipped() returns false for every pixel. As a result the !clipped && !bri arm 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 state bri == briT (and isPixelClipped() needs an active transition anyway), and at the end of the transition applyFinalBri() makes bri == briT, so behavior there is identical.

Testing

  • ESP32 (esp32dev), 313× WS2814, single segment, transition 1.5–5 s.
  • Before: dark flash right after turning off (Inside-out / Outside-in / Swipe).
  • After: transition starts cleanly from the current frame; on/off, interrupted transitions and Fade behave as before.

Summary by CodeRabbit

  • Bug Fixes
    • Improved LED transition handling so pixels are no longer briefly blanked before a segment transition actually starts.
    • Applied the fix consistently across both matrix and standard blending paths, resulting in smoother on/off brightness changes.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1cfdb6e3-86fa-40eb-bed0-53e09a9e1468

📥 Commits

Reviewing files that changed from the base of the PR and between bc2c80d and 72c83b7.

📒 Files selected for processing (1)
  • wled00/FX_fcn.cpp

Walkthrough

Modifies 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.

Changes

Transition Blanking Fix

Layer / File(s) Summary
On/Off transition blanking timing
wled00/FX_fcn.cpp
Adds topSegment.isInTransition() as an additional required condition before setting pixel color to BLACK in both the matrix (2D) and 1D slow-path blending loops, preventing blanking before the transition actually begins.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • wled/WLED#4889: Both PRs modify transition/fade handling within the same blendSegment() function in wled00/FX_fcn.cpp.

Suggested labels: bug, effect

Suggested reviewers: softhack007, willmmiles

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 clearly summarizes the main fix: preventing a blackout frame during on/off transitions with non-fade blending styles.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

@softhack007

Copy link
Copy Markdown
Member

stateUpdated() (led.cpp) sets bri = 0 first and only calls strip.setTransitionMode(true)

If the root cause is correct, would it be sufficient to change the order? Like strip.setTransitionMode(true); bri = 0; ?

@npabisz

npabisz commented Jul 9, 2026

Copy link
Copy Markdown
Author

stateUpdated() (led.cpp) sets bri = 0 first and only calls strip.setTransitionMode(true)

If the root cause is correct, would it be sufficient to change the order? Like strip.setTransitionMode(true); bri = 0; ?

Unfortunately a reorder alone can't fully close the race, for two reasons:

  1. bri isn't actually written inside stateUpdated() (my description was imprecise there - sorry). It is already changed by the callers before stateUpdated() is even entered: toggleOnOff() / handleSet() for the HTTP API, deserializeState() for JSON, nightlight, IR, UDP notifications, etc. stateUpdated() then relies on bri != briOld to decide whether a transition should be started at all, so "start the transition first, then change bri" would have to be implemented at every call site (or via a pending-brightness indirection). That changes the contract of the whole state-update path and touches many more places than this fix.

  2. Even a perfect reorder only shrinks the window, it doesn't remove it: on ESP32 the render loop runs on a different task than the web/UDP handlers, so a frame can be serviced between any two instructions of the request handler. deserializeState() for example does quite a bit of work between writing bri and reaching stateUpdated().

The blanking workaround in blendSegment() is the one place that assumes "a power transition is pending (bri != briT), therefore the segment transition has already started". The guard just makes that assumption explicit, so it holds no matter which caller changed bri and how long it takes to reach startTransition().

That said, moving strip.setTransitionMode(true) earlier inside stateUpdated() - before notify()/sendSysInfoUDP(), which are the slowest part of the window - would be a reasonable additional hardening; on its own it would make the flash rarer, but not impossible.

@DedeHai

DedeHai commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

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!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants