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

Source buffer out of bounds using Wayland viewporter with fractional scaling #9283

Closed
StackDoubleFlow opened this issue Mar 17, 2024 · 3 comments · Fixed by #9285
Closed
Assignees
Milestone

Comments

@StackDoubleFlow
Copy link

StackDoubleFlow commented Mar 17, 2024

If the SDL_WINDOW_ALLOW_HIGHDPI window flag is set while fractional scaling is taking place under wayland (therefore wl_viewporter is in use), then the window is resized to be larger, wp_viewport@33: error 2: source rectangle out of buffer bounds is logged and the program crashes.

I can reproduce the issue with the following diff on the testgl2.c test:

diff --git a/test/testgl2.c b/test/testgl2.c
index d14ae1675..874b31b83 100644
--- a/test/testgl2.c
+++ b/test/testgl2.c
@@ -259,6 +259,7 @@ int main(int argc, char *argv[])
 
     /* Set OpenGL parameters */
     state->window_flags |= SDL_WINDOW_OPENGL;
+    state->window_flags |= SDL_WINDOW_ALLOW_HIGHDPI;
     state->gl_red_size = 5;
     state->gl_green_size = 5;
     state->gl_blue_size = 5;
@@ -381,6 +382,8 @@ int main(int argc, char *argv[])
     ctx.glDepthFunc(GL_LESS);
     ctx.glShadeModel(GL_SMOOTH);
 
+    SDL_SetWindowSize(state->windows[0], 1000, 1000);
+
     /* Main render loop */
     frames = 0;
     then = SDL_GetTicks();

When running with WAYLAND_DEBUG=1, we can see the following happen:
The viewport is set up with the initial window size

[ 977499.834]  -> wp_viewporter@15.get_viewport(new id wp_viewport@33, wl_surface@32)
[ 977499.838]  -> wp_viewport@33.set_source(0.00000000, 0.00000000, 960.00000000, 720.00000000)
[ 977499.842]  -> wp_viewport@33.set_destination(640, 480)

The viewport is reconfigured with the new window size

[ 977502.558]  -> wp_viewport@33.set_source(0.00000000, 0.00000000, 1500.00000000, 1500.00000000)
[ 977502.562]  -> wp_viewport@33.set_destination(1000, 1000)

A frame buffer of the original window size which is too small to be the viewport source is created, attached, and commited:

[ 977512.878]  -> zwp_linux_dmabuf_v1@31.create_params(new id zwp_linux_buffer_params_v1@42)
[ 977512.920]  -> zwp_linux_buffer_params_v1@42.add(fd 11, 0, 0, 4096, 33554432, 273070852)
[ 977512.926]  -> zwp_linux_buffer_params_v1@42.add(fd 12, 1, 3145728, 1024, 33554432, 273070852)
[ 977512.929]  -> zwp_linux_buffer_params_v1@42.create_immed(new id wl_buffer@43, 960, 720, 875713112, 0)
[ 977512.932]  -> zwp_linux_buffer_params_v1@42.destroy()
[ 977512.934]  -> wl_surface@32.attach(wl_buffer@43, 0, 0)
[ 977512.938]  -> wl_surface@32.damage(0, 0, 2147483647, 2147483647)
[ 977512.940]  -> wl_surface@32.commit()
[ 977512.942]  -> wl_display@1.sync(new id wl_callback@44)
[ 977513.048] wl_display@1.delete_id(41)
[ 977513.052] wl_display@1.delete_id(42)
[ 977513.053] wl_display@1.error(wp_viewport@33, 2, "source rectangle out of buffer bounds")
wp_viewport@33: error 2: source rectangle out of buffer bounds

I noticed that without the enabling of fractional scaling, a small framebuffer is still used but does not cause issues on swap. Maybe this is something that should be allowed by wlroots?

Downstream, this issue affects both osu-lazer and Factorio for me, and has been ever since the bounds checking was added in wlroots.

@StackDoubleFlow
Copy link
Author

The above patch is for SDL2 testgl2.c, but the same happens in SDL3 testgl.c with the SDL_WINDOW_HIGH_PIXEL_DENSITY window flag.

@Kontrabant
Copy link
Contributor

The viewport is now set to always use the entire buffer, so this shouldn't happen anymore.

Most likely what was happening was that Wayland events would be processed internally due to some call that invoked a round trip, and the window would be resized along with the viewport, but the client wouldn't check SDL events and get the notification until the next frame, after a buffer with the old size was committed.

You example basically forced the issue by resizing the window without querying and resizing the backbuffer. You didn't see the issue with scaling off, because SDL2 doesn't create a viewport for unscaled windows.

@StackDoubleFlow
Copy link
Author

Thanks for the quick fix!

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 a pull request may close this issue.

2 participants