Use double type for better precision in local calculations in SVG and SXP

ITXT-CR-1425

Author: ar3em

styled-xml-parser/src/main/java/com/itextpdf/styledxmlparser/css/util/CssUtils.java

@@ -187,41 +187,42 @@ public static float parseAbsoluteLength(String length, String defaultMetric) {
             throw new StyledXMLParserException(MessageFormatUtil.format(LogMessageConstant.NAN, length));
         }
 
-        float f = Float.parseFloat(length.substring(0, pos));
+        // Use double type locally to have better precision of the result after applying arithmetic operations
+        double f = Double.parseDouble(length.substring(0, pos));
         String unit = length.substring(pos);
 
         //points
         if (unit.startsWith(CommonCssConstants.PT) || unit.equals("") && defaultMetric.equals(CommonCssConstants.PT)) {
-            return f;
+            return (float) f;
         }
         // inches
         if (unit.startsWith(CommonCssConstants.IN) || (unit.equals("") && defaultMetric.equals(CommonCssConstants.IN))) {
-            return f * 72f;
+            return (float) (f * 72);
         }
         // centimeters
         else if (unit.startsWith(CommonCssConstants.CM) || (unit.equals("") && defaultMetric.equals(CommonCssConstants.CM))) {
-            return (f / 2.54f) * 72f;
+            return (float) ((f / 2.54) * 72);
         }
         // quarter of a millimeter (1/40th of a centimeter).
         else if (unit.startsWith(CommonCssConstants.Q) || (unit.equals("") && defaultMetric.equals(CommonCssConstants.Q))) {
-            return (f / 2.54f) * 72f / 40;
+            return (float) ((f / 2.54) * 72 / 40);
         }
         // millimeters
         else if (unit.startsWith(CommonCssConstants.MM) || (unit.equals("") && defaultMetric.equals(CommonCssConstants.MM))) {
-            return (f / 25.4f) * 72f;
+            return (float) ((f / 25.4) * 72);
         }
         // picas
         else if (unit.startsWith(CommonCssConstants.PC) || (unit.equals("") && defaultMetric.equals(CommonCssConstants.PC))) {
-            return f * 12f;
+            return (float) (f * 12);
         }
         // pixels (1px = 0.75pt).
         else if (unit.startsWith(CommonCssConstants.PX) || (unit.equals("") && defaultMetric.equals(CommonCssConstants.PX))) {
-            return f * 0.75f;
+            return (float) (f * 0.75);
         }
 
         Logger logger = LoggerFactory.getLogger(CssUtils.class);
         logger.error(MessageFormatUtil.format(LogMessageConstant.UNKNOWN_ABSOLUTE_METRIC_LENGTH_PARSED, unit.equals("") ? defaultMetric : unit));
-        return f;
+        return (float) f;
     }
 
     /**
@@ -246,6 +247,7 @@ public static float parseRelativeValue(final String relativeValue, final float b
         int pos = determinePositionBetweenValueAndUnit(relativeValue);
         if (pos == 0)
             return 0f;
+        // Use double type locally to have better precision of the result after applying arithmetic operations
         double f = Double.parseDouble(relativeValue.substring(0, pos));
         String unit = relativeValue.substring(pos);
         if (unit.startsWith(CommonCssConstants.PERCENTAGE)) {
@@ -367,14 +369,14 @@ public static float parseResolution(String resolutionStr) {
         int pos = determinePositionBetweenValueAndUnit(resolutionStr);
         if (pos == 0)
             return 0f;
-        float f = Float.parseFloat(resolutionStr.substring(0, pos));
+        double f = Double.parseDouble(resolutionStr.substring(0, pos));
         String unit = resolutionStr.substring(pos);
         if (unit.startsWith(CommonCssConstants.DPCM)) {
-            f *= 2.54f;
+            f *= 2.54;
         } else if (unit.startsWith(CommonCssConstants.DPPX)) {
             f *= 96;
         }
-        return f;
+        return (float) f;
     }
 
     /**

styled-xml-parser/src/test/java/com/itextpdf/styledxmlparser/css/util/CssUtilTest.java renamed to styled-xml-parser/src/test/java/com/itextpdf/styledxmlparser/css/util/CssUtilsTest.java

@@ -60,7 +60,7 @@ This file is part of the iText (R) project.
 import static org.junit.Assert.assertNull;
 
 @Category(UnitTest.class)
-public class CssUtilTest extends ExtendedITextTest {
+public class CssUtilsTest extends ExtendedITextTest {
 
     public static float EPS = 0.0001f;
 
@@ -249,4 +249,21 @@ public void parseInvalidFloat() {
             Assert.fail();
         }
     }
+
+    @Test
+    public void parseAbsoluteLength12cmTest() {
+        // Calculations in CssUtils#parseAbsoluteLength were changed to work
+        // with double values instead of float to improve precision and eliminate
+        // the difference between java and .net. So the test verifies this fix.
+        assertEquals(340.15747f, CssUtils.parseAbsoluteLength("12cm"), 0f);
+    }
+
+
+    @Test
+    public void parseAbsoluteLength12qTest() {
+        // Calculations in CssUtils#parseAbsoluteLength were changed to work
+        // with double values instead of float to improve precision and eliminate
+        // the difference between java and .net. So the test verifies this fix
+        assertEquals(8.503937f, CssUtils.parseAbsoluteLength("12q"), 0f);
+    }
 }

svg/src/main/java/com/itextpdf/svg/renderers/impl/EllipseSvgNodeRenderer.java

@@ -60,37 +60,43 @@ public class EllipseSvgNodeRenderer extends AbstractSvgNodeRenderer {
     protected void doDraw(SvgDrawContext context) {
         PdfCanvas cv = context.getCurrentCanvas();
         cv.writeLiteral("% ellipse\n");
-        if(setParameters()) {
-            cv.moveTo(cx + rx, cy);
-            DrawUtils.arc(cx - rx, cy - ry, cx + rx, cy + ry, 0, 360, cv);
+        if (setParameters()) {
+            // Use double type locally to have better precision of the result after applying arithmetic operations
+            cv.moveTo((double) cx + (double) rx, cy);
+            DrawUtils.arc((double) cx - (double) rx, (double) cy - (double) ry, (double) cx + (double) rx,
+                    (double) cy + (double) ry, 0, 360, cv);
         }
     }
 
     /**
      * Fetches a map of String values by calling getAttribute(Strng s) method
      * and maps it's values to arc parmateter cx, cy , rx, ry respectively
+     *
      * @return boolean values to indicate whether all values exit or not
      */
-    protected boolean setParameters(){
-        cx=0; cy=0;
-        if(getAttribute(SvgConstants.Attributes.CX) != null){
+    protected boolean setParameters() {
+        cx = 0;
+        cy = 0;
+        if (getAttribute(SvgConstants.Attributes.CX) != null) {
             cx = CssUtils.parseAbsoluteLength(getAttribute(SvgConstants.Attributes.CX));
         }
-        if(getAttribute(SvgConstants.Attributes.CY) != null){
+        if (getAttribute(SvgConstants.Attributes.CY) != null) {
             cy = CssUtils.parseAbsoluteLength(getAttribute(SvgConstants.Attributes.CY));
         }
 
-        if(getAttribute(SvgConstants.Attributes.RX) != null
-                && CssUtils.parseAbsoluteLength(getAttribute(SvgConstants.Attributes.RX)) >0){
+        if (getAttribute(SvgConstants.Attributes.RX) != null
+                && CssUtils.parseAbsoluteLength(getAttribute(SvgConstants.Attributes.RX)) > 0) {
             rx = CssUtils.parseAbsoluteLength(getAttribute(SvgConstants.Attributes.RX));
-        }else{
-            return false; //No drawing if rx is absent
+        } else {
+            //No drawing if rx is absent
+            return false;
         }
-        if(getAttribute(SvgConstants.Attributes.RY) != null
-                &&CssUtils.parseAbsoluteLength(getAttribute(SvgConstants.Attributes.RY)) >0){
+        if (getAttribute(SvgConstants.Attributes.RY) != null
+                && CssUtils.parseAbsoluteLength(getAttribute(SvgConstants.Attributes.RY)) > 0) {
             ry = CssUtils.parseAbsoluteLength(getAttribute(SvgConstants.Attributes.RY));
-        }else{
-            return false; //No drawing if ry is absent
+        } else {
+            //No drawing if ry is absent
+            return false;
         }
         return true;
     }
@@ -102,6 +108,4 @@ public ISvgNodeRenderer createDeepCopy() {
         return copy;
     }
 
-
-
 }

svg/src/main/java/com/itextpdf/svg/utils/DrawUtils.java

@@ -57,6 +57,9 @@ private DrawUtils() {}
      * Draw an arc on the passed canvas,
      * enclosed by the rectangle for which two opposite corners are specified.
      * The arc starts at the passed starting angle and extends to the starting angle + extent
+     * @deprecated In {@link PdfCanvas} most of the path drawing methods accept {@code double}.
+     * So it is preferable to use {@link DrawUtils#arc(double, double, double, double, double, double, PdfCanvas)}.
+     * This method will be removed in iText 7.2
      * @param x1 corner-coordinate of the enclosing rectangle, first corner
      * @param y1 corner-coordinate of the enclosing rectangle, first corner
      * @param x2 corner-coordinate of the enclosing rectangle, second corner
@@ -65,12 +68,27 @@ private DrawUtils() {}
      * @param extent extent of the arc
      * @param cv canvas to paint on
      */
+    @Deprecated
     public static void arc(final float x1, final float y1, final float x2, final float y2, final float startAng, final float extent, PdfCanvas cv) {
+        arc((double) x1, (double) y1, (double) x2, (double) y2, (double) startAng, (double) extent, cv);
+    }
+
+    /**
+     * Draw an arc on the passed canvas,
+     * enclosed by the rectangle for which two opposite corners are specified.
+     * The arc starts at the passed starting angle and extends to the starting angle + extent
+     * @param x1 corner-coordinate of the enclosing rectangle, first corner
+     * @param y1 corner-coordinate of the enclosing rectangle, first corner
+     * @param x2 corner-coordinate of the enclosing rectangle, second corner
+     * @param y2 corner-coordinate of the enclosing rectangle, second corner
+     * @param startAng starting angle in degrees
+     * @param extent extent of the arc
+     * @param cv canvas to paint on
+     */
+    public static void arc(final double x1, final double y1, final double x2, final double y2, final double startAng, final double extent, PdfCanvas cv) {
         List<double[]> ar = PdfCanvas.bezierArc(x1, y1, x2, y2, startAng, extent);
         if (!ar.isEmpty()) {
-            double pt[];
-            for (int k = 0; k < ar.size(); ++k) {
-                pt = ar.get(k);
+            for (double[] pt : ar) {
                 cv.curveTo(pt[2], pt[3], pt[4], pt[5], pt[6], pt[7]);
             }
         }

svg/src/test/java/com/itextpdf/svg/renderers/impl/EllipseNodeRendererIntegrationTest.java renamed to svg/src/test/java/com/itextpdf/svg/renderers/impl/EllipseSvgNodeRendererIntegrationTest.java

@@ -43,19 +43,26 @@ This file is part of the iText (R) project.
 package com.itextpdf.svg.renderers.impl;
 
 import com.itextpdf.io.IOException;
+import com.itextpdf.kernel.pdf.PdfDocument;
+import com.itextpdf.kernel.pdf.PdfWriter;
+import com.itextpdf.kernel.pdf.canvas.PdfCanvas;
+import com.itextpdf.svg.SvgConstants;
+import com.itextpdf.svg.renderers.SvgDrawContext;
 import com.itextpdf.svg.renderers.SvgIntegrationTest;
 import com.itextpdf.test.ITextTest;
 import com.itextpdf.test.annotations.type.IntegrationTest;
 
+import java.nio.charset.StandardCharsets;
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 @Category(IntegrationTest.class)
-public class EllipseNodeRendererIntegrationTest extends SvgIntegrationTest {
+public class EllipseSvgNodeRendererIntegrationTest extends SvgIntegrationTest {
 
-    public static final String sourceFolder = "./src/test/resources/com/itextpdf/svg/renderers/impl/EllipseSvgNodeRendererTest/";
-    public static final String destinationFolder = "./target/test/com/itextpdf/svg/renderers/impl/EllipseSvgNodeRendererTest/";
+    public static final String sourceFolder = "./src/test/resources/com/itextpdf/svg/renderers/impl/EllipseSvgNodeRendererIntegrationTest/";
+    public static final String destinationFolder = "./target/test/com/itextpdf/svg/renderers/impl/EllipseSvgNodeRendererIntegrationTest/";
 
     @BeforeClass
     public static void beforeClass() {
@@ -147,4 +154,37 @@ public void ellipseSkewXTest() throws IOException, InterruptedException, java.io
     public void ellipseSkewYTest() throws IOException, InterruptedException, java.io.IOException {
         convertAndCompare(sourceFolder, destinationFolder, "ellipseSkewY");
     }
+
+    @Test
+    public void parseParametersAndCalculateCoordinatesWithBetterPrecisionEllipseTest() throws java.io.IOException, InterruptedException {
+        String filename = "parseParametersAndCalculateCoordinatesWithBetterPrecisionEllipseTest.pdf";
+        PdfDocument doc = new PdfDocument(new PdfWriter(destinationFolder + filename));
+        doc.addNewPage();
+
+        EllipseSvgNodeRenderer ellipseRenderer = new EllipseSvgNodeRenderer();
+        ellipseRenderer.setAttribute(SvgConstants.Attributes.CX, "170.3");
+        ellipseRenderer.setAttribute(SvgConstants.Attributes.CY, "339.5");
+        ellipseRenderer.setAttribute(SvgConstants.Attributes.RX, "6");
+        ellipseRenderer.setAttribute(SvgConstants.Attributes.RY, "6");
+
+        // Parse parameters with better precision (in double type) in the method CssUtils#parseAbsoluteLength
+        ellipseRenderer.setParameters();
+
+        SvgDrawContext context = new SvgDrawContext(null, null);
+        PdfCanvas cv = new PdfCanvas(doc, 1);
+        context.pushCanvas(cv);
+
+        // Calculate coordinates with better precision (in double type) in the method EllipseSvgNodeRenderer#doDraw
+        ellipseRenderer.draw(context);
+
+        String pageContentBytes = new String(doc.getPage(1).getContentBytes(), StandardCharsets.UTF_8);
+        doc.close();
+
+        String expectedResult = "132.22 254.63 m\n"
+                + "132.22 257.11 130.21 259.13 127.72 259.13 c\n"
+                + "125.24 259.13 123.22 257.11 123.22 254.63 c\n"
+                + "123.22 252.14 125.24 250.13 127.72 250.13 c\n"
+                + "130.21 250.13 132.22 252.14 132.22 254.63 c";
+        Assert.assertTrue(pageContentBytes.contains(expectedResult));
+    }
 }