Bug 68476 - Make -webkit-filter animatable
Summary: Make -webkit-filter animatable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on: 68475
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-20 14:25 PDT by Dean Jackson
Modified: 2011-12-14 13:16 PST (History)
8 users (show)

See Also:


Attachments
Work in progresss. Only blur() animates. (14.67 KB, patch)
2011-12-11 23:04 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Testcase (2.52 KB, text/html)
2011-12-12 18:06 PST, Simon Fraser (smfr)
no flags Details
Fixed testcase (2.52 KB, text/html)
2011-12-13 22:57 PST, Simon Fraser (smfr)
no flags Details
Patch (33.21 KB, patch)
2011-12-13 23:13 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (33.47 KB, patch)
2011-12-13 23:27 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (33.62 KB, patch)
2011-12-14 08:38 PST, Simon Fraser (smfr)
cmarrin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2011-09-20 14:25:47 PDT
We'll need some logic to animate filters, similar to the way transforms animate.
Comment 1 Radar WebKit Bug Importer 2011-09-20 14:26:30 PDT
<rdar://problem/10155818>
Comment 2 Simon Fraser (smfr) 2011-12-11 23:04:46 PST
Created attachment 118736 [details]
Work in progresss. Only blur() animates.
Comment 3 Dean Jackson 2011-12-12 10:09:12 PST
Cool
Comment 4 Simon Fraser (smfr) 2011-12-12 18:06:39 PST
Created attachment 118928 [details]
Testcase
Comment 5 Simon Fraser (smfr) 2011-12-13 22:57:09 PST
Created attachment 119158 [details]
Fixed testcase
Comment 6 Simon Fraser (smfr) 2011-12-13 23:13:14 PST
Created attachment 119160 [details]
Patch
Comment 7 WebKit Review Bot 2011-12-13 23:17:58 PST
Comment on attachment 119160 [details]
Patch

Attachment 119160 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10871121
Comment 8 Early Warning System Bot 2011-12-13 23:19:21 PST
Comment on attachment 119160 [details]
Patch

Attachment 119160 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10876139
Comment 9 Simon Fraser (smfr) 2011-12-13 23:27:47 PST
Created attachment 119166 [details]
Patch
Comment 10 WebKit Review Bot 2011-12-13 23:52:52 PST
Comment on attachment 119166 [details]
Patch

Attachment 119166 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10872146
Comment 11 Gyuyoung Kim 2011-12-14 00:51:30 PST
Comment on attachment 119166 [details]
Patch

Attachment 119166 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10874155
Comment 12 Gustavo Noronha (kov) 2011-12-14 02:10:20 PST
Comment on attachment 119166 [details]
Patch

Attachment 119166 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10872192
Comment 13 Early Warning System Bot 2011-12-14 03:05:09 PST
Comment on attachment 119166 [details]
Patch

Attachment 119166 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10871180
Comment 14 Csaba Osztrogonác 2011-12-14 03:26:24 PST
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
Comment 15 Simon Fraser (smfr) 2011-12-14 08:38:55 PST
Created attachment 119224 [details]
Patch
Comment 16 Simon Fraser (smfr) 2011-12-14 09:40:56 PST
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;
         }
Comment 17 Chris Marrin 2011-12-14 09:58:58 PST
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?
Comment 18 Simon Fraser (smfr) 2011-12-14 13:16:53 PST
Landed with a test: 
http://trac.webkit.org/changeset/102815