-
Notifications
You must be signed in to change notification settings - Fork 806
Support SVGs in XSSF #954
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
base: trunk
Are you sure you want to change the base?
Support SVGs in XSSF #954
Conversation
| public static final int PICTURE_TYPE_EPS = 10; | ||
| public static final int PICTURE_TYPE_BMP = 11; | ||
| public static final int PICTURE_TYPE_WPG = 12; | ||
| public static final int PICTURE_TYPE_SVG = 14; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
org.apache.poi.common.usermodel.PictureType.WDP has ooxmlId 13, which is missing here.
Should these constants be refactored to use org.apache.poi.common.usermodel.PictureType#ooxmlId in a seperate PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add the WDP one here as a placeholder for a later refactor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I added a comment as a placeholder. Hope that was what you were looking for :)
| XSSFSheet sheet = wb.createSheet(); | ||
| XSSFDrawing drawing = sheet.createDrawingPatriarch(); | ||
|
|
||
| byte[] data = "test svg data".getBytes(LocaleUtil.CHARSET_1252); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a valid svg image data (which should not be relevant for this test).
But maybe you can use a valid svg string, so it will not break if someone adds a SVG->PNG converter for backward compatibility
https://dev.w3.org/SVG/tools/svgweb/samples/svg-files/decimal.svg, for example:
<svg viewBox='0 0 125 80' xmlns='http://www.w3.org/2000/svg'>
<text y="75" font-size="100" font-family="serif"><![CDATA[10]]></text>
</svg>There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, adjusted the data
Currently, adding SVGs to a XSSF Workbook is not supported.
I added this feature similarly to #607 (Support SVGs in XWPF) with a basic test.
Do you have any tipps / suggestions to write a better test for this feature?
I have locally tried it out, generating svg_sample.xlsx.
I have read here that for full backwards compatibility, one would have to convert the SVG to a PNG, as Excel does it.
From my undestanding, the same should apply to the added feature in #607, so I decided to give it a try and open the PR similarly. Any thoughts on this?