Fix switch change animation occurring instantly with interval 0

[ForLoveOfCats]

This fixes an issue introduced by #1145

If the animation frame interval is zero the animation would not take place and it would snap to the new data at the next paint. There was like five different ways to fix this but I went with what seemed the simplest and most straightforward.

The reason this wasn't spotted when I was writing the original patch was because I was testing with the widget_gallery example which contains a spinner which ensured that the animation frames never stopped thus the switch never received an interval of zero except for the first one when the spinner started.

Getting an interval of zero makes sense logically but it can very easily cause some surprising bugs. I'm curious if it would be reasonable to skip sending the first animation frame allowing all intervals to be greater than 0.

[raphlinus]

Interesting. Longer term we'll want to factor some of this logic out into a reusable piece, with more capabilities such as easing curves. Also, I can imagine some more careful filtering at the platform level, but it will require more careful thought. I believe an interval of 0 means "animation is restarting after a period of idle", which seems like a potentially useful signal for a widget. It's possible this should be some other value than 0, for example an estimate of the likely frame-to-frame delay (~16ms), but we should work from principles about what will produce the highest quality results and best implementation factoring.

[ForLoveOfCats]

I had a thought, why not always track the frame to frame delay instead of only when there are animation frames running (perhaps this is already happening, I only have a high level understanding of this mechanism) and then the first animation frame can get the correct interval just like any other frame.

I also realized that there is a far better way to fix this specific case

[raphlinus]

I had a thought, why not always track the frame to frame delay instead of only when there are animation frames running

I actually had this in an earlier prototype. The problem is that a multi-second interval is likely to throw off animation logic, for example causing it to complete instantly. It's important to convey the difference between animation starting and continuously running. And I think the exact interval from the last time animation was running is rarely useful.

[ForLoveOfCats]

I don't mean time since the last animation frame but the time since the last frame period. That way the first animation frame gets a valid and accurate interval to work with.

I've force pushed the new fix, it is much simpler and more correct

[raphlinus]

I think I don't understand, could you clarify exactly what the "last frame period" means. Is that the display refresh even when the app is not presenting? I can see the value in that, but it requires fairly deep platform introspection into the compositor.

[ForLoveOfCats]

I'll have to look at the internals a bit to be able to enunciate it in more technical terms (as what you said implies more internal complexity than a simple refresh rate) but if we are counting the interval since the last animation frame then even when there are no animation frames running it could continue to run as though there were so when the first new animation frame triggers it can pass down the same interval it would have if there had been an animation frame just prior.

I'd like to get a review on the new smaller patch and this discussion can be moved over to an issue

[raphlinus]

Ah yes, I like this patch, it feels more robust. And yes, issues to continue discussion. I think there are (at least) two tracks. One is higher-level reusable animation components, and the other is potential changes at the druid-shell level to provide more useful or accurate animation timing.

[ForLoveOfCats]

I'll let you open the discussion issue for higher level animation utilities as that is a fair bit outside my current body of knowledge