Bug 140164 – Animation is stopped not at the destination position when CPU is low and busy

• Component: Animations
• Status: RESOLVED WONTFIX

[Mark Wang]

If -webkit-animation-iteration-count of an animation is not infinite, the animation should stop at the destination. But if CPU is low and very busy, the animation could stop in the middle of the origination and the destination.

The problem is in normalizedAnimationValue() of GraphicsLayerAnimation.cpp:
static double normalizedAnimationValue(double runningTime, double duration, Animation::AnimationDirection direction, double iterationCount)
{
    if (!duration)
        return 0;

    const int loopCount = runningTime / duration;
    const double lastFullLoop = duration * double(loopCount);
    const double remainder = runningTime - lastFullLoop;
    // Ignore remainder when we've reached the end of animation.
    const double normalized = (loopCount == iterationCount) ? 1.0 : (remainder / duration);

    return shouldReverseAnimationValue(direction, loopCount) ? 1 - normalized : normalized;
} 

if CPU is busy and low, loopCount could be bigger than iterationCount, for example iterationCount is 1.0 and loopCount is 2. For such case (iterationCount is 1.0 and loopCount is 2), normalized  should be 1.0, not remainder/duration which causes that animations stop in the middle of the origination and the destination. 

So the possible fix is:
    const double normalized = (loopCount >= iterationCount && iterationCount > 0) ? 1.0 : (remainder / duration);

[Simon Fraser (smfr)]

Can you provide a testcase that shows the bug (when the CPU is loaded).

[Alexey Proskuryakov]

Created attachment 244226 [details]
for EWS

I don't know this code; just posting as a patch to let EWS run regression tests with it.

[Mark Wang]

The tastcase can be any animation whose iteration_count is 1, but it must be run in the special environment of the low and busy CPU, like embedded system.

In order to reproduce this issue, you can manually change runningTime in normalizedAnimationValue() to be longer. 

For example:
static double normalizedAnimationValue(double runningTime, double duration, Animation::AnimationDirection direction, double iterationCount)
{
    if (!duration)
        return 0;

// changing runningTime to be longer manually. 
#if 1
    if (runningTime > 0.7* duration)
        runningTime = 2.7 * duration;
#endif
    
    const int loopCount = runningTime / duration;
    const double lastFullLoop = duration * double(loopCount);
    const double remainder = runningTime - lastFullLoop;
    // Ignore remainder when we've reached the end of animation.
    const double normalized = (loopCount == iterationCount) ? 1.0 : (remainder / duration);

    return shouldReverseAnimationValue(direction, loopCount) ? 1 - normalized : normalized;
}

Theortically, runningTime could be any value, which depends on the CPU and its running state, and the last frame of an animation must be drawn at the destination position, but the above case draws the last frame at the wrong position if animation's iteration_count is 1. 

Alexey Proskuryakov's attachment#244226 [details] can fix this issue.

[Dean Jackson]

As far as I can tell, this function is only used in the CoordinatedGraphics ports. They should probably review it, but I'll check it if applies to the base port.

[Dean Jackson]

I don't think we need this change in AnimationBase::progress.

In that, we first calculate an overall running time that is the iteration count times the duration. If the elapsed time is above that, then we return 0 or 1 depending on the direction and which iteration it is.

So, even if (speaking in the terms of the function above) runningTime was 2, we'd immediately return 1.0 for progress. We never do the division to check loopCount.

Now, this fix might still make sense for the ports that use GraphicsLayerAnimation, but those people should review it.

[Antoine Quint]

We don't have a test case and this code has been entirely re-written since then as a result of the Web Animations work. Marking as WONTFIX, but feel free to reopen with a test case if you believe the issue is still current.