Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[web-animations-2] AnimationWorklet integration - using GroupEffect and allowing control of localTime #2071

Open
birtles opened this Issue Dec 5, 2017 · 13 comments

Comments

Projects
None yet
2 participants
@birtles
Copy link
Contributor

birtles commented Dec 5, 2017

From @majido on July 11, 2017 20:52

Background

During last houdini F2F meeting it was decided that we should attempt to make animation worklet
API fit better within web animation model.

One of the key features of worklet animations is that they can have multiple effects which are
driven by the animate function in worklet scope. Our initial plan was to make a new Animation base
class that does not assume "a single effect" (see below for a initial API sketch). Since then I
realized that web animations level 2 spec has a GroupEffect construct which I believe worklet
animations can use to the same effect.

interface AnimationBase :  EventTarget{...};

interface Animation :  AnimationBase {
  attribute AnimationEffectReadOnly? effect;
  attribute AnimationTimeline? timeline;
};

interface WorkletAnimation : AnimationBase {
  attribute AnimationEffectReadOnly? effects;
  attribute array<AnimationTimeline> timelines;
}

Using GroupEffect with WorkletAnimations

The idea is to pass in a GroupEffect to WorkletAnimations for usecases that require controlling
multiple effects. However, In WorkletAnimation we want to provide a more flexible API where the
animation script has the option to set the local time for each individual child effects.

Here is a hypothetical API that can enable this:

// In Document scope
animationWorklet.addModule('my-custom-animator.js').then( _ => {
  const workletAnim = new WorkletAnimation(
    'my-custom-animation',
    new WorkletGroupEffect([new KeyframeEffect(), new KeyframeEffect()]),
    document.timeline);
});
// In Animation Worklet scope
registerAnimator('my-custom-animation', class {
  animate(timelines, groupEffect) {
    const time = timelines[0].currentTime;
    // Drive the output by setting group effect's children local times.
    groupEffect.children[0].localTime = time;
    groupEffect.children[1].localTime = time * 3.14;
  }
});

The current GroupEffect design only allows for two different scheduling models (i.e.,
parallel, sequence). These models govern how the group effect local time is translated to its
children effects local times by controlling the child effect start time. In parallel mode children
start times are the same as parent, and in sequence mode children start times form a sequence.

Setting local time directly on child effects does not fit in either of these models. One way to
think about it is that this model allows arbitrary per-child start time. Below is a set of proposed
changes to GroupEffect that allow this.

Proposal

Keep GroupEffect as is but introduce a new subclass WorkletGroupEffect (or CustomGroupEffect).
Unlike normal group effects where parent time dictates the child time the above will allow its
children local time to be mutated individually as well.

partial interface AnimationEffectReadOnly {
    // Intended for use inside Animation Worklet scope to drive the effect.
    [Exposed=AnimationWorklet]
    attribute localTime;
};


interface WorkletGroupEffectReadOnly :  GroupEffectReadOnly {}

interface WorkletGroupEffect :  WorkletGroupEffectReadOnly {}
WorkletGroupEffect implements AnimationEffectMutable;
WorkletGroupEffect implements GroupEffectMutable;

Setting localTime property on an effect to value t does the following:

  • If the animation effect does not have a parent group, then set the effect local time to t.
  • If the animation effect has a parent group
    • If the parent group is a WorkletGroupEffect, then
      set the effect local time to t, and the effect start time to (parent's transformed time - t).
    • Otherwise throw an exception indicating that the child effect time can only be controlled by
      its parent group.

Notes

  • Initially we don't want to expose WorkletGroupEffect directly, rather only allow it to be created
    if a sequence of effects are passed into WorkletAnimation.
  • Clone() method on WorkletGroupEffect needs to not only close timing and children but also the
    start time values.
  • Not clear to me what should happen if a child local time is set but its parent local time is
    still unresolved.

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

@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Dec 5, 2017

Thanks for writing this up.

Firstly I don't think we want to directly set the effect's local time, per se. The above proposal has the step, "set the effect local time to t, and the effect start time to (parent's transformed time - t)", however, in level 2 of the spec, the definition of the local time is, "local time of an animation effect is the animation effect’s inherited time minus its start time". That is, the local time is calculated from the start time so we can't set both without changing the model (i.e. setting the start time should be sufficient).

However, one awkward result of setting the start time is that there's a dependency chain between the timing calculations that goes:

child effect start time → child effect end time → group effect intrinsic iteration duration → group effect transformed time → child effect local time

That is, if you're in the second iteration of a WorkletGroupEffect and you set the local time of one of its children by adjusting its start time, it's local time could then jump to a different time than you set.

It's not quite the same, but I wrote a blog post a while back about similar sort of issues we encountered when trying to allow independent playback control on group children. Note that the terminology has since changed: the "players" referred to in that blog are now just called Animations.

Basically, seeking arbitrary nodes in the graph is hard. Seeking the children of the root-level group is probably doable provided the group doesn't repeat, or have timing functions applied etc.

@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Dec 5, 2017

From @alancutter on July 13, 2017 5:39

@birtles Is Web Animations level 2 hosted online somewhere like http://w3c.github.io/web-animations/?

@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Dec 5, 2017

@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Dec 5, 2017

From @alancutter on July 13, 2017 7:17

Perhaps making the relationship between a group animation effect and its children less constrained would be worth the flexibily it leaves for group animation effects in future. Let group animation effects specify how their duration and their children's local time are calculated and make the current level 2 specification specific to {Group,Sequence}Effects.

@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Dec 5, 2017

I don't understand what you're suggesting. It sounds like you're describing the current state of affairs.

@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Dec 5, 2017

From @alancutter on July 13, 2017 7:40

I'm suggesting to make an animation effect's local time determined by the parent group directly instead of having their start time determined by the parent group. This leaves the concept of a child animation effect's start time and inherited time as an implementation detail of {Sequence,Effect}Groups instead of something universal to all animation effects.

@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Dec 5, 2017

From @majido on July 17, 2017 21:32

Firstly I don't think we want to directly set the effect's local time, per se. The above proposal has the step, "set the effect local time to t, and the effect start time to (parent's transformed time - t)", however, in level 2 of the spec, the definition of the local time is, "local time of an animation effect is the animation effect’s inherited time minus its start time". That is, the local time is calculated from the start time so we can't set both without changing the model (i.e. setting the start time should be sufficient).

Makes sense. I think the expose API to Javascript should to be in terms of "local time" but in spec we will just modify the start time so that the desired local time is achieved.

However, one awkward result of setting the start time is that there's a dependency chain between the timing calculations that goes:
child effect start time → child effect end time → group effect intrinsic iteration duration → group effect transformed time → child effect local time
That is, if you're in the second iteration of a WorkletGroupEffect and you set the local time of one of its children by adjusting its start time, it's local time could then jump to a different time than you set.

Very interesting, I missed this dependency! One way around this is to define the intrinsic
iteration duration for a WorkletGroupEffect in such a way to break the dependency. For example if
the worklet group effect intrinsic duration was 0 then it will not be affected by its children.

I don't think this is a problem given that WorkletGroupEffect is really meant to be used with
in AnimationWorklet where it is expected that one directly sets its children localTime.

Basically, seeking arbitrary nodes in the graph is hard. Seeking the children of the root-level group is probably doable provided the group doesn't repeat, or have timing functions applied etc.

The proposal only allows seeking in immediate children of WorkletGroupEffect. Also If we make
it so that the seek has no visible effect to the group's ancestors then I don't believe we need to
limit it to the root-level.

In particular if we break the intrinsic iteration duration dependency you mentioned above, I don't
think ancestors see any other side effect given that we don't change the localTime of the
group.

I am not sure if I fully understand why we need to prevent repeat or application of timing functions.

I'm suggesting to make an animation effect's local time determined by the parent group directly instead of having their start time determined by the parent group. This leaves the concept of a child animation effect's start time and inherited time as an implementation detail of {Sequence,Effect}Groups instead of something universal to all animation effects.

Interesting idea. In particular if you do that then group effects are no longer limited to only
changing the children start offset. For example, you can have a group effect that plays two effects
alternatively depending on playback direction. Combined alternative playback rate this can lead
to interesting usecases.

Also as far as I understand it, a group scheduling have ramifications in both direction:

  1. parent -> children: transformed time -> local time
  2. children -> parent : active duration & delay -> intrinsic iteration duration

A particular group schedule strategy (e.g., parallel or sequence) determines both above
transformation.

@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Dec 5, 2017

Very interesting, I missed this dependency! One way around this is to define the intrinsic
iteration duration for a WorkletGroupEffect in such a way to break the dependency. For example if
the worklet group effect intrinsic duration was 0 then it will not be affected by its children.

I think the difficulty with making the intrinsic duration 0 is that authors will be required to manually set it. Animations use the duration of their children to perform reversing, finishing etc. Furthermore, groups use their own duration to perform repeating etc. Unless authors set the duration of these WorkletGroupEffects, a lot of stuff will break. But at the same time, I wonder how realistic it is to set an accurate duration manually for some of these use cases.

Groups as currently envisaged are primarily a timing construct and it feels a little bit like we're trying to retrofit them to be largely just an object container. I wonder if SMIL's <excl> containers might be a source of ideas here.

I agree Alan's proposal makes sense, but I think there are still a few unresolved issues (particularly what we do with regards to the group's duration).

@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Dec 5, 2017

From @flackr on July 28, 2017 17:15

If we implicitly construct the WorkletGroupEffect when we see an array of effects in the WorkletAnimation constructor I think we could set the duration to what we feel makes the most sense and avoid the need for the developer to worry about this.

@majido

This comment has been minimized.

Copy link
Contributor

majido commented May 29, 2018

I wanted to ping this thread in particular we are interested to start experimenting with this idea but only limited to AnimationWorklet for now.

I think the following API surface is safe to start with:

interface WorkletGroupEffect {
  readonly sequence<AnimationEffect> children;
}

partial interface AnimationEffect {
   [Exposed=AnimationWorklet] attribute double localTime;
}

This is consciously a conservative API. In particular:

  • Does not expose any constructor. The actual construction of group effect is done internally.
  • Does not allow nesting worklet group effects. So there can only be one nesting levels.
  • Have an interesic duration of 0 (which cannot be changes by the author code). See flackr@ comment
  • Only exposed API surface is to iterate over children and set their local time
  • setting local time is only exposed within animation worklet global scope.
  • The name is prefixed with "Worklet".
  • It does not expose and inherit from GroupEffect so as not to require anything there but in future can inherit from GroupEffect once it becomes available.

I appreciate any feedback you may have on this plan.

To implement something like this we need to so some ground work in Blink for general group effects. This should also give us valuable implementation feedback on GroupEffects in general beyond their
application with worklet animations.

Note: I choose to use a readonly sequence<AnimationEffect> instead of of per this issue

@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Jul 2, 2018

Sorry for the delay in responding here.

partial interface AnimationEffect {
   [Exposed=AnimationWorklet] attribute double localTime;
}

We already have the getTiming().localTime member. I wonder how this should interact. Should we set it through updateTiming() instead?

This is consciously a conservative API. In particular:

  • Does not expose any constructor. The actual construction of group effect is done internally.

Previous TAG feedback has been that we should not have objects that get created by the platform without constructors.

  • Does not allow nesting worklet group effects. So there can only be one nesting levels.

This seems reasonable (assuming we have a natural means of extending this to do nesting in future).

To implement something like this we need to so some ground work in Blink for general group effects. This should also give us valuable implementation feedback on GroupEffects in general beyond their
application with worklet animations.

Sounds great.

Note: I choose to use a readonly sequence instead of of per this issue

This seems reasonable to me but we'll need to think about how we expect this to be updated in future.

@majido

This comment has been minimized.

Copy link
Contributor

majido commented Aug 3, 2018

We already have the getTiming().localTime member. I wonder how this should interact. Should we set it through updateTiming() instead?

So I take it that the suggesting is that we add the localTime to OptionalEffectTiming and EffectTiming dictionary. So authors can do effect.updateTiming({localTime: 123.4});

This can work for animation worklet usecases. If making localTime writable is acceptable in general then I am in favor of having a consistent API here.

Previous TAG feedback has been that we should not have objects that get created by the platform without constructors.

This is an interim step until GroupEffect are fully specced and implemented. I didn't want to introduce a constructor that can later create conflict with what we end up using for GroupEffect. Alternatively we can allow an empty constructor which is not useful or one that take a list of AnimationEffects.

Note: I choose to use a readonly sequence instead of of per this issue
This seems reasonable to me but we'll need to think about how we expect this to be updated in future.

Turns out webidl sequence types are not allowed for attributes. So I suggest using a getter function for now and we can add an update function in the future. e.g.,

interface WorkletGroupEffect {
  sequence<AnimationEffect> getChildren();
}
@birtles

This comment has been minimized.

Copy link
Contributor Author

birtles commented Aug 6, 2018

We already have the getTiming().localTime member. I wonder how this should interact. Should we set it through updateTiming() instead?

So I take it that the suggesting is that we add the localTime to OptionalEffectTiming and EffectTiming dictionary. So authors can do effect.updateTiming({localTime: 123.4});

This can work for animation worklet usecases. If making localTime writable is acceptable in general then I am in favor of having a consistent API here.

I think it would be useful to be able to set localTime on AnimationEffects not bound to an Animation in general. This would let you have a completely free-standing way of driving an animation.

For an AnimationEffect that is associated with an Animation, I suspect we would just throw if you tried to set the localTime.

The question still remains whether you should set it by doing effect.localTime = 1000 or effect.updateTiming({ localTime: 1000 }), however.

Having thought about this a little further I start to lean towards effect.localTime = 1000 or something other than updateTiming().

My original comment was wrong when I suggested we already have getTiming().localTime. It's actually getComputedTiming().localTime.

Making it possible to set localTime through updateTiming() would also suggest it's no longer part of computed timing, and should be returned from getTiming(). That seems awkward.

There's also the concern that the following would begin to throw in some circumstances when it previously didn't:

effectB.updateTiming(effectA.getComputedTiming());

Authors should really be using getTiming() in that case but they may well end up doing the above.

In light of that, having a means to setting localTime outside of updateTiming() seems like the way to go.

Regarding the localTime attribute approach, the overlap between effect.getComputedTiming().localTime and effect.localTime seems unfortunate. I'm not sure how to fix that other than adding something like effect.setLocalTime(1000) however.

Turns out webidl sequence types are not allowed for attributes. So I suggest using a getter function for now and we can add an update function in the future. e.g.,

interface WorkletGroupEffect {
sequence getChildren();
}

Right. Web Animations Level 2 introduces the AnimationNodeList for this reason. That spec is out of date, but if we were to introduce that approach again we'd call it AnimationEffectList and get rid of the ReadOnly distinction.

I'm pretty sure you can use a frozen array as an attribute but I've no idea if that's a good idea or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.