Repository navigation
Conversation
…he GTK window The subsurface renderer presents each frame to a Wayland subsurface from the raster thread, but the subsurface was left in synchronized mode. A frame committed to a synchronized subsurface is only shown when the parent surface is next committed, so the renderer queued a GTK redraw of the view for every frame. That repaints the whole window with cairo on the GTK thread and damages the parent surface in full, which the compositor then has to re-upload. For an app animating in a 2560x1400 window this cost about 13% of a core in the app and another 13% in gnome-shell, with nothing drawn by GTK being visible. Make the subsurface desynchronized, so a frame is shown as soon as the raster thread commits it, and stop queuing a redraw per frame. The subsurface is synchronized again only while the widget changes size: it is set in size_allocate, before the new window metrics reach the engine, so the frame rendered at the new size is held back until GTK commits the resized parent surface, and it is desynchronized from the frame clock's after-paint signal once that commit has happened. Window and content therefore still change size in the same compositor update. Measured with an empty animating Flutter app in a 2560x1400 window on GNOME (Wayland): 15.6% -> 2.5% CPU in the app and 13.4% -> 2.8% in gnome-shell. Partial fix for flutter#101591 (Wayland only; the X11 renderer is unchanged).
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request configures the Wayland subsurface to be desynchronized by default, allowing frames to be shown immediately and keeping the GTK thread idle during animation. It also introduces a mechanism to synchronize the subsurface with the parent surface during resizing to ensure they change size together. The review feedback identifies an issue in the SetSynchronized test where CreateSubsurface is called before mock expectations are defined, which can lead to test warnings or failures.
robert-ancell
left a comment
robert-ancell
left a comment
There was a problem hiding this comment.
Works when I test it here. I don't think I considered we could switch synchronization during resize when I first implemented the subsurface. Some style changes and it should be good to land.
…sync and set it from the renderer
Thanks for the review @robert-ancell, all comments should be fixed now. :) |
The Wayland subsurface renderer (#191389) presents each frame to a
wl_subsurfacefrom the raster thread, but leaves the subsurface in synchronized mode, so a frame is only shown when the parent (GTK) surface is next committed. To make that happen the renderer queued a GTK redraw of the view for every frame. That repaints the whole window with cairo on the GTK thread and damages the parent surface in full, which the compositor then has to re-upload, and none of what GTK paints is visible since it is behind the subsurface.This change desynchronizes the subsurface, so a frame is shown as soon as the raster thread commits it, and stops queuing a redraw per frame. Window and content still change size in the same compositor update: the subsurface is synchronized again from
size_allocate(beforeFlViewsends the new window metrics to the engine, so the frame rendered at the new size is held back) until GTK has committed the resized parent surface, which it does from its ownafter-painthandler. Views sized to their content keep the subsurface synchronized and redraw per frame as before, since their window is resized to fit each frame.Measured with a release build of an app showing a
CircularProgressIndicatorin a 2560x1400 window on GNOME 50.1 (Wayland, Mesa 26.0.8 radeonsi), CPU as a percentage of one core:gnome-shell's idle baseline is 1.9%. An empty animating game (flame-engine/flame#4143) goes from 15.6% to 2.3%. A
perfprofile of master showed the cost aspixman_fillandcairo_fillundergdk_window_begin_draw_frameon the GTK thread, which is the per-frame repaint.Addresses the Wayland case of #101591. The X11 renderer (
FlViewRendererOpenGL) still draws frames through GTK and is unchanged by this PR.Tests:
fl_subsurface_test.cccovers the new default andfl_subsurface_set_synchronized(). The renderer's behaviour around a resize is not unit tested, asFlViewRendererSubsurfaceneeds a Wayland display and the tests run on X11 (as for the existing renderer tests). Checked manually on GNOME: continuous animation, drag resize, maximize and restore, minimize and restore, with no warnings logged.Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.