-
-
Notifications
You must be signed in to change notification settings - Fork 69
Send optimized icon size #249
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
Send optimized icon size #249
Conversation
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025111414-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025081011-4.3&flavor=update
Failed tests15 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/149225#dependencies 82 fixed
Unstable testsPerformance TestsPerformance degradation:11 performance degradations
Remaining performance tests:168 tests
|
window-icon-updater/icon-sender
Outdated
| chosen_size = sorted(icons.keys())[-1] | ||
| # Override the largest icon with our preferred sizes (if possible) | ||
| for size in ICON_PREFERRED_SIZES: | ||
| if size in icons.keys(): | ||
| chosen_size = size | ||
| break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| chosen_size = sorted(icons.keys())[-1] | |
| # Override the largest icon with our preferred sizes (if possible) | |
| for size in ICON_PREFERRED_SIZES: | |
| if size in icons.keys(): | |
| chosen_size = size | |
| break | |
| # Override the largest icon with our preferred sizes (if possible) | |
| for size in ICON_PREFERRED_SIZES: | |
| if size in icons.keys(): | |
| chosen_size = size | |
| break | |
| else: | |
| chosen_size = sorted(icons.keys())[-1] |
This way you avoid sorting if not necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. This is done. And I have to admit I did not know about for else in Python.
Lookup for the most common icon sizes and select one of them first. Then revert to whatever available size which does not violate our maximum. Unfortunately pre-SVG era icon designs for few legacy applications is totally different per icon size. So sending the highest resolution is not always desirable as it does not scale down well on low-DPI screens. Practically this patch does not change anything for most programs (compared to previous situation). Since the previous code started from 128x128 down to the lowest resolution. Background on various icon sizes in different OS: https://icons8.com/blog/articles/choosing-the-right-size-and-format-for-icons/ resolves: QubesOS/qubes-issues#10359
815efb7 to
8c1e9c1
Compare
|
PipelineRetry |
Lookup for the most common icon sizes and select one of them first. Then revert to whatever available size which does not violate our maximum.
Unfortunately pre-SVG era icon designs for few legacy applications is totally different per icon size. So sending the highest resolution is not always desirable as it does not scale down well on low-DPI screens.
Practically this patch does not change anything for most programs (compared to previous situation). Since the previous code started from 128x128 down to the lowest resolution.
Background on various icon sizes in different OS:
https://icons8.com/blog/articles/choosing-the-right-size-and-format-for-icons/
resolves: QubesOS/qubes-issues#10359