[web-animations-1] Replace AnimationEffectTiming(ReadOnly) with setTiming() ?

[birtles]

From @birtles on September 14, 2016 7:47

Just a thought about how we could simplify the API and make it more consistent.

Currently, we manage timing properties using the following interfaces:

• AnimationEffectTimingProperties — property bag for passing to constructors etc.
• ComputedTimingProperties — extends AnimationEffectTimingProperties with some extra properties we compute
• AnimationEffectTimingReadOnly — Live object with getters for reading the specified timing properties. Fortunately, this happens to have the same members as AnimationEffectTimingProperties so you can pass it to constructors/methods that expect a AnimationEffectTimingProperties and it should work.
• AnimationEffectTiming — Subclass of AnimationEffectTimingReadOnly that adds getters for the different parameters (used by mutable effects, i.e. those not tied to CSS)

In practice it looks like this:

// Set up a new animation
const anim = elem.animate({...}, { duration: 2000, fill: 'auto' });

// Read back the specified values
anim.effect.timing.duration; // 2000
anim.effect.timing.fill; // 'auto'

// Read back computed values
anim.effect.getComputedTiming().duration; // 2000
anim.effect.getComputedTiming().fill; // 'none'
// NOTE: |fill| and |duration| are a little bit special in that they allow 'auto'
// values that get computed into something else. It's not particularly useful, though,
// until we have groups (where, e.g., a group's auto duration computes to the length
// of its children).

// Read other computed values
anim.effect.getComputedTiming().activeDuration // 2000
anim.effect.getComputedTiming().progress // 0.0

// Update specified values
anim.effect.timing.duration *= 2;
anim.effect.timing.iterations++;

The parallel here is with style:

• anim.effect.timing =~ elem.style (sort of, kind of)
• anim.effect.getComputedTiming =~ window.getComputedStyle(elem)

However, for keyframes we do something completely different. We don't have live keyframe objects since that would be hard and instead we just use getKeyframes() and setKeyframes().

In addition, keyframe offsets can be computed. The author can omit them and we'll fill them in. We don't just fill in the offset member with the computed value when we return keyframes using getKeyframes(), however, since if we did that you couldn't take the result of getKeyframes() and pass it to setKeyframes() on another effect without freezing the offsets at that point (and losing the ability to auto-distribute keyframes from then on). Instead, we return the computed offset as a separate member: computedOffset.

So my thought here was:

• Drop AnimationEffectTimingReadOnly.
• Drop AnimationEffectTiming.
• Drop effect.timing.
• Rename effect.getComputedTiming() to effect.getTiming().
• Extend ComputedTimingProperties to include computedDuration and computedFill.
• Add effect.setTiming() that takes an AnimationEffectTimingProperties (which ComputedTimingProperties extends so you could pass the result of getTiming() to setTiming()).

Advantages:

• 2 fewer interfaces
• Fewer live objects
• Consistency within the API — keyframes and timing properties follow the same idiom
• Bulk setting of timing properties is easier

Disadvantages:

• More awkward to update an individual timing property, e.g.

// Current API
anim.effect.timing.duration *= 2;
// Proposed API
const updatedTiming = anim.effect.getTiming();
updatedTiming.duration *= 2;
anim.effect.setTiming(updatedTiming);

• More inefficient to update individual timing properties since you need to read and write these property bags. From what I understand, at least for Gecko, that can be quite slow.

I do wonder how common those last two items are, though. (And for the first one, if you're just blindingly overwriting the value, you can use Object.assign() or, one day, the object spread operator to write it more compactly.)

The other consideration is this proposal may be less forwards compatible. For example, suppose we extend iterations to take a keyword value. Presumably it will still compute to a float. Code that currently calls anim.effect.getComputedTiming().iterations would continue to work when this new property is used. However, under this proposal, code that calls anim.effect.getTiming().iterations would need to be updated to call anim.effect.getTiming.computedIterations instead.

That said, I think we already have this problem in that anyone currently calling anim.effect.timing.iterations and assuming it will be a float (which I expect is not uncommon since it's less to type than anim.effect.getComputedTiming().iterations) will have the same problem.

@shans, @dstockwell Any thoughts about this?

@heycam, @bzbarsky Is returning snapshot objects generally the preferred approach over maintaining live objects?

Copied from original issue: w3c/web-animations#168

[birtles]

From @alancutter on September 14, 2016 7:56

One slight downside to dropping effect.timing is its no longer possible to see the specified value of fill or duration as they get "resolved" in getTiming().

Dropping the live effect.timing object SGTM though, I don't think there are many use cases for it not being a snapshot.

[birtles]

One slight downside to dropping effect.timing is its no longer possible to see the specified value of fill or duration as they get "resolved" in getTiming().

The proposal here is that getTiming().fill returns the specified value, e.g. "auto", and we add a computedFill member for the computed value, e.g. "none". Likewise for duration.

[birtles]

From @bzbarsky on September 15, 2016 2:37

Is returning snapshot objects generally the preferred approach over maintaining live objects?

It really depends on the situation and the design considerations (performance tradeoffs, how the objects will typically be used, etc).

[birtles]

@BorisChiou, @hiikezoe let me know if you have any thoughts. Just to clarify one point of confusion, the idea is that you'd have:

anim.effect.getTiming().duration // "auto"
anim.effect.getTiming().computedDuration // 0

[birtles]

From @hiikezoe on September 20, 2016 22:39

I like this idea.
We do update individual property in many places in test code but I don't think it's common to change individual property dynamically in the real world.

[birtles]

From @shans on September 29, 2016 4:28

The More awkward to update an individual timing property disadvantage is a major disadvantage, I think.

Modification of animations in-flight is a major capability that the unshipped parts of the API will give us, and this proposal makes that significantly harder for timing properties.

Anecdata-ly, I found myself doing this a reasonable amount when playing with the polyfill.

In contrast, looking at the advantages:

• 2 fewer interfaces - this means nothing to web developers.
• Fewer live objects - sort of, but it's not likely that people will actually 'tear off' and store the timing.
• Consistency within the API — keyframes and timing properties follow the same idiom - obviously we're trading off consistency with the style approach here. Not sure we buy anything.
• Bulk setting of timing properties is easier - not really, unless you're talking about the very specific use case of bulk updating a large set of animations to all have exactly the same timing. Even here, wrapping the update in a loop will end up looking almost exactly the same.

[birtles]

From @dstockwell on September 29, 2016 5:32

The More awkward to update an individual timing property disadvantage is a major disadvantage, I think.

I thought that's how we ended up with the current design. Getting/setting timing being a more frequent use case than getting/setting keyframes, so we thought it OK to be different/harder.

[birtles]

I don't think setting individual timing properties after setting up the animation is common since in most cases doing that that will cause the animation to jump which is generally undesirable. (Test code etc. being obviously quite different since you're not concerned with smooth motion.)

Regarding bulk updating, it was partly an observation from seeing people like Paul & Surma use the API and store a separate timing object for passing to animate(). It seems a bit odd that you can pass that to animate() but beyond that point you can't use it to update an animation.

I think consistency within the API is more important than consistency with the style API since internal consistency trumps external consistency and it's not actually a good parallel with the style API anyway (since the style API doesn't actually represent specified style, but only style specified on the style attribute).

[birtles]

See also w3ctag/design-principles#34

[birtles]

Just to close the loop on this, I discussed this with Shane and Ian in November 2016 and I think we were thinking that with the addition of setTimingProperty for updating an individual property, this might be ok.
See meeting minutes.

[birtles]

I had a go at implementing this in Firefox just to see how it feels. The modified IDL I used is:

dictionary EffectTiming {
  double                              delay = 0.0;
  double                              endDelay = 0.0;
  FillMode                            fill = "auto";
  double                              iterationStart = 0.0;
  unrestricted double                 iterations = 1.0;
  (unrestricted double or DOMString)  duration = "auto";
  PlaybackDirection                   direction = "normal";
  DOMString                           easing = "linear";
};

dictionary PartialEffectTiming {
  double                              delay;
  double                              endDelay;
  FillMode                            fill;
  double                              iterationStart;
  unrestricted double                 iterations;
  (unrestricted double or DOMString)  duration;
  PlaybackDirection                   direction;
  DOMString                           easing;
};

dictionary ComputedEffectTiming : EffectTiming {
  unrestricted double   computedDuration = 0.0;
  FillMode              computedFillMode = "none";
  unrestricted double   endTime = 0.0;
  unrestricted double   activeDuration = 0.0;
  double?               localTime = null;
  double?               progress = null;
  unrestricted double?  currentIteration = null;
};

interface AnimationEffectReadOnly {
  ComputedEffectTiming getTiming();
  void setTiming(optional EffectTiming timing);
  void updateTiming(optional PartialEffectTiming timing);
};

As you can see I took the liberty of renaming some of the dictionaries at the same time. (Now that we no longer have AnimationEffectTiming(ReadOnly) we now longer need the Properties on AnimationEffectTimingProperties but I decided to try and simplify the names further still.)

The Partial naming is based on Partial from TypeScript.

The two questions I have before specing this are:

1. Obviously adding setTiming and updateTiming to AnimationEffectReadOnly don't make sense so where should they go? On KeyframeEffect itself? Or should we fix this and issue [web-animations-1] Drop the *ReadOnly interfaces #2068 at the same time?

   For solving [web-animations-1] Drop the *ReadOnly interfaces #2068 we'd rename the interface AnimationEffect and work out how these methods work when applied to animations tied to markup (CSS animations/transitions).

2. Is having computedDuration and computedFill a bit weird? Is it weird that activeDuration is a computed property but duration is not? E.g. if you do:

const timing = anim.effect.getTiming();
timing.duration = 3000;
timing.activeDuration = 6000;
anim.effect.updateTiming(timing);

only duration will have any effect, activeDuration will be ignored.

Also, it just occurred to me, might it be useful to have setTiming / updateTiming return the updated computed timing so you can chain calls together?

I'm not sure if there's a way of expressing partial dictionaries in WebIDL that's less redundant that what I have above.

[bzbarsky]

I'm not sure if there's a way of expressing partial dictionaries in WebIDL that's less redundant that what I have above.

There isn't, for the meaning of "partial" used here (which is different from what Web IDL means by "partial dictionary", as defined at https://heycam.github.io/webidl/#dfn-partial-dictionary).