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

Popups don't fit into screen #45199

Closed
Tracked by #37734
KoBeWi opened this issue Jan 14, 2021 · 1 comment · Fixed by #45201
Closed
Tracked by #37734

Popups don't fit into screen #45199

KoBeWi opened this issue Jan 14, 2021 · 1 comment · Fixed by #45201

Comments

@KoBeWi
Copy link
Member

KoBeWi commented Jan 14, 2021

Godot version:

011d201

OS/device including version:

Windows 10

Issue description:

image
nuff said

@EricEzaM
Copy link
Contributor

Ugh.... a regression from #44906.

The size of the popup menu is not set. set_as_minsize() worked because it called set_size(get_contents_minimum_size());, so then the popup size would be set correctly before the adjust_popup_rect method was called. But now, this code is being called before it is drawn (and it's size is not set manually), so the size is unknown.

I have investigated further and the flow of logic is pretty weird...

  1. PopupMenu::popup()
  2. Window::popup()
  3. In Window::popup(), calls Popup::_popup_adjust_rect(), which in this case does nothing because it still thinks the popup is tiny.
  4. In Window::popup(), calls set_size(adjust.size)
  5. set_size() calls _update_window_size, which enforces size limits.
  6. Then finally the size limit is get_contents_minimum_size(), and the size is set to the maximum of the contents size, the min_size and the current size.
  7. So this is weird because set_size is called with a size of [36, 12], but after that method returns, the size is then [310, 428] (or whatever the size should be)

This is a really weird way of doing it imo. To fix this, I just call _update_window_size() in Window::popup() before doing the _popup_adjust_rect(), so that the size is correctly calculated before adjusting.

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

Successfully merging a pull request may close this issue.

2 participants