swaywm / swaybg

Wallpaper tool for Wayland compositors
MIT License
490 stars 31 forks source link

Optimize solid color with single-pixel-buffer-v1 #43

Closed emersion closed 2 years ago

emersion commented 2 years ago

References: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/104

TODO: bump wayland-protocols version requirement.

mstoeckl commented 2 years ago

When I ran this branch of swaybg on an output with 2x scaling, swaybg -c '#ff00ff' failed to display a background; see following partial log with WAYLAND_DEBUG=1.

[  76026.226] zwlr_layer_surface_v1@11.configure(355, 1280, 720)
[  76026.242]  -> zwlr_layer_surface_v1@11.ack_configure(355)
[  76026.258]  -> wp_single_pixel_buffer_manager_v1@8.create_u32_rgba_buffer(new id wl_buffer@10, 4294967295, 0, 4294967295, 4294967295)
[  76026.275]  -> wl_surface@3.set_buffer_scale(2)
[  76026.289]  -> wl_surface@3.attach(wl_buffer@10, 0, 0)
[  76026.304]  -> wl_surface@3.damage_buffer(0, 0, 2147483647, 2147483647)
[  76026.319]  -> wp_viewporter@7.get_viewport(new id wp_viewport@12, wl_surface@3)
[  76026.333]  -> wp_viewport@12.set_destination(2560, 1440)
[  76026.346]  -> wl_surface@3.commit()
[  76026.357]  -> wp_viewport@12.destroy()
[  76026.368]  -> wl_buffer@10.destroy()
[  76027.542] wl_display@1.error(wl_surface@3, 2, "Buffer size (1x1) is not divisible by scale (2)")
wl_surface@3: error 2: Buffer size (1x1) is not divisible by scale (2)

Edit: looking at the wp-viewporter docs,

The coordinate transformations from buffer pixel coordinates up to the surface-local coordinates happen in the following order: 1. buffer_transform (wl_surface.set_buffer_transform) 2. buffer_scale (wl_surface.set_buffer_scale) 3. crop and scale (wp_viewport.set*) This means, that the source rectangle coordinates of crop and scale are given in the coordinates after the buffer transform and scale, i.e. in the coordinates that would be the surface-local coordinates if the crop and scale was not applied.

.. it seems the issue is that buffer scale is applied before wp_viewport operations, not after; so the following diff may be sufficient to fix the issue.

diff --git a/main.c b/main.c
index 65a6730..3a90c77 100644
--- a/main.c
+++ b/main.c
@@ -133,13 +133,12 @@ static void render_frame(struct swaybg_output *output, cairo_surface_t *surface)
                uint32_t a32 = (uint32_t)(a8 * f);
                struct wl_buffer *buffer = wp_single_pixel_buffer_manager_v1_create_u32_rgba_buffer(
                        output->state->single_pixel_buffer_manager, r32, g32, b32, a32);
-               wl_surface_set_buffer_scale(output->surface, output->scale);
                wl_surface_attach(output->surface, buffer, 0, 0);
                wl_surface_damage_buffer(output->surface, 0, 0, INT32_MAX, INT32_MAX);

                struct wp_viewport *viewport = wp_viewporter_get_viewport(
                        output->state->viewporter, output->surface);
-               wp_viewport_set_destination(viewport, buffer_width, buffer_height);
+               wp_viewport_set_destination(viewport, output->width, output->height);

                wl_surface_commit(output->surface);
mstoeckl commented 2 years ago

Also, a minor issue, not necessarily worth fixing: every time the output is resized, swaybg creates a new pair of wl_buffer and wl_viewport objects, and then promptly destroys them after use; it would be slightly more efficient to create one buffer/viewport pair on first use, and then call only wp_viewport.set_destination(...) and wl_surface.commit(...) when a size update is needed.

emersion commented 2 years ago

so the following diff may be sufficient to fix the issue.

Indeed! After discussing with pq in #wayland, it seems like it's a good practice to not use wl_surface.set_buffer_scale together with viewporter.

I've addressed all of the comments I think, thanks for the feedback!

emersion commented 2 years ago

Thanks for the review!