Update window properties on PropertyNotify, clipboard manipulation refactor, general cleanup

[ryze312]

Fixes #68471

[bruvzg]

Why is it called desired_fullscreen? _window_fullscreen_check is checking if it's current full screen and desired mode is wd.fullscreen.

The code seems to be wrong, it will never change mode.

[ryze312]

I've tested both on KDE and BSPWM, all modes are working.
The issue is that BSPWM sends MapNotify before ConfigureNotify, causing wd.minimized to be outdated. I've provided an explanation in the linked issue.

What is the proper order of these messages?

[bruvzg]

_validate_mode_on_map was added to fix initial window mode (so if it was not mapped instantly after _create_window):

1. _create_window sets wd.property values and attempts to change mode.
2. If window mode was not set in step 1, it's later fixed when window is mapped. ConfigureNotify should not be involved at all.

#68471 seems to be about switching workspaces, I'm not sure if _validate_mode_on_map should be called in this case.

[ryze312]

Do we need to validate mode only on window creation? If so, would something like this work?

WindowData &wd = windows[p_window];

if (wd.validated) return;

if (wd.fullscreen && !_window_fullscreen_check(p_window)) {
	_set_wm_fullscreen(p_window, true, wd.exclusive_fullscreen);
} else if (wd.maximized && !_window_maximize_check(p_window, "_NET_WM_STATE")) {
	_set_wm_maximized(p_window, true);
} else if (wd.minimized && !_window_minimize_check(p_window)) {
	_set_wm_minimized(p_window, true);
}

wd.validated = true;

[ryze312]

Or maybe we can just listen for PropertyNotify events involving WM_STATE changes to set them instantly? That would fix the problem with KDE as well.

[ryze312]

On Plasma wd.minimized is never set to true, unless window_set_mode is called from GDScript or by engine.
This is the log of opening Godot and minimizing it:

Godot Engine v4.0.beta.custom_build.ba4bd7f00 - https://godotengine.org
show_window: 88080387 (0) 
[UNK] VALIDATE MODE:
[UNK] wd.minimized: false
[UNK] minimize_check: false
OpenGL Renderer: AMD Radeon Graphics (renoir, LLVM 14.0.6, DRM 3.48, 6.0.12-zen1-1-zen)
 
[1] MapNotify window=88080387 (0) 
[UNK] VALIDATE MODE:
[UNK] wd.minimized: false
[UNK] minimize_check: false
[1] wd.minimized: false
[1] VisibilityNotify window=88080387 (0), state=1 
[1] wd.minimized: false
[1] Expose window=88080387 (0), count='0' 
[1] wd.minimized: false
[1] FocusIn window=88080387 (0), mode='0' 
[1] wd.minimized: false
[1] VisibilityNotify window=88080387 (0), state=0 
[1] wd.minimized: false
[1] Expose window=88080387 (0), count='1' 
[1] wd.minimized: false
[1] Expose window=88080387 (0), count='0' 
[1] wd.minimized: false
[1] ConfigureNotify window=88080387 (0), event=88080387, above=0, override_redirect=0 
[UNK] Calling _window_changed
[1] EnterNotify window=88080387 (0), mode='0' 
[1] wd.minimized: false
[51] LeaveNotify window=88080387 (0), mode='0' 
[51] wd.minimized: false
[157] FocusOut window=88080387 (0), mode='0' 
[157] wd.minimized: false
All focus lost, triggering NOTIFICATION_APPLICATION_FOCUS_OUT

Plasma just calls FocusOut when window is minimized.

[ryze312]

This approach should work for any kind of window managers. I think, we should use PropertyNotify to monitor other properties as well (wd.fullscreen, wd.maximized etc).

For finding out which property needs to be updated I just used comparison of Atoms. I am sure, there is a better way to approach this, but I am not aware of it.

[clayjohn]

Ping @bruvzg

[arisu0]

patched it into my current rc2, works as expected, thank you!

[ryze312]

@bruvzg
What's the ime_window_event boolean? Is it related to the mentioned issue and should it be kept?

[bruvzg]

What's the ime_window_event boolean? Is it related to the mentioned issue and should it be kept?

It's not related to the issue, it is used to ignore an event from the IME subwindow (which was not a thing when the issue was first reported), it should be used to skip all events except ConfigureNotify, and keyboard input (it's needed for input events, but probably redundant for the other stuff, since subwindow do not have mask for these events set).

[ryze312]

What's the ime_window_event boolean? Is it related to the mentioned issue and should it be kept?

It's not related to the issue, it is used to ignore an event from the IME subwindow (which was not a thing when the issue was first reported), it should be used to skip all events except ConfigureNotify, and keyboard input (it's needed for input events, but probably redundant for the other stuff, since subwindow do not have mask for these events set).

I see, I'll just remove these checks to resolve conflict then, since they were used to skip window properties checks, which should be redundant with this PR.

[ryze312]

Just rebased the PR properly, I did a merge instead of rebase yesterday duh.

[ryze312]

I refactored clipboard manipulation, so it updates and caches them using XFixes events. Also did a cleanup by separating certain parts into functions.

[ryze312]

This code is frankly a bit of mess to work with, I would like to refactor it more.