Bug 58054 - REGRESSION (WebKit2): Accelerated CSS animations have a lower framerate than in WebKit1
Summary: REGRESSION (WebKit2): Accelerated CSS animations have a lower framerate than ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Adam Roben (:aroben)
URL: http://webkit.org/blog-files/3d-trans...
Keywords: InRadar, PlatformOnly, Regression
: 58052 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-04-07 09:58 PDT by Adam Roben (:aroben)
Modified: 2011-06-01 11:55 PDT (History)
5 users (show)

See Also:


Attachments
Move WebView's window geometry updating code to a new class (10.86 KB, patch)
2011-06-01 08:07 PDT, Adam Roben (:aroben)
andersca: review+
Details | Formatted Diff | Diff
Route plugin window geometry updates through the DrawingArea (7.97 KB, patch)
2011-06-01 08:23 PDT, Adam Roben (:aroben)
andersca: review+
Details | Formatted Diff | Diff
Render accelerated content into a web process-owned child HWND (34.91 KB, patch)
2011-06-01 09:53 PDT, Adam Roben (:aroben)
no flags Details | Formatted Diff | Diff
Render accelerated content into a web process-owned child HWND (Take 2) (40.18 KB, patch)
2011-06-01 10:55 PDT, Adam Roben (:aroben)
andersca: review+
Details | Formatted Diff | Diff
Delete a bunch of dead code in DrawingAreaImpl (13.82 KB, patch)
2011-06-01 11:22 PDT, Adam Roben (:aroben)
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 2011-04-07 09:58:58 PDT
To reproduce:

1. Go to http://webkit.org/blog-files/3d-transforms/poster-circle.html in WebKit2, and compare to WebKit1

The framerate in WebKit2 is slower than in WebKit1. WebKit1 is running basically at 60fps continuously, while WebKit2 is somewhere closer to 30 or 40fps.
Comment 1 Adam Roben (:aroben) 2011-04-07 09:59:18 PDT
Fixing bug 58052 and/or bug 58053 would likely help with this.
Comment 2 Adam Roben (:aroben) 2011-04-07 10:00:34 PDT
<rdar://problem/9249839>
Comment 3 Adam Roben (:aroben) 2011-05-30 06:59:23 PDT
Kenneth Russell explained to me the strategy that Chrome is using, and I think something similar could work for WebKit2. The basic idea is that the web process would create an HWND that is a child of the WKView HWND. The web process would render into the child HWND directly using D3D. The UI process would be in charge of sizing the child HWND so that it completely covers the WKView HWND. The child HWND would be transparent to mouse events so that WKView's event handling can proceed normally.
Comment 4 Adam Roben (:aroben) 2011-05-30 07:03:09 PDT
Probably the trickiest thing with this approach will be making windowed plugins work correctly. They either need to be children of the GPU window, or be in front of it in the Z-order. And we need to make sure they still get scrolled correctly.
Comment 5 Adam Roben (:aroben) 2011-05-30 07:23:17 PDT
Placing plugins in front of the GPU window in the z-order seems like the simplest solution. That way we don't have to reparent plugins when we enter/exit accelerated compositing mode.
Comment 6 Simon Fraser (smfr) 2011-05-30 08:26:41 PDT
(In reply to comment #5)
> Placing plugins in front of the GPU window in the z-order seems like the simplest solution. That way we don't have to reparent plugins when we enter/exit accelerated compositing mode.

Will this cause any new rendering regressions: failure to be clipped by compositing layers, failure to be overlapped by compositing layers?
Comment 7 Adam Roben (:aroben) 2011-05-30 08:29:37 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > Placing plugins in front of the GPU window in the z-order seems like the simplest solution. That way we don't have to reparent plugins when we enter/exit accelerated compositing mode.
> 
> Will this cause any new rendering regressions: failure to be clipped by compositing layers, failure to be overlapped by compositing layers?

I don't think so; we clip plugins using window regions based on their render tree geometry, and that should continue to work even if they are in front of the GPU window. I would love to try some test cases if you have them!
Comment 8 Adam Roben (:aroben) 2011-05-30 08:30:18 PDT
I should note that windowed plugins are currently always clipped to a rectangular area. Non-rectangular clipping regions aren't supported at the moment.
Comment 9 Adam Roben (:aroben) 2011-05-30 11:24:04 PDT
(In reply to comment #4)
> And we need to make sure [windowed plugins] still get scrolled correctly.

This is a little tricky to get right. We need to scroll them when the WKCACFView renders to the screen, which can happen on a background thread.
Comment 10 Adam Roben (:aroben) 2011-05-31 13:59:15 PDT
(In reply to comment #9)
> (In reply to comment #4)
> > And we need to make sure [windowed plugins] still get scrolled correctly.
> 
> This is a little tricky to get right. We need to scroll them when the WKCACFView renders to the screen, which can happen on a background thread.

I tried making WKCACFView call ::SetWindowRgn/::DeferWindowPos from its background thread. This causes a deadlock because this causes the background thread to block on the main thread, but the main thread may already be blocked on a mutex that is held by the background thread.
Comment 11 Adam Roben (:aroben) 2011-06-01 07:57:22 PDT
It looks like moving the plugin windows in LayerTreeHostCAWin::contextDidChange (which is called on the main thread) works well enough. There's a slight asynchrony between the plugin windows and the accelerated content, but it's very hard to notice in most cases.
Comment 12 Adam Roben (:aroben) 2011-06-01 08:07:45 PDT
Created attachment 95601 [details]
Move WebView's window geometry updating code to a new class
Comment 13 Adam Roben (:aroben) 2011-06-01 08:23:14 PDT
Created attachment 95606 [details]
Route plugin window geometry updates through the DrawingArea
Comment 14 WebKit Review Bot 2011-06-01 08:27:17 PDT
Attachment 95606 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/WebProcess/WebPage/win/DrawingAreaImplWin.cpp:42:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Adam Roben (:aroben) 2011-06-01 09:53:04 PDT
Created attachment 95617 [details]
Render accelerated content into a web process-owned child HWND
Comment 16 Jeff Miller 2011-06-01 10:03:52 PDT
Comment on attachment 95617 [details]
Render accelerated content into a web process-owned child HWND

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

> Source/WebKit2/Shared/win/CoalescedWindowGeometriesUpdater.h:36
> +enum BringToTopOrNot { BringToTop, DoNotBringToTop };

Should this enum be defined inside the CoalescedWindowGeometriesUpdater class?
Comment 17 Adam Roben (:aroben) 2011-06-01 10:08:06 PDT
Comment on attachment 95617 [details]
Render accelerated content into a web process-owned child HWND

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

>> Source/WebKit2/Shared/win/CoalescedWindowGeometriesUpdater.h:36
>> +enum BringToTopOrNot { BringToTop, DoNotBringToTop };
> 
> Should this enum be defined inside the CoalescedWindowGeometriesUpdater class?

Given how specific this enum is, I don't think there's any danger of its name conflicting with some other use. If some other class wants to use this exact enum, we can move it to a shared header. Leaving the enum outside of the class makes code that uses the enum clearer.
Comment 18 Adam Roben (:aroben) 2011-06-01 10:55:44 PDT
Created attachment 95626 [details]
Render accelerated content into a web process-owned child HWND (Take 2)

Cleaned up WKCACFViewWindow a little.
Comment 19 Adam Roben (:aroben) 2011-06-01 11:22:44 PDT
Created attachment 95632 [details]
Delete a bunch of dead code in DrawingAreaImpl
Comment 20 Adam Roben (:aroben) 2011-06-01 11:39:35 PDT
*** Bug 58052 has been marked as a duplicate of this bug. ***
Comment 22 Adam Roben (:aroben) 2011-06-01 11:43:49 PDT
(In reply to comment #21)
> Additional cleanup note required for the fix in r87832
> http://trac.webkit.org/changeset/87832

*not* required...
Comment 23 Adam Roben (:aroben) 2011-06-01 11:55:19 PDT
Build fix in r87834
http://trac.webkit.org/changeset/87834