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

Always wait for effective resizing? #83692

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rakkarage
Copy link
Contributor

@rakkarage rakkarage commented Oct 20, 2023

#83693

This fix is probably wrong but it seems to work... called twice first time waits 50 attempts second times waits ~3 attempts

@@ -2278,7 +2278,8 @@ void DisplayServerX11::_set_wm_maximized(WindowID p_window, bool p_enabled) {

XSendEvent(x11_display, DefaultRootWindow(x11_display), False, SubstructureRedirectMask | SubstructureNotifyMask, &xev);

if (p_enabled && window_is_maximize_allowed(p_window)) {
if (p_enabled) {
window_is_maximize_allowed(p_window);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi! window_is_maximize_allowed returns a bool on whether it can be done and that's it, so I'm not really sure that calling it standalone makes any sense.

I can't find any meaningful side-effect in that method in the X11 implementation either, so I think that you're pretty much removing it from the condition and that's it, which isn't very useful.

Why are you changing this part? I'm not really sure how this is related to the splash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ya i should have just removed it from the if... works.
Main::setup2 sets the resolution and then draws the splash...
this i guess adds an extra delay between? idk

Copy link
Contributor

Choose a reason for hiding this comment

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

this i guess adds an extra delay between? idk

I think it... shouldn't? It's all done in the main thread I think.

Not really sure what's going on 🤷

Perhaps someone more experienced with the X11 backend could help.

Copy link
Contributor Author

@rakkarage rakkarage Oct 22, 2023

Choose a reason for hiding this comment

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

ya i guess the underlying problem is maybe that _window_maximize_check always returns false (on wayland?) and idk how to fix... so i just cut it out

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this might be some XWayland quirk. It looks oddly familiar to #80036 so perhaps it could be a good starting point, although from a quick glance we might already be using EWMH for _window_maximize_check. Not sure though.

Either way, I'm not sure that completely disabling such a condition because of XWayland is a good idea, especially as we're going to get soon (4.3) a proper native Wayland backend.

Does window_is_maximize_allowed work at all on XWayland?

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

Successfully merging this pull request may close these issues.

4 participants