w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.52k stars 672 forks source link

[web-animations-2] Progress APIs #8799

Open flackr opened 1 year ago

flackr commented 1 year ago

In #8669, #8765 and #8201 there seems to be a common desire to have more convenient APIs for getting some representation of progress. Currently you can get the progress of the current iteration of an effect using the getComputedTiming() API as follows:

let progress = animation.effect.getComputedTiming().progress;

You can also get the overall effect progress by doing a calculation such as:

let progress = animation.currentTime / animation.effect.getComputedTiming().endTime;

However, with several of the other linked issues it seems that it would be convenient for developers to have some form of access to the progress through the attached animation range, or to get that progress directly from the timeline.

We could add a convenience currentProgress helper alongside the currentTime API on Animation:

attribute double? currentProgress;

This could return the progress of currentTime through the entire range before which it is considered complete, e.g. calculated as follows:

currentProgress = currentTime / effect endTime

Note that endTime is infinite when either iterations or duration is infinite. In these cases currentProgress should probably be null or undefined?

Open question - should it be read only or should we allow setting the progress which would be equivalent to setting currentTime to currentProgress * endTime?

As per discussion on #8669 a progress reported on animation should be mostly independent of the effect timing, so we calculate it as the progress from startTime to the time when the animation would resolve its finished promise. This would intentionally not match the progress through the developer specified keyframes.

@birtles @bramus @fantasai

I also propose that given the uncertainty around what's expected from the getCurrentTime API on timeline, that we remove it until we have a better understanding of the specific use cases that developers need it for. Perhaps the animation progress API above will address some of them where it seems developers were trying to work this out through the timeline.

birtles commented 1 year ago

Overall it seems reasonable to me. I think defining it in terms of startTime and endTime is a nice way of keeping it at arms length from effect timing.

Note that endTime is infinite when either iterations or duration is infinite. In these cases currentProgress should probably be null or undefined?

Mathematically, should it be zero? Not sure if that would be the most useful result, however.

Open question - should it be read only or should we allow setting the progress which would be equivalent to setting currentTime to currentProgress * endTime?

I'd suggest making it readonly. As you pointed out, there are some cases where this value would be null / undefined / 0 or somesuch, and in those cases if set made it writeable I guess the setter would throw?

We already have enough ways of changing the animation position (setting currentTime, setting startTime, calling the various playback methods etc.) and it's easy enough to make a read-only attribute writeable later if compelling use cases present themselves.

flackr commented 1 year ago

Mathematically, should it be zero? Not sure if that would be the most useful result, however.

Yeah, mathematically it would be 0. I don't have a strong preference for it to be null/undefined, just thought it might be marginally more meaningful than 0 to distinguish from the case of a 0 that can make progress.

bramus commented 1 year ago

In favor of this general approach as described here, to tackle #8669.

Fine with making it readonly as a starting point.

flackr commented 1 year ago

Thanks @bramus. Adding to agenda to discuss/resolve:

  1. Add read-only currentProgress to Animation calculated as currentTime / effect endTime.
  2. Should it return 0 (technically the mathematical result), null, or undefined when the animation end time is infinite?
ydaniv commented 1 year ago

Regarding 1. +1 on adding, with 2 suggestions:

  1. Make it read/write. I think it should reflect the currentTime behavior and allow setting progress through this endpoint. It will both be, IMHO, more handy for authors, and if I'm not mistaken, should be necessary for animations with duration: auto.
  2. How about naming it simply progress?
css-meeting-bot commented 1 year ago

The CSS Working Group just discussed [web-animations-2] Progress APIs, and agreed to the following:

The full IRC log of that discussion <faceless> present-
<bramus> flackr: So WAAPI is very focused on absolute times, but with scroll-driven aninamations, developers often just work with progress instead of time
<bramus> flackr: seems like a reasonable thing to add, so there is a proposal to get the current progress which is a calcuation between start and and end time
<bramus> flackr: also need to specify what hsould happen for infinitely repeating animations
<bramus> flackr: Proposal is adding currentProgress to Animation
<bramus> flackr: it is calculated as currentTime / effect endTime
<bramus> flackr: propose to make it readonly for now
<bramus> s/repeating/duration/
<bramus> Rossen_: Any feedback?
<ydaniv> I proposed "progress" instead of "currentProgress"
<TabAtkins> +1
<TabAtkins> (to the current proposal, no opinion on naming)
<bramus> Rossen_: You are taking into account the feedback from ydaniv?
<bramus> flackr: Yes, he proposed currentProgress instead of progress
<bramus> flackr: dont object to this shorter name and computedTIming value from the effect is also called progress
<bramus> Rossen_: what about read/write vs readonly?
<bramus> flackr: then have to decide how to handle infinite duration animation
<bramus> Rossen_: starting with readonly gives up path to go forward later
<bramus> Rossen_: Should we formulate as readonly? not hearing objections from yehonathan
<bramus> flackr: would be happy with that
<bramus> bramus: me too
<bramus> Rossen_: objections to adding as readonly?
<bramus> Rossen_: Not hearing anything
<bramus> bramus: and the name?
<bramus> flackr: we said progress
<bramus> flackr: Proposed resolution: add progress to Animation and make it readonly
<bramus> RESOLVED: Add progress to Animation and make it readonly
flackr commented 9 months ago

On the PR from @DavMila there is one point where we don't have a clear answer. Should the progress be clamped to [0, 1]. For the full discussion you can view the PR review https://github.com/w3c/csswg-drafts/pull/9937#discussion_r1491825782 however I'll do my best to summarize the pros and cons of each.

  1. Don't clamp the progress. This makes it similar to the currentTime which progress exists alongside (which also can be negative or greater than end time) and is the natural result of the expressed formula. This could be useful for example to distinguish between being at the start time (progress = 0) where the effect is active and visibly shown, and being before the start time (progress < 0) where the effect is not applied (unless fill = backwards | both). However, this also means that to correctly reflect the progress for 0 duration animations we would likely have to return -Infinity and Infinity when you are before or at/after the start time respectively.
  2. Clamp the progress to [0, 1]. A developer won't be able to tell when at 0 progress whether the animation is active or not without also checking whether the currentTime is less than the start time. We could consider adding another member to animation to reflect this in the future.

Ultimately, I think either works, but which is better comes back to how authors will use this. @bramus @fantasai @birtles any thoughts?

ydaniv commented 8 months ago

I'm leaning towards 2, since this will probably be used to sync a scroll/view timeline with another effect that can't be done with CSS, e.g.: canvas, SVG, video.

So, I think that as default behavior, having this perform as a sort of "manual" CustomEffect, with a clamped progress like KeyframeEffect's is a good way to start, and we could add a new property in the future, like you said, to reflect the more complex cases.

birtles commented 8 months ago

I agree that 2 makes most sense. I also think, as per the current PR, we shouldn't try to reflect the fill mode of the associated effect.

ydaniv commented 8 months ago

However, if we consider the above use-cases, having enter/exit events on the animations, similar requested by @bramus on #7962 may become quite crucial for easily creating such effects without extra IntersectionObserver or scroll handler. Although you can currently get a similar outcome with start/end events on CSS Animations like here: https://jsbin.com/jizahopixe/edit?css,js,console,output I think it's currently not possible to achieve with WAAPI.

ydaniv commented 8 months ago

We could also resolve on adding start/end events on elements as per #9011 which should also help there

DavMila commented 2 months ago

I'll agenda+ this issue so we can get a formal resolution on the question on whether to clamp to [0,1].

ydaniv commented 2 months ago

Perhaps we should extract the idea of checking/specifying an effect in a before/after state? Currently it requires hacks to achieve this, like setting delay: 1ms; play-state: paused for before. For WAAPI you have to play and pause immediately.

ydaniv commented 2 months ago

If it helps, I can add an example from a research we started on doing exactly what I mentioned above:

I'm leaning towards 2, since this will probably be used to sync a scroll/view timeline with another effect that can't be done with CSS, e.g.: canvas, SVG, video.

Here: https://codepen.io/ydaniv/pen/VwNraKB - trying to scrub a <video>'s progress in-sync with a ScrollTimeline (spoiler: it doesn't work, at least OOTB). This is the kind of usages I think we'll mostly see for this API.

css-meeting-bot commented 1 month ago

The CSS Working Group just discussed [web-animations-2] Progress APIs, and agreed to the following:

The full IRC log of that discussion <fserb> github-bot take up https://github.com/w3c/csswg-drafts/issues/8799
<fantasai> astearns: remaining question is whether we are clamping
<fantasai> flackr: we decided to add a progress accessor to animations
<fantasai> flackr: question was what to do outside the range of the animation
<fantasai> flackr: before animation starts or after it ends
<fantasai> flackr: I believe everyone is agreed that clamping is what makes sense, so want to resolve on it
<fantasai> flackr: [0%, 100%]
<fantasai> flackr: maps to the keyframe you're showing, assuming animation is in effect
<fantasai> astearns: any disagreement?
<fantasai> RESOLVED: Clamp progress to [0,1]
DavMila commented 3 weeks ago

Got some feedback that having AnimationEffect.getComputedTiming().progress and Animation.progress which mean different things could be confusing. Could we consider renaming progress? Perhaps totalProgress to reflect that it accounts for all the animation's iterations as opposed to AnimationEffect.getComputedTiming().progress which only accounts for the current iteration's progress.

flackr commented 3 weeks ago

Perhaps currentProgress would better align it with currentTime? I think we should also consider that if we want to later expose the effect phase (i.e. before/active/after), so if we went with currentProgress, currentPhase would make sense.

bramus commented 3 weeks ago

I like Rob’s suggestion

ydaniv commented 3 weeks ago

I don't know, IMHO, currentProgress looks awkward. And don't really think that those specifically meed to be consistent. I've seen usage of progress in libraries before. time by itself could be ambiguous, like start orend, but don't think progress is.

jyasskin commented 2 weeks ago

The concern isn't that progress itself is confusing, it's that myAnimation.effect.getComputedTiming().progress means one kind of progress, while myAnimation.progress means a different kind, and nothing in the names indicates the difference. I actually think it'd be ideal for getComputedTiming().progress to mention that it's the iteration's progress, but since that has been shipping for several years, I don't see much prospect for getting it renamed.

I'm not sure that currentProgress expresses the relevant difference here either (where totalProgress would), but the WG knows better than I do what context authors are likely to have when reading this code, and whether "current" is going to inspire the right intuition.

css-meeting-bot commented 2 weeks ago

The CSS Working Group just discussed [web-animations-2] Progress APIs, and agreed to the following:

The full IRC log of that discussion <TabAtkins> DavidA: we added a property field, it reflects the progress of the animation in a way that reflects the iterations of the animation
<TabAtkins> DavidA: there is a .progress that exists in AnimationEffect that has a .getComputedTiming() function
<TabAtkins> DavidA: but that only reflects the progress of the current animation
<TabAtkins> DavidA: got some feedback that it coudl be confusing to share the name since they mean different things
<TabAtkins> DavidA: wanted to consider renaming the Animation progress field either totalProgress or currentProgress
<TabAtkins> DavidA: not sure which is better to go with
<TabAtkins> DavidA: currentProgress is related to currentTime
<TabAtkins> DavidA: but totalProgress more strongly reflects taht it covers all the iterations
<TabAtkins> fantasai: okay so it's a .progress on AnimationEffect...
<TabAtkins> DavidA: no, on Animation itself
<TabAtkins> fantasai: is there one on AnimationEffect?
<TabAtkins> DavidA: indirectly, kinda
<DavidA> AnimationEffect.getComputedTiming().progress
<TabAtkins> vmpstr: yehonatan also said he thinks progress if fine and he'd find currentProgress confusing
<dholbert> https://www.w3.org/TR/web-animations-1/#calculating-the-overall-progress
<TabAtkins> dholbert: WebAnim 1 uses overallProgress
<TabAtkins> dholbert: it differentiates that from "simple" iteration progress
<fantasai> https://drafts.csswg.org/web-animations-1/#dictdef-computedeffecttiming
<TabAtkins> astearns: "overall" is just a name in the algo, not part of the exposed API?
<TabAtkins> dholbert: yeah
<fantasai> .endTie
<fantasai> .endTime
<fantasai> .activeDuration
<fantasai> .localTime
<fantasai> .progress
<TabAtkins> fantasai: so this is on an object called ComputedEffectTiming, which has...
<fantasai> .currentIteration
<TabAtkins> fantasai: and we're suggesting renaming .progress
<TabAtkins> fantasai: we currently have .activeDuration, .localTime, .currentIteration
<TabAtkins> DavidA: Oh, not the progress field on this object
<TabAtkins> DavidA: The one on the Animation class
<astearns> I kind of like `overallProgress`
<TabAtkins> yeah i'm kinda leaning toward overall
<fantasai> https://drafts.csswg.org/web-animations-1/#the-animation-interface
<TabAtkins> DavidA: .progress on ComputedEffectTiming has existed for a while, changing woudl be hard
<astearns> s/ComputedEffectTiming/getComputedTiming/
<TabAtkins> fantasai: can you string multiple Animations together?
<TabAtkins> DavidA: I'm not sure...
<TabAtkins> dholbert: i'm slightly uneasy about "total" because it sounds like a summation
<TabAtkins> +1
<fantasai> +1
<TabAtkins> astearns: I'd expect "total progress" to not change over time
<TabAtkins> DavidA: okay so overallProgress is still something we could consider, or currentProgress
<TabAtkins> fantasai: since timingeffect has currentTime and progress, what's the problem with just using progress on this?
<fantasai> s/currentTime/localTime/
<TabAtkins> fantasai: you'd get the context off of which object you're getting from
<astearns> request for something other than `progress`: https://github.com/w3ctag/design-reviews/issues/994#issuecomment-2427287323
<TabAtkins> astearns: [summarizes Jeffrey's comment]
<fantasai> TabAtkins: effect timing can run an animation multipel times, and progress gives you progress of the iteration
<fantasai> TabAtkins: this is a 0-1 and done thing, so it's a different concept than what the effect timing one is
<fantasai> TabAtkins: can't change that one, but maybe use a different word here
<TabAtkins> fantasai: i don't mind overall progress, i'm just saying we both have startTimes, they mean different things
<fantasai> fantasai: can tell from context
<fantasai> TabAtkins: progress has a different interpretation, because [missed]
<fantasai> TabAtkins: AnimationEffect has a .endTime
<fantasai> TabAtkins: Effect timing and animation have same interpretation of those
<dbaron> (last 2 lines were answering vmpstr asking about whether startTime has the same meaning on both)
<fantasai> TabAtkins: Animation has a .startTime and no .endTime / AnimationEffect has .endTime and no .startTime
<fantasai> vmpstr: Progress, one cycles per iteration, so wouldn't be the same value at any particular point
<fantasai> TabAtkins: for progress, right
<fantasai> vmpstr: but if startTime and endTime both existed on the same object, they would be the same, right?
<fantasai> TabAtkins: AFAICT from a quick read, they refer to the same type of concept
<fantasai> TabAtkins: it's beginning of whole thing it's doing, vs iteratin
<fantasai> astearns: gist I'm getting, is ppl think there's enough difference to have a different name
<TabAtkins> fantasai: i'm okay with overallProgress
<TabAtkins> astearns: so proposed resolution is we change Animation.progress to Animation.overallProgress
<TabAtkins> RESOLVED: change Animation.progress to Animation.overallProgress
yisibl commented 1 week ago

Can we add these APIs to CSS?

ydaniv commented 1 week ago

Can we add these APIs to CSS?

Do you mean to the CSSAnimation interface? It inherits from Animation, so naturally yes.

graouts commented 7 hours ago

I'm making the required WPT change also in https://github.com/web-platform-tests/wpt/pull/49353.