swaywm / swaybg

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

Use fractional-scale-v1 to estimate buffer sizes #56

Closed mstoeckl closed 4 months ago

mstoeckl commented 1 year ago

This PR lets swaybg use wp-fractional-scale-v1 with wp-viewporter to submit buffers that more closely match the real output pixel dimensions. The first commit refactors the code to decouple buffer creation from surface scale configuration further.

Note: This is marked as a draft, and introduces a temporary commit to deal with the probable bug in which, with current sway/wlroots, the first fractional-scale-v1 event arrives after the first configure event and is also missing after output scale changes.

For example: ``` [3700439.334] -> wl_compositor@5.create_surface(new id wl_surface@3) [3700439.340] -> wl_compositor@5.create_region(new id wl_region@11) [3700439.347] -> wl_surface@3.set_input_region(wl_region@11) [3700439.354] -> wl_region@11.destroy() [3700439.361] -> wp_fractional_scale_manager_v1@9.get_fractional_scale(new id wp_fractional_scale_v1@12, wl_surface@3) [3700439.369] -> wp_viewporter@7.get_viewport(new id wp_viewport@13, wl_surface@3) [3700439.377] -> zwlr_layer_shell_v1@6.get_layer_surface(new id zwlr_layer_surface_v1@14, wl_surface@3, wl_output@10, 0, "wallpaper") [3700439.386] -> zwlr_layer_surface_v1@14.set_size(0, 0) [3700439.394] -> zwlr_layer_surface_v1@14.set_anchor(15) [3700439.399] -> zwlr_layer_surface_v1@14.set_exclusive_zone(-1) [3700439.406] -> wl_surface@3.commit() [3700439.518] wl_display@1.delete_id(11) [3700439.526] zwlr_layer_surface_v1@14.configure(87935, 2560, 1440) [3700439.531] zwlr_layer_surface_v1@14.configure(87938, 2560, 1440) [3700439.537] -> zwlr_layer_surface_v1@14.ack_configure(87938) [3700468.552] -> wl_shm@4.create_pool(new id wl_shm_pool@11, fd 5, 14745600) [3700468.567] -> wl_shm_pool@11.create_buffer(new id wl_buffer@15, 0, 2560, 1440, 10240, 0) [3700468.573] -> wl_shm_pool@11.destroy() [3700481.901] -> wl_surface@3.attach(wl_buffer@15, 0, 0) [3700481.916] -> wl_surface@3.damage_buffer(0, 0, 2560, 1440) [3700481.920] -> wl_buffer@15.destroy() [3700481.924] -> wp_viewport@13.set_destination(2560, 1440) [3700481.927] -> wl_surface@3.commit() [3700496.018] wl_display@1.delete_id(11) [3700496.038] wl_display@1.delete_id(15) [3700496.042] wp_fractional_scale_v1@12.preferred_scale(120) [3700496.046] zwlr_layer_surface_v1@14.configure(87941, 2560, 1440) [3700496.051] -> zwlr_layer_surface_v1@14.ack_configure(87941) ```

Edit: diagram of how this PR applies the viewport in the worst case, for comparison with #57. Note: this assumes the compositor exactly applies the scale it claims, and stretches e.g 5 horizontal logical pixels onto 7.5 physical pixels. Current sway does not do this, and this PR appears to work fine there.

![grids-ugly](https://user-images.githubusercontent.com/7674289/220794826-aa738729-5679-4e1c-9375-eb077de0e70c.png)
emersion commented 1 year ago

the first fractional-scale-v1 event arrives after the first configure event

https://github.com/swaywm/sway/pull/7466

is also missing after output scale changes

https://github.com/swaywm/sway/issues/7464

mstoeckl commented 4 months ago

Note: while I've rebased this MR, currently it does not work for me, because no wp_fractional_scale_v1::preferred_scale events are arriving, even though I think they should.

emersion commented 4 months ago

Hm. These events are sent by wlr_scene IIRC, but only after the surface is mapped.

emersion commented 4 months ago

We're supposed to draw with scale=1 if we don't receive an event. It would be good to make wlroots send an event with a guess but the protocol doesn't require it (same for wl_surface.preferred_buffer_scale and wl_surface.enter).

mstoeckl commented 4 months ago

We're supposed to draw with scale=1 if we don't receive an event. It would be good to make wlroots send an event with a guess but the protocol doesn't require it (same for wl_surface.preferred_buffer_scale and wl_surface.enter).

I am in the process of testing the following diff:

diff --git a/main.c b/main.c
index f15669e..43460b1 100644
--- a/main.c
+++ b/main.c
@@ -419,6 +419,8 @@ static void handle_global(void *data, struct wl_registry *registry,
        } else if (strcmp(interface, wl_output_interface.name) == 0) {
                struct swaybg_output *output = calloc(1, sizeof(struct swaybg_output));
                output->state = state;
+               output->scale = 1;
+               output->pref_fract_scale = 120;
                output->wl_name = name;
                output->wl_output =
                        wl_registry_bind(registry, 
emersion commented 4 months ago

For the record, the related protocol discussion is: https://gitlab.freedesktop.org/wayland/wayland/-/issues/133

mstoeckl commented 4 months ago

1.) For layer shell surfaces which are created for a specific output, the compositor should have enough information to immediately provide a fractional scale.

2.) I think the best scale code change would be the following -- to initially try the integer output scale until the preferred fractional scale arrives. This will render twice for non-integral scales, which is currently unavoidable, but only render once when the scale is e.g. 2 or 3.

diff --git a/main.c b/main.c
index f15669e..fc6725e 100644
--- a/main.c
+++ b/main.c
@@ -159,7 +159,7 @@ static void get_buffer_size(const struct swaybg_output *output,
                        output->state->viewporter) {
                *buffer_width = 1;
                *buffer_height = 1;
-       } else if (output->fract_scale && output->state->viewporter) {
+       } else if (output->pref_fract_scale && output->state->viewporter) {
                // rounding mode is 'round half up'
                *buffer_width = (output->width * output->pref_fract_scale +
                        FRACT_DENOM / 2) / FRACT_DENOM;
@@ -419,6 +419,7 @@ static void handle_global(void *data, struct wl_registry *registry,
        } else if (strcmp(interface, wl_output_interface.name) == 0) {
                struct swaybg_output *output = calloc(1, sizeof(struct swaybg_output));
                output->state = state;
+               output->scale = 1;
                output->wl_name = name;
                output->wl_output =
                        wl_registry_bind(registry, name, &wl_output_interface, 4);

Should I make a new PR for the latter, or should we revert/modify the last commit? (I did test the first commit in this PR, and it was OK; only the second had an issue.) (Edit: have made a new PR.)

emersion commented 4 months ago

For layer shell surfaces which are created for a specific output, the compositor should have enough information to immediately provide a fractional scale.

Right. Layer shell is a special case here. wlr_scene doesn't have a special codepath for it, yet (Sway used to for wl_output.enter IIRC).