Windows: Add support for enabling Alt+Space menu and fix borderless maximize #88329

ManpreetXSingh · 2024-02-14T14:53:10Z

Fixes #88277
~~For now, I have added the option in display/window/size/. Where would be the best place for it?~~

Works in the editor
Works when the project is run
Works in the project manager
Works in borderless mode
Fixes an issue where a ~~borderless~~ window might get stuck in a given mode
Separates mode borderless maximized (does not cover taskbar) from mode borderless fullscreen
Fixes an issue where a newly created maximized borderless window had incorrect size (if mode was set through project settings)

doc/classes/ProjectSettings.xml

Mickeon · 2024-02-14T14:58:56Z

Where would be the best place for it?

Good question. I don't think there's a good place for it. application/run would be more suitable but it's not perfect, in my opinion.

Mickeon · 2024-02-14T15:01:56Z

It's also most definitely not GLOBAL_DEF_BASIC for being very niche. That macro shorthand is for defining a project setting that will always appear regardless of this toggle:

You may want to use GLOBAL_DEF, instead.

doc/classes/ProjectSettings.xml

ManpreetXSingh · 2024-02-14T15:59:25Z

Why does this not work in the project manager though? I have no idea how to enable that.
Edit: I think, this should work Engine::get_singleton()->is_project_manager_hint() 😅

AThousandShips · 2024-02-14T16:03:08Z

The project manager doesn't use the settings of any specific project so it defaults to false I think, you need to handle it specifically there

ManpreetXSingh · 2024-02-14T17:51:42Z

I was able to make it work with borderless mode, but it requires modifications to this line (adding WS_SYSMENU, WS_MAXIMIZEBOX and WS_MINIMIZEBOX styles)

godot/platform/windows/display_server_windows.cpp

Line 1869 in 907db8e

r_style |= WS_POPUP; // p_borderless was WS_EX_TOOLWINDOW in the past.

This might break other stuff. So, should I make these changes?

I did some testing with borderless mode 😊:
The window is in borderless mode for the entire duration.

Windowed, Minimized : Good
Exclusive Fullscreen : While the alt+tab menu is displayed the visible region changes size to same as what it was in windowed mode and the rest of the region appears black.
Fullscreen : The alt+tab menu allows moving the window, so the menu should probably be disabled in both fullscreen modes
Maximized
- Through alt+space menu:
  - Going to maximized:
    - (Before fix) Sets mode to Exclusive Fullscreen instead of Maximized.
    - (After fix) Mode is set to Maximized. (If the taskbar is hidden, then mode switches to Exclusive Fullscreen)
  - Coming back (to windowed): Good
- Through code:
  - Going to maximized:
    - (Before fix) Mode is set to Fullscreen (has 1px border)
    - (After fix) Mode is set to Windowed (also has a 1px border, so the window remains 2px smaller than the expected size in Maximized mode)
  - Coming back (to windowed):
    - (Before fix) Window size and position both get set to (0, 0)
    - (After fix) Mode remains Windowed (the 1px border also sustains)

I've pushed changes (for now) to allow testing.
I'm not sure what should be done.

ManpreetXSingh · 2024-02-15T04:46:46Z

The above listed issues must be fixed now. Also fixed an issue with window mode getting stuck. I tested with the following code:

func _input(event: InputEvent) -> void:
	if event is InputEventKey:
		if event.keycode == KEY_C and event.pressed:
			print("Mode: ", get_window().mode)
			print("Position: ", get_window().position)
			print("Size: ", get_window().size)
		
		if event.keycode == KEY_Q and event.pressed:
			get_tree().quit()

		if event.keycode == KEY_B and event.pressed:
			get_window().borderless = !get_window().borderless

		if event.keycode == KEY_F and event.pressed:
			get_window().mode = Window.MODE_FULLSCREEN

		if event.keycode == KEY_E and event.pressed:
			get_window().mode = Window.MODE_EXCLUSIVE_FULLSCREEN

		if event.keycode == KEY_W and event.pressed:
			get_window().mode = Window.MODE_WINDOWED

		if event.keycode == KEY_M and event.pressed:
			get_window().mode = Window.MODE_MAXIMIZED

		if event.keycode == KEY_N and event.pressed:
			get_window().mode = Window.MODE_MINIMIZED

platform/windows/display_server_windows.cpp

Calinou

Tested locally on Windows 11 22H2, it works but I can't use keyboard shortcuts in the editor such as F5 after using Alt + Space then dismissing the popup. I need to unfocus the editor window then focus it again to be able to do that. This occurs regardless of how the popup is dismissed (by pressing Escape or clicking outside the popup).

The behavior regarding where it's enabled is good still. (It's enabled in the editor and project manager and disabled by default in projects.)

ManpreetXSingh · 2024-02-16T12:29:10Z

@Calinou Thanks for testing this.
I noticed that this occurs because the alt key is still held after the popup is dismissed. Pressing and releasing alt after the popup is dismissed restores the original state.

I'm trying to figure out a fix for this 👍.

ManpreetXSingh · 2024-02-16T21:16:21Z

I noticed that SC_KEYMENU is received even when alt alone is pressed and released. This also sends SC_ENTERMENULOOP, and does not even display the menu 🤦‍♂️ (my bad). The menu loop is exited only after alt is pressed and released again. This is fixed ✅.

The issue that @Calinou described should also be fixed ✅.
Note: You have to press Esc twice or alt once to exit the menu completely.

This does change the keyboard events that gdscript will receive:

KEY PRESSED: Alt
(menu is displayed)
(menu is closed)
KEY PRESSED: Alt+Space    ## alt+space is pressed before the menu is displayed, but the event is received after
KEY RELEASED: Alt+Space
KEY RELEASED: Alt
KEY RELEASED: Escape | Alt etc..    ## this is the key used to close the menu, key pressed is not received

with menu disabled:

KEY PRESSED: Alt
KEY PRESSED: Alt+Space
KEY RELEASED: Alt+Space
KEY RELEASED: Alt

The following code was used:

func _input(event: InputEvent) -> void:
	if event is InputEventKey:
		print("KEY ", "PRESSED: " if event.pressed else "RELEASED: ", event.as_text_keycode())

ManpreetXSingh · 2024-02-16T21:36:49Z

One issue introduced by this PR is with the animations that are played (in bordered mode)

Transitioning from Maximized to Fullscreen triggers the Unmaximizing animation
Transitioning from Fullscreen to Maximized triggers the Maximizing animation.

This behavior is a result of fixing an issue where transitioning Maximized -> Fullscreen -> Windowed was not possible before this PR.

popcar2 · 2024-02-16T23:03:26Z

Coincidentally I found this PR while writing about how wacky and nonsensical Godot's borderless fullscreen is on Windows

I tried this PR on my MRP for fullscreen modes (you can find it on that comment) and while this PR does technically fix a bug, it also unknowingly removes the only band-aid solution for having a borderless fullscreen mode on Windows without a 1px border.

I know fixing that is not in the scope of this PR but by fixing maximized borderless windowed mode, this technically breaks compatibility for a workaround people are using until someone fixes borderless fullscreen. ¯\_(ツ)_/¯

Mickeon · 2024-02-16T23:10:58Z

I wonder if it's worth to revert borderless-related changes and only keeping the Alt-Space menu in most cases (for now), or just addressing everything all at once. This relatively simple addition is turning into quite the rabbit hole...

bruvzg

Exclusive Fullscreen : While the alt+tab menu is displayed the visible region changes size to same as what it was in windowed mode and the rest of the region appears black.

That's how it should be, exclusive fs disables composition and only one window can be displayed, if it's not like this with your change, you broke exclusive fs.

bruvzg · 2024-02-16T23:25:44Z

platform/windows/display_server_windows.cpp

@@ -3473,6 +3480,14 @@ LRESULT DisplayServerWindows::WndProc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARA
 					min_max_info->ptMaxTrackSize.x = windows[window_id].max_size.x + decor.x;
 					min_max_info->ptMaxTrackSize.y = windows[window_id].max_size.y + decor.y;
 				}
+				if (windows[window_id].borderless) {
+					Rect2i screen_rect = screen_get_usable_rect(window_get_current_screen(window_id));
+					int border = ((screen_get_size(window_get_current_screen(window_id)) == screen_rect.size) ? 1 : 0);


What's that supposed to do? If you are trying to make window bigger to hide border in non-exclusive fullscreen, it will not work on many GPUs (depends on a driver, some account only for client area) and will break mulitwindow apps like Godot editor.

Here actually, I am trying to add the border instead in maximized mode (when the taskbar and other panels are hidden), this border is not needed when the taskbar is present already (is it needed? 😅).
The border was previously being added within the if block at line 1870.
Now it is only added if maximized size == screen size, to prevent automatically switching to exclusive fs (no border is added, actually the window size is made 1px smaller in all directions😅)

This code seems problematic.. right?
Making the window smaller shouldn't be the right way.
should we undo this? we will have to disable maximization support (through the menu) in borderless mode, if we decide to undo this.
Or is there a better solution to allow maximization in borderless mode when the taskbar is hidden?

Changing window size is not a good option, that's why WS_BORDER was applied to maximized window (to keep window size and reduce client area), but I'm not sure if it would work with WS_MAXIMIZE.

I would always disable Alt+Space for borderless and full-screen if someone is using borderless window, it's not expected to be maximizable (and if it is, a custom decorations and Alt+Space menu should be implemented).

WS_BORDER does work with WS_MAXIMIZE just fine (I tried just now). But the problem then becomes how do we update the style when maximization is done through the menu. Because our _update_window_style is not called (I think) when maximization happens through the menu.

The common part that runs in both cases (through code and through menu) is here.

The problematic border has been removed, now we use WS_BORDER if the maximized window covers the entire screen👍

Hi 😊

if someone is using borderless window, it's not expected to be maximizable

I think it is expected to be borderless maximizable, as on linux, godot does support borderless maximization, and window menu also works with borderless windows.
For fullscreen, the menu has been disabled ✅

ManpreetXSingh · 2024-02-17T11:07:26Z

I had to make these changes because the maximizing behavior through the menu and through code was different. See: #88329 (comment) .
I'm willing to discuss the possible solutions ✅.

That's how it should be, exclusive fs disables composition and only one window can be displayed, if it's not like this with your change, you broke exclusive fs.

I've removed the ability to use the window menu in fullscreen and exclusive fs modes. ✅

This relatively simple addition is turning into quite the rabbit hole...

You are right 😅

I wonder if it's worth to revert borderless-related changes

I wish we don't have to. 🥲

ManpreetXSingh · 2024-02-17T11:40:29Z

I'm curious about why the workaround by @popcar2 is effective, considering the window size is larger than the screen size. Wouldn't it be expected for some GPUs to switch to exclusive fullscreen mode since there's no border? @bruvzg

@popcar2 as a workaround this can be used (for now), I hope it is good enough:

var curr_screen := DisplayServer.window_get_current_screen(get_window().get_window_id())
var screen_size := DisplayServer.screen_get_size(curr_screen)
var screen_pos := DisplayServer.screen_get_position(curr_screen)
get_window().mode = Window.MODE_FULLSCREEN
get_window().mode = Window.MODE_WINDOWED
get_window().size = screen_size + Vector2i(2, 2)
get_window().position = screen_pos - Vector2i(1, 1)

To check if the window is in fullscreen mode:

get_window().size == screen_size + Vector2i(1, 1) and get_window().position == screen_pos - Vector2i(1, 1)

Also, you'll need to save the original window size in case it needs to be restored.

bruvzg · 2024-02-17T11:54:41Z

Exclusive fs is triggered by client area size, making it bigger or smaller (by adding border like we do) will prevent it. But if it's bigger it will stick to the other displays, and might cause GPU driver to bug out and fullscreen it on the wrong screen.

ManpreetXSingh · 2024-02-17T12:04:52Z

But if it's bigger it will stick to the other displays, and might cause GPU driver to bug out and fullscreen it on the wrong screen.

Thanks for clarifying 👍.

I noticed, @popcar2 's workaround (this one):

get_window().mode = Window.MODE_MAXIMIZED
get_window().borderless = true
get_window().mode = Window.MODE_MAXIMIZED

actually switches to Exclusive fs mode (on 4.3 dev 2 and 4.2 stable), Incorrect mode Windowed is reported by gdscript. (Notice flicker when alt+tab). (on 4.1 stable the mode is also Exclusive fs but gdscript reports Fullscreen)

(this one):

get_window().mode = Window.MODE_MAXIMIZED
get_window().borderless = true
get_window().mode = Window.MODE_MAXIMIZED
get_window().borderless = true

makes the window size 2px bigger than the screen size. The mode reported is Windowed (In 4.1 stable the 1px border exists, and mode is reported Fullscreen). So, the above described workaround (of setting the window size and position) should be the same.

Exclusive fs is triggered by client area size, making it bigger or smaller (by adding border like we do) will prevent it.

~~I think I did it (fixes in PR) in the correct way then.~~ Does this PR require any additional changes?
Regarding the border addition, I'm wondering if it should be included in maximized mode when the window size matches the screen size, instead of making the window smaller. Also, should the border be added when the maximized window is smaller than the screen size due to the taskbar?

Test project (The background color is set to white make the border easily noticeable, and the script is attached with the appropriate keybindings for changing window modes):
DevTestProject.zip

ManpreetXSingh · 2024-02-17T12:58:19Z

(BUG) Also exists in v4.3 dev2 and v4.2 stable, (window is in bordered mode before running this) (quite the rabbit hole):

get_window().mode = Window.MODE_MAXIMIZED
get_window().borderless = true

This code sets the window position offscreen, (-7, -7) in my case. I believe it would be beneficial to create a separate issue for this.

This should undo the effect:

get_window().mode = Window.MODE_WINDOWED
get_window().mode = Window.MODE_MAXIMIZED

ManpreetXSingh · 2024-02-19T14:06:15Z

I was about to undo the maximization stuff, but then I figured it out a better solution (I think).
Also fixed a bug where if a borderless window was set to maximized mode in the project settings, the window size was not correct (It remained in windowed mode actually, only the 1px border appeared around the window). (Tested in 4.0.3 stable and 4.3 dev2)

ManpreetXSingh · 2024-02-19T15:53:39Z

platform/windows/display_server_windows.cpp

@@ -4742,7 +4781,7 @@ DisplayServer::WindowID DisplayServerWindows::_create_window(WindowMode p_mode,
 	DWORD dwExStyle;
 	DWORD dwStyle;

-	_get_window_style(window_id_counter == MAIN_WINDOW_ID, (p_mode == WINDOW_MODE_FULLSCREEN || p_mode == WINDOW_MODE_EXCLUSIVE_FULLSCREEN), p_mode != WINDOW_MODE_EXCLUSIVE_FULLSCREEN, p_flags & WINDOW_FLAG_BORDERLESS_BIT, !(p_flags & WINDOW_FLAG_RESIZE_DISABLED_BIT), p_mode == WINDOW_MODE_MAXIMIZED, (p_flags & WINDOW_FLAG_NO_FOCUS_BIT) | (p_flags & WINDOW_FLAG_POPUP), dwStyle, dwExStyle);
+	_get_window_style(window_id_counter == MAIN_WINDOW_ID, (p_mode == WINDOW_MODE_FULLSCREEN || p_mode == WINDOW_MODE_EXCLUSIVE_FULLSCREEN), p_mode != WINDOW_MODE_EXCLUSIVE_FULLSCREEN, p_flags & WINDOW_FLAG_BORDERLESS_BIT, !(p_flags & WINDOW_FLAG_RESIZE_DISABLED_BIT), p_mode == WINDOW_MODE_MAXIMIZED, false, (p_flags & WINDOW_FLAG_NO_FOCUS_BIT) | (p_flags & WINDOW_FLAG_POPUP), dwStyle, dwExStyle);


maximized_fs is handled inside WM_WINDOWPOSCHANGED [This is required because maximization could occur through external means (via the menu) which bypasses our state management logic at window_set_mode👍].

In case needed the following can be used here instead of false (not needed for now because it is handled at WM_WINDOWPOSCHANGED):

bool is_maximized_fs = (p_mode == WINDOW_MODE_MAXIMIZED) && (p_flags & WINDOW_FLAG_BORDERLESS_BIT) && (screen_get_size(rq_screen) == screen_get_usable_rect(rq_screen).size) && (screen_get_position(rq_screen) == screen_get_usable_rect(rq_screen).position);

doc/classes/ProjectSettings.xml

Calinou

Tested locally (rebased on top of master 655e93d), it works as expected.

I've tested the following scenarios:

Window (resizable)
Window (non-resizable)
Fullscreen (Alt + Space is ineffective in this mode, as expected)
Exclusive Fullscreen (Alt + Space is ineffective in this mode, as expected)

platform/windows/display_server_windows.cpp

doc/classes/ProjectSettings.xml

platform/windows/display_server_windows.cpp

AThousandShips · 2024-04-06T08:36:34Z

Please squash your commits into one, see here

AThousandShips · 2024-04-06T10:03:51Z

Please remove all the unnecessary text from the commit message, just keep the title

AThousandShips · 2024-04-06T10:27:41Z

Looks great!

akien-mga · 2024-04-06T11:05:41Z

There seems to be a remaining style issue: https://github.com/godotengine/godot/actions/runs/8580514699/job/23516655245?pr=88329

You can fix it with clang-format or manually, and then amend the commit with that change to keep the PR with a single fixed commit.

akien-mga · 2024-04-08T09:26:08Z

Thanks! And congrats for your first merged Godot contribution 🎉

ManpreetXSingh · 2024-04-08T17:37:16Z

Thanks everyone!! 🎉🎉
That means a lot to me! 😊

ManpreetXSingh requested review from a team as code owners February 14, 2024 14:53

AThousandShips added enhancement platform:windows topic:porting labels Feb 14, 2024

AThousandShips added this to the 4.x milestone Feb 14, 2024

AThousandShips reviewed Feb 14, 2024

View reviewed changes

doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved

AThousandShips reviewed Feb 14, 2024

View reviewed changes

doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved

ManpreetXSingh requested a review from AThousandShips February 14, 2024 17:41

ManpreetXSingh changed the title ~~Add optional support to enable alt+space menu (Windows)~~ Add support to enable alt+space menu and fix borderless maximize (Windows) Feb 15, 2024

AThousandShips reviewed Feb 15, 2024

View reviewed changes

platform/windows/display_server_windows.cpp Outdated Show resolved Hide resolved

ManpreetXSingh requested a review from AThousandShips February 15, 2024 18:48

Calinou requested changes Feb 15, 2024

View reviewed changes

Mickeon requested a review from Calinou February 16, 2024 21:54

bruvzg reviewed Feb 16, 2024

View reviewed changes

ManpreetXSingh requested a review from bruvzg February 17, 2024 13:18

AThousandShips changed the title ~~Add support to enable alt+space menu and fix borderless maximize (Windows)~~ Windows: Add support for enabling alt+space menu and fix borderless maximize Feb 17, 2024

popcar2 mentioned this pull request Feb 19, 2024

Enabling fullscreen via code shows window border at screen edges (DisplayServer regression) #63500

Open

ManpreetXSingh commented Feb 19, 2024

View reviewed changes

Calinou reviewed Apr 5, 2024

View reviewed changes

doc/classes/ProjectSettings.xml Outdated Show resolved Hide resolved

Calinou approved these changes Apr 5, 2024

View reviewed changes

AThousandShips approved these changes Apr 6, 2024

View reviewed changes

ManpreetXSingh force-pushed the key-window-menu branch from adf503b to c7cea04 Compare April 6, 2024 10:02

ManpreetXSingh force-pushed the key-window-menu branch from c7cea04 to 6319d38 Compare April 6, 2024 10:27

akien-mga modified the milestones: 4.x, 4.3 Apr 6, 2024

Windows: Add alt+space menu and fix window modes

329d04d

ManpreetXSingh force-pushed the key-window-menu branch from 6319d38 to 329d04d Compare April 6, 2024 11:07

akien-mga changed the title ~~Windows: Add support for enabling alt+space menu and fix borderless maximize~~ Windows: Add support for enabling Alt+Space menu and fix borderless maximize Apr 6, 2024

akien-mga merged commit 7d96ec4 into godotengine:master Apr 8, 2024
16 checks passed

alvinhochun mentioned this pull request Jul 10, 2024

WINDOW_MODE_WINDOWED acting weird #86418

Open

Windows: Add support for enabling Alt+Space menu and fix borderless maximize #88329

Windows: Add support for enabling Alt+Space menu and fix borderless maximize #88329

Conversation

ManpreetXSingh commented Feb 14, 2024 • edited Loading

Mickeon commented Feb 14, 2024

Mickeon commented Feb 14, 2024

ManpreetXSingh commented Feb 14, 2024 • edited Loading

AThousandShips commented Feb 14, 2024

ManpreetXSingh commented Feb 14, 2024 • edited Loading

ManpreetXSingh commented Feb 15, 2024 • edited Loading

Calinou left a comment • edited Loading

Choose a reason for hiding this comment

ManpreetXSingh commented Feb 16, 2024 • edited Loading

ManpreetXSingh commented Feb 16, 2024 • edited Loading

ManpreetXSingh commented Feb 16, 2024 • edited Loading

popcar2 commented Feb 16, 2024 • edited Loading

Mickeon commented Feb 16, 2024

bruvzg left a comment

Choose a reason for hiding this comment

bruvzg Feb 16, 2024

Choose a reason for hiding this comment

ManpreetXSingh Feb 17, 2024 • edited Loading

Choose a reason for hiding this comment

ManpreetXSingh Feb 17, 2024

Choose a reason for hiding this comment

bruvzg Feb 17, 2024

Choose a reason for hiding this comment

bruvzg Feb 17, 2024

Choose a reason for hiding this comment

ManpreetXSingh Feb 17, 2024 • edited Loading

Choose a reason for hiding this comment

ManpreetXSingh Feb 19, 2024

Choose a reason for hiding this comment

ManpreetXSingh Feb 20, 2024

Choose a reason for hiding this comment

ManpreetXSingh commented Feb 17, 2024 • edited Loading

ManpreetXSingh commented Feb 17, 2024 • edited Loading

bruvzg commented Feb 17, 2024

ManpreetXSingh commented Feb 17, 2024 • edited Loading

ManpreetXSingh commented Feb 17, 2024 • edited Loading

ManpreetXSingh commented Feb 19, 2024 • edited Loading

ManpreetXSingh Feb 19, 2024 • edited Loading

Choose a reason for hiding this comment

Calinou left a comment

Choose a reason for hiding this comment

AThousandShips commented Apr 6, 2024

AThousandShips commented Apr 6, 2024

AThousandShips commented Apr 6, 2024

akien-mga commented Apr 6, 2024

akien-mga commented Apr 8, 2024

ManpreetXSingh commented Apr 8, 2024

ManpreetXSingh commented Feb 14, 2024 •

edited

Loading

ManpreetXSingh commented Feb 14, 2024 •

edited

Loading

ManpreetXSingh commented Feb 14, 2024 •

edited

Loading

ManpreetXSingh commented Feb 15, 2024 •

edited

Loading

Calinou left a comment •

edited

Loading

ManpreetXSingh commented Feb 16, 2024 •

edited

Loading

ManpreetXSingh commented Feb 16, 2024 •

edited

Loading

ManpreetXSingh commented Feb 16, 2024 •

edited

Loading

popcar2 commented Feb 16, 2024 •

edited

Loading

ManpreetXSingh Feb 17, 2024 •

edited

Loading

ManpreetXSingh Feb 17, 2024 •

edited

Loading

ManpreetXSingh commented Feb 17, 2024 •

edited

Loading

ManpreetXSingh commented Feb 17, 2024 •

edited

Loading

ManpreetXSingh commented Feb 17, 2024 •

edited

Loading

ManpreetXSingh commented Feb 17, 2024 •

edited

Loading

ManpreetXSingh commented Feb 19, 2024 •

edited

Loading

ManpreetXSingh Feb 19, 2024 •

edited

Loading