Bug 70213 - [GTK] Switch to a backing store approach for painting WebKitWebView
Summary: [GTK] Switch to a backing store approach for painting WebKitWebView
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords:
Depends on:
Blocks: 68757
  Show dependency treegraph
 
Reported: 2011-10-16 19:34 PDT by Martin Robinson
Modified: 2011-11-02 09:12 PDT (History)
5 users (show)

See Also:


Attachments
Patch (39.67 KB, patch)
2011-10-16 20:34 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (39.67 KB, patch)
2011-10-16 22:23 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch fixing paint before realize (39.73 KB, patch)
2011-10-22 12:40 PDT, Martin Robinson
gustavo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 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.
Comment 1 Martin Robinson 2011-10-16 20:34:38 PDT
Created attachment 111203 [details]
Patch
Comment 2 Gustavo Noronha (kov) 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
Comment 3 Martin Robinson 2011-10-16 22:23:39 PDT
Created attachment 111209 [details]
Patch
Comment 4 Alejandro G. Castro 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.
Comment 5 Martin Robinson 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.
Comment 6 Martin Robinson 2011-10-21 09:08:49 PDT
Comment on attachment 111209 [details]
Patch

Clearing review flag for the time being.
Comment 7 Martin Robinson 2011-10-22 12:40:43 PDT
Created attachment 112096 [details]
Patch fixing paint before realize
Comment 8 Gustavo Noronha (kov) 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?
Comment 9 Alejandro G. Castro 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.
Comment 10 Martin Robinson 2011-10-30 09:29:41 PDT
Committed r98827: <http://trac.webkit.org/changeset/98827>
Comment 11 Philippe Normand 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
Comment 12 Martin Robinson 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.