Re: [csswg-drafts] [web-animations-1] Make updating the playback rate asynchronous

The more I work on this, the more I think it's not a good idea for setting a member to trigger an async operation like this.

There are a few reasons but perhaps the most concrete issue is that by doing that we would provide no obvious way to achieve synchronous behavior for applications that require it.

Consider the asynchronous operations.

**1. Playing**

Async play:
```js
anim.play();
```

Sync play (i.e. start on this main thread frame):
```js
anim.startTime = document.timeline.currentTime;
```

**2. Pausing**

Async pause:
```js
anim.pause();
```

Sync pause (i.e. pause at this main thread frame):
```js
anim.startTime = null;
```

**3. Changing the playback rate**

Currently we only have:
```js
anim.playbackRate = 2.0;
```
which we are trying to make asynchronously update timing.

If you want to synchronously update the playback rate with a compensatory seek you would need to do something like:

```js
// Record current time before we update startTime
const previousCurrentTime = anim.currentTime;
// Pause first since updating the playbackRate can be done synchronously when paused
anim.startTime = null;
anim.playbackRate = 2.0;
// Do compensatory seek
anim.startTime = (document.timeline.currentTime - previousCurrentTime) / 2.0;
```

At a glance it *seems* like if we just define setting the `currentTime` to cancel the async compensatory seek, then the following would work:

```js
const previousCurrentTime = anim.currentTime;
anim.playbackRate = 2.0;
// Define the following to cancel the async playback rate change
anim.currentTime = previousCurrentTime;
```

However, that's not the case because in the above example `anim` would be paused since setting the playbackRate causes the `startTime` to become unresolved (and there would be no async operation to resolve the `startTime`, unless we make setting the `currentTime` here _also_ resolve the `startTime` in this case, which seems at least a little odd).

I've checked what Chrome does in this case, and it [doesn't appear to cancel the async update](https://jsbin.com/fudihununa/edit?html,js,console,output). That is, there's no obvious way to update the `playbackRate` in a synchronous way (except perhaps the pause-first approach, which I haven't tried).

Updating the `playbackRate` without doing a seek is not too bad though:

```js
const previousStartTime = anim.startTime;
anim.playbackRate = 2.0;
// Define the following to cancel the async playback rate change
anim.startTime = previousStartTime;
```

**Preferred solution**

I think the way this interface *should* work is:

```webidl
partial interface Animation {
   // setting synchronously updates the playbackRate with NO compensatory seek
   attribute double playbackRate;
   // asynchronously updates the playbackRate and does a compensatory seek
   void updatePlaybackRate(double playbackRate);
}
```

Async playback rate change:
```js
anim.updatePlaybackRate(2.0);
```

Sync playback rate change:
```js
anim.playbackRate = 2.0;
```

Or, sync playback rate change with compensatory seek:
```js
const previousCurrentTime = anim.currentTime;
anim.playbackRate = 2.0;
anim.currentTime = previousCurrentTime;
```

Doing this would mean you have a clear distinction where:
* Setting members is always synchronous. The model is updated immediately based on main-thread state even if that means it causes the animation to jump.
* Methods form an asynchronous layer that update members at different times with appropriate values.

If we introduce the API changes suggested in #196, it would mean that `isPending` is only ever set to `true` by calling methods; never by setting members.

I'm dubious, however, that we can drop the compensatory seek arising from setting `playbackRate` without breaking content. I'd be very very glad to be proven wrong, however.

**Realistic solution**

Assuming we can't drop the compensatory seek behavior from `playbackRate`, then I think we should make this interface as follows:

```webidl
partial interface Animation {
   // setting synchronously updates the playbackRate with compensatory seek based
   // on main thread state (currently specified behavior)
   attribute double playbackRate;
   // asynchronously updates the playbackRate and does a compensatory seek
   void updatePlaybackRate(double playbackRate);
}
```

Async playback rate change:
```js
anim.updatePlaybackRate(2.0);
```

Sync playback rate change with compensatory seek:
```js
anim.playbackRate = 2.0;
```

Sync playback rate change without compensatory seek:
```js
const previousStartTime = anim.startTime;
anim.playbackRate = 2.0;
anim.startTime = previousStartTime;
```

The downside of this is that some authors will never notice `updatePlaybackRate` and just set `playbackRate` even when they probably want the async behavior.

@alancutter what do you think?

-- 
GitHub Notification of comment by birtles
Please view or discuss this issue at https://github.com/w3c/csswg-drafts/issues/2059#issuecomment-349193530 using your GitHub account

Received on Tuesday, 5 December 2017 04:41:01 UTC