-
Notifications
You must be signed in to change notification settings - Fork 206
Add all SVGs for Platform UI Icons #2643
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
base: master
Are you sure you want to change the base?
Conversation
bbd6f40
to
1f803fe
Compare
1f803fe
to
1ee7d7c
Compare
1ee7d7c
to
2182008
Compare
2182008
to
be4eed4
Compare
be4eed4
to
8e7e6f4
Compare
8e7e6f4
to
cf3e508
Compare
cf3e508
to
f92d029
Compare
f92d029
to
ff6d35a
Compare
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.
I've just tested the SVG replacement with the recent addition of SVG support to SWT. To do so, I had to add the SVG fragment to the product. Note that this PR needs to be extended by adding the required capability for every bundle using SVGs in order to ensure that products using those bundles have a rasterized available.
I found that some icons are not rasterized in proper resolution but upscaled from 100%. When moving to a 175% monitor (from 100% primary monitor with monitor-specific scaling enabled), I got the following:
In particular, several icons from the org.eclipse.search
bundle are not scaled properly in toolbars (probably it also affects others). May this require further changes such as the ones proposed in ...?
Test Results 1 824 files ±0 1 824 suites ±0 1h 37m 50s ⏱️ + 6m 1s For more details on these failures, see this check. Results for commit f0639c6. ± Comparison against base commit 9730d5f. ♻️ This comment has been updated with latest results. |
This pull request changes some projects for the first time in this development cycle.
An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patch
Further information are available in Common Build Issues - Missing version increments. |
f8d657a
to
f513ec7
Compare
I updated the all manifests of bundles where I added SVGs with
I tried loading the SVGs on 125% and 175% zoom respectively and it worked fine without blurry icons. Then I reproduced your setup with activated monitor-specific UI scaling and it also works fine. Can you please try again if the issue still persists and if so, can you maybe tell me how I can reproduce the problem?
As as all the icons of |
f513ec7
to
d127140
Compare
With the following setup, I can still reproduce it:
When moving the windows to the secondary monitor, the marked icons are blurry. |
You can ignore my previous comment. I found the issue for the wrongly scaled SVGs: the Image implementation was erroneous. If the data for an image at the zoom level of the file (usually 100) was already loaded, this data is scaled instead of loading the file in proper zoom anew (as required for SVGs). That's easy to fix with a PR I provided today, so I added a fix that PR: |
Thats great, thank you! :) |
d127140
to
6929b43
Compare
6929b43
to
f0639c6
Compare
This PR adds SVGs for testing the new SVG feature.
See this PR for the feature.