Bug 77662

Summary: [chromium] Add impl-thread support for fill-mode and direction css animation properties
Product: WebKit Reporter: vollick
Component: WebKit Misc.Assignee: vollick
Status: RESOLVED FIXED    
Severity: Normal CC: cc-bugs, jamesr, nduca, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 77646, 79819    
Bug Blocks: 85813    
Attachments:
Description Flags
Patch
none
Patch none

Description vollick 2012-02-02 11:38:08 PST
There are css animation properties that aren’t yet supported by the cc-thread animation code. If we detect an unsupported feature, it is possible to return false in GraphicsLayer::addAnimation to revert to the usual, main-thread animation code, but we should support most/all css animation properties. Specifically: animation-direction, animation-delay, animation-timing-function, and animation-fill-mode.
Comment 1 vollick 2012-03-15 19:49:33 PDT
This should not block turning the feature on by default, since we gracefully degrade.
Comment 2 vollick 2012-04-03 10:23:15 PDT
Created attachment 135354 [details]
Patch
Comment 3 James Robinson 2012-04-03 11:15:01 PDT
Comment on attachment 135354 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [chromium] Add impl-thread support for all css animation properties.

I think "all" is a bit ambitious. Can you say specifically what this patch is adding support for?

> Source/WebKit/chromium/src/TextFieldDecoratorImpl.cpp:36
> +#include "Image.h"

this looks unrelated

> Source/WebKit/chromium/tests/CCActiveAnimationTest.cpp:72
> +TEST(CCActiveAnimationTest, TrimTimeAlternating)

is this sufficient test coverage?  should we have some tests for reverse as well?
Comment 4 vollick 2012-05-09 08:25:10 PDT
Created attachment 140950 [details]
Patch

(In reply to comment #3)
> (From update of attachment 135354 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135354&action=review
>
> > Source/WebCore/ChangeLog:3
> > +        [chromium] Add impl-thread support for all css animation properties.
>
> I think "all" is a bit ambitious. Can you say specifically what this patch is adding support for?
Done.
>
> > Source/WebKit/chromium/src/TextFieldDecoratorImpl.cpp:36
> > +#include "Image.h"
>
> this looks unrelated
Removed.
>
> > Source/WebKit/chromium/tests/CCActiveAnimationTest.cpp:72
> > +TEST(CCActiveAnimationTest, TrimTimeAlternating)
>
> is this sufficient test coverage?  should we have some tests for reverse as well?
It isn't. Added tests to cover reverse.
Comment 5 James Robinson 2012-05-09 11:47:35 PDT
Comment on attachment 140950 [details]
Patch

Cool, R=me. Thanks for the extra tests
Comment 6 WebKit Review Bot 2012-05-09 13:26:18 PDT
Comment on attachment 140950 [details]
Patch

Clearing flags on attachment: 140950

Committed r116554: <http://trac.webkit.org/changeset/116554>
Comment 7 WebKit Review Bot 2012-05-09 13:26:22 PDT
All reviewed patches have been landed.  Closing bug.