Bug 70213

Summary: [GTK] Switch to a backing store approach for painting WebKitWebView
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: WebKitGTKAssignee: Martin Robinson <mrobinson>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, cgarcia, gustavo, pnormand, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 68757    
Attachments:
Description Flags
Patch
none
Patch
none
Patch fixing paint before realize gustavo: review+

Martin Robinson
Reported 2011-10-16 19:34:11 PDT
The way that the WebKit1 WebKitWebView operates now is that a draw (expose) event can trigger a relayout. This is bad for several reasons: 1. If an animation is happening, it can cause a relayout during expose. This leads to more layouts than necessary and slower framerates. 2. If plugins move during the relayout, we will call gtk_widget_size_allocate during expose events. This shouldn't happen. My approach is to add a backing store to the widget itself. We only draw into the backing store when WebCore tells us the contents of the backing store are invalid. This is roughly the approach that WebKit2 takes. This comes with many benefits: 1. Fixed position objects don't jump around while scrolling. This happens on boingboing currently if you scroll down far enough for the right column to become fixed. 2. Scrolling is faster since we no longer have a need to call gdk_window_process_updates during every single scroll. This fixes lag on the twitter stream and the facebook timeline information page. 3. We have greater contorl over what happens during resizing. This code improves opaque resize performance greatly. This is important in Gnome Shell. 4. We can easily switch to the image backend if we want to with only a tiny code changes in WebKit.
Attachments
Patch (39.67 KB, patch)
2011-10-16 20:34 PDT, Martin Robinson 		no flags
DetailsFormatted DiffDiff
Patch (39.67 KB, patch)
2011-10-16 22:23 PDT, Martin Robinson 		no flags
DetailsFormatted DiffDiff
Patch fixing paint before realize (39.73 KB, patch)
2011-10-22 12:40 PDT, Martin Robinson 		gustavo: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Martin Robinson
Comment 1 2011-10-16 20:34:38 PDT
Created attachment 111203 [details] Patch
Gustavo Noronha (kov)
Comment 2 2011-10-16 20:56:48 PDT
Comment on attachment 111203 [details] Patch Attachment 111203 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10074825
Martin Robinson
Comment 3 2011-10-16 22:23:39 PDT
Created attachment 111209 [details] Patch
Alejandro G. Castro
Comment 4 2011-10-21 08:08:55 PDT
Tried to test the patch but it crashes for me with: #0 0x00007ffff6e573a0 in WebCore::WidgetBackingStore::cairoSurface() () from /opt/gnome3/lib/libwebkitgtk-3.0.so.0 #1 0x00007ffff5ff4187 in WebKit::ChromeClient::paint () from /opt/gnome3/lib/libwebkitgtk-3.0.so.0 #2 0x00007ffff666bbf2 in WebCore::ThreadTimers::sharedTimerFiredInternal() () from /opt/gnome3/lib/libwebkitgtk-3.0.so.0 #3 0x00007ffff6e53132 in WebCore::timeout_cb(void*) () from /opt/gnome3/lib/libwebkitgtk-3.0.so.0 #4 0x00007ffff45fa1f4 in g_timeout_dispatch (source=0x1a375d0, callback=0x7ffff6e53120 <WebCore::timeout_cb(void*)>, user_data=0x0) at gmain.c:3907 #5 0x00007ffff45f6c05 in g_main_dispatch (context=0x550e10) at gmain.c:2441 #6 0x00007ffff45f814b in g_main_context_dispatch (context=0x550e10) at gmain.c:3011 #7 0x00007ffff45f85f8 in g_main_context_iterate (context=0x550e10, block=1, dispatch=1, self=0x502450) at gmain.c:3089 #8 0x00007ffff45f8d51 in g_main_loop_run (loop=0x625790) at gmain.c:3297 #9 0x00007ffff5807a56 in gtk_main () at gtkmain.c:1362 #10 0x00007ffff56dc1a2 in gtk_application_run_mainloop (application=0x627000) at gtkapplication.c:115 #11 0x00007ffff4c539ec in g_application_run (application=0x627000, argc=1, argv=0x7fffffffdcb8) at gapplication.c:1323 #12 0x0000000000436b0c in main (argc=1, argv=0x7fffffffdcb8) at ephy-main.c:475 Sorry, it is my laptop with release build. I'll try to check the issue.
Martin Robinson
Comment 5 2011-10-21 09:08:35 PDT
(In reply to comment #4) > Tried to test the patch but it crashes for me with: > > > #0 0x00007ffff6e573a0 in WebCore::WidgetBackingStore::cairoSurface() () from /opt/gnome3/lib/libwebkitgtk-3.0.so.0 > #1 0x00007ffff5ff4187 in WebKit::ChromeClient::paint () from /opt/gnome3/lib/libwebkitgtk-3.0.so.0 > #2 0x00007ffff666bbf2 in WebCore::ThreadTimers::sharedTimerFiredInternal() () from /opt/gnome3/lib/libwebkitgtk-3.0.so.0 > #3 0x00007ffff6e53132 in WebCore::timeout_cb(void*) () from /opt/gnome3/lib/libwebkitgtk-3.0.so.0 > #4 0x00007ffff45fa1f4 in g_timeout_dispatch (source=0x1a375d0, callback=0x7ffff6e53120 <WebCore::timeout_cb(void*)>, user_data=0x0) at gmain.c:3907 > #5 0x00007ffff45f6c05 in g_main_dispatch (context=0x550e10) at gmain.c:2441 > #6 0x00007ffff45f814b in g_main_context_dispatch (context=0x550e10) at gmain.c:3011 > #7 0x00007ffff45f85f8 in g_main_context_iterate (context=0x550e10, block=1, dispatch=1, self=0x502450) at gmain.c:3089 > #8 0x00007ffff45f8d51 in g_main_loop_run (loop=0x625790) at gmain.c:3297 > #9 0x00007ffff5807a56 in gtk_main () at gtkmain.c:1362 > #10 0x00007ffff56dc1a2 in gtk_application_run_mainloop (application=0x627000) at gtkapplication.c:115 > #11 0x00007ffff4c539ec in g_application_run (application=0x627000, argc=1, argv=0x7fffffffdcb8) at gapplication.c:1323 > #12 0x0000000000436b0c in main (argc=1, argv=0x7fffffffdcb8) at ephy-main.c:475 Interesting. It seems like the WebView is painting before it's realized. This shouldn't be a problem. I'll update the patch to work in this situation.
Martin Robinson
Comment 6 2011-10-21 09:08:49 PDT
Comment on attachment 111209 [details] Patch Clearing review flag for the time being.
Martin Robinson
Comment 7 2011-10-22 12:40:43 PDT
Created attachment 112096 [details] Patch fixing paint before realize
Gustavo Noronha (kov)
Comment 8 2011-10-24 06:09:32 PDT
Comment on attachment 112096 [details] Patch fixing paint before realize ♥ I guess we should obsolete Joone's port of the tiled backend, and invest on adding features to this new backing store if we want tiling and such, btw?
Alejandro G. Castro
Comment 9 2011-10-24 12:26:13 PDT
Works well, and result is awesome! Good job. I'm adding this as proposed for 1.6.2.
Martin Robinson
Comment 10 2011-10-30 09:29:41 PDT
Committed r98827: <http://trac.webkit.org/changeset/98827>
Philippe Normand
Comment 11 2011-11-02 01:32:54 PDT
(In reply to comment #10) > Committed r98827: <http://trac.webkit.org/changeset/98827> Looks like this broke the fast/css/webkit-mask-crash-table.html test: #0 0x00002ab2a1658058 in WebCore::GraphicsContext::~GraphicsContext (this=0x7fff9b7a15a0, __in_chrg=<optimized out>) at ../../Source/WebCore/platform/graphics/GraphicsContext.cpp:91 91 ASSERT(!m_transparencyCount); Thread 1 (Thread 0x2ab2aedd68e0 (LWP 11828)): #0 0x00002ab2a1658058 in WebCore::GraphicsContext::~GraphicsContext (this=0x7fff9b7a15a0, __in_chrg=<optimized out>) at ../../Source/WebCore/platform/graphics/GraphicsContext.cpp:91 #1 0x00002ab2a0de88f8 in WebKit::paintWebView (webView=0x97a050, frame=0x9b2780, dirtyRegion=...) at ../../Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:522 #2 0x00002ab2a0de8bb4 in WebKit::ChromeClient::paint (this=0x97c5f0) at ../../Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:560 #3 0x00002ab2a0de7d3d in WebKit::repaintEverythingSoonTimeout (client=0x97c5f0) at ../../Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:387 #4 0x00002ab2a60c9ddb in g_timeout_dispatch (source=0x3b3039d0, callback=<optimized out>, user_data=<optimized out>) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3882 #5 0x00002ab2a60c84a3 in g_main_dispatch (context=0x8f9d70) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:2440 #6 g_main_context_dispatch (context=0x8f9d70) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3013 #7 0x00002ab2a60c8c80 in g_main_context_iterate (context=0x8f9d70, block=1, dispatch=1, self=<optimized out>) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3091 #8 0x00002ab2a60c92f2 in g_main_loop_run (loop=0x3ad4b1f0) at /tmp/buildd/glib2.0-2.28.6/./glib/gmain.c:3299 #9 0x00002ab2a3e764cd in gtk_main () from /usr/lib/libgtk-3.so.0 #10 0x000000000042f480 in runTest (testPathOrURL=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:712 #11 0x000000000042eab8 in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:504 #12 0x0000000000430de9 in main (argc=2, argv=0x7fff9b7a2358) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1208
Martin Robinson
Comment 12 2011-11-02 09:12:36 PDT
(In reply to comment #11) > (In reply to comment #10) > > Committed r98827: <http://trac.webkit.org/changeset/98827> > > Looks like this broke the fast/css/webkit-mask-crash-table.html test: I'll take a look at this soon.
Note You need to log in before you can comment on or make changes to this bug.