Bug 74049 - Enable animations of CSS images using -webkit-cross-fade
Summary: Enable animations of CSS images using -webkit-cross-fade
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-12-07 19:49 PST by Tim Horton
Modified: 2011-12-08 15:09 PST (History)
4 users (show)

See Also:


Attachments
initial patch (168.63 KB, patch)
2011-12-07 20:01 PST, Tim Horton
simon.fraser: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2011-12-07 19:49:06 PST
Using -webkit-cross-fade, we can allow -webkit-animation between background-image, list-style-image, border-image, and -webkit-mask-(box-)image (If I missed any, let me know!)

<rdar://problem/10209303>
Comment 1 Tim Horton 2011-12-07 20:01:08 PST
Created attachment 118315 [details]
initial patch

I think I still need more tests, but this is a start.
Comment 2 WebKit Review Bot 2011-12-07 21:15:36 PST
Comment on attachment 118315 [details]
initial patch

Attachment 118315 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10800082

New failing tests:
animations/cross-fade-webkit-mask-image.html
animations/cross-fade-webkit-mask-box-image.html
animations/cross-fade-background-image.html
animations/cross-fade-border-image-source.html
animations/cross-fade-list-style-image.html
Comment 3 Simon Fraser (smfr) 2011-12-08 11:12:59 PST
Comment on attachment 118315 [details]
initial patch

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

r=me but some transition tests wouldn't hurt.

> LayoutTests/animations/cross-fade-background-image.html:37
> +    @-webkit-keyframes "anim" {
> +        from { background-image: url(resources/blue-100.png); }
> +        to   { background-image: url(resources/green-100.png); }
> +    }
> +    @-webkit-keyframes "animShorthand" {
> +        from { background: url(resources/blue-100.png); }
> +        to   { background: url(resources/green-100.png); }

Animation names are IDENT, not STRING, so should not be quoted.

> LayoutTests/animations/resources/animation-test-helpers.js:54
> +	var matches = s.match("-webkit-cross-fade\\((.*)\\s*,\\s*(.*)\\s*,\\s*(.*)\\)");

Is the double escaping necessary?

> Source/WebCore/css/CSSCrossfadeValue.cpp:112
> +    if (fromImageSize.width() == toImageSize.width() && fromImageSize.height() == toImageSize.height())

Doesn't IntSize have an operator== ?

> Source/WebCore/page/animation/AnimationBase.cpp:239
> +    if (progress == 1.)

1. -> 1 or 1.0

> Source/WebCore/page/animation/AnimationBase.cpp:251
> +    RefPtr<StyleGeneratedImage> generatedImage = StyleGeneratedImage::create(crossfadeValue.get());

Could just return StyleGeneratedImage::create(crossfadeValue.get());

> Source/WebCore/page/animation/AnimationBase.cpp:274
> +    // FIXME: Support transitioning between NinePieceImages that differ by more than image content.

Please file a bug for this.

> Source/WebCore/page/animation/AnimationBase.cpp:286
> +    return newNinePieceImage;

return NinePieceImage(....) ?
Comment 4 Tim Horton 2011-12-08 15:09:43 PST
Landed with Simon's changes and two transition tests in http://trac.webkit.org/changeset/102388.