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

[4.0.beta9] Minimum window size not applied at startup #70347

Closed
DarkMessiah opened this issue Dec 20, 2022 · 5 comments · Fixed by #70863
Closed

[4.0.beta9] Minimum window size not applied at startup #70347

DarkMessiah opened this issue Dec 20, 2022 · 5 comments · Fixed by #70863

Comments

@DarkMessiah
Copy link
Contributor

Godot version

v4.0.beta9

System information

Windows 10

Issue description

Minimum window size not applied at startup and DisplayServer.window_set_min_size isn't work as expected.

v4.0.beta9 v4.0.beta8
image image

Steps to reproduce

  1. Run game
  2. Minimize window size

Minimum size must be (64, 64).


Isn't work in beta9:

func _ready() -> void:
       DisplayServer.window_set_min_size(Vector2i(100, 100))

Works in beta9:

func _ready() -> void:
       await get_tree().process_frame
       DisplayServer.window_set_min_size(Vector2i(100, 100))

Minimal reproduction project

window_min_size_bug.zip

@akien-mga
Copy link
Member

This could be good to bisect, between:

@DarkMessiah
Copy link
Contributor Author

@YuriSizov #69353

commit 29cc86fa6c19c32b44522bdac9726e2392b7182b
Author: Yuri Sizov <yuris@humnom.net>
Date:   Tue Nov 29 23:01:45 2022 +0300

    Copy local theme overrides from Control to Window

 doc/classes/Control.xml |   3 +-
 doc/classes/Window.xml  | 210 ++++++++++++++++---
 scene/gui/control.cpp   | 158 +++++++--------
 scene/gui/control.h     |  12 +-
 scene/main/window.cpp   | 522 +++++++++++++++++++++++++++++++++++++++++++++---
 scene/main/window.h     |  61 +++++-
 6 files changed, 815 insertions(+), 151 deletions(-)

@YuriSizov
Copy link
Contributor

YuriSizov commented Dec 21, 2022

I don't think this is supposed to work to begin with. If you want to enforce a minimum size of a window node, you need to do it on the node itself. Because the moment _update_window_size() is called, DisplayServer::get_singleton()->window_set_min_size() will also be called with a new value, rewriting your setting.

This was the only thing that the offending PR did that could cause the issue to appear, it calls _update_window_size() on theme changes. But I bet you'll reproduce this issue if you add a child node to your window after calling DisplayServer::get_singleton()->window_set_min_size(), or indeed changing window's own min or max size settings.

So it worked before by a happenstance. And now we're almost guaranteed to call for an update on the window size for any window, which highlights the issue in your code immediately.

@DarkMessiah
Copy link
Contributor Author

But it works in 4.0 beta 8 and 3.5.1. If you run the project without any code, the window should be (64, 64), but it near (0, 0).

https://docs.godotengine.org/en/latest/classes/class_displayserver.html#class-displayserver-method-window-set-min-size
Note: By default, the main window has a minimum size of Vector2i(64, 64). This prevents issues that can arise when the window is resized to a near-zero size.

@YuriSizov
Copy link
Contributor

YuriSizov commented Dec 21, 2022

Windowing works differently in 3.5, so it's not relevant here.

As for 4.0, as I've explained, this worked by accident. Window nodes do not take the display server into account, they tell the display server their minimum size after computing it themselves. Telling the display server that your window should have some minimum size without telling your window the same is not going to work as expected. I'm not sure if there is an underlying implementation issue with the windowing, but it's not the linked PR that created the issue.

You can reproduce the same problem in beta 8 with

extends Node

func _enter_tree() -> void:
	TranslationServer.set_locale('es')

Because NOTIFICATION_TRANSLATION_CHANGED also causes the window to recompute its size, just like NOTIFICATION_THEME_CHANGED does now. The same will also happen if you enable control wrapping for the main window and add or remove its children.

So to summarize, the main window needs to be told that it has a min size, otherwise it will, at some point, recalculate its size using its internal data and tell DisplayServer a new value, ignoring whatever you've set in the server directly. The way the engine enforces 64 by 64 minimum size is also faulty.

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