Bug 111905 - [CA] Animations of CSS filters don't work correctly
Summary: [CA] Animations of CSS filters don't work correctly
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:
Blocks:
 
Reported: 2013-03-08 17:19 PST by Simon Fraser (smfr)
Modified: 2013-03-12 10:57 PDT (History)
6 users (show)

See Also:


Attachments
Testcase (2.58 KB, text/html)
2013-03-08 20:42 PST, Simon Fraser (smfr)
no flags Details
More complete testcase (2.80 KB, text/html)
2013-03-08 23:11 PST, Simon Fraser (smfr)
no flags Details
Patch (81.32 KB, patch)
2013-03-10 23:10 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (81.30 KB, patch)
2013-03-11 08:57 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (81.29 KB, patch)
2013-03-11 10:55 PDT, Simon Fraser (smfr)
dino: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2013-03-08 17:19:31 PST
Transitioning filters to/from filter: none should show a nice smooth progression from no filter to the applied filter. It doesn't; it either jumps, or animates jankily.
Comment 1 Radar WebKit Bug Importer 2013-03-08 17:20:21 PST
<rdar://problem/13383975>
Comment 2 Dirk Schulze 2013-03-08 19:17:31 PST
What happens with a SVG Filter reference? Do we just snap to another filter after the half of the time?
Comment 3 Simon Fraser (smfr) 2013-03-08 20:42:11 PST
Created attachment 192331 [details]
Testcase
Comment 4 Simon Fraser (smfr) 2013-03-08 20:57:46 PST
We're adding filters to layers OK:
contents (surface 8 [200 200] BGRA8888))
                                                          (filters (array
                                                            (filter colorMonochrome
                                                              (inputs (array
                                                                (inputAmount (vector 0.999753058)))))))
                                                          (animations (array
                                                            (basic-animation
                                                              (timing
                                                                (beginTime 2488.306979)
                                                                (duration 1.000000)
                                                                (fillMode forwards)
                                                                (repeatCount 1.000000))
                                                              (timingFunction (vector 0.25 0.1000000015 0.25 1))
                                                              (removedOnCompletion false)
                                                              (keyPath filters.filter_0.inputIntensity)
                                                              (from (vector 1))
                                                              (to (vector 0)

but I think the key path is wrong:  filters.filter_0.inputIntensity)
Comment 5 Simon Fraser (smfr) 2013-03-08 21:07:20 PST
I think the keyPath should be [filterType]-[index]
Comment 6 Simon Fraser (smfr) 2013-03-08 21:27:12 PST
The filter properties returned by PlatformCAAnimation::animatedFilterPropertyName() are also wrong.
Comment 7 Dean Jackson 2013-03-08 22:34:13 PST
I screwed up when we moved to CAFilters and forgot to change PlatformCAAnimation stuff. Simon is fixing this right now.
Comment 8 Simon Fraser (smfr) 2013-03-08 23:11:41 PST
Created attachment 192337 [details]
More complete testcase
Comment 9 Simon Fraser (smfr) 2013-03-09 11:22:47 PST
The sepia values for CI filters seem way off, and the matrix we're using doesn't seem to correspond to https://dvcs.w3.org/hg/FXTF/raw-file/tip/filters/index.html#sepiaEquivalent
Comment 10 Simon Fraser (smfr) 2013-03-09 11:25:57 PST
CI invert seems broken too.
Comment 11 Simon Fraser (smfr) 2013-03-09 11:30:17 PST
CI opacity animations were also broken (need to use a CIVector).
Comment 12 Simon Fraser (smfr) 2013-03-09 11:44:07 PST
And I think we'll have to do brightness in CI as a color matrix. CI does brightness in color adjust by just adding/subtracting the RGB values, which doesn't match the slope/intersect stuff that the spec says.
Comment 13 Dirk Schulze 2013-03-09 21:21:01 PST
(In reply to comment #12)
> And I think we'll have to do brightness in CI as a color matrix. CI does brightness in color adjust by just adding/subtracting the RGB values, which doesn't match the slope/intersect stuff that the spec says.

It sounds a bit like "how could that ever work" :P
Comment 14 Simon Fraser (smfr) 2013-03-10 23:10:12 PDT
Created attachment 192405 [details]
Patch
Comment 15 Simon Fraser (smfr) 2013-03-11 08:57:05 PDT
Created attachment 192479 [details]
Patch
Comment 16 Build Bot 2013-03-11 10:13:43 PDT
Comment on attachment 192479 [details]
Patch

Attachment 192479 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17121240
Comment 17 Simon Fraser (smfr) 2013-03-11 10:55:05 PDT
Created attachment 192506 [details]
Patch
Comment 18 Dean Jackson 2013-03-11 14:12:23 PDT
Comment on attachment 192506 [details]
Patch

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

> Source/WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm:387
> +    NSLog(@"Adding animation %@ for key %@", propertyAnimation, (NSString*)key);

Oops.
Comment 19 Simon Fraser (smfr) 2013-03-12 10:57:54 PDT
http://trac.webkit.org/changeset/145472 and followups