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

Avoid freeze when interacting with menus on Wayland by re-aquiring next swapchain image after updating swapchain #79143

Merged
merged 1 commit into from Jul 9, 2023

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Jul 7, 2023

Fixes: #78487
Fixes: #75752

Tested on Windows and Linux (X11), this change does not reintroduce #77790

I am a little uncertain about this change. I am confident that it is minimal and should not cause any problems. But I am unsure if there is an even better fix for the issue. Wayland behaves differently from every other windowing system here, and since I am not running Wayland, it is very difficult to figure out how to make Wayland happy

@clayjohn clayjohn added bug platform:linuxbsd topic:porting regression cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Jul 7, 2023
@clayjohn clayjohn added this to the 4.2 milestone Jul 7, 2023
@clayjohn clayjohn requested a review from a team as a code owner July 7, 2023 07:11
@Riteo

This comment was marked as outdated.

Copy link
Contributor

@Riteo Riteo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for before. I wrote a huge wall of text speculating why this would work when I forgot that I could try on my new shiny desktop wayland setup with xwayland and all bells and whistles.

I can replicate the bug on the aforementioned shiny wayland setup, which is very similar to the one there (arch linux, RDNA2, mesa vulkan radeon) and i confirm that this PR fixes the issue.

As I expected, the branch never gets called (tested with a print_line right after the condition, before free(presentModes);). I think that this fix is somehow related to the fact that we never break from the loop, not the opposite as expected.

That said, this is pretty high priority so I'd merge in the meantime as it indeed fixes the issue.

IMO further investigation is warranted (I can help if you want).

Edit: Rereading the original reasoning behind the patch I might've misunderstood as usual. If the idea was exactly to account for the fact that we never get any null extents in this case... I'm sorry, this stuff is complicated and lately I'm really getting tangled in it.

@akien-mga akien-mga merged commit 1453dc9 into godotengine:master Jul 9, 2023
13 checks passed
@akien-mga
Copy link
Member

Thanks!

@clayjohn clayjohn deleted the wayland-menu branch July 9, 2023 11:15
@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jul 10, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.1.

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