always_on_top breaks non-embedded popups

[KoBeWi]

Godot version

307dfa9

System information

Windows 10 x64

Issue description

I guess sub-windows should inherit the always_on_top flag.

Steps to reproduce

1. Add any node with popup menu (e.g. TextEdit)
2. Enable always_on_top in project settings
3. Disable embed_subwindows
4. Run
5. Open the popup menu (right-click TextEdit etc)

Minimal reproduction project

No response

[rainlizard]

Is this the same issue?

Enabling the Always on Top checkbox of a FileDialog window makes it so I can't switch the hard drive!

[YuriSizov]

cc @Sauermann you may be interested in this.

[DmitriySalnikov]

PowerToys Always on Top also breaks popups

Godot_v4.2-rc1_win64_r0VTuBiaEq.mp4

[AlexOtsuka]

I was taking a look at this issue last night, and while I wasn't able to come up with a clean fix, here are a few things I've learned that might be helpful to know.

First off, all the previously mentioned issues are still current, and can be consistently reproduced on current master.
Both normal windows and popup menus are Window objects. Another important thing to note is that popups are always transient, and a transient window is not allowed to also be on top. For some reason, it's possible to enable both transient and always_on_top on FileDialog, but it does output an error message, at least in Windows:

Here's a reference for the hierarchy trees below, so that we're on the same page as to which windows I'm referring to.

The red arrow represents the drawing order, and the black lines are the parent-child relationships between each window. I use the setup from OP in my example, but the bug mentioned in one of the comments above happens for the exact same reason.

Using the default settings, the windows are drawn according to their hierarchy, and siblings in the order they were created. For this setup, the result is very straightforward:

When always_on_top is set to true on a window, it'll always be drawn after every window without that setting, although hierarchy still takes precedence. That means that since transient popups are still children objects of the always_on_top main window, they're still drawn in the correct order;

When embed_subwindows is disabled, new pop-ups aren't associated with the main window anymore. Instead, I believe they are considered direct children of the screen they're drawn on. However, when the always_on_top setting is false, the draw order remains unchanged:

When always_on_top is true and embed_subwindows = false is when it all breaks. This is because as there is no longer a relationship between the main window and its popup windows, the draw order takes precedence and we end up with something like this:

One thing I've attempted that looked somewhat promising, was to set transient to false and always_on_top to true whenever a popup window isn't embedded. While it looked okay, it completely broke submenu popups which is why I mentioned those in this comment. Note that this is a VERY crude approach and completely breaks editor popups as well:

godot/scene/main/window.cpp

Line 1811 in 655e93d

set_transient(true);

changed to

set_transient(is_embedded());
set_flag(Flags::FLAG_ALWAYS_ON_TOP, !is_embedded());

I don't believe this is the way to go at all, but it's how far I've gotten for now.

Perhaps, forcing the parent Viewport to embed a transient popup no matter what, or at the very least, figuring out a clean way for it to be taken into consideration when determining the draw order might be an acceptable solution.

Contextual popups are used everywhere in Godot, it's very important for them to be transient, some of this stuff is potentially OS specific and I'm not sure what the correct approach to the problem is. Would love to hear your thoughts and ideas.

[KoBeWi]

Seems fixed in 4.3.

[CsloudX]

Seems fixed in 4.3.

Seems it not fixed in 4.3 rc2.
plese test: #73335

[KoBeWi]

Yeah it's not fixed. I probably didn't test enough, sorry.

[Delsin-Yu]

Any reason this was postponed to 4.x? This is a pretty serious issue.

[Sauermann]

Moving the issue from 4.3 to 4.x was just a way to let people know, that this issue was not resolved during 4.3, so it was just a "documentation"-update.

Godot is a community-driven project, where most bugfixes and enhancements are created by volunteers who try to improve the engine in their spare time. There is no way to guess, when someone from the community finds a way to fix this specific issue.

[KoBeWi]

Note that the issue originally had 4.1 milestone. If the milestone is only going to be bumped with every release, there is no reason for assigning specific one.

[Delsin-Yu]

I once thought a milestone meant the Godot Officials considered it needed to be done before the associated version; thanks for the explanation.
I guess this issue is help wanted for now.

Why not just remove the milestone in this case? Many Issues don't have one anyway.

[KoBeWi]

I once thought a milestone meant the Godot Officials considered it needed to be done before the associated version

This mostly applies to recent regressions. If e.g. there is a new bug introduced in 4.4, it will get regression label and 4.4 milestone, with the intention that it's going to be fixed soon (and it mostly is). Recent regressions are also easier to fix, because it's easy to identify what exactly caused them and the original contributor is usually available to fix it.

For older issues though, the milestones were assigned more arbitrarily and we had many cases where they were just bumped with each release. This particular issue is likely as old as DisplayServer changes in Godot 4.0 and is missing someone who would be able to investigate and fix it.

[bruvzg]

Reproducible on macOS as well.