#592 Render linked SVG images with unit (px, cm, em, etc) sizes.

We do this by firing up the CSS parser for values of the width and height attributes that are not plain numbers. With test proof (output identical to Chrome).
danfickle · Nov 4, 2020 · 1e7e5d6 · 1e7e5d6
1 parent 16bc704
commit 1e7e5d6
Show file tree

Hide file tree

Showing 8 changed files with 83 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@
 + [#542](https://github.com/danfickle/openhtmltopdf/pull/542) Improve list-decoration placement. Thanks for PR @syjer and reporting @mndzielski.
 + [#458](https://github.com/danfickle/openhtmltopdf/issues/458) Fix for list-decorations being output (clipped) in page margin area.
 + [#525](https://github.com/danfickle/openhtmltopdf/pull/525) Remove unused schema/DTDs. Significantly reduces size of jar. Thanks for PR @syjer.
++ [#592](https://github.com/danfickle/openhtmltopdf/issues/592) Allow unit (px, cm, em, etc) values in the width/height attributes of linked SVG images. Thanks @DanielWulfert.
 
 
 ## 1.0.4 (2020-July-25)

diff --git a/README.md b/README.md
@@ -85,6 +85,7 @@ from ````/openhtmltopdf-examples/src/main/java/com/openhtmltopdf/testcases/Testc
 + [#542](https://github.com/danfickle/openhtmltopdf/pull/542) Improve list-decoration placement. Thanks for PR @syjer and reporting @mndzielski.
 + [#458](https://github.com/danfickle/openhtmltopdf/issues/458) Fix for list-decorations being output (clipped) in page margin area.
 + [#525](https://github.com/danfickle/openhtmltopdf/pull/525) Remove unused schema/DTDs. Significantly reduces size of jar. Thanks for PR @syjer.
++ [#592](https://github.com/danfickle/openhtmltopdf/issues/592) Allow unit (px, cm, em, etc) values in the width/height attributes of linked SVG images. Thanks @DanielWulfert.
 
 
 ## 1.0.4 (2020-July-25)

diff --git a/openhtmltopdf-examples/src/main/resources/demos/images/half-circle.svg b/openhtmltopdf-examples/src/main/resources/demos/images/half-circle.svg
diff --git a/...-examples/src/main/resources/visualtest/expected/issue-592-svg-linked-image-unit-size.pdf b/...-examples/src/main/resources/visualtest/expected/issue-592-svg-linked-image-unit-size.pdf
diff --git a/...pdf-examples/src/main/resources/visualtest/html/issue-592-svg-linked-image-unit-size.html b/...pdf-examples/src/main/resources/visualtest/html/issue-592-svg-linked-image-unit-size.html
@@ -0,0 +1,16 @@
+<html>
+<head>
+<style>
+@page {
+  size: 250px 6cm;
+  margin: 0;
+}
+img {
+  border: 1px solid red;
+}
+</style>
+</head>
+<body>
+  <img src="../../demos/images/half-circle.svg" alt="Half circle" />
+</body>
+</html>
diff --git a/...-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java b/...-examples/src/test/java/com/openhtmltopdf/visualregressiontests/VisualRegressionTest.java
@@ -759,6 +759,15 @@ public void testIssue493SVGStylesLinkedImage() throws IOException {
         assertTrue(vt.runTest("issue-493-svg-styles-linked-image", TestSupport.WITH_SVG));
     }
 
+    /**
+     * Tests that a svg image linked from an image element containing unit
+     * sizes (px, cm, etc) in the width and height attribute is correctly sized.
+     */
+    @Test
+    public void testIssue592SVGLinkedImageWithUnitSize() throws IOException {
+        assertTrue(vt.runTest("issue-592-svg-linked-image-unit-size", TestSupport.WITH_SVG));
+    }
+
     /**
      * Tests that a broken image inside a table cell renders with a zero sized image rather
      * than crashing with a NPE. See issue 336.

diff --git a/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/BatikSVGDrawer.java b/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/BatikSVGDrawer.java
@@ -88,9 +88,10 @@ public SVGImage buildSVGImage(Element svgElement, Box box, CssContext c,
 
     	double cssMaxWidth = CalculatedStyle.getCSSMaxWidth(c, box);
     	double cssMaxHeight = CalculatedStyle.getCSSMaxHeight(c, box);
-
-        BatikSVGImage img = new BatikSVGImage(svgElement, box, cssWidth, cssHeight,
-                cssMaxWidth, cssMaxHeight, dotsPerPixel);
+
+        BatikSVGImage img = new BatikSVGImage(
+                svgElement, box, cssWidth, cssHeight,
+                cssMaxWidth, cssMaxHeight, dotsPerPixel, c);
         img.setFontResolver(fontResolver);
         img.setUserAgentCallback(userAgentCallback);
         img.setSecurityOptions(allowScripts, allowExternalResources, allowedProtocols);

diff --git a/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/BatikSVGImage.java b/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/BatikSVGImage.java
@@ -16,6 +16,12 @@
 import org.w3c.dom.Node;
 import org.w3c.dom.Text;
 
+import com.openhtmltopdf.css.constants.CSSName;
+import com.openhtmltopdf.css.parser.CSSParser;
+import com.openhtmltopdf.css.parser.PropertyValue;
+import com.openhtmltopdf.css.sheet.StylesheetInfo;
+import com.openhtmltopdf.css.style.CssContext;
+import com.openhtmltopdf.css.style.derived.LengthValue;
 import com.openhtmltopdf.extend.OutputDevice;
 import com.openhtmltopdf.extend.SVGDrawer.SVGImage;
 import com.openhtmltopdf.render.Box;
@@ -34,12 +40,17 @@ public class BatikSVGImage implements SVGImage {
     private final PDFTranscoder pdfTranscoder;
     private UserAgentCallback userAgentCallback;
 
-    public BatikSVGImage(Element svgElement, Box box, double cssWidth, double cssHeight,
-            double cssMaxWidth, double cssMaxHeight, double dotsPerPixel) {
+    public BatikSVGImage(
+            Element svgElement, Box box,
+            double cssWidth, double cssHeight,
+            double cssMaxWidth, double cssMaxHeight,
+            double dotsPerPixel,
+            CssContext ctx) {
+
         this.svgElement = svgElement;
         this.dotsPerPixel = dotsPerPixel;
-
         this.pdfTranscoder = new PDFTranscoder(box, dotsPerPixel, cssWidth, cssHeight);
+
         if (cssWidth >= 0) {
             this.pdfTranscoder.addTranscodingHint(
                     SVGAbstractTranscoder.KEY_WIDTH,
@@ -61,7 +72,7 @@ public BatikSVGImage(Element svgElement, Box box, double cssWidth, double cssHei
                     (float) (cssMaxHeight / dotsPerPixel));
         }
 
-        Point dimensions = parseDimensions(svgElement);
+        Point dimensions = parseDimensions(svgElement, box, ctx);
         double w;
         double h;
 
@@ -111,46 +122,65 @@ public void setSecurityOptions(boolean allowScripts, boolean allowExternalResour
     public void setUserAgentCallback(UserAgentCallback userAgentCallback) {
         this.userAgentCallback = userAgentCallback;
     }
-
-    public Integer parseLength(String attrValue) {
-        // TODO read length with units and convert to dots.
-        // length ::= number (~"em" | ~"ex" | ~"px" | ~"in" | ~"cm" | ~"mm" |
-        // ~"pt" | ~"pc")?
+
+    private Integer parseLength(
+            String attrValue,
+            CSSName property,
+            Box box,
+            CssContext ctx) {
+
         try {
             return Integer.valueOf(attrValue);
         } catch (NumberFormatException e) {
-            XRLog.log(Level.WARNING, LogMessageId.LogMessageId1Param.GENERAL_INVALID_INTEGER_PASSED_AS_DIMENSION_FOR_SVG, attrValue);
-            return null;
+            // Not a plain number, probably has a unit (px, cm, etc), so
+            // try with css parser.
+
+            CSSParser parser = new CSSParser((uri, msg) -> 
+                XRLog.log(Level.WARNING, LogMessageId.LogMessageId1Param.GENERAL_INVALID_INTEGER_PASSED_AS_DIMENSION_FOR_SVG, attrValue));
+
+            PropertyValue value = parser.parsePropertyValue(property, StylesheetInfo.AUTHOR, attrValue);
+
+            if (value == null) {
+                // CSS parser couldn't deal with value either.
+                return null;
+            }
+
+            LengthValue length = new LengthValue(box.getStyle(), property, value);
+            float pixels = length.getFloatProportionalTo(property, box.getContainingBlock() == null ? 0 : box.getContainingBlock().getWidth(), ctx);
+
+            return (int) Math.round(pixels / this.dotsPerPixel);
         }
     }
-    
-    public Point parseWidthHeightAttributes(Element e) {
+
+    private Point parseWidthHeightAttributes(Element e, Box box, CssContext ctx) {
         String widthAttr = e.getAttribute("width");
-        Integer width = widthAttr.isEmpty() ? null : parseLength(widthAttr);
+        Integer width = widthAttr.isEmpty() ? null :
+            parseLength(widthAttr, CSSName.WIDTH, box, ctx);
 
         String heightAttr = e.getAttribute("height");
-        Integer height = heightAttr.isEmpty() ? null : parseLength(heightAttr);
+        Integer height = heightAttr.isEmpty() ? null : 
+            parseLength(heightAttr, CSSName.HEIGHT, box, ctx);
 
         if (width != null && height != null) {
             return new Point(width, height);
         }
-        
+
         return DEFAULT_DIMENSIONS;
     }
 
-    public Point parseDimensions(Element e) {
+    private Point parseDimensions(Element e, Box box, CssContext ctx) {
         String viewBoxAttr = e.getAttribute("viewBox");
         String[] splitViewBox = viewBoxAttr.split("\\s+");
         if (splitViewBox.length != 4) {
-            return parseWidthHeightAttributes(e);
+            return parseWidthHeightAttributes(e, box, ctx);
         }
         try {
             int viewBoxWidth = Integer.parseInt(splitViewBox[2]);
             int viewBoxHeight = Integer.parseInt(splitViewBox[3]);
 
             return new Point(viewBoxWidth, viewBoxHeight);
         } catch (NumberFormatException ex) {
-            return parseWidthHeightAttributes(e);
+            return parseWidthHeightAttributes(e, box, ctx);
         }
     }