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

Support for setting SVG dimensions via CSS #128

Closed
harbulot opened this issue Sep 14, 2017 · 4 comments
Closed

Support for setting SVG dimensions via CSS #128

harbulot opened this issue Sep 14, 2017 · 4 comments

Comments

@harbulot
Copy link
Contributor

I'm trying to set the size of an SVG image using CSS styles (e.g. style="width: 25%"), but this is ignore.
Only the width and height attributes of the <svg /> element seem to have an effect. The default values seem to be 400, but I'm not sure what unit this uses, or what it is in relation to the page width.

Example

Here is a simple HTML example:

<!DOCTYPE html>
<html>
<head>
<style type="text/css">
@page {
	size: A4 portrait;
	margin: 2cm 1cm;
}
</style>
</head>
<body>
    <h1>Test</h1>
    <h2>Text</h2>
    <div style="width: 25%; border: 1px solid black;">
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit.
            Donec venenatis velit enim, a placerat lectus viverra non.
            Proin varius porta ligula, in fringilla erat suscipit a.</p>
    </div>
    <h2>SVG</h2>
    <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 30 20"
        style="width: 25%; border: 1px solid black;">
        <rect x="1" y="1" rx="5" ry="5" width="28" height="18"
            stroke="blue" fill="green" stroke-width="2" />
    </svg>
</body>
</html>

Here is the output in a browser:

image

Here is the PDF output:

image

Here is the code I've used:

        try (InputStream is = getClass()
                .getResourceAsStream("/html/" + baseName + ".html");
                FileOutputStream os = new FileOutputStream(baseName + ".pdf")) {

            String docHtml = IOUtils.toString(is, StandardCharsets.UTF_8);
            PdfRendererBuilder builder = new PdfRendererBuilder();
            builder.useSVGDrawer(new BatikSVGDrawer());

            builder.withHtmlContent(docHtml, "");

            builder.toStream(os);

            builder.run();
        }

Improvement attempt 1

I've looked into the SVG support code to investigate, and here is a partial fix:

--- a/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/PDFTranscoder.java
+++ b/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/PDFTranscoder.java
@@ -46,6 +46,9 @@ public class PDFTranscoder extends SVGAbstractTranscoder {
                this.outputDevice = od;

                this.fontResolver = fontResolver;
+
+                this.hints.put(KEY_WIDTH, (float)(width / dotsPerInch));
+                this.hints.put(KEY_HEIGHT, (float)(height / dotsPerInch));
        }

        public static class OpenHtmlFontResolver implements FontFamilyResolver {

This seems to get the correct width at least:

image

The height of the containing element still seems to be a problem.
This seems to come from getting the height from PdfBoxSVGReplacedElement.getIntrinsicHeight() (which in this case is the default value of 400).

Improvement attempt 2

This consists of 3 steps:

  1. We pass the CSS width/height values (even if it's -1) to svgDraw.
  2. We make pseudo default width/height attributes on the SVG element based on the viewBox values.
  3. We only set the width/height hints if the values are strictly positive.

The result is slightly better in that the image is now rendered at the top of its container.

One of the main outcomes is that the transcoder's width and height fields are now correct, as set via SVGAbstractTranscoder.setImageSize(...). (In this particular example, they're now 175.55 and 117.03, which is the correct aspect ratio.)

image

diff --git a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxSVGReplacedElement.java b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxSVGReplacedElement.java
index 181a3330..389dffda 100644
--- a/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxSVGReplacedElement.java
+++ b/openhtmltopdf-pdfbox/src/main/java/com/openhtmltopdf/pdfboxout/PdfBoxSVGReplacedElement.java
@@ -79,6 +79,6 @@ public class PdfBoxSVGReplacedElement implements PdfBoxReplacedElement {

     @Override
     public void paint(RenderingContext c, PdfBoxOutputDevice outputDevice, BlockBox box) {
-        svg.drawSVG(e, outputDevice, c, point.getX(), point.getY(), getIntrinsicWidth(), getIntrinsicHeight(), dotsPerPixel);
+        svg.drawSVG(e, outputDevice, c, point.getX(), point.getY(), this.width, this.height, dotsPerPixel);
     }
 }
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
index 1bcce513..fb3545b0 100644
--- a/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/BatikSVGDrawer.java
+++ b/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/BatikSVGDrawer.java
@@ -55,19 +55,29 @@ public class BatikSVGDrawer implements SVGDrawer {
                                Node importedAttr = svgElement.getAttributes().item(i);
                                newDocument.getDocumentElement().setAttribute(importedAttr.getNodeName(), importedAttr.getNodeValue());
                        }
-
+
+                       String defaultVpWidth = DEFAULT_VP_WIDTH;
+                        String defaultVpHeight = DEFAULT_VP_HEIGHT;
+                        if (svgElement.hasAttribute("viewBox")) {
+                            String[] viewBoxComponents = svgElement.getAttribute("viewBox").split(" ");
+                            if (viewBoxComponents.length >= 4) {
+                                defaultVpWidth = viewBoxComponents[2];
+                                defaultVpHeight = viewBoxComponents[3];
+                            }
+                        }
+
                        if (svgElement.hasAttribute("width")) {
                                newDocument.getDocumentElement().setAttribute("width", svgElement.getAttribute("width"));
                        }
                        else {
-                               newDocument.getDocumentElement().setAttribute("width", DEFAULT_VP_WIDTH);
+                               newDocument.getDocumentElement().setAttribute("width", defaultVpWidth);
                        }

                        if (svgElement.hasAttribute("height")) {
                                newDocument.getDocumentElement().setAttribute("height", svgElement.getAttribute("height"));
                        }
                        else {
-                               newDocument.getDocumentElement().setAttribute("height", DEFAULT_VP_HEIGHT);
+                               newDocument.getDocumentElement().setAttribute("height", defaultVpHeight);
                        }

                        TranscoderInput in = new TranscoderInput(newDocument);
diff --git a/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/PDFTranscoder.java b/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/PDFTranscoder.java
index 1594fab1..edb03a95 100644
--- a/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/PDFTranscoder.java
+++ b/openhtmltopdf-svg-support/src/main/java/com/openhtmltopdf/svgsupport/PDFTranscoder.java
@@ -46,6 +46,13 @@ public class PDFTranscoder extends SVGAbstractTranscoder {
                this.outputDevice = od;

                this.fontResolver = fontResolver;
+
+               if (width > 0) {
+                    this.hints.put(KEY_WIDTH, (float)(width / dotsPerInch));
+               }
+               if (height > 0) {
+                    this.hints.put(KEY_HEIGHT, (float)(height / dotsPerInch));
+               }
        }

        public static class OpenHtmlFontResolver implements FontFamilyResolver {

Remaining problem

Being able to get transformer.width and transformer.height after those calculations would be great, but it's too late for the first call to PdfBoxSVGReplacedElement.getIntrinsicHeight().

I'm not suggesting the above patches should be integrated in the existing code base, but hopefully, they might help investigate the issue, if possible.

@harbulot
Copy link
Contributor Author

I've investigated this issue a bit further, and I've found two ways of solving it.

Both approach try to read the <svg /> element's width and height attributes, and fallback on the viewBox attribute to get a reasonable aspect ratio if width and height are not suitable.

Option 1: minimal changes to existing interfaces

This approach more or less duplicates the logic being applied to <img /> in PdfBoxReplacedElementFactory for evaluating width, height, max-width, max-height and also copies some of the scale logic from FSImage into PdfBoxSVGReplacedElement.

It leaves SVGDrawer.getSVGWidth(Element) and SVGDrawer.getSVGHeight(Element) as they are, but doesn't really use them for PDFs. That code should still be in use with Java 2D.

My aim with this was to minimise API changes. However, I think it leads to some code duplication and spread the logic related to the scaling across multiple classes (and specific implementations).

Option 2: relying on Batik for scaling, and possibly better encapsulation

Part of the problem with the existing implementation is that we try to get the dimensions from PdfBoxSVGReplacedElement's intrinsict dimensions without necessarily having read or interpreted their initial scale, and by always giving priority to the CSS value (even if only one is specified).

Batik's SVGAbstractTranscoder (the superclass of PDFTranscoder) actually already has the logic for scaling and adapting to width, height, max-width, max-height, via "hints".
The problem is that the PDFTranscoder is only called afterwards for drawing, after the box dimensions have been evaluated. Essentially, drawSVG(Element svgElement, OutputDevice outputDevice, RenderingContext ctx, double x, double y, double width, double height, double dotsPerPixel) doesn't really make any use of what happens beforehands with PdfBoxSVGReplacedElement.

To address this, I've added an SVGImage interface (within SVGDrawer) that models the image, parses its dimensions and scales them via the PDFTransformer, using the CSS values passed as transformer hints.
This is used for the dimensions of the PdfBoxSVGReplacedElement, and then also used (with all the dimensions context and the existing transformer) to draw, later on.

(Note: I have not tested this with Java2D.)

Example

    <h2>Text</h2>
    <div style="width: 25%; border: 1px solid black;">
        <p>Lorem ipsum dolor sit amet, consectetur adipiscing elit.
            Donec venenatis velit enim, a placerat lectus viverra non.
            Proin varius porta ligula, in fringilla erat suscipit a.</p>
    </div>
    <h2>SVG</h2>
    <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 30 20"
        style="width: 25%; border: black 1px solid;">
        <rect x="1" y="1" rx="5" ry="5" width="28" height="18"
            stroke="blue" fill="yellow" stroke-width="2" />
    </svg>
    <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 30 20"
        style="width: 80%; max-width: 8cm; border: black 1px solid;">
        <rect x="1" y="1" rx="5" ry="5" width="28" height="18"
            stroke="blue" fill="yellow" stroke-width="2" />
    </svg>
    <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 30 20" width="300" height="200"
        style="height: 4cm; border: black 1px solid;">
        <rect x="1" y="1" rx="5" ry="5" width="28" height="18"
            stroke="blue" fill="yellow" stroke-width="2" />
    </svg>

The width in %, the height in cm and max-width seem to work just fine:

image

Feedback welcome, please let me know if you're interested in a pull request.

@danfickle
Copy link
Owner

Thanks @harbulot

I think option 2 is the more correct way to go, as you suggest. However, @rototor is mostly responsible for getting SVG working fully, so I'd like to get an opinion from him before making a definitive decision.

@rototor
Copy link
Contributor

rototor commented Oct 1, 2017

@danfickle I don't really know the "get the size calculation right" stuff. I mostly did the PDFBoxGraphics2D bridge to make using Batik possible. You debugged and fixed the sizing stuff :)

@harbulot You could run the TestcaseRunner and compare the written png files before and after your change. If they match everything should be fine. The TestcaseRunner writes both PDFs and PNG files, so that you can compare how different the output is. From my point of view a pullrequest for svg_issue128_option2 would be nice. If your changes breaks something in the Java2D side I will investigate it. (I should have time on Tuesday - it's a holiday here in Germany)

danfickle added a commit that referenced this issue Oct 19, 2017
… max-width and max-height.

There are now helper functions for max-width and max-height in
CalculatedStyle, to stop us all re-implementing this functionality all
over the place. Thanks again @harbulot
@danfickle
Copy link
Owner

Cleaning up old issues. I think we can close this now. If anyone wants information on how SVG sizing is handled see the SVG wiki page.

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

No branches or pull requests

3 participants