Skip to content

Add internal API in SVGrasterizer to rasterize images at given height and width #2514

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

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

Add internal API in SVGrasterizer to rasterize images at given height and width #2514

wants to merge 1 commit into from
+162 −28

Conversation

arunjose696
Copy link
Contributor

@arunjose696 arunjose696 commented Sep 15, 2025 •
edited
Loading

It shall be possible to draw images at a custom zoom or size while maintaining the best achievable quality. To support this, images originating from an SVG source must be rasterized at the required size.

The internal API introduced in this PR enables loading SVGs at custom sizes, allowing images to be drawn at any requested zoom level or resolution, as discussed in in vi-eclipse/Eclipse-Platform#326

Goal It shall be possible to draw images at a custom zoom/size at the best achievable quality. This means either

  • rescaling from the most fitting zoomed version of the image that exists
  • rasterizing the image at the according size from an SVG source

This PR specifically adds the internal API to handle rasterization for images coming from SVG sources.

@laeubi
Copy link
Contributor

laeubi commented Sep 15, 2025

@arunjose696 can we please mirror the (relevant) information in the PR? Its always a bit hard to follow for others what are is discussed if it happens in another repository with a different organization.

laeubi
laeubi reviewed Sep 15, 2025
View reviewed changes
bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/image/SVGRasterizer.java Outdated
*/
public ImageData rasterizeSVG(InputStream stream, int zoom) throws IOException;

public ImageData rasterizeSVG(InputStream stream, int width, int height) throws IOException;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add javadoc here, also if there are any limitations (e.g. what happens when size is negative).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added javadoc

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 15, 2025 •
edited
Loading

Test Results

  118 files  ±0    118 suites  ±0   11m 9s ⏱️ + 1m 57s
4 432 tests ±0  4 413 ✅ +2  17 💤 ±0  2 ❌  - 2 
  298 runs  ±0    292 ✅ +2   4 💤 ±0  2 ❌  - 2 

For more details on these failures, see this check.

Results for commit 1d8eb8c. ± Comparison against base commit 7d4190a.

♻️ This comment has been updated with latest results.

@laeubi laeubi mentioned this pull request Sep 15, 2025
Open
@arunjose696 arunjose696 mentioned this pull request Sep 15, 2025
Open
@arunjose696 arunjose696 linked an issue Sep 15, 2025 that may be closed by this pull request
rasterize the image at the according size from an SVG source vi-eclipse/Eclipse-Platform#449
Open
@arunjose696 arunjose696 force-pushed the arunjose696/326/rasterizer branch from 663817f to c04bcdf Compare September 15, 2025 14:11
This was referenced Sep 15, 2025
Draft
Draft
@HeikoKlare HeikoKlare force-pushed the arunjose696/326/rasterizer branch from c04bcdf to a0f0f0b Compare September 22, 2025 15:04
@HeikoKlare
Copy link
Contributor

@arunjose696 can you please fix the compilation errors?

Error:  D:\a\eclipse.platform.swt\eclipse.platform.swt\bundles\org.eclipse.swt\Eclipse SWT\common\org\eclipse\swt\internal\image\SVGFileFormat.java:[57] 
Error:  	} catch (IOException e) {
Error:  	         ^^^^^^^^^^^
Error:  Unreachable catch block for IOException. This exception is never thrown from the try statement body

@arunjose696 arunjose696 force-pushed the arunjose696/326/rasterizer branch 2 times, most recently from 74d2f86 to 2f57b89 Compare September 22, 2025 16:08
HeikoKlare
HeikoKlare reviewed Sep 22, 2025
View reviewed changes
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.

In general, the change looks good. I have rather minor comments.

bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/image/SVGRasterizer.java Outdated
Comment on lines 41 to 42
* If {@code width} or {@code height}
* is zero or negative, this method throws an {@link SWT#ERROR_INVALID_ARGUMENT}.
Copy link
Contributor

Choose a reason for hiding this comment

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

You could document this in a @exception documentation for SWTException just like for other SWT APIs (such as the Control constructor to only mention one example). You could also add the explanation for SWT#ERROR_INVALID_IMAGE there.

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/JSVGRasterizerTest.java
assertEquals(300, data.width);
assertEquals(150, data.height);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering if it's a good idea to tighly couple the general SWT test bundle to the current SVG fragment with one possible SVGRasterizer implementation. But I think that's better than adding some unnecessary abstraction for the test setup, as they are easy to adapt if necessary anyway.

Since you have already add tests for the new constructors, maybe you could also add similiar tests for the existing constructors and the exceptional cases (width/height < 0, invalid stream etc.)?

tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/JSVGRasterizerTest.java Outdated
@@ -0,0 +1,52 @@
/*******************************************************************************
* Copyright (c) 2024, 2025 Yatta Solutions and others.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Copyright (c) 2024, 2025 Yatta Solutions and others.
* Copyright (c) 2025 Yatta Solutions and others.

@arunjose696
Add internal API in SVGrasterizer to rasterize images at given height…
1d8eb8c 
… and width

Adding internal API to SVGRasterizer and simple tests to see if the svgs
are loaded at right sizes by the rasterizer
@arunjose696 arunjose696 force-pushed the arunjose696/326/rasterizer branch from 2f57b89 to 1d8eb8c Compare September 23, 2025 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HeikoKlare HeikoKlare HeikoKlare left review comments

@laeubi laeubi laeubi left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rasterize the image at the according size from an SVG source
3 participants
@arunjose696 @laeubi @HeikoKlare