Wayland: fix monitor and scaling detection #356
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stacyharper
force-pushed
the
fix-multiple-outputs
branch
from
3e6346b
to
b302806
Compare
|
To reproduce the issue I had, connect two outputs with different scaling values, and try bemenu. On one or the other output, bemenu is blurry. |
stacyharper
changed the title
|
Draft for now. We should probably wait for https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/220 |
|
Perhaps the xdg-output protocol could be updated as well #297 |
|
Would this also fix the bug that on wayland (wlroots, river) with |
Depends. I only reproduce this when two monitor got different scaling. |
|
@stacyharper if use single monitor, but with |
stacyharper
force-pushed
the
fix-multiple-outputs
branch
from
b302806
to
c554552
Compare
stacyharper
changed the title
I dropped this completly. Wayland v4 added the "name" event to wl_outputs a long time ago now.
That seems completely expected if your output isn't scaled to 2. |
|
I rewrote this patch, and adapted the commit message, and merge request body.
Nak, we don't need this because we still need to fetch the max_height value. |
stacyharper
force-pushed
the
fix-multiple-outputs
branch
from
c554552
to
37dbe0e
Compare
|
The Flatpak image wayland protocol seems outdated. Wayland protocol v4 has been merged upstream the 24 jan 2021. @Cloudef: it is possible to bump it a bit? |
|
@stacyharper does it work if you bump the runtime version in both of the .yml files to |
The whole management of monitor_name with the Wayland backend seems heavily broken, and doesn't allow to (1) select the target output with -m, and (2) to correctly detect the used output and use correct attributes. When used with two monitors with different scale values, depending on the output order, you get a blurry menu. To fix this we attach a listener to wayland surfaces to store which outputs it belongs to. This rewrite almost completly the "recreate_windows" function. This also drop completly the xdg-output-unstable-v1 protocol, cause v4 already add the "name" event to fetch the output name. I did not pushed this to support v6, with the new "preferred_buffer_scale" event, cause anyway we need to fetch the output max_height.
stacyharper
force-pushed
the
fix-multiple-outputs
branch
2 times, most recently
from
4798e77
to
81ca5ff
Compare
Ah nice, it works! |
|
Thanks, merged |
|
With the recent update to bemenu v0.6.17 (and I believe due to this commit), I'm getting logged as My setup is unchanged from prior working version, and it has a single monitor and |
|
@dkwo could you run this command, and paste here the full output? $ sh -c 'grep -v '\''^#'\'' ~/bookmarks | WAYLAND_DEBUG=1 bemenu -i -l 50 | cut -d'\'' '\'' -f1' |
|
@stacyharper Sure: |
|
I don't reproduce. Even when I end up with a 1920/651 buffer, on an output 3840/2160, there is no crash. Can you give your output resolution, scale value, your $BEMENU_OPTS environmental value, and any other specific config you might have? |
|
Mhh, it looks like River send surface.enter events after layer_surface.ack_configure. So bemenu create buffers with the wrong scale values |
|
just to give more details, this is on river 0.2.4 (current version on void linux) and the shell is zsh. and |
|
Here the output on Sway 1.9-rc1, with a similar screen output, with the same BEMENU_OPTS, and the same command line arguments: We can see that here we never create a buffer with scale=1. We receive the surface enter first, so that bemenu knows that we use scale=2. My opinion is that River doesn't trigger the events in the correct order. I'm not sure yet if it is possible to work-around in this circumstance. It would probably means the first frame would be rendered incorrectly anyway. |
|
Also, I don't really understand why River would fails that lately about the 1920x625 scale=2. I mean this buffer has already been configured as scale=1. And it probably already has been displayed on the output. But the next frame, that is correctly created as 3840x1250 scale=2, cause a crash that speak about 625 not divisible by 2. Maybe there really is something wrong with River here? wl_buffer@14 has already been destroyed |
|
hmm, maybe @ifreund has ideas? |
|
Note that |
|
Regardless of what river is doing here, bemenu is committing a protocol error. From the In the log shared by @dkwo, this request is made but no buffer with proper dimensions is attached to the surface despite the fact that a buffer with proper dimensions ( I think this is pretty clearly indicative of a bemenu bug. As for the relative ordering the If there is a meaningful user-facing improvement that can be had by sending it earlier feel free to open a river issue, note however that river's master branch now leverages the wlroots scene graph API to send enter events though and note that compositors supporting |
|
Here is the offending commit, likely all that needs to be done to fix this is to attach the new buffer before this line: bemenu/lib/renderers/wayland/window.c Line 262 in ec6d7f9 Or perhaps the proper fix is to move the (also, that wl_display_roundtrip() is some major code smell, roundtrips should not be necessary after initial wl_registry setup) |
|
Thank you both so much. |
|
I see also conceptual issue here, the scale is queried after the surface enter event, but the surface enter is not available unless we have commited a buffer to a compositor, thus we need to recreate the buffer with new scale immediately... Is this because we want to avoid using some xdg protocol? |
|
I haven't done wayland dev in long time, but is wlr layer protocol's interpretation of buffer scale somehow different? yet the surface isn't being rendered 4x bigger neither in hyprland or sway. EDIT: |
This behavior depend on the compositor. Sway 1.9-rc1 doesn't do this: Here, we can see that bemenu receive AFAIK if the compositor require to attach a buffer, then it become impossible for us to render a correct first frame. |
|
Perhaps the issue is then only if you use for example |
Mhh no: diff --git a/lib/renderers/wayland/window.c b/lib/renderers/wayland/window.c
index dd2bea0..d3e3e4a 100644
--- a/lib/renderers/wayland/window.c
+++ b/lib/renderers/wayland/window.c
@@ -253,6 +253,7 @@ bm_wl_window_render(struct window *window, struct wl_display *display, struct bm
window->notify.render(&buffer->cairo, buffer->width, window->max_height, menu, &result);
window->displayed = result.displayed;
+ fprintf(stderr, "bm_wl_window_render loop scale %d\n", buffer->cairo.scale);
if (window->height == result.height / buffer->cairo.scale)
break;
|
|
3156dac @stacyharper Nevermind I just realized the bm_wl_window_render has to create the buffer twice in scenario where it still does not know the final height. |
|
Okay! I think now I understand how BEMENU_SCALE was miss-behaving. It doensn't mess with the wayland scaling, but it upscale the bemenu window completly, while still being scaled pixel perfect. Am I right? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The whole management of monitor_name with the Wayland backend seems heavily
broken, and doesn't allow to (1) select the target output with -m, and (2) to
correctly detect the used output and use correct attributes.
When used with two monitors with different scale values, depending on
the output order, you get a blurry menu.
To fix this we attach a listener to wayland surfaces to store which outputs
it belongs to. This rewrite almost completly the "recreate_windows"
function.
This also drop completly the xdg-output-unstable-v1 protocol, cause v4
already add the "name" event to fetch the output name.
I did not pushed this to support v6, with the new
"preferred_buffer_scale" event, cause anyway we need to fetch the
output max_height.