[webkit-reviews] review granted: [Bug 78826] [GTK] [AC] Events can starve accelerated compositing updates : [Attachment 127404] Patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 78826: [GTK] [AC] Events can starve accelerated compositing updates
https://bugs.webkit.org/show_bug.cgi?id=78826

Attachment 127404: Patch
https://bugs.webkit.org/attachment.cgi?id=127404&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=127404&action=review


> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextGL.cpp:175
>     
core(m_webView)->mainFrame()->view()->syncCompositingStateIncludingSubframes();


One thing I've noticed while working with the clutter AC stuff is that this
call may some times not go through because of a pending layout, and the sync
will in those cases not go through untill we are lucky to have another sync
requested with no layout pending. I am unsure tbh whether that is caused by a
bug in our implementation, but it looks like something that is expected, since
it's a check instead of an ASSERT() inside
FrameView::syncCompositingStateForThisFrame. Unrelated to this patch, but
something to keep in mind.

> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextGL.cpp:188
> +	   m_syncTimerCallbackId = g_timeout_add_full(GDK_PRIORITY_EVENTS,
1000.0 / 60.0, reinterpret_cast<GSourceFunc>(syncLayersTimeoutCallback), this,
0);

OK, I guess. I was thinking here right now that I tend to consider layer
synchronization more a relayout-like thing than a redraw-like thing. Using
GTK_PRIORITY_RESIZE might make more sense if we ever figure out the appropriate
balance among the existing priorities =(