Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to swaynag rendering #8437

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Improvements to swaynag rendering #8437

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2535a2e
swaynag: account for height of lower border
mstoeckl Nov 9, 2024
131026b
swaynag: reduce roundtrips when rendering frames
mstoeckl Nov 9, 2024
49464d2
swaynag: coalesce surface redraw operations
mstoeckl Nov 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 32 additions & 27 deletions swaynag/render.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,39 +260,44 @@ void render_frame(struct swaynag *swaynag) {
cairo_paint(cairo);
cairo_restore(cairo);
uint32_t height = render_to_cairo(cairo, swaynag);
if (height != swaynag->height) {
if (swaynag->height == 0) {
// for first commit, set required size but do not attach a buffer
zwlr_layer_surface_v1_set_size(swaynag->layer_surface, 0, height);
zwlr_layer_surface_v1_set_exclusive_zone(swaynag->layer_surface,
height);
height);
wl_surface_commit(swaynag->surface);
wl_display_roundtrip(swaynag->display);
} else {
swaynag->current_buffer = get_next_buffer(swaynag->shm,
swaynag->buffers,
swaynag->width * swaynag->scale,
swaynag->height * swaynag->scale);
if (!swaynag->current_buffer) {
sway_log(SWAY_DEBUG, "Failed to get buffer. Skipping frame.");
goto cleanup;
}
goto cleanup;
}
if (height != swaynag->height) {
zwlr_layer_surface_v1_set_size(swaynag->layer_surface, 0, height);
zwlr_layer_surface_v1_set_exclusive_zone(swaynag->layer_surface,
height);
Comment on lines +273 to +275
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the resize case, shouldn't we wait for the compositor's configure event as well?

Copy link
Contributor Author

@mstoeckl mstoeckl Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

}

cairo_t *shm = swaynag->current_buffer->cairo;
cairo_save(shm);
cairo_set_operator(shm, CAIRO_OPERATOR_CLEAR);
cairo_paint(shm);
cairo_restore(shm);
cairo_set_source_surface(shm, recorder, 0.0, 0.0);
cairo_paint(shm);

wl_surface_set_buffer_scale(swaynag->surface, swaynag->scale);
wl_surface_attach(swaynag->surface,
swaynag->current_buffer->buffer, 0, 0);
wl_surface_damage(swaynag->surface, 0, 0,
swaynag->width, swaynag->height);
wl_surface_commit(swaynag->surface);
wl_display_roundtrip(swaynag->display);
swaynag->current_buffer = get_next_buffer(swaynag->shm,
swaynag->buffers,
swaynag->width * swaynag->scale,
height * swaynag->scale);
if (!swaynag->current_buffer) {
sway_log(SWAY_DEBUG, "Failed to get buffer. Skipping frame.");
goto cleanup;
}

cairo_t *shm = swaynag->current_buffer->cairo;
cairo_save(shm);
cairo_set_operator(shm, CAIRO_OPERATOR_CLEAR);
cairo_paint(shm);
cairo_restore(shm);
cairo_set_source_surface(shm, recorder, 0.0, 0.0);
cairo_paint(shm);

wl_surface_set_buffer_scale(swaynag->surface, swaynag->scale);
wl_surface_attach(swaynag->surface,
swaynag->current_buffer->buffer, 0, 0);
wl_surface_damage(swaynag->surface, 0, 0,
swaynag->width, swaynag->height);
wl_surface_commit(swaynag->surface);

cleanup:
cairo_surface_destroy(recorder);
cairo_destroy(cairo);
Expand Down