Bug 179973 – [Web Animations] Perform accelerated animations when possible

• Component: Animations
• Status: RESOLVED FIXED

[Antoine Quint]

Certain layer properties (opacity, transform, filter, etc.) can be animated using hardware compositing when using CSS Animations, but not currently with Web Animations. It should be possible to use the same approach used for CSS Animations with Web Animations and have the same performance gains.

[Antoine Quint]

<rdar://problem/34953922>

[Antoine Quint]

Created attachment 327498 [details]
Patch

[Dean Jackson]

Comment on attachment 327498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=327498&action=review

I know you're unsure about the term "hardware compositing" vs "accelerated". I think more people are familiar with the term accelerated, or maybe just "composited".

> Source/WebCore/animation/DocumentTimeline.cpp:170
> +        animation->toggleHardwareCompositing();

I don't like the name "toggle", because it's not clear what is actually happening. Is this turning it on or off? Or does it depend on the animation?

> Source/WebCore/animation/DocumentTimeline.cpp:182
> +bool DocumentTimeline::elementRunningAnimationsAreAllHardwareComposited(Element& element)

This is also a weird name. It's asking if the Element has only hardware composited animations. Maybe elementUsingHardwareCompositingForAnimations?

> Source/WebCore/animation/KeyframeEffect.cpp:150
> +    if (m_startedHardwareCompositedAnimation && localTime >= timing()->duration()) {

I wonder if relativeTime is a better name than localTime. I had to think a bit to work out what localTime >= duration meant (because it makes no sense to compare a general time to a duration).

> Source/WebCore/animation/KeyframeEffect.cpp:184
> +    if (needsToStartWithHardwareCompositing)
> +        animation()->hardwareCompositingActiveStateDidChange();

I assume you have to call this after you flip the Z index? Otherwise it makes more sense to go up where you set needsToStartWithHardwareCompositing.

[Antoine Quint]

(In reply to Dean Jackson from comment #3)
> Comment on attachment 327498 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=327498&action=review
> 
> I know you're unsure about the term "hardware compositing" vs "accelerated".
> I think more people are familiar with the term accelerated, or maybe just
> "composited".

I definitely don't want to use "composited" because of the notion of "effect composition" in Web Animations (https://w3c.github.io/web-animations/#effect-composition). I think accelerated is the way to go. I'll rename everything to use that term instead.

> > Source/WebCore/animation/DocumentTimeline.cpp:170
> > +        animation->toggleHardwareCompositing();
> 
> I don't like the name "toggle", because it's not clear what is actually
> happening. Is this turning it on or off? Or does it depend on the animation?

It turns it on if it's off, and off if it's on, hence the use of "toggle". Specifically, it starts the accelerated animation if it's not running yet, and it ends it if it's already running.

> > Source/WebCore/animation/DocumentTimeline.cpp:182
> > +bool DocumentTimeline::elementRunningAnimationsAreAllHardwareComposited(Element& element)
> 
> This is also a weird name. It's asking if the Element has only hardware
> composited animations. Maybe elementUsingHardwareCompositingForAnimations?

I think areAllRunningAnimationsForElementAccelerated()?

> > Source/WebCore/animation/KeyframeEffect.cpp:150
> > +    if (m_startedHardwareCompositedAnimation && localTime >= timing()->duration()) {
> 
> I wonder if relativeTime is a better name than localTime. I had to think a
> bit to work out what localTime >= duration meant (because it makes no sense
> to compare a general time to a duration).

The term "local time" comes from the spec (https://w3c.github.io/web-animations/#local-time-section).

> > Source/WebCore/animation/KeyframeEffect.cpp:184
> > +    if (needsToStartWithHardwareCompositing)
> > +        animation()->hardwareCompositingActiveStateDidChange();
> 
> I assume you have to call this after you flip the Z index? Otherwise it
> makes more sense to go up where you set needsToStartWithHardwareCompositing.

Actually, we don't. This code was there because the needsToStartWithHardwareCompositing was the return value for a previous iteration of this patch. Now we no longer use a return value, I can set it earlier as suggested.

[Antoine Quint]

Committed r225128: <https://trac.webkit.org/changeset/225128>