Cloudef / bemenu

Dynamic menu library and client program inspired by dmenu
GNU General Public License v3.0
1.16k stars 90 forks source link

Reapply "implement wayland fractional scaling" #381

Closed stacyharper closed 5 months ago

stacyharper commented 5 months ago

This reverts commit 943d74600eb3791f3fe8344a844e5bc6d17899bb.

This fix the BEMENU_SCALE that was left unused when fractionnal scale support was detected.

This is rebased over origin/master.

Cloudef commented 5 months ago

FYI I separated BEMENU_SCALE from wayland buffer scale in this commit https://github.com/Cloudef/bemenu/commit/3156dac7d4cb94e323ca08087aacb15824a901c7 they aren't the same thing as compositor will render forced 2x buffers downscaled for example if your output is scale 1. BEMENU_SCALE should always give scaled buffer and window size (override). Forcing wayland buffer scale doesn't seem to ever make sense to me.

stacyharper commented 5 months ago

FYI I separated BEMENU_SCALE from wayland buffer scale in this commit 3156dac they aren't the same thing as compositor will render forced 2x buffers downscaled for example if your output is scale 1. BEMENU_SCALE should always give scaled buffer and window size (override). Forcing wayland buffer scale doesn't seem to ever make sense to me.

Perfect! Now I understand better BEMENU_SCALE, and it finaly also make sense to me.

I rebased this over your patch. And it seems to works as we expect it, with fractionnal support too.

One question I have. To preserve default BEMENU_SCALE, we have to use the output scale value (in my case BEMENU_SCALE=1.4). I would expect 1 would be the default, while 2 means "double dimensions".

Cloudef commented 5 months ago

Hmm, not sure I understand. BEMENU_SCALE has no default, so by default on X11 we use 1 (no scale), and on wayland whatever is output's scale.

What BEMENU_SCALE effectively does is:

This is the only way to do "hidpi" on X11 (I guess?) And on wayland this allows you to force hires buffers even on scale 1 outputs or if the output scale detection is wonky for whatever reason.

That said I haven't tried commit https://github.com/Cloudef/bemenu/commit/3156dac7d4cb94e323ca08087aacb15824a901c7 on outputs that are not 1x scale .. it may not work :thinking: Perhaps if BEMENU_SCALE is set, wl_buffer.scale should be set to 1 and we ignore window->scale (or we can force window->scale to always be 1)

stacyharper commented 5 months ago

That said I haven't tried commit https://github.com/Cloudef/bemenu/commit/3156dac7d4cb94e323ca08087aacb15824a901c7 on outputs that are not 1x scale .. it may not work 🤔

It works. But BEMENU_SCALE=1 doesn't give the "default". ex: output is 2x scale, you use BEMENU_SCALE=1, then bemenu is 2 time tinier than without the variable. The "default" in that situation is 2.0.

Cloudef commented 5 months ago

Yeah that might be slightly unexpected behavior, which I don't think is useful. I'll add commit that forces window->scale to 1 if BEMENU_SCALE is set. What do you think?

stacyharper commented 5 months ago

Yeah that might be slightly unexpected behavior, which I don't think is useful. I'll add commit that forces window->scale to 1 if BEMENU_SCALE is set.

No I don't think that is what we want. if you enforce window->scale=1 then the window will be blurry

stacyharper commented 5 months ago

I think what we want is this:

diff --git a/lib/renderers/wayland/window.c b/lib/renderers/wayland/window.c
index 0043c2b..cb3500e 100644
--- a/lib/renderers/wayland/window.c
+++ b/lib/renderers/wayland/window.c
@@ -149,7 +149,7 @@ create_buffer(struct wl_shm *shm, struct buffer *buffer, int32_t width, int32_t

     const char *env_scale = getenv("BEMENU_SCALE");
     if (env_scale) {
-        buffer->cairo.scale = fmax(strtof(env_scale, NULL), 1.0f);
+        buffer->cairo.scale = scale * fmax(strtof(env_scale, NULL), 1.0f);
     } else {
         buffer->cairo.scale = scale;
     }
Cloudef commented 5 months ago

Is it blurry even if you do BEMENU_SCALE=2 on output with scale 2? Shouldn't that effectively be same as wl_buffer with scale 2? I'd like to keep the value absolute scale if possible, as in you don't normally use it, but when you do you want it to exactly give the scale you request for.

stacyharper commented 5 months ago

Why someone would want to have a blurry or sharped surface? You can not enforce a hires surface with an output 1x scaled.

The current state, with the diff I gave earlier, seems ideal to me. Even with an 1x output, the user could enforce larger text and dimensions with BEMENU_SCALE=2.

Cloudef commented 5 months ago

Like I said, you normally do not use the env variable. But if you do, you want it to give the exact scale you request for. If you ask 1x scale on 2x output, you shall receive it. It's still possible to do relative scale by doing the math and inputting the result to BEMENU_SCALE.

stacyharper commented 5 months ago

Like I said, you normally do not use the env variable. But if you do, you want it to give the exact scale you request for. If you ask 1x scale on 2x output, you shall receive it. It's still possible to do relative scale by doing the math and inputting the result to BEMENU_SCALE.

I see what you mean. With output scale 1, or 2, using BEMENU_SCALE=1 gives a similar situation.

Why not! Both suits me. I never used this anyway :3

edit: then no change is required right? this is the current situation

Cloudef commented 5 months ago

No change needed. I add the scale = 1 scenario separately.

Cloudef commented 5 months ago

Merged, thanks!

Cloudef commented 5 months ago

This seems to fail in hyprland currently. I think this is bug in hyprland though.

[3013176.290]  -> wp_fractional_scale_manager_v1@9.get_fractional_scale(new id wp_fractional_scale_v1@18, wl_surface@17)
[3013176.293]  -> wp_viewporter@6.get_viewport(new id wp_viewport@19, wl_surface@17)
[3013176.296]  -> zwlr_layer_shell_v1@8.get_layer_surface(new id zwlr_layer_surface_v1@20, wl_surface@17, nil, 3, "menu")
[3013176.299]  -> zwlr_layer_surface_v1@20.set_anchor(13)
[3013176.302]  -> zwlr_layer_surface_v1@20.set_size(0, 32)
[3013176.305]  -> wl_surface@17.commit()
[3013176.307]  -> wl_display@1.sync(new id wl_callback@21)
[3013176.334] wl_display@1.delete_id(16)
[3013176.336] wl_display@1.delete_id(3)
[3013176.338] wl_display@1.error(wp_fractional_scale_manager_v1@9, 0, "Fractional scale exists.")
wp_fractional_scale_manager_v1@9: error 0: Fractional scale exists.

The compositor says the surface already has fractional scaling, but it does not, it was applied to different surface. I'll make this feature under BEMENU_WL_FRACTIONAL_SCALING env variable for now.

stacyharper commented 5 months ago

@Cloudef I don't understand aac7d73afcbd34f1662e994d235d9ee0eb98065a. This make the surface blurry all the time. What's the point of this?

Cloudef commented 5 months ago

Like I said, you normally do not use the env variable. But if you do, you want it to give the exact scale you request for. If you ask 1x scale on 2x output, you shall receive it. It's still possible to do relative scale by doing the math and inputting the result to BEMENU_SCALE.

Note it only forces scale = 1 if you use BEMENU_SCALE

stacyharper commented 5 months ago

Note it only forces scale = 1 if you use BEMENU_SCALE

Who would use this? What is the use case?

Cloudef commented 5 months ago
  1. Test bemenu scaled cairo drawing bypassing any other scaling mechanism
  2. Allow people to scale bemenu output to whatever scale, even if you don't have hidpi display
  3. So that the behavior aligns what BEMENU_SCALE says in the manpage / readme and what it does on x11
Cloudef commented 5 months ago

Hmm, I just realized the compositor will in this case resize the surface 2x on 2x output...? .. So the original behavior without that commit would be the correct behavior.

EDIT: reverted the commit and made new release ...

stacyharper commented 5 months ago

Hmm, I just realized the compositor will in this case resize the surface 2x on 2x output...? .. So the original behavior without that commit would be the correct behavior.

Yes, on output scaled x2, the compositor upscale it itself, which cause a blurry surface. Without that commit, BEMENU_SCALE give control over the scaling, while staying pixel perfect for the compositor.

Cloudef commented 5 months ago

FYI, I opened issue here https://github.com/hyprwm/Hyprland/issues/4600