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.

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

Reapply "implement wayland fractional scaling" #381

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

stacyharper
Copy link
Contributor

This reverts commit 943d746.

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

This is rebased over origin/master.

@Cloudef
Copy link
Owner

Cloudef commented Feb 2, 2024

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.

@stacyharper stacyharper force-pushed the wayland-fractional-scaling-v2 branch from 5fec8fb to 2062b6e Compare February 2, 2024 08:34
This reverts commit 943d746.

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

This is rebased over origin/master.
@stacyharper stacyharper force-pushed the wayland-fractional-scaling-v2 branch from 2062b6e to 79409be Compare February 2, 2024 08:38
@stacyharper
Copy link
Contributor Author

stacyharper commented Feb 2, 2024

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
Copy link
Owner

Cloudef commented Feb 2, 2024

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:

  • It creates buffer with multiplier of BEMENU_SCALE
  • Passes BEMENU_SCALE to cairo
  • Draws the buffer into a window that's equal of the buffer scale

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 3156dac on outputs that are not 1x scale .. it may not work 🤔
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
Copy link
Contributor Author

That said I haven't tried commit 3156dac 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
Copy link
Owner

Cloudef commented Feb 2, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Owner

Cloudef commented Feb 2, 2024

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
Copy link
Contributor Author

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
Copy link
Owner

Cloudef commented Feb 2, 2024

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
Copy link
Contributor Author

stacyharper commented Feb 2, 2024

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
Copy link
Owner

Cloudef commented Feb 2, 2024

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

@stacyharper stacyharper changed the title WIP: Reapply "implement wayland fractional scaling" Reapply "implement wayland fractional scaling" Feb 2, 2024
@Cloudef Cloudef merged commit 877ca82 into Cloudef:master Feb 2, 2024
4 checks passed
@Cloudef
Copy link
Owner

Cloudef commented Feb 2, 2024

Merged, thanks!

@Cloudef
Copy link
Owner

Cloudef commented Feb 2, 2024

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

[3013176.290]  -> [email protected]_fractional_scale(new id wp_fractional_scale_v1@18, wl_surface@17)
[3013176.293]  -> [email protected]_viewport(new id wp_viewport@19, wl_surface@17)
[3013176.296]  -> [email protected]_layer_surface(new id zwlr_layer_surface_v1@20, wl_surface@17, nil, 3, "menu")
[3013176.299]  -> [email protected]_anchor(13)
[3013176.302]  -> [email protected]_size(0, 32)
[3013176.305]  -> [email protected]()
[3013176.307]  -> [email protected](new id wl_callback@21)
[3013176.334] [email protected]_id(16)
[3013176.336] [email protected]_id(3)
[3013176.338] [email protected](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
Copy link
Contributor Author

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

@Cloudef
Copy link
Owner

Cloudef commented Feb 2, 2024

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
Copy link
Contributor Author

Note it only forces scale = 1 if you use BEMENU_SCALE

Who would use this? What is the use case?

@Cloudef
Copy link
Owner

Cloudef commented Feb 2, 2024

  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
Copy link
Owner

Cloudef commented Feb 2, 2024

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
Copy link
Contributor Author

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
Copy link
Owner

Cloudef commented Feb 3, 2024

FYI, I opened issue here hyprwm/Hyprland#4600

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants