Bug 68476 – Make -webkit-filter animatable

• Component: CSS
• Status: RESOLVED FIXED

[Dean Jackson]

We'll need some logic to animate filters, similar to the way transforms animate.

[Radar WebKit Bug Importer]

<rdar://problem/10155818>

[Simon Fraser (smfr)]

Created attachment 118736 [details]
Work in progresss. Only blur() animates.

[Dean Jackson]

Cool

[Simon Fraser (smfr)]

Created attachment 118928 [details]
Testcase

[Simon Fraser (smfr)]

Created attachment 119158 [details]
Fixed testcase

[Simon Fraser (smfr)]

Created attachment 119160 [details]
Patch

[WebKit Review Bot]

Comment on attachment 119160 [details]
Patch

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

[Early Warning System Bot]

Comment on attachment 119160 [details]
Patch

Attachment 119160 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10876139

[Simon Fraser (smfr)]

Created attachment 119166 [details]
Patch

[WebKit Review Bot]

Comment on attachment 119166 [details]
Patch

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

[Gyuyoung Kim]

Comment on attachment 119166 [details]
Patch

Attachment 119166 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10874155

[Gustavo Noronha (kov)]

Comment on attachment 119166 [details]
Patch

Attachment 119166 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10872192

[Early Warning System Bot]

Comment on attachment 119166 [details]
Patch

Attachment 119166 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10871180

[Csaba Osztrogonác]

Comment on attachment 119166 [details]
Patch

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

> Source/WebCore/Target.pri:3265
>          rendering/style/FilterOperations.cpp
> +        rendering/style/FilterOperation.cpp

rendering/style/FilterOperations.cpp \
rendering/style/FilterOperation.cpp

[Simon Fraser (smfr)]

Created attachment 119224 [details]
Patch

[Simon Fraser (smfr)]

Patch needs:

diff --git a/Source/WebCore/page/animation/KeyframeAnimation.cpp b/Source/WebCore/page/animation/KeyframeAnimation.cpp
index 9393e4a..f0231e2 100644
--- a/Source/WebCore/page/animation/KeyframeAnimation.cpp
+++ b/Source/WebCore/page/animation/KeyframeAnimation.cpp
@@ -452,7 +452,7 @@ void KeyframeAnimation::checkForMatchingFilterFunctionLists()
 
     for (size_t i = 0; i < numKeyframes; ++i) {
         const KeyframeValue& currentKeyframe = m_keyframes[i];
-        if (currentKeyframe.style()->transform().operations().size()) {
+        if (currentKeyframe.style()->filter().operations().size()) {
             firstNonEmptyFilterKeyframeIndex = i;
             break;
         }

[Chris Marrin]

Comment on attachment 119224 [details]
Patch

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

Looks good. Just a few nits

> Source/WebCore/page/animation/AnimationBase.cpp:201
> +                RefPtr<FilterOperation> identityOp = PassthroughFilterOperation::create();

You should avoid creating this object when not needed

> Source/WebCore/page/animation/ImplicitAnimation.cpp:273
> +    // Transform lists match.

Add a FIXME here so we can change this variable and get rid of this useless comment when isTransformFunctionListValid gets renamed

> Source/WebCore/rendering/style/FilterOperation.cpp:35
> +PassRefPtr<FilterOperation> BasicColorMatrixFilterOperation::blend(const FilterOperation* from, double progress, bool blendToPassthrough)

Why "Basic"? I don't see any other forms of this object that would cause you to have to discriminate it like this.

> Source/WebCore/rendering/style/FilterOperation.cpp:63
> +PassRefPtr<FilterOperation> BasicComponentTransferFilterOperation::blend(const FilterOperation* from, double progress, bool blendToPassthrough)

Same here

> Source/WebCore/rendering/style/FilterOperation.h:-113
> -

Spurious change?

[Simon Fraser (smfr)]

Landed with a test: 
http://trac.webkit.org/changeset/102815