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

Fix rounding of ImageButton #3531

Merged
merged 5 commits into from
Nov 10, 2023
Merged

Fix rounding of ImageButton #3531

merged 5 commits into from
Nov 10, 2023

Conversation

chriscate
Copy link
Contributor

@chriscate chriscate commented Nov 5, 2023

Updated ImageButton to use rounding from the corresponding image via self.image.image_options().rounding and added a rounding() method to ImageButton.

Closes #3486 if we agree the addition of a rounding() method on the ImageButton itself, which the issue also mentions, can be discussed and handled separately.

This now fully closes #3486

@chriscate
Copy link
Contributor Author

chriscate commented Nov 5, 2023

One thing I noticed with the current approach in this PR is that when using e.g., rounding(50.0), the border bumps up against the available space and the edge of the circle flattens out ever so slightly (see bottom circle in below screenshot). Is there a way to auto-scale this to account for that?

I thought this existing logic would allow it all to fit:

.align_size_within_rect(image_size, rect.shrink2(padding));

image

Also, any insight into why rounding was called out separately from all the other image options to begin with? There must have been a reason.

@v-kat
Copy link
Contributor

v-kat commented Nov 5, 2023

I'm fine with this although I'd also like to see the rounding() method. I have no clue why the original code separated out rounding....

@chriscate
Copy link
Contributor Author

I actually agree we need the rounding() method at the same time or else there's no quick way to override if the user chooses. I went ahead and added it in this same PR.

So, now this fully closes #3486

@v-kat let me know what you think and then I'll open for review.

@v-kat
Copy link
Contributor

v-kat commented Nov 6, 2023

I haven't tried it but it looks okay

@chriscate chriscate marked this pull request as ready for review November 6, 2023 05:04
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one, thanks!

crates/egui/src/widgets/button.rs Outdated Show resolved Hide resolved
@emilk emilk added bug Something is broken egui labels Nov 10, 2023
@emilk emilk changed the title ImageButton rounding fix Fix rounding of ImageButton Nov 10, 2023
@emilk emilk merged commit 9ee6669 into emilk:master Nov 10, 2023
21 checks passed
@chriscate
Copy link
Contributor Author

You're welcome! Happy to contribute.

@chriscate chriscate deleted the image-button-3486 branch November 10, 2023 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken egui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ImageButton ignoring input Image rounding
3 participants