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

Improve button min. size calculation. #92009

Merged
merged 1 commit into from
May 17, 2024

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented May 16, 2024

Fixes #91022

Changes button min. size calculation to take into account all stiles and use the largest possible margin. _get_current_stylebox() can't be used for the min. size calculation, since min. size is not recalculated on hover.

@bruvzg bruvzg added this to the 4.3 milestone May 16, 2024
@bruvzg bruvzg requested a review from a team as a code owner May 16, 2024 07:48
@akien-mga
Copy link
Member

I trust that checking all these is necessary, but how expensive is it computationally? I'm worried about how the cost might scale for button heavy UIs, like the Godot editor.

@bruvzg
Copy link
Member Author

bruvzg commented May 16, 2024

but how expensive is it computationally?

I'll check it later, but StyleBox::get_minimum_size is doing only simple math, so should not have any significant impact, unless it's custom StyleBox and _get_minimum_size is implemented in a script. And it's only used it the buttons get_minimum_size, which is only called when layout is changing.

@KoBeWi
Copy link
Member

KoBeWi commented May 16, 2024

This looks like something that can be cached when theme/layout changes.

@bruvzg
Copy link
Member Author

bruvzg commented May 16, 2024

Moved it to theme cache.

but how expensive is it computationally?

With default theme, it takes 0-2 μs to call it.

@passivestar
Copy link
Contributor

Works on my end, thanks.

P.S. There's also a problem with icons shifting in "hover-pressed". I was hoping it could be a part of the same problem as text, but this one still occurs:

Screen.Recording.mp4

@bruvzg
Copy link
Member Author

bruvzg commented May 16, 2024

There's also a problem with icons shifting in "hover-pressed". I was hoping it could be a part of the same problem as text, but this one still occurs

Probably draw should use max. style size for alignment as well.

@bruvzg
Copy link
Member Author

bruvzg commented May 16, 2024

There's also a problem with icons shifting in "hover-pressed". I was hoping it could be a part of the same problem as text, but this one still occurs

Updated to also fix this by caching the largest margins as well.

@passivestar
Copy link
Contributor

Updated to also fix this by caching the largest margins as well.

It works 🎉

@akien-mga akien-mga merged commit 0d5e910 into godotengine:master May 17, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

Text clipping on buttons in the "hover_pressed" state
4 participants