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

wayland: Refactor toplevel mapping, implement HideWindow #4319

Merged
merged 1 commit into from
Apr 21, 2021

Conversation

flibitijibibo
Copy link
Collaborator

This moves around the xdg_toplevel management in a way that allows us to implement HideWindow/ShowWindow properly.

For the most part this actually ended up being isolated from the rest of the driver; we have some early returns when the toplevel hasn't been made yet but it turns out it's more robust to just push all window state at ShowWindow time anyway. As a result this is really just changing the location and not really changing core functionality. The one exception is an extremely annoying behavior in the Wayland protocol where you have to manually detach any buffers from the wl_surface when creating the toplevel a second time, even though unmapping is supposed to be a full reset.

This was tested in both windowed/fullscreen mode on GNOME and Plasma and behavior was consistent.

@flibitijibibo
Copy link
Collaborator Author

flibitijibibo commented Apr 20, 2021

This patch revealed a bug in Plasma where destroying a toplevel object causes the application to freeze when a Vulkan context attempts to call vkAcquireNextImageKHR when using FIFO presentation. No errors are printed and GNOME works correctly, as does using a different presentation mode, and the issue was reproduced on both RADV and AMDVLK. Avoiding reallocation of the xdg_surface and only recreating the toplevel had no effect.

WAYLAND_DEBUG output is attached: kdehang.txt

AFAICT this is outside the boundaries of SDL, as we're doing the same thing as other toolkits and we're following the xdg-shell spec as written.

@flibitijibibo
Copy link
Collaborator Author

KWin bug has been filed: https://bugs.kde.org/show_bug.cgi?id=435978

@bl4ckb0ne
Copy link
Contributor

bl4ckb0ne commented Apr 21, 2021

I ran wltest on sway 1.5.1 and the bug is also there, the window never reappears. The trace is a bit different https://paste.sr.ht/~bl4ckb0ne/abb38f8982ba4e481468c21eaa365c1617c4ee9b

Would it be possible to have access to the sources of wltest?

@flibitijibibo
Copy link
Collaborator Author

wltest.tar.bz2 is MonoKickstart with FNA and a wltest.cs, provided in the tarball.

MonoKickstart: https://github.com/flibitijibibo/MonoKickstart

FNA: https://github.com/FNA-XNA/FNA

FNA3D, which may also matter: https://github.com/FNA-XNA/FNA3D

@icculus
Copy link
Collaborator

icculus commented Apr 21, 2021

I'm merging this on the assumption this last issue either isn't our bug, or it is but we're still better off than we were. In the latter case, we can fix it when more information arrives later.

@flibitijibibo
Copy link
Collaborator Author

Right, so it turns out I'm a moron (big surprise), and the OpenGL case wasn't working as intended because I forgot we never fixed the lack of EGL_EXT_swap_control_tear:

FNA-XNA/FNA3D@fddf4e0

So OpenGL and Vulkan behave the same, for the same reason. It turns out this is actually #4335 in disguise. This patch is good, it's just that Wayland screwed up big time with their vsync implementation.

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.

3 participants