Skip to content

Add SVGs to org.eclipse.ui.intro bundles #1815

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

Merged

Conversation

Michael5601
Copy link
Contributor

This PR adds SVGs for all icons in the bundles org.eclipse.ui.intro and org.eclipse.ui.intro.universal except for the following as it is not available as SVG yet:

org.eclipse.ui.intro/etool16/restore_welcome.svg

See also this PR for more information.

@eclipse-platform-bot
Copy link
Contributor

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

ua/org.eclipse.ui.intro.universal/META-INF/MANIFEST.MF
ua/org.eclipse.ui.intro/META-INF/MANIFEST.MF
ua/org.eclipse.ui.intro/pom.xml

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
From ae5b2ae08392d4613bc105e65a69a00f0729bf02 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Mon, 14 Apr 2025 08:38:49 +0000
Subject: [PATCH] Version bump(s) for 4.36 stream


diff --git a/ua/org.eclipse.ui.intro.universal/META-INF/MANIFEST.MF b/ua/org.eclipse.ui.intro.universal/META-INF/MANIFEST.MF
index be1fa7660e..c74030cd69 100644
--- a/ua/org.eclipse.ui.intro.universal/META-INF/MANIFEST.MF
+++ b/ua/org.eclipse.ui.intro.universal/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %plugin_name
 Bundle-SymbolicName: org.eclipse.ui.intro.universal;singleton:=true
-Bundle-Version: 3.5.400.qualifier
+Bundle-Version: 3.5.500.qualifier
 Bundle-Vendor: %provider_name
 Bundle-Localization: plugin
 Export-Package: org.eclipse.ui.internal.intro.universal;x-friends:="org.eclipse.ua.tests",
diff --git a/ua/org.eclipse.ui.intro/META-INF/MANIFEST.MF b/ua/org.eclipse.ui.intro/META-INF/MANIFEST.MF
index e84a27c27a..21bebbc014 100644
--- a/ua/org.eclipse.ui.intro/META-INF/MANIFEST.MF
+++ b/ua/org.eclipse.ui.intro/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %plugin_name
 Bundle-SymbolicName: org.eclipse.ui.intro; singleton:=true
-Bundle-Version: 3.7.500.qualifier
+Bundle-Version: 3.7.600.qualifier
 Bundle-Activator: org.eclipse.ui.internal.intro.impl.IntroPlugin
 Bundle-Vendor: %provider_name
 Bundle-Localization: plugin
diff --git a/ua/org.eclipse.ui.intro/pom.xml b/ua/org.eclipse.ui.intro/pom.xml
index f83aafa4b8..43980665b8 100644
--- a/ua/org.eclipse.ui.intro/pom.xml
+++ b/ua/org.eclipse.ui.intro/pom.xml
@@ -18,6 +18,6 @@
   </parent>
   <groupId>org.eclipse.platform</groupId>
   <artifactId>org.eclipse.ui.intro</artifactId>
-  <version>3.7.500-SNAPSHOT</version>
+  <version>3.7.600-SNAPSHOT</version>
   <packaging>eclipse-plugin</packaging>
 </project>
-- 
2.49.0

Further information are available in Common Build Issues - Missing version increments.

Copy link
Contributor

github-actions bot commented Apr 14, 2025

Test Results

 1 758 files  ±0   1 758 suites  ±0   1h 29m 11s ⏱️ - 4m 4s
 4 173 tests ±0   4 150 ✅ ±0   23 💤 ±0  0 ❌ ±0 
13 119 runs  ±0  12 952 ✅ ±0  167 💤 ±0  0 ❌ ±0 

Results for commit 1ed62c8. ± Comparison against base commit 26a376c.

♻️ This comment has been updated with latest results.

@akurtakov
Copy link
Member

There are code changes which are unrelated to svgs. Please drop them from this PR.

@Michael5601
Copy link
Contributor Author

There are code changes which are unrelated to svgs. Please drop them from this PR.

Doing it right now :D

@Michael5601 Michael5601 force-pushed the org.eclipse.ui.intro.SVGs branch from cbbe4c6 to ecd45ef Compare April 15, 2025 09:24
@laeubi
Copy link
Contributor

laeubi commented Apr 15, 2025

Seeing all these changes from png > svg (and previously from gif > png), maybe we should have a ImageDescriptor with autodetection e.g. if no extension is given it first tries svg, then png, then gif?

Then one can already remove all extensions and if ther will ever be a svg provided it is picked up automatically.

@Michael5601 Michael5601 force-pushed the org.eclipse.ui.intro.SVGs branch 4 times, most recently from 4ab71ef to c5b42d2 Compare April 15, 2025 09:33
@Michael5601
Copy link
Contributor Author

There are code changes which are unrelated to svgs. Please drop them from this PR.

Should be clean now. I did not notice the changes from the autoformatter thus the unwanted changes.
I will do this today for all other PRs too.

@Michael5601
Copy link
Contributor Author

Seeing all these changes from png > svg (and previously from gif > png), maybe we should have a ImageDescriptor with autodetection e.g. if no extension is given it first tries svg, then png, then gif?

Then one can already remove all extensions and if ther will ever be a svg provided it is picked up automatically.

Sounds like a good idea but wouldn't the performance be affected by this? I think we would need to try loading the SVG and if it fails we load the PNG.

@laeubi
Copy link
Contributor

laeubi commented Apr 15, 2025

Of course it could require some more lookups, but the same is true for looking for higher res images and so on, and given we use it not on a cloud-image it should be not that costly given that we have svg for most of the things already.

But that was just an idea that came into my mind seeing all these extension changes :-)

@Michael5601
Copy link
Contributor Author

Of course it could require some more lookups, but the same is true for looking for higher res images and so on, and given we use it not on a cloud-image it should be not that costly given that we have svg for most of the things already.

But that was just an idea that came into my mind seeing all these extension changes :-)

I would handle this as a follow up for now but keep it in mind. Thanks :)

@laeubi
Copy link
Contributor

laeubi commented Apr 15, 2025

I would handle this as a follow up for now but keep it in mind. Thanks :)

What just came into my mind: If we remove the png now and people reference them in their plugin (what of course is bad practice) this might fail now.

So what might be useful as well is that if one looks for myImage.png but it is not found, we give another try for myImage.svg... I think this would be acceptable to have it "slower" here instead of fail completely.

@Michael5601
Copy link
Contributor Author

I would handle this as a follow up for now but keep it in mind. Thanks :)

What just came into my mind: If we remove the png now and people reference them in their plugin (what of course is bad practice) this might fail now.

So what might be useful as well is that if one looks for myImage.png but it is not found, we give another try for myImage.svg... I think this would be acceptable to have it "slower" here instead of fail completely.

The PNGs are not removed yet for the exact reason you mentioned but of course this is a goal in the future. We already discussed the approach of redirecting to the SVG if the PNG is not available and referenced somewhere. We don't have a final decision here but thanks for the note.

@HeikoKlare HeikoKlare force-pushed the org.eclipse.ui.intro.SVGs branch from c5b42d2 to 4af74cd Compare April 25, 2025 08:14
Michael5601 and others added 3 commits April 26, 2025 10:37
This commit adds SVGs for all icons in the bundles `org.eclipse.ui.intro` and `org.eclipse.ui.intro.universal` except for the following as these are not available as SVG yet:

org.eclipse.ui.intro/etool16/restore_welcome.svg
Use on-the-fly-generated disabled version of SVG-rasterized icons
instead.
@HeikoKlare HeikoKlare force-pushed the org.eclipse.ui.intro.SVGs branch from 4af74cd to 1ed62c8 Compare April 26, 2025 08:37
@HeikoKlare HeikoKlare merged commit f8e6177 into eclipse-platform:master Apr 26, 2025
11 checks passed
@Michael5601 Michael5601 deleted the org.eclipse.ui.intro.SVGs branch April 26, 2025 13:27
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.

5 participants