egl-wayland: Use the correct coordinates when setting the damage region

[Kontrabant]

Using the buffer width and height with wl_surface_damage is incorrect, as it expects surface-local coordinates. This causes issues when scaling using a viewport with an output region larger than the source region, as only a portion of the output region equal to the unscaled buffer size will be updated.

Use wl_surface_damage_buffer, when available, to set the damage region using buffer-local coordinates, otherwise, use wl_surface_damage with a sufficiently large width and height to declare the entire output region as damaged.

This is simpler than #50 while still fixing the underlying issue for clients using viewports.

[amshafer]

Do you have an example app/environment for triggering the scaling issue you mention?

My impression was that the surface size we used there was indeed in surface coordinates, @cubanismo do you know the answer to this? It seems that we get that from the wl_egl_window, so that would seem to also be window/surface coordinates?

As I understand it wl_surface_damage_buffer is the recommended request to use now, so I do think it is a good idea to switch to it.

[Kontrabant]

Any scenario where the viewport protocol is used to scale a smaller source buffer to a larger destination region.

If the destination size is set, it causes the surface size to become dst_width, dst_height. The source (rectangle) is scaled to exactly this size. This overrides whatever the attached wl_buffer size is, unless the wl_buffer is NULL.

This can be seen in the SDL tests by commenting out lines 661-665 in src/video/wayland/SDL_waylandwindow.c (a workaround for this issue), building the tests with -DSDL_TESTS=ON, running testsprite, and hitting ctrl+enter to enter a fullscreen mode that uses a viewport to scale a 640x480 buffer to the desktop dimensions. Only the upper-left 640x480 region is updated.

[cubanismo]

Sorry, I don't have any additional knowledge to add.

[kbrenneman]

Right, wl_surface::damage takes coordinates after any viewport transformations. If the window is scaled up, then the buffer size will be smaller than the window size, and only part of the window gets updated. And, since egl-wayland isn't the part that sends the viewport parameters, it has no way to know what those transformations might be. wl_surface::damage_buffer was added specifically to fix that particular design flaw.

wl_surface::damage does, however, let you specify something bigger than the window, so passing INT32_MAX is a reliable way to specify the whole window.

Note that we could simplify this even further by just using wl_surface::damage unconditionally, with INT32_MAX as the size. Having an option to use wl_surface::damage_buffer would mainly be interesting if we ever get around to wiring it up to EGL_KHR_swap_buffers_with_damage.

[amshafer]

Ah okay makes sense why the viewport protocol triggers this. I agree that simplifying to only use the wl_surface::damage request with INT32_MAX is probably the best approach for now.

[Kontrabant]

This was fixed in a more thorough manner by #138, so closing.