Fix: #8194 by using a stepped animation for the wiggle

[RoboErikG]

The basics

• I validated my changes

The details

Resolves

Fixes #8194

Proposed Changes

Instead of calculating a skew from the elapsed time, step through an array of animation values
and change the update period to 15ms. This creates a more consistent animation even if frames
are misses.

Reason for Changes

The wiggle animation could end up calculating a skew of 0 a lot of the time. If there were also
some skipped render frames this could lead to the wiggle being much smaller than intended.
By stepping through pre-calculated animation steps we can guarantee more frames will include
a non-zero value making it much less likely to have no apparent wiggle.

Updating every 15ms better aligns with the 60 fps limit (16.67ms/frame) making it unlikely we'll
double update between frames.

Test Coverage

N/A

Documentation

No

Additional Information

[BenHenning]

Thanks @RoboErikG! I have a few follow-up questions:

1. Might you have a before and after video that could be attached to the PR comment to demonstrate the previous (broken) and current (fixed) behaviors? I think this would be nice to help contextualize the fix. One potential issue that I see is the lack of smooth interpolation between the different states now that the implementation is using a fixed cycle table and longer delays between updates, but over a 200ms duration I'm guessing this won't be easily noticeable.
2. Do you have a sense as to why skew was being calculated at 0? I don't see any issue with the new implementation other than what I mentioned above, but a skewX of 0 seems like something's broken with the old implementation. I think it's important to clarify both what and why the previous implementation is broken so that it can be better understood why the new solution is preferred.

[RoboErikG]

1)Yep, here's the before and after videos:

• wiggle before.webm
• wiggle after.webm

In theory, there should be very little difference between the two approaches. The original code was targeting 3 half cycles. Given JS render updates are usually limited to 60hz (16.67ms/frame) that gives you about 4 frames per half cycle over 200ms, which is also what you'd see for my change in the ideal case. Using 0.66 as the intermediate value approximates a sin curve in that context (you spend more time near the top and bottom of the curve and less time near 0).

JS won't refresh faster than your monitor's refresh rate (typically 60hz). There are monitors with faster refresh rates, but they're multiples of 60 (120Hz, 240Hz) and human eyes can't see that fast. Because of this, updating faster than 60hz isn't really useful and can cause double updates in between paints, hence the longer 15ms steps.

2)In practice, the magnitude is pretty small. A single If block has a magnitude of ~8.5 and three if blocks have a magnitude of ~3. If you're a little bit unlucky with the period of the sin that gets rounded to 1 pretty often. The calculation that was being used also scaled the wiggle down over the duration because it was trying to model a dampened sinusoid, which is what a real object would do. However, that meant that if the first couple frames are missed (because it takes extra time to calculate things after dragging out a block) you only get the tail end of the animation, which can easily get rounded down to 1 or 0 for larger stacks and make the wiggle not noticeable.

Given all of that, this trades off a theoretically more realistic wiggle for a more consistent wiggle. Some other options for trade-offs would be:

• Increase the magnitude of the wiggle. This makes it less likely to be zero, but looks weird at the start.
• Increase the duration of the wiggle. This would give more frames, but could get distracting if it's too long.