Fix Window::get_min_size causing AcceptDialog::_get_contents_minimum_size autocorrelation

[Rindbee]

Fix #55894.

Previously, the calculation is based on the previous min size, which does not seem appropriate.

Edit:

Previously, the min_size in Winodw may end up being reset automatically in some cases, this may be inappropriate.

And the new value of the setting is related to the calculation of _get_contents_minimum_size, which in turn depends on min_size, that is, the current min_size is relate to the previous min_size, same for _get_contents_minimum_size. Eventually, min_size/_get_contents_minimum_size may get bigger and bigger.

[akien-mga]

Seems sensible to me, but I wonder if that won't break assumptions and introduce regressions. E.g. would it always resize dialogs when changing the contents (e.g. switching tabs)?

[Rindbee]

if that won't break assumptions and introduce regressions. E.g. would it always resize dialogs when changing the contents (e.g. switching tabs)?

May be as bad as you guessed, but _get_contents_minimum_size still should have a clear purpose like its name.

[Rindbee]

I looked further into the source code and I was confused.

If AcceptDialog::_get_contents_minimum_size is similar to Control::get_combined_minimum_size, there is no problem to call get_min_size.

But get_min_size should return a custom value, that is min_size, default value is (0,0). For some reason, it returns the result of the last call to _get_contents_minimum_size.

So it messes up between min_size and the results returned by _get_contents_minimum_size.

[Rindbee]

The issue is caused by these codes:

godot/scene/gui/dialogs.cpp

Lines 233 to 234 in 362f53f

	Size2 wmsize = get_min_size();
	minsize.x = MAX(wmsize.x, minsize.x);

godot/scene/main/window.cpp

Lines 127 to 140 in 362f53f

	void Window::set_min_size(const Size2i &p_min_size) {
	min_size = p_min_size;
	if (!wrap_controls && window_id != DisplayServer::INVALID_WINDOW_ID) {
	DisplayServer::get_singleton()->window_set_min_size(min_size, window_id);
	}
	_update_window_size();
	}
	Size2i Window::get_min_size() const {
	if (window_id != DisplayServer::INVALID_WINDOW_ID) {
	min_size = DisplayServer::get_singleton()->window_get_min_size(window_id);
	}
	return min_size;
	}

godot/scene/main/window.cpp

Lines 601 to 603 in 362f53f

	if (wrap_controls) {
	size_limit = get_contents_minimum_size();
	}

godot/scene/main/window.cpp

Line 623 in 362f53f

DisplayServer::get_singleton()->window_set_min_size(size_limit, window_id);

So min_size ends up being the largest get_combined_minimum_size of the control, and it won't get smaller even if the control is removed later.

Maybe we should use min_size_limit (custom min_size) and min_size (the actual minimum size that can be resized to) to distinguish the existing min_size_limit and window_set_min_size (both are min_size). At the moment, it seems that the difference and usefulness of the two are confused.

[Rindbee]

In the code, I searched for related setters/getters:

It seems that there is no case to get the result of window_get_max_size/window_get_min_size through get_max_size/get_min_size before. The purpose of these calls seems to be just to get the custom max_size/min_size.

So the current change is no problem.

[akien-mga]

Thanks!

[akien-mga]

Oops I merged thinking that this was still the version I had review before, my bad. I guess we'll see how it goes in the master branch (but CC @bruvzg to see if the merged changes make sense).

[bruvzg]

It seems fine and fixes the issue.