Bug 79048

Summary: Enable compositing when a CSS filter is animating
Product: WebKit Reporter: Dean Jackson <dino>
Component: CSSAssignee: Dean Jackson <dino>
Severity: Normal CC: dglazkov, simon.fraser, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
WK2 No Worky Testcase
Patch simon.fraser: review+, webkit.review.bot: commit-queue-

Description Dean Jackson 2012-02-20 15:05:53 PST
Now that we have hardware accelerated filters, and support for animation (at least on OS X), we should turn on compositing for animated filters.
Comment 1 Dean Jackson 2012-02-20 15:06:10 PST
Comment 2 Dean Jackson 2012-02-20 15:28:44 PST
Created attachment 127858 [details]
Comment 3 Dean Jackson 2012-02-20 15:38:45 PST
This patch seems too easy (but it works as verified by CA_PRINT_TREE). Simon, have I missed something?
Comment 4 Simon Fraser (smfr) 2012-02-20 16:05:36 PST
Comment on attachment 127858 [details]

Don't you need to test the layer output (layerTreeAsText())?.  You'll also have to be careful that the first animating thing doesn't throw the rest into layers for the test.
Comment 5 Dean Jackson 2012-02-20 17:50:47 PST
(In reply to comment #4)
> (From update of attachment 127858 [details])
> Don't you need to test the layer output (layerTreeAsText())?. 

Added a new test for that, but I've run into a problem.

> You'll also have to be careful that the first animating thing doesn't throw the rest into layers for the test.

I've added a couple of non-filtered elements.

The problem is that the fix doesn't work in WebKit2. It works fine in WK1, but with WK2 the layer updates don't get painted. I do see the object jump into compositing mode, but only repaint if I do something tricky in the inspector (like highlight the object, then it paints that frame once).

I've attached a simple example that exercises the feature. Hover over the first blue rectangle and it should animate. Doesn't work in WK2.
Comment 6 Dean Jackson 2012-02-20 17:52:12 PST
Created attachment 127879 [details]
Comment 7 Dean Jackson 2012-02-20 17:53:08 PST
Created attachment 127880 [details]
WK2 No Worky Testcase
Comment 8 Dean Jackson 2012-03-05 14:40:06 PST
Created attachment 130206 [details]
Comment 9 WebKit Review Bot 2012-03-05 14:43:17 PST
Attachment 130206 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/css3..." exit_code: 1
Source/WebCore/rendering/RenderLayerCompositor.cpp:1514:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 7 files

If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Dean Jackson 2012-03-05 15:01:06 PST
Style checker failure is because I have the semicolon for the return statement on a blank line, due to the #if. I think that's ok.
Comment 11 WebKit Review Bot 2012-03-05 16:02:32 PST
Comment on attachment 130206 [details]

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

New failing tests:
Comment 12 Dean Jackson 2012-03-05 17:03:30 PST
Comment on attachment 130206 [details]

Patch no worky in the case where an element is already composited. Also, layer tree output is platform-specific (as shown in failing test for chromium)
Comment 13 Dean Jackson 2012-03-05 21:37:41 PST
Created attachment 130281 [details]
Comment 14 Dean Jackson 2012-03-05 21:39:00 PST
The platform that will produce interesting results for the layout tests is mac-future, but those results are not included here.
Comment 15 WebKit Review Bot 2012-03-05 22:31:02 PST
Comment on attachment 130281 [details]

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

New failing tests:
Comment 16 Dean Jackson 2012-03-06 12:53:09 PST
I'm not sure why the chromium tests are failing. Since the results are platform specific anyway, I'll put them in the platform dir.
Comment 17 Dean Jackson 2012-03-06 13:01:38 PST