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.

Sign up for GitHub

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 #2593

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3e77046
Logic for SVG rasterization
Michael5601 Nov 5, 2024
284f5da
added svgs
Michael5601 Dec 3, 2024
4b0ce82
Refactor names of SVGRasterizer and ISVGRasterizer
Michael5601 Dec 11, 2024
2b71399
Remove SVGUtil and move utility methods into JSVGRasterizer
Michael5601 Dec 11, 2024
ad8bbae
[FIXUP|SUGGESTION] Adapt to conversion of swt.svg to be a fragment
HannesWell Dec 17, 2024
f809177
change isSVGFile-Check to simple file-extension check
Michael5601 Dec 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bundles/org.eclipse.jface/src/org/eclipse/jface/resource/CompositeImageDescriptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ protected boolean supportsZoomLevel(int zoom) {
// Currently only support integer zoom levels, because getZoomedImageData(..)
// suffers from Bug 97506: [HiDPI] ImageData.scaledTo() should use a
// better interpolation method.
return zoom > 0 && zoom % 100 == 0;
return true;
}

private ImageData getZoomedImageData(ImageDataProvider srcProvider) {
Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FileImageDescriptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public ImageData getImageData(int zoom) {
InputStream in = getStream(zoom);
if (in != null) {
try (BufferedInputStream stream = new BufferedInputStream(in)) {
return new ImageData(stream);
return new ImageData(stream, zoom);
} catch (SWTException e) {
if (e.code != SWT.ERROR_INVALID_IMAGE) {
throw e;
Expand Down
34 changes: 30 additions & 4 deletions bundles/org.eclipse.jface/src/org/eclipse/jface/resource/URLImageDescriptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import org.eclipse.swt.graphics.ImageData;
import org.eclipse.swt.graphics.ImageDataProvider;
import org.eclipse.swt.graphics.ImageFileNameProvider;
import org.eclipse.swt.graphics.SVGRasterizer;
import org.eclipse.swt.graphics.SVGRasterizerRegistry;

/**
* An ImageDescriptor that gets its information from a URL. This class is not
Expand All @@ -59,6 +61,16 @@ public URLImageFileNameProvider(String url) {
public String getImagePath(int zoom) {
URL tempURL = getURL(url);
if (tempURL != null) {
SVGRasterizer rasterizer = SVGRasterizerRegistry.getRasterizer();
if (rasterizer != null) {
try (InputStream in = getStream(tempURL)) {
if (rasterizer.isSVGFile(in)) {
Copy link
Member

Choose a reason for hiding this comment

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

If we would just check the file-extension here we would save creating an InputStream for each image to render. The latter is probably a significant performance overhead.
Furthermore it would save us from making SVGRasterizerRegistry and SVGRasterizer public.

Copy link
Author

@Michael5601 Michael5601 Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Please see my comment.

We could nevertheless change the isSVGFile() call in Platform UI to a file-extension check. This way the method can't be deleted but SVGRasterizerRegistry and SVGRasterizer could be internal.

Edit: How can we make SVGRasterizer internal? It is implemented by JSVGRasterizer so it must be visible.

Copy link
Author

Choose a reason for hiding this comment

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

I made SVGRasterizerRegistry internal and changed isSVGFile to a file-extension check.

return getFilePath(tempURL, false);
}
} catch (IOException e) {
// ignore.
}
}
final boolean logIOException = zoom == 100;
if (zoom == 100) {
return getFilePath(tempURL, logIOException);
Expand Down Expand Up @@ -139,12 +151,22 @@ public ImageData getImageData(int zoom) {
private static ImageData getImageData(String url, int zoom) {
URL tempURL = getURL(url);
if (tempURL != null) {
SVGRasterizer rasterizer = SVGRasterizerRegistry.getRasterizer();
if (rasterizer != null) {
try (InputStream in = getStream(tempURL)) {
if (rasterizer.isSVGFile(in)) {
return getImageData(tempURL, zoom);
}
} catch (IOException e) {
// ignore.
}
}
if (zoom == 100) {
return getImageData(tempURL);
return getImageData(tempURL, zoom);
}
URL xUrl = getxURL(tempURL, zoom);
if (xUrl != null) {
ImageData xdata = getImageData(xUrl);
ImageData xdata = getImageData(xUrl, zoom);
if (xdata != null) {
return xdata;
}
Expand All @@ -153,18 +175,22 @@ private static ImageData getImageData(String url, int zoom) {
if (xpath != null) {
URL xPathUrl = getURL(xpath);
if (xPathUrl != null) {
return getImageData(xPathUrl);
return getImageData(xPathUrl, zoom);
}
}
}
return null;
}

private static ImageData getImageData(URL url) {
return getImageData(url, 0);
}

private static ImageData getImageData(URL url, int zoom) {
ImageData result = null;
try (InputStream in = getStream(url)) {
if (in != null) {
result = new ImageData(in);
result = new ImageData(in, zoom);
}
} catch (SWTException e) {
if (e.code != SWT.ERROR_INVALID_IMAGE) {
Expand Down
Loading
Loading