Merge pull request #1405 from gritGmbH/enhancement/svg-renderer-289

Prevent improper rendering of SVG graphics
deegree · Nov 16, 2022 · de33f5f · de33f5f
2 parents b23927c + 6227190
commit de33f5f
Show file tree

Hide file tree

Showing 9 changed files with 249 additions and 33 deletions.
diff --git a/...e-core/deegree-core-rendering-2d/src/main/java/org/deegree/rendering/r2d/SvgRenderer.java b/...e-core/deegree-core-rendering-2d/src/main/java/org/deegree/rendering/r2d/SvgRenderer.java
@@ -1,4 +1,3 @@
-//$HeadURL$
 /*----------------------------------------------------------------------------
  This file is part of deegree, http://deegree.org/
  Copyright (C) 2001-2010 by:
@@ -64,6 +63,7 @@ Occam Labs UG (haftungsbeschränkt)
 import org.apache.batik.transcoder.TranscoderOutput;
 import org.apache.batik.transcoder.image.PNGTranscoder;
 import org.deegree.commons.utils.ComparablePair;
+import org.deegree.commons.utils.TunableParameter;
 import org.deegree.style.styling.components.Graphic;
 import org.slf4j.Logger;
 
@@ -73,34 +73,36 @@ Occam Labs UG (haftungsbeschränkt)
  * Renders svg images onto buffered images.
  * 
  * @author <a href="mailto:[email protected]">Andreas Schmitz</a>
- * @author last edited by: $Author: mschneider $
- * 
- * @version $Revision: 31882 $, $Date: 2011-09-15 02:05:04 +0200 (Thu, 15 Sep 2011) $
  */
 class SvgRenderer {
 
     private static final Logger LOG = getLogger( SvgRenderer.class );
 
-    final LinkedHashMap<ComparablePair<String, Integer>, BufferedImage> svgCache = new LinkedHashMap<ComparablePair<String, Integer>, BufferedImage>(
-                                                                                                                                                      256 ) {
+    private final int cacheSize = TunableParameter.get( "deegree.cache.svgrenderer", 256 );
+
+    final LinkedHashMap<String, BufferedImage> svgCache = new LinkedHashMap<>( cacheSize ) {
         private static final long serialVersionUID = -6847956873232942891L;
 
         @Override
-        protected boolean removeEldestEntry( Map.Entry<ComparablePair<String, Integer>, BufferedImage> eldest ) {
-            return size() > 256; // yeah, hardcoded max size... TODO
+        protected boolean removeEldestEntry( Map.Entry<String, BufferedImage> eldest ) {
+            return size() > cacheSize;
         }
     };
 
     BufferedImage prepareSvg( Rectangle2D.Double rect, Graphic g ) {
         BufferedImage img = null;
-        ComparablePair<String, Integer> cp = new ComparablePair<String, Integer>( g.imageURL, round( g.size ) );
-        if ( svgCache.containsKey( cp ) ) {
-            img = svgCache.get( cp );
+        final String cacheKey = createCacheKey( g.imageURL, rect.width, rect.height );
+        if ( svgCache.containsKey( cacheKey ) ) {
+            img = svgCache.get( cacheKey );
         } else {
             PNGTranscoder t = new PNGTranscoder();
 
-            t.addTranscodingHint( KEY_WIDTH, new Float( rect.width ) );
-            t.addTranscodingHint( KEY_HEIGHT, new Float( rect.height ) );
+            if ( rect.width > 0.0d ) {
+                t.addTranscodingHint( KEY_WIDTH, new Float( rect.width ) );
+            }
+            if ( rect.height > 0.0d ) {
+                t.addTranscodingHint( KEY_HEIGHT, new Float( rect.height ) );
+            }
 
             TranscoderInput input = new TranscoderInput( g.imageURL );
 
@@ -110,15 +112,14 @@ BufferedImage prepareSvg( Rectangle2D.Double rect, Graphic g ) {
             TranscoderOutput output = new TranscoderOutput( out );
             InputStream in = null;
 
-            // TODO cache images
             try {
                 t.transcode( input, output );
                 out.flush();
                 in = new ByteArrayInputStream( out.toByteArray() );
                 MemoryCacheSeekableStream mcss = new MemoryCacheSeekableStream( in );
                 RenderedOp rop = create( "stream", mcss );
                 img = rop.getAsBufferedImage();
-                svgCache.put( cp, img );
+                svgCache.put( cacheKey, img );
             } catch ( TranscoderException e ) {
                 LOG.warn( "Could not rasterize svg '{}': {}", g.imageURL, e.getLocalizedMessage() );
             } catch ( IOException e ) {
@@ -131,4 +132,7 @@ BufferedImage prepareSvg( Rectangle2D.Double rect, Graphic g ) {
         return img;
     }
 
+    String createCacheKey( String url, double width, double height ) {
+        return String.format( "%s_%d_%d", url, round( width ), round( height ) );
+    }
 }
diff --git a/...e/deegree-core-rendering-2d/src/test/java/org/deegree/rendering/r2d/SvgRendererTests.java b/...e/deegree-core-rendering-2d/src/test/java/org/deegree/rendering/r2d/SvgRendererTests.java
@@ -0,0 +1,81 @@
+package org.deegree.rendering.r2d;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import java.awt.geom.Rectangle2D;
+import java.awt.image.BufferedImage;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+
+import org.deegree.style.styling.components.Graphic;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@RunWith(Parameterized.class)
+public class SvgRendererTests {
+
+    private static final Logger LOG = LoggerFactory.getLogger( SvgRendererTests.class );
+
+    @Parameters(name = "{index}: {4} {2}x{3} => {0}x{1}")
+    public static Collection<Object[]> data() {
+        // target width, height, rectWidth, rectHeight, file
+        return Arrays.asList( new Object[][] {
+                                               //
+                                               { 183, 100, 0, 100, "svg_w200_h100_border10.svg" },
+                                               { 100, 55, 100, 0, "svg_w200_h100_border10.svg" },
+                                               { 100, 100, 100, 100, "svg_w200_h100_border10.svg" },
+                                               { 220, 120, 0, 0, "svg_w200_h100_border10.svg" },
+                                               { 200, 100, 0, 100, "svg_w200_h100_no_border.svg" },
+                                               { 100, 50, 100, 0, "svg_w200_h100_no_border.svg" },
+                                               { 100, 100, 100, 100, "svg_w200_h100_no_border.svg" },
+                                               { 200, 100, 0, 0, "svg_w200_h100_no_border.svg" },
+                                               //
+                                               { 100, 183, 100, 0, "svg_w100_h200_border10.svg" },
+                                               { 55, 100, 0, 100, "svg_w100_h200_border10.svg" },
+                                               { 100, 100, 100, 100, "svg_w100_h200_border10.svg" },
+                                               { 120, 220, 0, 0, "svg_w100_h200_border10.svg" },
+                                               { 100, 200, 100, 0, "svg_w100_h200_no_border.svg" },
+                                               { 50, 100, 0, 100, "svg_w100_h200_no_border.svg" },
+                                               { 100, 100, 100, 100, "svg_w100_h200_no_border.svg" },
+                                               { 100, 200, 0, 0, "svg_w100_h200_no_border.svg" }, } );
+    }
+
+    @Parameter(0)
+    public int requiredWidth;
+
+    @Parameter(1)
+    public int requiredHeight;
+
+    @Parameter(2)
+    public int requestedWidth;
+
+    @Parameter(3)
+    public int requestedHeight;
+
+    @Parameter(4)
+    public String fileName;
+
+    @Test
+    public void testGeneratedImage()
+                            throws IOException {
+        Rectangle2D.Double rect = new Rectangle2D.Double( 0, 0, requestedWidth, requestedHeight );
+        Graphic g = new Graphic();
+        //
+        g.size = requestedHeight > 0 ? requestedHeight : -requestedWidth;
+        g.imageURL = getClass().getResource( "svgtests/" + fileName ).toExternalForm();
+
+        BufferedImage img = ( new SvgRenderer() ).prepareSvg( rect, g );
+
+        assertNotNull( img );
+        LOG.info( "generated image w: {} h: {} from: {}", img.getWidth(), img.getHeight(), fileName );
+        assertEquals( requiredWidth, img.getWidth() );
+        assertEquals( requiredHeight, img.getHeight() );
+    }
+}
diff --git a/...rc/test/resources/org/deegree/rendering/r2d/svgtests/svg_w100_h200_border10.svg b/...rc/test/resources/org/deegree/rendering/r2d/svgtests/svg_w100_h200_border10.svg
diff --git a/...c/test/resources/org/deegree/rendering/r2d/svgtests/svg_w100_h200_no_border.svg b/...c/test/resources/org/deegree/rendering/r2d/svgtests/svg_w100_h200_no_border.svg
diff --git a/...rc/test/resources/org/deegree/rendering/r2d/svgtests/svg_w200_h100_border10.svg b/...rc/test/resources/org/deegree/rendering/r2d/svgtests/svg_w200_h100_border10.svg
diff --git a/...c/test/resources/org/deegree/rendering/r2d/svgtests/svg_w200_h100_no_border.svg b/...c/test/resources/org/deegree/rendering/r2d/svgtests/svg_w200_h100_no_border.svg
diff --git a/deegree-core/deegree-core-style/src/main/java/org/deegree/style/utils/ShapeHelper.java b/deegree-core/deegree-core-style/src/main/java/org/deegree/style/utils/ShapeHelper.java
@@ -1,4 +1,3 @@
-//$HeadURL: svn+ssh://[email protected]/deegree/deegree3/trunk/deegree-core/deegree-core-rendering-2d/src/main/java/org/deegree/rendering/r2d/RenderHelper.java $
 /*----------------------------------------------------------------------------
  This file is part of deegree, http://deegree.org/
  Copyright (C) 2001-2009 by:
@@ -40,6 +39,7 @@
 import static java.lang.Math.PI;
 import static java.lang.Math.max;
 import static java.lang.Math.toRadians;
+import static org.deegree.commons.utils.TunableParameter.get;
 import static org.slf4j.LoggerFactory.getLogger;
 
 import java.awt.Shape;
@@ -55,16 +55,16 @@
 import java.io.InputStream;
 import java.net.URL;
 import java.util.HashSet;
-
+import org.apache.batik.anim.dom.SAXSVGDocumentFactory;
 import org.apache.batik.bridge.BridgeContext;
 import org.apache.batik.bridge.DocumentLoader;
 import org.apache.batik.bridge.GVTBuilder;
 import org.apache.batik.bridge.UserAgent;
 import org.apache.batik.bridge.UserAgentAdapter;
-import org.apache.batik.anim.dom.SAXSVGDocumentFactory;
 import org.apache.batik.gvt.GVTTreeWalker;
 import org.apache.batik.gvt.GraphicsNode;
 import org.apache.batik.gvt.RootGraphicsNode;
+import org.apache.xerces.parsers.SAXParser;
 import org.deegree.style.styling.components.Mark;
 import org.slf4j.Logger;
 import org.w3c.dom.svg.SVGDocument;
@@ -79,6 +79,8 @@
  */
 public class ShapeHelper {
 
+    protected static boolean SVG_TO_SHAPE_FALLBACK = get( "deegree.rendering.svg-to-shape.previous", false );
+
     private static final Logger LOG = getLogger( ShapeHelper.class );
 
     /**
@@ -263,7 +265,7 @@ public static Shape getShapeFromSvg( String url, double size, double rotation )
      */
     public static Shape getShapeFromSvg( InputStream in, String url ) {
         try {
-            SAXSVGDocumentFactory fac = new SAXSVGDocumentFactory( "org.apache.xerces.parsers.SAXParser" );
+            SAXSVGDocumentFactory fac = new SAXSVGDocumentFactory( SAXParser.class.getName() );
             SVGDocument doc = fac.createSVGDocument( url, in );
             GVTBuilder builder = new GVTBuilder();
             UserAgent userAgent = new UserAgentAdapter();
@@ -278,22 +280,30 @@ public static Shape getShapeFromSvg( InputStream in, String url ) {
             t.scale( 1 / max, 1 / max );
             t.translate( -rect.getX(), -rect.getY() );
 
-            root.setTransform( t );
-
-            GVTTreeWalker walker = new GVTTreeWalker( root );
-            GraphicsNode node = root;
-            // should not include root's shape in the path as it doesn't always work properly
-            GeneralPath shape = new GeneralPath();
-            while ( ( node = walker.nextGraphicsNode() ) != null ) {
-                AffineTransform t2 = (AffineTransform) t.clone();
-                if ( node.getTransform() != null ) {
-                    t2.concatenate( node.getTransform() );
+            if ( SVG_TO_SHAPE_FALLBACK ) {
+                // TRICKY setting transform on elements interferes with the svg coordinate system / viewbox
+                // use only as fallback if all styles are already adapted to previous scaling
+                // NOTE if the walk-through is needed or dead code is unclear
+
+                root.setTransform( t );
+                GVTTreeWalker walker = new GVTTreeWalker( root );
+                GraphicsNode node = root;
+                // should not include root's shape in the path as it doesn't always work properly
+                GeneralPath shape = new GeneralPath();
+                while ( ( node = walker.nextGraphicsNode() ) != null ) {
+                    AffineTransform t2 = (AffineTransform) t.clone();
+                    if ( node.getTransform() != null ) {
+                        t2.concatenate( node.getTransform() );
+                    }
+                    node.setTransform( t2 );
+                    shape.append( node.getOutline(), false );
                 }
-                node.setTransform( t2 );
-                shape.append( node.getOutline(), false );
-            }
 
-            return root.getOutline();
+                return root.getOutline();
+            } else {
+                Shape sizeOneShape = t.createTransformedShape( root.getOutline() );
+                return sizeOneShape;
+            }
         } catch ( IOException e ) {
             LOG.warn( "The svg image at '{}' could not be read: {}", url, e.getLocalizedMessage() );
             LOG.debug( "Stack trace", e );

diff --git a/deegree-core/deegree-core-style/src/test/java/org/deegree/style/utils/ShapeHelperTests.java b/deegree-core/deegree-core-style/src/test/java/org/deegree/style/utils/ShapeHelperTests.java
@@ -0,0 +1,94 @@
+package org.deegree.style.utils;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.closeTo;
+
+import java.awt.Shape;
+import java.awt.geom.Rectangle2D;
+import java.io.StringReader;
+import org.junit.Before;
+import org.junit.Test;
+import org.postgresql.util.ReaderInputStream;
+
+public class ShapeHelperTests {
+
+    private final String TUNABLE_OLD_SCALE = "deegree.rendering.svg-to-shape.previous";
+
+    @Before
+    public void resetTunable() {
+        ShapeHelper.SVG_TO_SHAPE_FALLBACK = false;
+    }
+
+    // NOTE this test can be removed if the fallback is removed form ShapeHelper
+    @Test
+    public void testSvgToShapeConversionViewboxWidthHeightFallbackBad() {
+        ShapeHelper.SVG_TO_SHAPE_FALLBACK = true;
+        String svg;
+        svg = "<svg width=\"61.2\" height=\"59.4\" version=\"1.1\" viewBox=\"0 0 16.2 15.7\" xmlns=\"http://www.w3.org/2000/svg\">\n";
+        svg += " <rect x=\".132\" y=\".132\" width=\"11.1\" height=\"10.5\" fill=\"#ff0\" stroke=\"#ff0\" stroke-width=\".265\"/>\n";
+        svg += " <ellipse cx=\"13.6\" cy=\"13.1\" rx=\"2.42\" ry=\"2.49\" fill=\"#808000\" stroke=\"#808000\" stroke-width=\".254\"/>\n";
+        svg += "</svg>\n";
+
+        Shape shp = ShapeHelper.getShapeFromSvg( new ReaderInputStream( new StringReader( svg ) ), "dummy.svg" );
+        Rectangle2D bounds = shp.getBounds2D();
+
+        assertThat( bounds.getMinX(), closeTo( 0.0d, 0.02 ) );
+        assertThat( bounds.getMinY(), closeTo( 0.0d, 0.02 ) );
+        assertThat( bounds.getMaxX(), closeTo( 0.02d, 0.02 ) );
+        assertThat( bounds.getMaxY(), closeTo( 0.02d, 0.02 ) );
+    }
+
+    @Test
+    public void testSvgToShapeConversionViewboxWidthHeight() {
+        String svg;
+        svg = "<svg width=\"61.2\" height=\"59.4\" version=\"1.1\" viewBox=\"0 0 16.2 15.7\" xmlns=\"http://www.w3.org/2000/svg\">\n";
+        svg += " <rect x=\".132\" y=\".132\" width=\"11.1\" height=\"10.5\" fill=\"#ff0\" stroke=\"#ff0\" stroke-width=\".265\"/>\n";
+        svg += " <ellipse cx=\"13.6\" cy=\"13.1\" rx=\"2.42\" ry=\"2.49\" fill=\"#808000\" stroke=\"#808000\" stroke-width=\".254\"/>\n";
+        svg += "</svg>\n";
+
+        Shape shp = ShapeHelper.getShapeFromSvg( new ReaderInputStream( new StringReader( svg ) ), "dummy.svg" );
+        Rectangle2D bounds = shp.getBounds2D();
+
+        // bounds should be 1 x 1 in size
+        assertThat( bounds.getMinX(), closeTo( 0.0d, 0.05 ) );
+        assertThat( bounds.getMinY(), closeTo( 0.0d, 0.05 ) );
+        assertThat( bounds.getMaxX(), closeTo( 1.0d, 0.05 ) );
+        assertThat( bounds.getMaxY(), closeTo( 1.0d, 0.05 ) );
+    }
+
+    @Test
+    public void testSvgToShapeConversionWidthHeight() {
+        String svg;
+        svg = "<svg width=\"16.2\" height=\"15.7\" xmlns=\"http://www.w3.org/2000/svg\">\n";
+        svg += " <rect x=\".132\" y=\".132\" width=\"11.1\" height=\"10.5\" fill=\"#ff0\" stroke=\"#ff0\" stroke-width=\".265\"/>\n";
+        svg += " <ellipse cx=\"13.6\" cy=\"13.1\" rx=\"2.42\" ry=\"2.49\" fill=\"#808000\" stroke=\"#808000\" stroke-width=\".254\"/>\n";
+        svg += "</svg>\n";
+
+        Shape shp = ShapeHelper.getShapeFromSvg( new ReaderInputStream( new StringReader( svg ) ), "dummy.svg" );
+        Rectangle2D bounds = shp.getBounds2D();
+
+        // bounds should be 1 x 1 in size
+        assertThat( bounds.getMinX(), closeTo( 0.0d, 0.05 ) );
+        assertThat( bounds.getMinY(), closeTo( 0.0d, 0.05 ) );
+        assertThat( bounds.getMaxX(), closeTo( 1.0d, 0.05 ) );
+        assertThat( bounds.getMaxY(), closeTo( 1.0d, 0.05 ) );
+    }
+
+    @Test
+    public void testSvgToShapeConversionViewbox() {
+        String svg;
+        svg = "<svg viewBox=\"0 0 16.2 15.7\" xmlns=\"http://www.w3.org/2000/svg\">\n";
+        svg += " <rect x=\".132\" y=\".132\" width=\"11.1\" height=\"10.5\" fill=\"#ff0\" stroke=\"#ff0\" stroke-width=\".265\"/>\n";
+        svg += " <ellipse cx=\"13.6\" cy=\"13.1\" rx=\"2.42\" ry=\"2.49\" fill=\"#808000\" stroke=\"#808000\" stroke-width=\".254\"/>\n";
+        svg += "</svg>\n";
+
+        Shape shp = ShapeHelper.getShapeFromSvg( new ReaderInputStream( new StringReader( svg ) ), "dummy.svg" );
+        Rectangle2D bounds = shp.getBounds2D();
+
+        // bounds should be 1 x 1 in size
+        assertThat( bounds.getMinX(), closeTo( 0.0d, 0.05 ) );
+        assertThat( bounds.getMinY(), closeTo( 0.0d, 0.05 ) );
+        assertThat( bounds.getMaxX(), closeTo( 1.0d, 0.05 ) );
+        assertThat( bounds.getMaxY(), closeTo( 1.0d, 0.05 ) );
+    }
+}
diff --git a/deegree-services/deegree-webservices-handbook/src/main/asciidoc/appendix.adoc b/deegree-services/deegree-webservices-handbook/src/main/asciidoc/appendix.adoc
@@ -45,4 +45,7 @@ f
 
 |deegree.sqldialect.oracle.optimized_point_storage |java.lang.Boolean |true |Use optimized point storage for 2D points in oracle database.
 
+|deegree.cache.svgrenderer |java.lang.Integer |256 |Maximum number of rendered SVG images to be cached for speed
+
+|deegree.rendering.svg-to-shape.previous |java.lang.Boolean |false |Enables the behavior of previously used versions when scaling SVG graphics for the rendering of strokes
 |===