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

Force scaling icon #812

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Force scaling icon #812

wants to merge 3 commits into from

Conversation

ogios
Copy link

@ogios ogios commented Dec 13, 2024

fix #809

this make sure that all the icon's height are the same.
but some might considered "ok" size before may seem a bit off than usual:

BEFORE:
image

AFTER:
image

But we can change the icon size ourself (used to be 16, now 24):
image

So this is just one way of fixing the problem
i can close this if it's not a proper solution.

@ogios
Copy link
Author

ogios commented Dec 13, 2024

and also, should we find a proper size in pixmap function instead of just use the first one?

.and_then(|pixmap| pixmap.first())

or since we have re-scale as post-process, we can just choose the largest one.

@JakeStanger
Copy link
Owner

Thanks for this, haven't given a full review yet but looks good.

I think it would make sense to pick the next biggest size and scale that down. That'd be cheaper than always scaling the largest.

@ogios
Copy link
Author

ogios commented Dec 15, 2024

I think it would make sense to pick the next biggest size and scale that down.

i've added the functionality for this if i understand correctly.

One thing bothering me is that, whether the Vec<IconPixmap> is sorted with height, if so then we can use a better way to speed up the search.

@JakeStanger
Copy link
Owner

One thing bothering me is that, whether the Vec is sorted with height, if so then we can use a better way to speed up the search.

The spec is so incredibly vague and has so many incorrect implementations that I'd keep away from trying to do anything like that. The good news is in reality there will only be a few entries on rare occasions so the search will be fast enough always.

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

Successfully merging this pull request may close these issues.

vlc tray icon not respecting icon_size
2 participants