Improvements to swaynag rendering #8437
Conversation
This draws the lower border below the message and buttons instead of slightly overlapping them when bar_border_thickness is large.
This reduces flicker when toggling the detail view.
This avoids exhausting the buffer pool when multiple events that would trigger redraws occur rapidly (for example: scrolling, resizing output).
| zwlr_layer_surface_v1_set_size(swaynag->layer_surface, 0, height); | ||
| zwlr_layer_surface_v1_set_exclusive_zone(swaynag->layer_surface, | ||
| height); |
There was a problem hiding this comment.
In the resize case, shouldn't we wait for the compositor's configure event as well?
There was a problem hiding this comment.
I believe we do not need to and the cost is greater than the benefit.
I assume the layer shell configure event is fundamentally a hint, similar to xdg-shell. On rereading, I find I can't exactly parse the grammar of the protocol text for the configure event, but it seems to point in the right direction.
The size is a hint, in the sense that the client is free to ignore it if it doesn't resize, pick a smaller size (to satisfy aspect ratio or resize in steps of NxM pixels).
At any rate, there is no "dimension_mismatch" error for ignoring the size as there is for ext-session-lock.
Consequently, if a layer shell client wants to use a specific size, it can just send a buffer (and, optionally, a set_size request), and acknowledge and disregard any following configure events. (The risk is that the compositor might clip the surface if it the client's provided size is infeasible.) In practice, sway appears to just reply with a configure event with height matching set_size, so there is no disagreement.
In fact, the documentation for the layer shell set_size request says
If you pass 0 for either value, the compositor will assign it and inform you of the assignment in the configure event.
so it looks like the compositor should always match the suggested height from set_size in its next configure event, unless there is some scale change, output layout change, or other unusual event (in which case the width might change and the compositor send a new configure event.)
Therefore: waiting for the compositor's configure event introduces an extra roundtrip of delay before a new buffer can be committed, and its benefit (the client not committing a wrong-sized buffer) only occurs in the case where the configure width depends on the set_size height.
(Waiting also brings a theoretical risk of livelock, where the client could never commit a new buffer if the compositor never gives it the height it asked for -- but this should not happen per set_size documentation.)
Misc note: swaynag's current rendering code seems to be a weird mix of attempting to adapt to the recommended height value from the configure (see comparisons with swaynag->height in render_message, render_button) and ignoring it (render_detailed).
Swaynag did not redraw buffers promptly when the output scale was changed or when scrolling the detail view. The root cause was wl_buffer exhaustion -- trying to commit too many updates before the compositor could respond. Fixing this (commit 3) required eliminating a roundtrip in
render_frame(commit 2) which required adjusting the way the lower border is drawn (commit 1).Observable improvements:
swaynag --border-bottom-size 15 -m "Test", the bottom border no longer overlaps the messageinput.txt, and runningswaymsg -d -m 'test' -l < input.txt: scrolling the detail view with the pointer axis should be less jerky, and there should be fewer "Failed to get buffer. Skipping frame." warning messages.