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

Feature Proposal: Rasterization of SVGs at Runtime for Eclipse Icons #1638

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Michael5601
Copy link
Contributor

@Michael5601 Michael5601 commented Dec 6, 2024

Eclipse currently loads icons exclusively as raster graphics (e.g., .png), without support for vector formats like .svg. A major drawback of raster graphics is their inability to scale without degrading image quality. Additionally, generating icons of different sizes requires manually rasterizing SVGs outside Eclipse, leading to unnecessary effort and many icon files.

This PR introduces support for vector graphics in Eclipse, enabling SVGs to be used for icons. Existing PNG icons will continue to be loaded alongside SVGs, allowing the use of the new functionality without the need to replace all PNG files at once.


Key Features

  • Example Images:
    Screenshots showcasing the new functionality can be found below. These screenshots were taken with 125% monitor-zoom and scaling enabled with the flag -Dswt.autoScale=quarter.
PNG SVG
PNG SVG
PNG2 SVG2
PNG3 SVG3
PNG4 SVG4
  • How It Works:

    • To use SVG icons, simply place the SVG file in the bundle and reference it in the plugin.xml and other necessary locations, as is done for PNGs. No additional configuration is required.
    • At runtime, Eclipse uses the library JSVG to rasterize the SVG into a raster image of the desired size, eliminating the need for scaling. My analysis shows that JSVG is the most suitable Java library for this purpose.
    • You need to write the flag -Dswt.autoScale=quarter into your eclipse.ini file or into the run arguments of a new configuration.
  • Demonstration:
    This PR includes SVG versions of icons for the following bundles:

    • org.eclipse.search
    • org.eclipse.ui
    • org.eclipse.ui.ide
    • org.eclipse.ui.ide.application
    • org.eclipse.ui.editors
    • org.eclipse.ui.navigator

Due to this the PR of Platform UI is very large. If preferred, I can limit the changes to org.eclipse.search, allowing SVG icons to be tested specifically in the search result window.
I did not delete any exisiting PNGs.


Architecture

Eclipse icons fall into three categories, all three adressed by this PR:

  1. Standard Icons:
    These is the most common type of icons. They are loaded, scaled if needed, and displayed in the UI.

  2. Composite Icons:
    Composite icons combine a base icon with up to four overlay icons. Each component is loaded separately and then combined for display.

  3. Graphically Customized Icons:
    These icons represent disabled or "grayed-out" states. They can either be provided as pre-designed raster images or generated dynamically at runtime by applying transformations to the "enabled" version of the icon. Eclipse does not have no pre-designed SVGs. As soon as the path of the pre-designed raster graphic is removed the icon will be created at runtime.

Standard-Icons and Composite Icons are fully supported by this PR. Graphically Customized Icons are only supported if the pre-designed raster graphic is removed. The original image loaded before the customization, is generated by rasterizing a SVG. Afterwards the current automatic customization functionality creates the "grayed-out" style. This currenct functionality for customization produces bad results as seen here. This is why I want to change this behaviour in a future PR.

The solution of my future PR applies a runtime filter to SVGs before rasterization (pre-processing), which can emulate GTK or Cocoa (macOS)/Windows styles. Specifically, GTK uses a unique style for disabled icons and I can only produce results that fit to either GTK or Windows and Cocoa. I will release this feature as a follow up to this PR as soon as a solution is found for the different styles for different OS. As discussion for this topic can be found here. Alternatively I can release the PR before and we live with the other visuals for one OS. For now I recommend to use the pre-designed PNGs as usual. The future PR for the customization of icons with SVGs is nearly complete but requires further refinement before release.

The sequence diagrams below illustrate the architecture for loading Standard and Composite Icons. The new functionality introduced in this PR is highlighted in blue. The new functionality changes the ImageLoader process on the right side of the diagramms. You can see that the implementation can happen in the same place for these two icon groups:

Sequence-Diagram: Standard-Icons (Currenct Workflow)
20241206_Standard-Icons_Current

Sequence-Diagram: Standard-Icons (Improved Workflow)
20241206_Standard-Icons_Improved

Sequence-Diagram: Composite-Icons (Currenct Workflow. See also the two little referenced workflows below)
20241206_Composite-Icons_Current-1
20241206_Composite-Icons_Current-2

Sequence-Diagram: Composite-Icons (Improved Workflow)
20241206_Composite-Icons_Improved

For Graphically Customized Icons, only the current workflow will be shown in a sequence diagram here. The new workflow as described above will be part of the follow up PR. You can see that by calling URLImageDescriptor.createImage() the Standard-Icons workflow is called.

Sequence-Diagram: Disabled-Icons (Current Workflow)
20241206_Disabled-Icons_Current


Dependency Management

To enable SVG rasterization, the library JSVG is required as an external dependency. Since all rasterization logic needs to be part of Eclipse SWT, the dependency must also be included in SWT. However, SWT currently has no external dependencies, which created challenges for the integration of the Dependency:

Unsuccessful Approaches:

  1. Fragment Approach:
    Adding a fragment to SWT with the JSVG dependency failed, as SWT itself is composed of fragments, and fragments cannot depend on other fragments.

  2. Service Loader Approach:
    Adding a new bundle with the functionality and the dependency failed because SWT fragments lack a class loader to locate the new bundle. (I am not sure if this reason is correct but I tried for some hours and it did not work)

Successful Approach:

I created a new bundle containing the rasterization logic and the JSVG dependency. This bundle registers itself via a hook mechanism during initialization. To ensure the bundle is loaded, I added an initialization line to Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor).

Once JSVG is included in the target platform (with help from @HannesWell), this functionality will become fully operational.


Outstanding Issues

  1. Target Platform:
    The JSVG library must be included in the target platform.

  2. Existing PNGs:
    PNG files cannot yet be deleted, as some are referenced across bundles via hardcoded paths e.g. in platform code. Removing these files would result in errors.

  3. Early-Loaded Icons:
    A few icons (e.g., eclipse16.png) are loaded before the workbench is initialized. At this point, the rasterizer has not yet been registered. I only experienced three icons that were loaded this early but there might be more.

  4. Scaling Support for Composite Icons:
    I modified the behavior of the CompositeImageDescriptor.supportsZoomLevel(int zoom) method in Platform UI to address a limitation where it only allowed zoom levels that were exact multiples of 100 (e.g., 100, 200, 300). This restriction, caused by an unresolved bug, prevented scaling to zoom levels such as 125. Although I haven't analyzed the underlying bug yet, this change was necessary to enable proper scaling functionality.

  5. Missing or broken SVGs
    There are some icons that don't have a SVG-File or the SVG-File needs to be improved. The feature can still be used as there will be raster graphics that are loaded instead but finally this issue needs to be fixed. I will provide PRs if I find these icons.


Planned Tasks for this PR

  1. Regression Testing:
    I will add regression tests that allow automatic testing of the feature.

  2. Performance Evaluation:
    I will perform performance tests to find out if this feature is faster or slower than the current implementation. From manual testing I can say that it feels rather the same.

  3. Testing for different OS
    The ImageLoader class has specific implementations for cocoa, win32 and gtk that I needed to change. All three implementations are very similar, so I don't think there will be an issue but the cocoa and gtk implementation was no yet tested by me. I only tested on a windows system.


See also this PR for the necessary changes in Platform UI. All changes for this feature were performed in SWT and Platform UI.

Fixes #1438.

Let me know if you'd like further clarification or additional adjustments!

@BeckerWdf
Copy link
Contributor

2. PNG files cannot yet be deleted, as some are referenced across bundles via hardcoded paths e.g. in platform code. Removing these files would result in errors.

We could clean up (most of) these references across bundles so that we can delete (most of) the PNGs.

@HannesWell
Copy link
Member

Thank you @Michael5601 for this PR I'm really looking forward to this.

Withouth having it tried out yet, I have a few remarks/questions/suggestions, all based on the assumption that providing support to render vector graphics is an 'implementation' detail that does not need any or only little new APIs where technically necessary (e.g. to specify a zoom factor).

Therefore I wonder for example a new public API type ISVGRasterizer is necessary or if it should better be an internal class, just like the other new types.
I assume it's not intended to allow external contributions of custom rasterizers? And as far as I understood our discussion, on the long run SVG support could become fully built in in SWT and then we have to decide again about the way JSVG is integrated.

Splitting the steps to handle all kind of icons sounds very reasonable for me. I think this should just focus on adding support for rendering vector graphics, just as it's done.

  1. Fragment Approach:
    Adding a fragment to SWT with the JSVG dependency failed, as SWT itself is composed of fragments, and fragments cannot depend on other fragments.

It's correct that Fragments can't have own Fragments, but a (host) bundle can have an arbitrary number of fragments.
So it would be possible to make o.e.swt.svg another fragment of o.e.swt and since all fragments and the their host bundle share the same classloader code from the corresponding native SWT fragments can then access classes from the svg fragment.
Then you could use the plain Java ServiceLoader without further addo or even just load a fixed class reflectivly using Class.forName(). This would avoid the need for adding a SVGRasterizerRegistry and making that the registration happens and happens early enough.

Setting up the java ServiceLoader between OSGi bundles (not fragments) on the other hand is a bit more complicated, but also possible. Just for reference, this blog post should explain it (I don't want to go into the details here):
https://blog.osgi.org/2013/02/javautilserviceloader-in-osgi.html

Copy link
Contributor

github-actions bot commented Dec 7, 2024

Test Results

   493 files   -  1     493 suites   - 1   11m 42s ⏱️ + 2m 47s
 4 294 tests  - 37   4 283 ✅  - 35   10 💤  - 3  0 ❌ ±0  1 🔥 +1 
16 535 runs   - 37  16 430 ✅  - 34  104 💤  - 4  0 ❌ ±0  1 🔥 +1 

For more details on these errors, see this check.

Results for commit 945ee0c. ± Comparison against base commit 6d374d9.

This pull request removes 37 tests.
org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageDataForDifferentFractionalZoomsShouldBeDifferent
org.eclipse.swt.graphics.ImageWin32Tests ‑ testImageShouldHaveDimesionAsPerZoomLevel
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testByteArrayTransfer
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testFileTransfer
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testHtmlTransfer
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromCopiedImage
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImage
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageData
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testImageTransfer_fromImageDataFromImage
org.eclipse.swt.tests.win32.Test_org_eclipse_swt_dnd_DND ‑ testRtfTransfer
…

♻️ This comment has been updated with latest results.

@Michael5601
Copy link
Contributor Author

Thank you for the comment, I will check all your remarks soon.

Setting up the java ServiceLoader between OSGi bundles (not fragments) on the other hand is a bit more complicated, but also possible. Just for reference, this blog post should explain it (I don't want to go into the details here): https://blog.osgi.org/2013/02/javautilserviceloader-in-osgi.html

To this point I can already say that I tried it with the same article but I wasn't successful.

@HeikoKlare
Copy link
Contributor

Therefore I wonder for example a new public API type ISVGRasterizer is necessary or if it should better be an internal class, just like the other new types. I assume it's not intended to allow external contributions of custom rasterizers? And as far as I understood our discussion, on the long run SVG support could become fully built in in SWT and then we have to decide again about the way JSVG is integrated.

That's something we should discuss and agree on. I would not in favor of tightly integrating a specific SVG rasterizer into SWT. Having it interchangeable ensures low coupling and gives the flexibility to exchange it without rebuilding SWT (by just providing some other fragment/bundle/... that contains an alternative rasterizer implementation). We could still think of making that interface internal for now, so that the interface is not public but in case you would want to provide a different rasterizer it would still be possible (with potentially a warning).

  1. Fragment Approach:
    Adding a fragment to SWT with the JSVG dependency failed, as SWT itself is composed of fragments, and fragments cannot depend on other fragments.

It's correct that Fragments can't have own Fragments, but a (host) bundle can have an arbitrary number of fragments. So it would be possible to make o.e.swt.svg another fragment of o.e.swt and since all fragments and the their host bundle share the same classloader code from the corresponding native SWT fragments can then access classes from the svg fragment. Then you could use the plain Java ServiceLoader without further addo or even just load a fixed class reflectivly using Class.forName(). This would avoid the need for adding a SVGRasterizerRegistry and making that the registration happens and happens early enough.

For some more clarifiation on using a fragment: If we are not mistaken, to provide a fragment with a concrete rasterizer implementation, the rasterizer interface needs to be placed directly in the SWT host bundle, not in some common package that is mapped into all the native fragments like done for all the other SWT code. Otherwise, the SVG fragment would be tied to the presence of another fragment providing that interface, which (reasonably) seems to be prohibited. As a consequence, that interface would become the first source element to be placed directly in the SWT host bundle, while all the other code is placed in the native fragments. In addition, we need to reference that interface from within the native fragments (to make use a rasterizer in SWT code). With the current configuration of the bundles/fragment, this is not possible. The SWT host bundle has to be added to the classpath of the native fragments to access the interface via adding the PDE required plugins container to its classpath (i.e., to add the classpath entry <classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>). All this may not be prohitibitive, but it's all changes to the current configuration of the SWT host and fragments that need to be considered.

Setting up the java ServiceLoader between OSGi bundles (not fragments) on the other hand is a bit more complicated, but also possible. Just for reference, this blog post should explain it (I don't want to go into the details here): https://blog.osgi.org/2013/02/javautilserviceloader-in-osgi.html

I just want to add that I also tried to create a minimal reproducer for using a ServiceLoader across OSGi bundles, but I also failed, just like Michael. The necessary capabilities added to the manifests do not even show up when listing the capabilities of the bundles via the OSGi console. Probably, I am / we are doing something wrong here.

@Michael5601
Copy link
Contributor Author

I edited the PR message and added the following points:

  1. Planned Tasks for this PR: I will include regression tests and I will perform a performance evaluation. The feature also needs to be tested for gtk and cocoa as they have different ImageLoader implementations.
  2. New Outstanding issue: There are missing or broken SVGs that do not need to be fixed for this feature to work but if we plan to remove all PNGs finally this issue needs to be resolved.

Copy link
Contributor

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

I think we should follow the java recommendation for interface names here the I prefix is more an eclipse thing and even there we do not follow this anymore and I don't have seen it in SWT aynwhere

Copy link
Contributor

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

I think the stream handling is flawed at the moment and reading SVG documents should be done using streams or URLs.

@Michael5601
Copy link
Contributor Author

Michael5601 commented Dec 11, 2024

Please see this Draft for the Follow-up-functionality for the automatic customization of graphically customized icons as announced previously in this PR.

The functionality is complete but the draft is meant as a base for discussion. All changes to this PR will be done also in the draft.

@HannesWell
Copy link
Member

HannesWell commented Dec 11, 2024

Therefore I wonder for example a new public API type ISVGRasterizer is necessary or if it should better be an internal class, just like the other new types. I assume it's not intended to allow external contributions of custom rasterizers? And as far as I understood our discussion, on the long run SVG support could become fully built in in SWT and then we have to decide again about the way JSVG is integrated.

That's something we should discuss and agree on.

Yes, absolutely. I suggest we discuss this in a separate issue, in parallel to this PR.

Setting up the java ServiceLoader between OSGi bundles (not fragments) on the other hand is a bit more complicated, but also possible. Just for reference, this blog post should explain it (I don't want to go into the details here): https://blog.osgi.org/2013/02/javautilserviceloader-in-osgi.html

I just want to add that I also tried to create a minimal reproducer for using a ServiceLoader across OSGi bundles, but I also failed, just like Michael. The necessary capabilities added to the manifests do not even show up when listing the capabilities of the bundles via the OSGi console. Probably, I am / we are doing something wrong here.

Besides adding the correct required and provided capabilities to the provider and consumer of the service (implementation), you also have to make sure that aries.spifly as well as the provider of the service (implementation) are auto-started at an appropriated level, see for example how it's done for slf4j-simple providing a logging-provider consumed by the slf4j-api bundle:
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/02e51912906b367d441fb0d120c7fd3bc3990dad/eclipse.platform.releng.tychoeclipsebuilder/eclipse.platform.repository/sdk.product#L193-L200

Everyone building products or launching Eclipse-apps from a workspace that include this SVG support for SWT would have to configure this. This would IMHO be cumbersome and to avoid all these difficulties I strongly would like to avoid the need for the OSGI Service Loader mediator in this case.

  1. Fragment Approach:
    Adding a fragment to SWT with the JSVG dependency failed, as SWT itself is composed of fragments, and fragments cannot depend on other fragments.

It's correct that Fragments can't have own Fragments, but a (host) bundle can have an arbitrary number of fragments. So it would be possible to make o.e.swt.svg another fragment of o.e.swt and since all fragments and the their host bundle share the same classloader code from the corresponding native SWT fragments can then access classes from the svg fragment. Then you could use the plain Java ServiceLoader without further addo or even just load a fixed class reflectivly using Class.forName(). This would avoid the need for adding a SVGRasterizerRegistry and making that the registration happens and happens early enough.

For some more clarifiation on using a fragment: If we are not mistaken, to provide a fragment with a concrete rasterizer implementation, the rasterizer interface needs to be placed directly in the SWT host bundle, not in some common package that is mapped into all the native fragments like done for all the other SWT code.

That's right. The only disadvantage I can think of for having code in the o.e.swt host bundle to, would be that for plain-Java/non-OSGi apps o.e.swt would then also need o.e.swt on the classpath, while they only need the native fragment of the current platform now. So I guess that wouldn't be ideal, altough I don't know if it's forbidden by a rule.
But maybe we could make that change only for the compilation in the workspace, because at runtime because all fragments and their host share the same classloader, the actual location of a class-file among them, is irrelevant (maybe with multiple occurrences of the same class).
But it would also lead to another problem: Because SVGRasterizer returns an ImageData object that class must be moved to o.e.swt too and with it it's whole dependency-closure and eventually we would more or less revert:

But I tried a few things out and had another idea:
If o.e.swt.svg is a fragment of o.e.swt, we could simply add an explicit requirement to the native fragments to the build-path of o.e.swt.svg. See the details below in the code-remarks.
But that should be a solution that seems feasible.

Another option would be to not provide an interface SVGRasterizer at all and just load the class JSVGRasterizer reflectively using Class.forName() and also calling all relevant methods reflectively (or a method handle to improve performance).
This way a special interface to implement would not be necessary at all, but it would also be a bit more 'bare-metal'.

Edit: Thinking about this again, this wouldn't save much since e.g. the ImageData type is still required, which leads to the same problem again.

Yet another option would be to avoid a new o.e.swt.svg bundle/fragment and add all code to interact with JSVG, which is not that much, directly into o.e.swt respectivly it's native fragments and make the dependency to JSVG optional and be prepared for it's absence! This way one would only have to add jsvg to an application/product for OSGi-runtimes and plain Java apps and not also o.e.swt.svg. Hopefully it has OSGi metadata soon:

Of course this would more or less make it impossible to supply alternative rasterizer. But I cannot tell if it's realistic that custom ones will be supplied or not and we are just over-engineering here?
Of course it shouldn't be integrated to tightly, so that an exchange in the future is feasible if necessary. But that's maybe part of the discussion of the very first point of this comment.

@laeubi
Copy link
Contributor

laeubi commented Dec 12, 2024

For some more clarifiation on using a fragment: If we are not mistaken, to provide a fragment with a concrete rasterizer implementation, the rasterizer interface needs to be placed directly in the SWT host bundle, not in some common package that is mapped into all the native fragments like done for all the other SWT code.

This is not true, all fragments are merged with the host and therefore you have one big class space, it might be that the IDE (or Tycho) currently have some limitations in this regard but we could/should fix that instead of making things harder than required.

A ServiceLoader approach seem suitable also here for me, no need to complicate the thing with OSGi SPI, this is only for applications that are not aware of OSGi at all and this is not the case here and also not needed.

@BeckerWdf
Copy link
Contributor

It looks like scaling the PNGs almost costs nothing ( 9.529 compared to 9.5555)

Scaling the PNGs is pretty cheap performance wise. I am also surprised by this.

But scaling the SVGs is more expensive ( 9.9035 compared to 10.3165). Why is this the case?

Rendering SVGs in a higher resolution is more expensive performance wise. For JSVG this effect is the smallest from all libraries that I analyzed but it is still noticeable.

I can also imagine that the conversion from BufferedImage to SWT ImageData is more expensive when increasing the image size.

Can we somehow improve on this?

@Michael5601
Copy link
Contributor Author

Michael5601 commented Jan 7, 2025

I added a new regression test for this feature in commit 07f860c. Right now this test does not work as the SVGRasterizer-Fragment ist not loaded at runtime, so no SVGRasterizer can be returned.

@HannesWell Do you know what I need to include for it to be loaded? I thought this should happen as soon as SWT is used.

The reason for my problem was that I did not run the test as a JUnit Plugin Test. Sadly the JUnit Plugin Test lead to a new problem as loading the icons from the test plugin was difficult. I moved the test to a new plugin and changed the loading mechanism. Now it works.

See d084f7d for the commit.

@Michael5601 Michael5601 force-pushed the IconScalingBackup branch 2 times, most recently from 83cee5c to d084f7d Compare January 8, 2025 14:09
@Michael5601
Copy link
Contributor Author

Performance Test Results Update

I have automated the performance tests, making these results more reliable than the initial measurements in #1638 (comment). I plan to discuss this update during Thursday's community call, where it can serve as a basis for our discussion.


Measurement Modes

The performance tests measured the following scenarios:

  • PNG 100%: Current implementation with native scaling at 100%. Icons are loaded at 16x16px.
  • SVG 100%: SVG-based implementation with native scaling at 100%. Icons are rasterized to 16x16px.
  • PNG 125%: Current implementation with native scaling at 125%. Icons are loaded at 200% (32x32px) and scaled down to 20x20px.
  • SVG 125%: SVG-based implementation with native scaling at 125%. Icons are rasterized to 20x20px.
  • PNG 200%: Current implementation with native scaling at 200%. Icons are loaded at 32x32px.
  • SVG 200%: SVG-based implementation with native scaling at 200%. Icons are rasterized to 32x32px.

Each mode was measured 50 times using an automated JUnit test. Each test involved loading 105 icons from the default Java perspective.


Results

The following results differ from the first measurement as they focus on UI startup time only rather than the full Eclipse application startup time.

newplot (2)

Groups PNG 100% SVG 100% PNG 125% SVG 125% PNG 200% SVG 200%
Minimum 6.195 6.984 6.240 7.091 7.030 8.096
Median 6.515 7.346 6.584 7.496 7.645 8.715
Mean 6.523 7.359 6.617 7.495 7.619 8.706
Maximum 6.916 7.683 7.149 7.850 8.106 9.239

Evaluation

The additional time required for SVGs, compared to PNGs, is as follows:

  • 100% scaling: +0.831s (12.75%)
  • 125% scaling: +0.912s (13.85%)
  • 200% scaling: +1.070s (13.99%)

I performed manual testing at 125% with the SVG feature to find out where the performance bottlenecks are.

  • BufferedImage to ImageData conversion: ~0.1 seconds for 100 icons.
  • Initial rasterization overhead: The first icon rasterization takes ~0.1 seconds. I think this due to library initialization. Subsequent rasterizations average ~5ms per icon, totaling ~0.6 seconds for 100 icons.

These factors account for ~0.7s of the 0.912s overhead for SVG 125, with the remaining time likely due to additional File I/O operations. The File I/O is likely higher for SVGs than for PNGs as the file size is bigger.


Optimization Ideas

To improve the performance, I thought of the following ideas:

  1. Reduce SVG file sizes:

    • Remove unused elements in the SVGs (e.g., empty groups or elements outside the view area) to decrease file size and improve File I/O performance.
  2. Use a faster rasterization library:

    • JSVG is significantly faster than Apache Batik (~2x) and EchoSVG (~3.5x) and also the fastest Java library I found. Non-Java libraries could further improve performance but would likely add complexity for integration.
  3. Simplify icon designs:

    • 58.26% of SVG elements in Eclipse icons are used for color gradients. These gradients are often not seen at small sizes. Simplifying these to monochrome surfaces would reduce complexity and file size.
  4. Implement a caching mechanism:

    • Caching rasterized icons (e.g., in the file system) for multiple eclipse applications could reduce the need for repeated rasterizations. However, the performance benefit depends on whether cached File I/O is faster than rasterization. The integration of a caching mechanism also leads to some complex challenges.

@HannesWell
Copy link
Member

HannesWell commented Jan 19, 2025

I'm also looking into the build failures and tried out a few things locally but didn't succeed. Will report back once I have found a working solution.

@Michael5601 Michael5601 force-pushed the IconScalingBackup branch 7 times, most recently from 31fdbdf to 032d1c2 Compare January 21, 2025 18:17
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.

Thanks for the update. I this is heading into the right direction.

I have added a few more comments below.

@HannesWell
Copy link
Member

Now that we have a release of JSVG 1.7.0 with proper OSGi-metadata, it is added to the TP via:

Feature Proposal: Rasterization of SVGs at Runtime for Eclipse Icons
Fixes eclipse-platform#1438

Eclipse currently loads icons exclusively as raster graphics (e.g., `.png`), without support for vector formats like `.svg`. A major drawback of raster graphics is their inability to scale without degrading image quality. Additionally, generating icons of different sizes requires manually rasterizing SVGs outside Eclipse, leading to unnecessary effort and many icon files.

This PR introduces support for vector graphics in Eclipse, enabling SVGs to be used for icons. Existing PNG icons will continue to be loaded alongside SVGs, allowing the use of the new functionality without the need to replace all PNG files at once.

---
- **How It Works**:
  - To use SVG icons, simply place the SVG file in the bundle and reference it in the `plugin.xml` and other necessary locations, as is done for PNGs. No additional configuration is required.
  - At runtime, Eclipse uses the library JSVG to rasterize the SVG into a raster image of the desired size, eliminating the need for scaling. My analysis shows that JSVG is the most suitable Java library for this purpose.
  - You need to write the flag `-Dswt.autoScale=quarter` into your `eclipse.ini` file or into the run arguments of a new configuration.
*
* @since 3.129
*/
public ImageData(String filename, int zoom) {
Copy link
Member

Choose a reason for hiding this comment

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

In general using String to model file-system paths is a bad idea. But that's how it's done in the existing API.
I have created a PR with the proposal to replace it:

If that is successful I think we should use File directly in new methods.

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.

Improving Eclipse Icon Scaling by Supporting Vectorized Icons
8 participants