[Bug] Calling play() then cancel() on an animation has different results in chrome and in safari

[xaviergonz]

1. Describe the bug

If an animation calls play() and then, at a later time, calls cancel() it has different results in Safari and Chrome:

• In chrome it will reset the animation to the original frame
• In safari it will reset the animation to the original frame for a single frame, then apply the last played frame styles.

Note that it doesn't matter where play() is called (might even be at the very beginning of the animation). However, if play() is not called then them both behave like in Chrome.

Also note that setting allowWebkitAcceleration to true seems to have no effect.

2. IMPORTANT: Provide a CodeSandbox reproduction of the bug

https://codesandbox.io/s/agitated-shtern-5ktj6v?file=/src/App2.tsx

3. Steps to reproduce

Open the sandbox with Chrome and Safari and see how in Chrome the box ends unrotated and in safari it ends rotated.

4. Expected behavior

I expect them both to behave in the same way.

6. Browser details

Latest Chrome and Safari in OSX 12.4

[xaviergonz]

I just checked using the web animation API in safari and in this case it works ok, so it must be a bug with the polyfill.
What I don't understand is how it still fails then when using allowWebkitAcceleration: true, I thought doing that the polyfill was not used?

[mattgperry]

The polyfill is only there to animate individual transforms outside of Chrome, so it’s never used in other cases. allowWebkitAccelerarion is for running WAAPI on the cpu vs the gpu in Safari for all other values

[xaviergonz]

A total wild guess, but maybe it is because this line

motionone/packages/animation/src/Animation.ts

Line 169 in d6306f3

requestAnimationFrame(this.tick)

should assign the returned value to this.frameRequestId?

or maybe it should even clear this.frameRequestId every time that it is not enqueued (at the beginning of this.tick), set it to undefined when it is cleared (in cancel) and not re-enqueue it if it is already enqueued (in play)

[xaviergonz]

Since I can't make PRs due to fork restrictions... (untested)

import type {
  AnimationControls,
  AnimationOptions,
  Easing,
} from "@motionone/types"
import {
  isEasingGenerator,
  isEasingList,
  defaults,
  noopReturn,
} from "@motionone/utils"
import { getEasingFunction } from "./utils/easing"
import { interpolate as createInterpolate } from "./utils/interpolate"

export class Animation implements Omit<AnimationControls, "stop" | "duration"> {
  private resolve?: (value: any) => void

  private reject?: (value: any) => void

  startTime: number | null = null

  private pauseTime: number | undefined

  private rate = 1

  private tick: (t: number) => void

  private t = 0

  private cancelTimestamp: number | null = null

  private frameRequestId?: number

  playState: AnimationPlayState = "idle"

  constructor(
    output: (v: number) => void,
    keyframes: number[] = [0, 1],
    {
      easing = defaults.easing as Easing,
      duration = defaults.duration,
      delay = defaults.delay,
      endDelay = defaults.endDelay,
      repeat = defaults.repeat,
      offset,
      direction = "normal",
    }: AnimationOptions = {}
  ) {
    if (isEasingGenerator(easing)) {
      const custom = easing.createAnimation(keyframes, () => "0", true)
      easing = custom.easing
      if (custom.keyframes !== undefined) keyframes = custom.keyframes
      if (custom.duration !== undefined) duration = custom.duration
    }

    const animationEasing = isEasingList(easing)
      ? noopReturn
      : getEasingFunction(easing)

    const totalDuration = duration * (repeat + 1)

    const interpolate = createInterpolate(
      keyframes,
      offset,
      isEasingList(easing) ? easing.map(getEasingFunction) : noopReturn
    )

    this.tick = (timestamp: number) => {
      this.frameRequestId = undefined;

      // TODO: Temporary fix for OptionsResolver typing
      delay = delay as number

      if (this.pauseTime) timestamp = this.pauseTime

      let t = (timestamp - this.startTime!) * this.rate

      this.t = t

      // Convert to seconds
      t /= 1000

      // Rebase on delay
      t = Math.max(t - delay, 0)

      /**
       * If this animation has finished, set the current time
       * to the total duration.
       */
      if (this.playState === "finished") t = totalDuration

      /**
       * Get the current progress (0-1) of the animation. If t is >
       * than duration we'll get values like 2.5 (midway through the
       * third iteration)
       */
      const progress = t / duration

      // TODO progress += iterationStart

      /**
       * Get the current iteration (0 indexed). For instance the floor of
       * 2.5 is 2.
       */
      let currentIteration = Math.floor(progress)

      /**
       * Get the current progress of the iteration by taking the remainder
       * so 2.5 is 0.5 through iteration 2
       */
      let iterationProgress = progress % 1.0

      if (!iterationProgress && progress >= 1) {
        iterationProgress = 1
      }

      /**
       * If iteration progress is 1 we count that as the end
       * of the previous iteration.
       */
      iterationProgress === 1 && currentIteration--

      /**
       * Reverse progress if we're not running in "normal" direction
       */
      const iterationIsOdd = currentIteration % 2
      if (
        direction === "reverse" ||
        (direction === "alternate" && iterationIsOdd) ||
        (direction === "alternate-reverse" && !iterationIsOdd)
      ) {
        iterationProgress = 1 - iterationProgress
      }

      const p = t >= totalDuration ? 1 : Math.min(iterationProgress, 1)
      const latest = interpolate(animationEasing(p))

      output(latest)

      const isAnimationFinished =
        this.playState === "finished" || t >= totalDuration + endDelay

      if (isAnimationFinished) {
        this.playState = "finished"
        this.resolve?.(latest)
      } else if (this.playState !== "idle") {
        this.frameRequestId = requestAnimationFrame(this.tick)
      }
    }

    this.play()
  }

  finished = new Promise((resolve, reject) => {
    this.resolve = resolve
    this.reject = reject
  })

  play() {
    const now = performance.now()
    this.playState = "running"

    if (this.pauseTime) {
      this.startTime = now - (this.pauseTime - (this.startTime ?? 0))
    } else if (!this.startTime) {
      this.startTime = now
    }

    this.cancelTimestamp = this.startTime
    this.pauseTime = undefined
    if (this.frameRequestId === undefined) {
      this.frameRequestId = requestAnimationFrame(this.tick)
    }
  }

  pause() {
    this.playState = "paused"
    this.pauseTime = performance.now()
  }

  finish() {
    this.playState = "finished"
    this.tick(0)
  }

  stop() {
    this.playState = "idle"

    if (this.frameRequestId !== undefined) {
      cancelAnimationFrame(this.frameRequestId)
      this.frameRequestId = undefined;
    }

    this.reject?.(false)
  }

  cancel() {
    this.stop()
    this.tick(this.cancelTimestamp!)
  }

  reverse() {
    this.rate *= -1
  }

  commitStyles() {}

  get currentTime() {
    return this.t
  }

  set currentTime(t: number) {
    if (this.pauseTime || this.rate === 0) {
      this.pauseTime = t
    } else {
      this.startTime = performance.now() - t / this.rate
    }
  }

  get playbackRate() {
    return this.rate
  }

  set playbackRate(rate) {
    this.rate = rate
  }
}

[xaviergonz]

@mattgperry I can confirm the fix above works

[xaviergonz]

Basically what I did was to ensure that requestAnimationFrame is not called again when it is already scheduled to run so it doesn't double run.

[mattgperry]

Thanks for the fix! It'll go out in 10.12.0 hopefully in the next few days.

[xaviergonz]

I see that you applied the missing assignation, but not setting it to undefined when it is cleared and when the tick begins. is that on purpose or because you didn't notice?

[mattgperry]

Ah yeah this was on purpose, all the ids are unique so once it’s cleared it doesn’t really matter if we don’t delete it’s reference

[xaviergonz]

I think you could still queue two ticks if you call play() twice in a row if the requestAnimationFrame is not protected by a check through an if though (and that'd make the first tick un-cancelable since the id would be lost)

[xaviergonz]

Here's the code adapted to the latest version of the file

import type {
  AnimationControls,
  AnimationOptions,
  EasingFunction,
} from "@motionone/types";
import {
  isEasingGenerator,
  isEasingList,
  defaults,
  noopReturn,
  interpolate as createInterpolate,
} from "@motionone/utils";
import { getEasingFunction } from "./utils/easing";

export class Animation implements Omit<AnimationControls, "stop" | "duration"> {
  private resolve?: (value: any) => void;

  private reject?: (value: any) => void;

  startTime: number | null = null;

  private pauseTime: number | undefined;

  private rate = 1;

  private tick: (t: number) => void;

  private t = 0;

  private cancelTimestamp: number | null = null;

  private frameRequestId?: number;

  private easing: EasingFunction = noopReturn;

  private duration: number = 0;

  private totalDuration: number = 0;

  private repeat: number = 0;

  playState: AnimationPlayState = "idle";

  constructor(
    output: (v: number) => void,
    keyframes: number[] = [0, 1],
    {
      easing,
      duration: initialDuration = defaults.duration,
      delay = defaults.delay,
      endDelay = defaults.endDelay,
      repeat = defaults.repeat,
      offset,
      direction = "normal",
    }: AnimationOptions = {}
  ) {
    easing = easing || defaults.easing;

    if (isEasingGenerator(easing)) {
      const custom = easing.createAnimation(keyframes, () => "0", true);
      easing = custom.easing;
      if (custom.keyframes !== undefined) keyframes = custom.keyframes;
      if (custom.duration !== undefined) initialDuration = custom.duration;
    }

    this.repeat = repeat;

    this.easing = isEasingList(easing) ? noopReturn : getEasingFunction(easing);

    this.updateDuration(initialDuration);

    const interpolate = createInterpolate(
      keyframes,
      offset,
      isEasingList(easing) ? easing.map(getEasingFunction) : noopReturn
    );

    this.tick = (timestamp: number) => {
      this.frameRequestId = undefined;

      // TODO: Temporary fix for OptionsResolver typing
      delay = delay as number;

      let t = 0;
      if (this.pauseTime !== undefined) {
        t = this.pauseTime;
      } else {
        t = (timestamp - this.startTime!) * this.rate;
      }

      this.t = t;

      // Convert to seconds
      t /= 1000;

      // Rebase on delay
      t = Math.max(t - delay, 0);

      /**
       * If this animation has finished, set the current time
       * to the total duration.
       */
      if (this.playState === "finished" && this.pauseTime === undefined) {
        t = this.totalDuration;
      }

      /**
       * Get the current progress (0-1) of the animation. If t is >
       * than duration we'll get values like 2.5 (midway through the
       * third iteration)
       */
      const progress = t / this.duration;

      // TODO progress += iterationStart

      /**
       * Get the current iteration (0 indexed). For instance the floor of
       * 2.5 is 2.
       */
      let currentIteration = Math.floor(progress);

      /**
       * Get the current progress of the iteration by taking the remainder
       * so 2.5 is 0.5 through iteration 2
       */
      let iterationProgress = progress % 1.0;

      if (!iterationProgress && progress >= 1) {
        iterationProgress = 1;
      }

      /**
       * If iteration progress is 1 we count that as the end
       * of the previous iteration.
       */
      iterationProgress === 1 && currentIteration--;

      /**
       * Reverse progress if we're not running in "normal" direction
       */
      const iterationIsOdd = currentIteration % 2;
      if (
        direction === "reverse" ||
        (direction === "alternate" && iterationIsOdd) ||
        (direction === "alternate-reverse" && !iterationIsOdd)
      ) {
        iterationProgress = 1 - iterationProgress;
      }

      const p = t >= this.totalDuration ? 1 : Math.min(iterationProgress, 1);
      const latest = interpolate(this.easing(p));

      output(latest);

      const isAnimationFinished =
        this.pauseTime === undefined &&
        (this.playState === "finished" || t >= this.totalDuration + endDelay);

      if (isAnimationFinished) {
        this.playState = "finished";
        this.resolve?.(latest);
      } else if (this.playState !== "idle") {
        this.requestAnimationFrame();
      }
    };

    this.play();
  }

  finished = new Promise((resolve, reject) => {
    this.resolve = resolve;
    this.reject = reject;
  });

  play() {
    const now = performance.now();
    this.playState = "running";

    if (this.pauseTime !== undefined) {
      this.startTime = now - this.pauseTime;
    } else if (!this.startTime) {
      this.startTime = now;
    }

    this.cancelTimestamp = this.startTime;
    this.pauseTime = undefined;
    this.requestAnimationFrame();
  }

  pause() {
    this.playState = "paused";
    this.pauseTime = this.t;
  }

  finish() {
    this.playState = "finished";
    this.tick(0);
  }

  stop() {
    this.playState = "idle";

    this.cancelAnimationFrame();

    this.reject?.(false);
  }

  cancel() {
    this.stop();
    this.tick(this.cancelTimestamp!);
  }

  reverse() {
    this.rate *= -1;
  }

  commitStyles() {}

  private updateDuration(duration: number) {
    this.duration = duration;
    this.totalDuration = duration * (this.repeat + 1);
  }

  get currentTime() {
    return this.t;
  }

  set currentTime(t: number) {
    if (this.pauseTime !== undefined || this.rate === 0) {
      this.pauseTime = t;
    } else {
      this.startTime = performance.now() - t / this.rate;
    }
  }

  get playbackRate() {
    return this.rate;
  }

  set playbackRate(rate) {
    this.rate = rate;
  }

  private requestAnimationFrame() {
    if (this.frameRequestId === undefined) {
      this.frameRequestId = requestAnimationFrame(this.tick);
    }
  }

  private cancelAnimationFrame() {
    if (this.frameRequestId !== undefined) {
      cancelAnimationFrame(this.frameRequestId);
      this.frameRequestId = undefined;
    }
  }
}

basically the class methods requestAnimationFrame and cancelAnimationFrame ensure you cannot double request an animation frame

[xaviergonz]

And here's the diff
https://www.diffchecker.com/6w8ylKos