Bug 161683 – [GTK] Clarify frame callbacks behaviour in Wayland compositor

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Emanuele Aina]

The way we fire frame callbacks in the nested Wayland compositor can be puzzling to developers expecting Wayland semantics, but since we have our own mechanism to handle synchronization we don't care much
about them. Adding a comment would avoid surprised Wayland developers.

[Emanuele Aina]

Created attachment 288124 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 288124 [details]
Patch

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

> w/Source/WebKit2/UIProcess/gtk/WaylandCompositor.cpp:237
> +    // From a Wayland point-of-view firing frame callbacks here is very weird,
> +    // but our WebProcess clients don't rely on them for synchronization and
> +    // set the EGL swap interval to zero, relying on the RenderNextFrame events
> +    // in the WebKit IPC instead

The comment adds more confusion to me. It says that the firing frame callbacks here is weird, but it doesn't say why or where they should be fired. The only reason why I didn't remove all frame callbacks handling from the nested compositor is because I was not sure we can rely that eglSwapInterval is always going to work for all drivers. But if it works, frame callbacks are never emitted, so this list is always empty. The reason why we do this, is not because we rely on RenderNextFrame events, adn there isn't any WebKit IPC involved. The reason is that we are rendering to an offscreen context always in the web process, and when we have to render a new frame, we schedule a redraw in the web view, and then we render in the screen. So syncing to vblank in the web process doesn't help, because we will not render at that time to the screen, we will render into the offscreen context, and the  schedule a redraw on the widget. It's at that point where we want to vblank sync, and that's already done by GTK+. This avoids unnecessary throttling in the web process. When there's a new frame rendered in the web process, eglSwapBuffers is what triggers the repaint in the nested compositor using the wayland protocol, not the WebKit IPC.

[Emanuele Aina]

Frame callbacks should be fired where you know the compositor has used the committed content. E.g. in Weston it the place where it has queued the GL commands to update the screen and so the next screen update contents have been locked in place.

To be specific, it should not just be any screen update, it should be the particular update that first uses the content from the same commit as where the frame callback was filed. Frame callbacks should also not be fired if the surface is not shown.

What I was trying to say is that despite frame callbacks being the main throttling mechanism in normal Wayland usage, we don't need them for that purpose as throttling is already handled elsewhere by WebKit.

[Emanuele Aina]

Created attachment 288131 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 288131 [details]
Patch

Much clearer, thanks!

[WebKit Commit Bot]

Comment on attachment 288131 [details]
Patch

Clearing flags on attachment: 288131

Committed r205547: <http://trac.webkit.org/changeset/205547>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.