jackw01 / led-control

Advanced WS2812/SK6812 RGB/RGBW LED controller with on-the-fly Python animation programming, web code editor/control interface, 1D, 2D, and 3D display support, and E1.31 sACN support
https://jackw01.github.io/led-control/
MIT License
162 stars 35 forks source link

prev_state is not reinitialized to 0 upon pattern change... when is the first animation frame? #19

Closed ohthehugemanatee closed 2 years ago

ohthehugemanatee commented 2 years ago

For example, create a pattern like this, and hit "Compile pattern".

def pattern(t, dt, x, y, z, prev_state):
    val = prev_state[2]
    if prev_state[2] < 1:
        val = prev_state[2] + 0.001
    return (1,1,val), hsv

If lights are V=0, this will slowly bring them up to maximum brightness.

Then hit "Compile pattern" again, and nothing will change. Try switching to a pattern with variable V, like blackbody cycle 1d, and then jump right to this newly defined pattern. The initial pattern of V values will stay in place.

This is not what I expected from the README. prev_state should be "Initialized to (0, 0, 0) on the first animation frame."

Perhaps the real clarification needed is, when is the first animation frame? Is it at program start, or at pattern start? The former makes sense for smooth transitions, but makes t values hard to work with and breaks isolation for prev_state. The latter keeps pattern functions isolated, but makes for uglier transitions.

Proposal: you could have a checkbox/api parameter for "continuous animation" which controls this.

jackw01 commented 2 years ago

Changed the default behavior to reset both time and state when the animation is changed or recompiled since this makes more sense, clarified the README, and added a command-line flag to revert back to the original behavior.

ohthehugemanatee commented 2 years ago

Thank you for the amazingly fast response.

May I suggest: the desire to transition smoothly between two animations is a property of the transition, or maybe of the pattern. It's not a global property of the whole execution lifetime. There's a reasonable use case for both (and a mixture of both, which is my own case).

Consider a pattern like "fade to off," or "fade in from end to end". There's no way to implement these if t and prev_state are linked like this.

If it's globally set to blackout at the first cycle, I could check for t==1 and set the color... but I have no access to the previous state, so I don't know what that color is. If it's globally set to preserve prev_state and t, I can't determine time from start of animation to do that progressive fade in.

But if the t value resets and prev_state is preserved, then I have options. I can manually set a blackout at beginning, or just inherit. And I always know my time from animation start.

I suggest that t should always reset at start of pattern/compile (after all, what's the value of a cycle count from execution start?), and the global option should only control if prev_state is zeroed out. In fact the global option could disappear, since the pattern code can manually zero it out.