Skip to content

Conversation

@Michael5601
Copy link
Contributor

@Michael5601 Michael5601 commented Apr 23, 2025

This PR adds SVGs for all icons in the bundle org.eclipse.pde.api.tools.ui except for the following as it does not exist as SVG yet:

obj16/eclipse16.svg

See also this PR for more information.

@github-actions
Copy link

github-actions bot commented Apr 23, 2025

Test Results

   273 files   -  12     273 suites   - 12   45m 54s ⏱️ - 3m 25s
 3 608 tests  -   1   3 531 ✅  -   1   76 💤 ± 0  1 ❌ +1 
10 628 runs   - 389  10 454 ✅  - 331  172 💤  - 59  2 ❌ +2 

For more details on these failures, see this check.

Results for commit 0867ccc. ± Comparison against base commit 1e8d28a.

This pull request removes 1 test.
[8: View using browser technology] ‑ Unknown test

♻️ This comment has been updated with latest results.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

The change looks sound.

Regarding missing:

obj16/eclipse16.svg

I wonder whether we really don't have the Eclipse logo as SVG to place here?

@HannesWell HannesWell force-pushed the org.eclipse.pde.api.tools.ui.SVGs branch from 421bdb1 to daec848 Compare April 28, 2025 17:54
Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

obj16/eclipse16.svg

I wonder whether we really don't have the Eclipse logo as SVG to place here?

I also assume that we already have such an icon, even if not explicitly defined in the images repo. Maybe it can be found out from which SVG the current PNGs are generated when looking at their git-history?
@Michael5601 I would prefer to add that SVG with this change.
Besides that this change looks good.

@HannesWell HannesWell force-pushed the org.eclipse.pde.api.tools.ui.SVGs branch from daec848 to b64900a Compare April 28, 2025 19:37
@merks
Copy link
Contributor

merks commented Apr 28, 2025

I’m not sure from where I got this from

https://github.com/eclipse-platform/www.eclipse.org-eclipse/blob/master/eclipse.svg

somewhere in the images repo.

@Michael5601
Copy link
Contributor Author

I intentionally marked icons like eclipse16/32/48... as missing as there is no SVG with exactly this name. I proceeded like this for every icon that does not have an SVG with the same name even though there could be duplicates with a different name so we can discuss this topic.

For the eclipse icons in particular I am aware of one special SVG file (eclipse-svg/eclipse.platform.releng.tychoeclipsebuilder/eclipse.platform.repository/icons/Eclipse.icns.svg) which is 1024x1024px in size that looks like the needed icons. The same folder in the eclipse-png package contains the same icon in many different sizes as PNG. We would need to check if we can just reduce the SVG in size and use it.

@merks I couldn't find the SVG that you sent in the images repo. The SVG you sent can of course also be used for the eclipse16/32/48 icons but it would also need to be adjusted in size.

@HeikoKlare
Copy link
Contributor

I intentionally marked icons like eclipse16/32/48... as missing as there is no SVG with exactly this name.

Might be that the Eclipse icons is actually the only one that really used in a "shared" way, i.e., placed as SVG at some central place and then used ot generated PNGs at different sizes out of it.
In addition, most of the eclipse16/32/48 PNGs are probably fine as they not used as icons within the application but, if I am not mistaken, also embedded into the application to be used by the OS as program icon and the like. That's why those can probably not be replaced easily. Those will usually be placed in the root folder of the application rather than the icons folder.

@Michael5601 Michael5601 force-pushed the org.eclipse.pde.api.tools.ui.SVGs branch from b64900a to a847760 Compare April 29, 2025 08:33
@Michael5601
Copy link
Contributor Author

I now added the missing SVG obj16/eclipse16.svg by adjusting the SVG @merks sent.

@HannesWell
Copy link
Member

I think what has been said is right. When search the eclipse-platform organization for eclipse16 (https://github.com/search?q=org%3Aeclipse-platform%20eclipse16&type=code) I found. The two branding plugins that provide it:

But for both I didn't find an SVG source, but I think that's what you said before.
Btw. I noticed that in org.eclipse.ant.ui's icons/full/obj16 folder, there is one eclipse16.png, one eclipse_obj.svg and one eclipse_obj@2x.png. This group doesn't fit in the usual pattern and the latter two Eclipse icons looks much older than the former. @Michael5601 could you please have a look at that and check what's correct respectively used?

This commit adds SVGs for all icons in the bundle `org.eclipse.pde.api.tools.ui`.
@HannesWell HannesWell force-pushed the org.eclipse.pde.api.tools.ui.SVGs branch from a847760 to 0867ccc Compare April 29, 2025 23:00
Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

This change looks good. Thank you.

@Michael5601
Copy link
Contributor Author

I noticed that in org.eclipse.ant.ui's icons/full/obj16 folder, there is one eclipse16.png, one eclipse_obj.svg and one eclipse_obj@2x.png. This group doesn't fit in the usual pattern and the latter two Eclipse icons looks much older than the former. @Michael5601 could you please have a look at that and check what's correct respectively used?

I don't know if the eclipse16.png and eclipse_obj.png are connected. I viewed them as different icons as both of them have duplicates that are used in other bundles. I think both should have a 100% and a 200% size, which is not the case. In the end also both SVGs should be in the folder but I could only add the SVG for eclipse_obj.svg as the other one was missing (documented in this PR).

@HannesWell HannesWell merged commit 23371e1 into eclipse-pde:master Apr 30, 2025
10 of 12 checks passed
@HannesWell
Copy link
Member

I don't know if the eclipse16.png and eclipse_obj.png are connected. I viewed them as different icons as both of them have duplicates that are used in other bundles. I think both should have a 100% and a 200% size, which is not the case.

I think we have to check the exact usage and see what that reveals. If you don't have time, I can try to do it in the next days.

@Michael5601
Copy link
Contributor Author

I don't know if the eclipse16.png and eclipse_obj.png are connected. I viewed them as different icons as both of them have duplicates that are used in other bundles. I think both should have a 100% and a 200% size, which is not the case.

I think we have to check the exact usage and see what that reveals. If you don't have time, I can try to do it in the next days.

I can do it on monday if this has time to wait. :) If it doesn't you can do it if you want.

@Michael5601 Michael5601 deleted the org.eclipse.pde.api.tools.ui.SVGs branch May 1, 2025 07:26
@Michael5601
Copy link
Contributor Author

Michael5601 commented May 5, 2025

I think we have to check the exact usage and see what that reveals. If you don't have time, I can try to do it in the next days.

I think you are right that the eclipse_obj.png is the old variant. I can only find it used in the fieldassist menu.
image

Also I found the icon in the bundle org.eclipse.ant.ui in the folder icons/full/obj16/ but it is not used anywhere.


The icon eclipse16.png is used in various places like the help menu:
image

or in the top right corner of the window:
image

The icons are placed in
org.eclipse.ant.ui/obj16/
org.eclipse.pde.api.tools.ui/obj16/


Also there is a eclipse.png in org.eclipse.pde.ui/etool16/eclipse.svg and in org.eclipse.pde.ui/obj16/eclipse.svg that looks like the eclipse16.png.

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.

4 participants