Allow specifying composite/offset/easing when using property-indexed keyframes #148

Open
birtles opened this Issue Apr 25, 2016 · 6 comments

Projects

None yet

3 participants

@birtles
Contributor
birtles commented Apr 25, 2016

I can't remember exactly how the proposal works but I think this is the relevant IRC log for which @shans was going to write up a PR:

15:39 <alancutter> Should element.animate({color: 'green', offset: 0.5}, 1000) be equivalent to element.animate([{color: 'green', offset: 0.5}])?
15:40 <alancutter> I think the current spec drops the offset in the former case: http://w3c.github.io/web-animations/#process-a-frames-argument
15:41 <alancutter> Missed a ", 1000" in the second example.
17:09 <birtles> alancutter: yeah, you can't specify offsets when using the property-indexed format
08:49 <alancutter> birtles: To clarify, these two examples should not be equivalent?
08:51 <birtles> Right, offset is ignored in the property-indexed case
08:51 <alancutter> Is it the same for composite?
08:51 <alancutter> The spec seems to read the composite member and make use it though it's not in the dictionary.
08:52 <alancutter> *use of
08:52 <birtles> Yeah, I think the spec might be wrong there
08:53 <birtles> it probably should be in that BasePropertyIndexedKeyframe dictionary
08:53 <birtles> I haven't really checked that because we don't implement composite operations yet
08:53 <alancutter> Okay, so for a non-iterable non-SharedKeyframeList object the offset and composite members are ignored (no TypeErrors thrown).
08:54 <birtles> No, I think composite is supposed to be read?
08:55 <alancutter> Okay, sorry misunderstood which direction "wrong" was.
08:56 <birtles> I think we probably should change the spec so that BasePropertyIndexedKeyframe has a composite member, right?
08:56 <birtles> And that applies to all values specified?
08:56 <birtles> I honestly can't remember how we decided that should work
08:57 <alancutter> We can do that, though it does mean there's a duplicate way to do the same thing with specifying it in the keyframe options.
08:57 <birtles> Yeah, good point
08:57 <birtles> I just realised that too
08:57 <birtles> So, then I guess there's no reason to put it on BasePropertyIndexedKeyframe
08:58 <alancutter> Okay, so if composite is specified we'll ignore it.
08:58 <alancutter> Same with offset.
08:58 <birtles> Right
08:58 <birtles> And the step, "Let composite operation be the value of the “composite” member of property-indexed keyframe, or null if there is no member of that name."
08:58 <birtles> is wrong
08:58 <alancutter> Personally I think a TypeError or something should be thrown, silently ignoring it feels like a trap to me.
08:59 <birtles> So, throw a type error simply if it exists?
08:59 <alancutter> I've used element.animate({color: 'red', composite: 'add', offset: 0.5}) numerous times when making demos.
09:00 <birtles> I think implementations could test if the property exists and report a console error without that being observable? As long as we don't get the property?
09:00 <alancutter> You mean a warning?
09:00 <birtles> Yeah
09:00 <alancutter> That sounds reasonable to me.
09:02 <alancutter> Thanks for the clarifications, I'll make sure our implementation matches.
09:02 <birtles> Thanks!
09:02 <birtles> I still need to do some work on ours so don't rely on it too much!
09:02 <birtles> (Actually, I need to rewrite the entire frame handling code :/)
09:03 <alancutter> Branch point is in a couple of days so whatever we end up with will be what ships in a few months.
09:03 <alancutter> Let me know if that's an issue.
09:04 <birtles> Sure
09:04 <birtles> I know there were one or two small issues we came across but I don't know if they affected the exposed API surface area
09:05 <birtles> One definitely didn't
09:07 <alancutter> Actually none of what we discussed here is getting shipped anyway since it involves composite modes other than replace.
09:23 → webanimbot joined (~webanimbot@public.cloak)
09:23 <webanimbot> [web-animations] birtles pushed 1 new commit to master: https://git.io/v20lL
09:23 <webanimbot> web-animations/master 14a1188 Brian Birtles: Drop step about reading composite member from property-indexed keyframes...
09:23 ← webanimbot left (~webanimbot@public.cloak)
09:41 <shane> won't the same behavior be required for element.animate({color: 'red', offset: 0.5})?
09:42 <birtles> what same behaviour?
09:42 <shane> console error
09:42 <shane> (i.e. this is going to be shipped)
09:43 <shane> birtles: FWIW I think that should work. I think that obviously element.animate({color: ['red', 'green'], offset: 0.5}) can't. But the single value case should.
09:43 <birtles> console warnings are implementation-dependent
09:43 <shane> Or if it doesn't work, then we should throw an exception to make it obvious
09:43 <shane> rather than just logging a warning
09:43 <shane> (or whatever)
09:44 <birtles> I thought when we introduced property-indexed keyframes we said you can't set offsets and make the single keyframe case a variant of property-indexed keyframes
09:44 <birtles> I'm not opposed to the change but it's a change that needs to be spec'ed and implemented
09:45 <shane> birtles: I'd wanted the two cases to collapse to the same thing - i.e. {foo: bar} == [{foo: bar}]
09:45 <shane> but I think in the case of offset that's not possible (it probably is for everything else)
09:45 <shane> but the bigger point here is alan's comment: "I've used element.animate({color: 'red', composite: 'add', offset: 0.5}) numerous times when making demos."
09:45 <shane> I'm not comfortable with silently failing if this is something commonly used.
09:46 <birtles> the suggestion was implementations can make that a console warning
09:46 <shane> (or that is likely to be commonly used)
09:46 <shane> yeah. That's pretty much silently failing
09:46 <shane> we've biased towards exceptions everywhere else, it'd be weird to do something different in this one case.
09:46 <birtles> ok, well let me know what you want
09:47 <shane> birtles: to make sure I understand correctly, we have 2 issues: what to do with composite, and what to do with offset.
09:47 <shane> and this is specifically in the case of a single keyframe with only single values per property
09:47 <shane> (single keyframe not in a list)
09:49 <shane> so it seems like composite can be just supported for single keyframes, because the meaning would be well defined. Which harmonizes both the list of keyframes and single keyframe cases.
09:49 <birtles> what would the meaning be?
09:49 <shane> That just leaves offset. Personally I'd slightly lean towards allowing it in the single-keyframe only-single-values case, but I'm perfectly happy with throwing an exception there too. So whichever of those two options are better?
09:50 <shane> element.animate([{color: 'red', composite: 'add'}]) === element.animate({color: 'red', composite: 'add'})
09:51 <shane> and element.animate({color: ['red', 'green'], composite: 'add'}) == element.animate([{color: 'red', composite: 'add'}, {color: 'green', composite: 'add'}])
09:54 <birtles> Hmm, I just noticed that element.animate({color: 'red', easing: 'ease'}) doesn't actually do what I'd expect
09:54 <birtles> i.e. the easing doesn't apply
09:54 <shane> yeah
09:54 <shane> same as if you do
09:55 <shane> element.animate([{color: 'red', easing: 'ease'}])
09:55 <birtles> because we only apply it to the specified keyframes, not the synthesized ones
09:55 <shane> this is pretty much by design, I think. Basically, the two cases should be the same as much as possible.
09:56 <birtles> the asymmetry comes when you go from element.animate({color: 'red', easing 'ease'}) to element.animate({color: ['red', 'blue'], easing 'ease'})
09:56 <birtles> both cases are acceptable
09:56 <birtles> but if you do the same with offset or composite you throw?
09:56 <shane> but again, you have the same thing when you do element.animate([{color: 'red', easing: 'ease'}, {color: 'blue', easing: 'ease'}])
09:56 <shane> nah I'm suggesting you don't throw ever with composite
09:57 <shane> and with offset, you only throw if there are properties that have lists
09:57 <birtles> so composite just start silently failing?
09:57 <shane> hey?
09:57 <shane> no?
09:57 <shane> element.animate({color: ['red', 'green'], composite: 'add'}) == element.animate([{color: 'red', composite: 'add'}, {color: 'green', composite: 'add'}])
09:58 <shane> in fact, if you wanted to
09:58 <shane> you could say that in general, you:
09:58 <shane> (1) separate out the properties
09:58 <shane> (2) duplicate composite/offset/easing to each property channel
09:59 <shane> (3) synthesize offsets for each value in the list (per-property), and create keyframes for each
09:59 <shane> (4) duplicate composite/offset/easing to each keyframe
09:59 <shane> i.e. element.animate({color: ['red', 'green'], offset: 0.5}) would become element.animate([{color: 'red', offset: 0.5}, {color: 'green', offset: 0.5}])
10:00 <birtles> yeah, that makes more sense to me than throwing
10:00 <shane> then it's all nice and regular
10:00 <shane> great, let's ship it :)
@shans shans was assigned by birtles Apr 25, 2016
@birtles
Contributor
birtles commented May 23, 2016

Hi @shans, how can I help with this? If you want to write something in a Google Doc or even just here I'm happy to work it into the spec.

@shans
Contributor
shans commented Aug 12, 2016

@birtles are you still waiting for something from me here? Sorry! I'll try to get to it next week.

@birtles
Contributor
birtles commented Aug 15, 2016

@shans Yes, thanks. I was planning on picking this up in Q4 if you didn't get to it by then.

@birtles
Contributor
birtles commented Aug 15, 2016

Thinking about the following example:

{ width: [ '20px', '30px', '50px' ], 
  offset: [ 0, 0.2 ] }

I wonder if it would work to say that for offset, if there are not enough values, you make the last offset 1 and distribute the remaining synthesized offsets? i.e. the above becomes offset: [ 0, 0.2, 1 ]. Just thinking aloud.

@AmeliaBR

I wonder if it would work to say that for offset, if there are not enough values, you make the last offset 1 and distribute the remaining synthesized offsets? i.e. the above becomes offset: [ 0, 0.2, 1 ]

On first glance, this makes sense to me. So

{ width: [ '20px', '30px', '50px' ], 
  offset: [ 0, 0.2 ] }

would be equivalent to

[{ width: '20px', offset:0 },
 { width: '30px', offset:0.2 }, 
 { width: '50px' }]

Which makes the array-of-objects and object-of-arrays syntaxes directly invertable, as if it was the difference between defining a table by rows or by columns.

However, I would also expect a similar assignment of array values into keyframes when different properties have different numbers of values in their array. In other words, the total number of frames is the maximum length of all the arrays, and all other arrays are null-padded to match. But if I'm reading the algorithm correctly each property's array of values ends up distributed as an independent set of keyframes? (It's not clearly stated in the human-readable sections!)

Which begs the question, how would offsets be distributed in this case:

{ width: [ '20px', '30px', '50px' ], 
  opacity: [0, 1],
  offset: [ 0, 0.2 ] }
@birtles
Contributor
birtles commented Feb 20, 2017

I guess it depends where we apply the offsets. If we do it after merging (see process a keyframes argument) then I guess we'd have:

  [ { width: '20px', opacity: '0' },
    { width: '30px' },
    { width: '50px', opacity: '1' } ]

so then doing the "make the last offset 1 and distribute the remaining synthesized offsets" behavior would give:

  [ { width: '20px', opacity: '0', offset: 0 },
    { width: '30px', offset: 0.2 },
    { width: '50px', opacity: '1', computedOffset: 1 } ]

which seems to be ok?

Although, as you mentioned, we wouldn't even need any special "make the last offset 1 and distribute the remaining synthesized offsets". I think we could just assign the first few offsets and the usual behavior for computing offsets would produce the expected result (especially once we drop spacing modes).

For 'easing' and 'composite', I definitely think if there's a single value we should apply it to all keyframes. At least for easing that matches up nicely with how animation-timing-function and transition-timing-function work when you specify them on the element, i.e. it gets copied to all keyframes.

If you specify two values though, easing: [ 'ease', 'ease-out' ] what is the most useful behavior? To repeat the list: ease, ease-out, ease, ease-out, ease etc. Or to repeat the last value: ease, ease-out, ease-out, ease-out, ease-out. I suppose repeating the list matches better with what CSS does?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment