[github-601] XDGF: handle elliptical arcs that have colinear points. Thanks to Dmitrii Komarov. This closes #601

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1916144 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 243 insertions, 74 deletions

diff --git a/poi-ooxml/src/main/java/org/apache/poi/xdgf/usermodel/section/geometry/EllipticalArcTo.java b/poi-ooxml/src/main/java/org/apache/poi/xdgf/usermodel/section/geometry/EllipticalArcTo.java
index dcce505f64..1dcb6cbdfc 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/xdgf/usermodel/section/geometry/EllipticalArcTo.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/xdgf/usermodel/section/geometry/EllipticalArcTo.java
@@ -30,6 +30,8 @@ import com.microsoft.schemas.office.visio.x2012.main.RowType;
 
 public class EllipticalArcTo implements GeometryRow {
 
+    private static final double EPS = 1e-10;
+
     EllipticalArcTo _master;
 
     // The x-coordinate of the ending vertex on an arc.
@@ -169,6 +171,13 @@ public class EllipticalArcTo implements GeometryRow {
         double x0 = last.getX();
         double y0 = last.getY();
 
+        if (isColinear(x0, y0, x, y, a, b)) {
+            // All begin, end, and control points lie on the same line.
+            // Skip calculating an arc and just replace it with line.
+            path.lineTo(x, y);
+            return;
+        }
+
         // translate all of the points to the same angle as the ellipse
         AffineTransform at = AffineTransform.getRotateInstance(-c);
         double[] pts = new double[] { x0, y0, x, y, a, b };
@@ -219,6 +228,10 @@ public class EllipticalArcTo implements GeometryRow {
         path.append(at.createTransformedShape(arc), false);
     }
 
+    private static boolean isColinear(double x1, double y1, double x2, double y2, double x3, double y3) {
+        return Math.abs((y1 - y2) * (x1 - x3) - (y1 - y3) * (x1 - x2)) < EPS;
+    }
+
     protected static double computeSweep(double startAngle, double endAngle,
             double ctrlAngle) {
         double sweep;
diff --git a/poi-ooxml/src/main/java/org/apache/poi/xdgf/usermodel/shape/ShapeRenderer.java b/poi-ooxml/src/main/java/org/apache/poi/xdgf/usermodel/shape/ShapeRenderer.java
index a6a76642d9..502be0e67c 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/xdgf/usermodel/shape/ShapeRenderer.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/xdgf/usermodel/shape/ShapeRenderer.java
@@ -24,6 +24,7 @@ import java.awt.geom.Path2D;
 
 import org.apache.poi.xdgf.usermodel.XDGFShape;
 import org.apache.poi.xdgf.usermodel.XDGFText;
+import org.apache.poi.xdgf.usermodel.section.GeometrySection;
 
 /**
  * To use this to render only particular shapes, override it and provide an
@@ -60,7 +61,27 @@ public class ShapeRenderer extends ShapeVisitor {
     }
 
     protected Path2D drawPath(XDGFShape shape) {
-        Path2D.Double path = shape.getPath();
+        Path2D path = null;
+
+        for (GeometrySection geometrySection : shape.getGeometrySections()) {
+            if (geometrySection.getNoShow()) {
+                continue;
+            }
+
+            // We preserve only first drawn path
+            if (path == null) {
+                path = drawPath(geometrySection, shape);
+            } else {
+                drawPath(geometrySection, shape);
+            }
+
+        }
+
+        return path;
+    }
+
+    private Path2D drawPath(GeometrySection geometrySection, XDGFShape shape) {
+        Path2D path = geometrySection.getPath(shape);
         if (path != null) {
 
             // setup the stroke for this line
@@ -69,7 +90,6 @@ public class ShapeRenderer extends ShapeVisitor {
             _graphics.setStroke(shape.getStroke());
             _graphics.draw(path);
         }
-
         return path;
     }
 
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xdgf/usermodel/section/geometry/GeometryTestUtils.java b/poi-ooxml/src/test/java/org/apache/poi/xdgf/usermodel/section/geometry/GeometryTestUtils.java
new file mode 100644
index 0000000000..71444f8088
--- /dev/null
+++ b/poi-ooxml/src/test/java/org/apache/poi/xdgf/usermodel/section/geometry/GeometryTestUtils.java
@@ -0,0 +1,102 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.xdgf.usermodel.section.geometry;
+
+import com.microsoft.schemas.office.visio.x2012.main.CellType;
+import com.microsoft.schemas.office.visio.x2012.main.RowType;
+import org.apache.poi.util.LocaleUtil;
+import org.junit.jupiter.api.Assertions;
+
+import java.awt.geom.Path2D;
+import java.awt.geom.PathIterator;
+import java.util.Arrays;
+import java.util.Map;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public final class GeometryTestUtils {
+
+    private static final double EPS = 1e-6;
+
+    private GeometryTestUtils() {
+    }
+
+    public static RowType createRow(long index, Map<String, Object> cells) {
+        RowType row = RowType.Factory.newInstance();
+        row.setIX(index);
+        row.setDel(false);
+
+        CellType[] cellsArray = cells
+                .entrySet()
+                .stream()
+                .map(entry -> createCell(entry.getKey(), entry.getValue().toString()))
+                .toArray(CellType[]::new);
+        row.setCellArray(cellsArray);
+
+        return row;
+    }
+
+    private static CellType createCell(String name, String value) {
+        CellType cell = CellType.Factory.newInstance();
+        cell.setN(name);
+        cell.setV(value);
+        return cell;
+    }
+
+    public static void assertPath(Path2D expected, Path2D actual) {
+        PathIterator expectedIterator = expected.getPathIterator(null);
+        PathIterator actualIterator = actual.getPathIterator(null);
+
+        double[] expectedCoordinates = new double[6];
+        double[] actualCoordinates = new double[6];
+        while (!expectedIterator.isDone() && !actualIterator.isDone()) {
+            int expectedSegmentType = expectedIterator.currentSegment(expectedCoordinates);
+            int actualSegmentType = actualIterator.currentSegment(actualCoordinates);
+
+            assertEquals(expectedSegmentType, actualSegmentType);
+            assertCoordinates(expectedCoordinates, actualCoordinates);
+
+            expectedIterator.next();
+            actualIterator.next();
+        }
+
+        if (!expectedIterator.isDone() || !actualIterator.isDone()) {
+            Assertions.fail("Path iterators have different number of segments");
+        }
+    }
+
+    private static void assertCoordinates(double[] expected, double[] actual) {
+        if (expected.length != actual.length) {
+            Assertions.fail(String.format(
+                    LocaleUtil.getUserLocale(),
+                    "Given coordinates arrays have different length: expected=%s, actual=%s",
+                    Arrays.toString(expected), Arrays.toString(actual)));
+        }
+        for (int i = 0; i < expected.length; i++) {
+            double e = expected[i];
+            double a = actual[i];
+
+            if (Math.abs(e - a) > EPS) {
+                Assertions.fail(String.format(
+                        LocaleUtil.getUserLocale(),
+                        "expected <%f> but found <%f>", e, a));
+            }
+        }
+    }
+
+}
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xdgf/usermodel/section/geometry/TestArcTo.java b/poi-ooxml/src/test/java/org/apache/poi/xdgf/usermodel/section/geometry/TestArcTo.java
index 6c9f08705d..9decd1cef2 100644
--- a/poi-ooxml/src/test/java/org/apache/poi/xdgf/usermodel/section/geometry/TestArcTo.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/xdgf/usermodel/section/geometry/TestArcTo.java
@@ -17,27 +17,20 @@
 
 package org.apache.poi.xdgf.usermodel.section.geometry;
 
-import com.microsoft.schemas.office.visio.x2012.main.CellType;
 import com.microsoft.schemas.office.visio.x2012.main.RowType;
 import com.microsoft.schemas.office.visio.x2012.main.SectionType;
 import com.microsoft.schemas.office.visio.x2012.main.TriggerType;
 
-import org.apache.poi.util.LocaleUtil;
 import org.apache.poi.xdgf.usermodel.section.GeometrySection;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 import java.awt.geom.Path2D;
-import java.awt.geom.PathIterator;
-import java.util.Arrays;
+import java.util.HashMap;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 public class TestArcTo {
 
-    private static final double EPS = 0.000001;
-
     // We draw a circular arc with radius 100 from (0, 0) to (100, 100)
     private static final double X0 = 0.0;
     private static final double Y0 = 0.0;
@@ -61,7 +54,7 @@ public class TestArcTo {
         expectedPath.curveTo(26.521649, 0.0, 51.957040, 10.535684, 70.710678, 29.289321);
         expectedPath.curveTo(89.464316, 48.042960, 100.000000, 73.478351, X, Y);
 
-        assertPath(expectedPath, actualPath);
+        GeometryTestUtils.assertPath(expectedPath, actualPath);
     }
 
     @Test
@@ -80,7 +73,7 @@ public class TestArcTo {
         expectedPath.curveTo(0.0, 26.521649, 10.535684, 51.957040, 29.289321, 70.710678);
         expectedPath.curveTo(48.042960, 89.464316,  73.478351, 100.000000,  X, Y);
 
-        assertPath(expectedPath, actualPath);
+        GeometryTestUtils.assertPath(expectedPath, actualPath);
     }
 
     @Test
@@ -98,7 +91,7 @@ public class TestArcTo {
         expectedPath.moveTo(X0, Y0);
         expectedPath.lineTo(X, Y);
 
-        assertPath(expectedPath, actualPath);
+        GeometryTestUtils.assertPath(expectedPath, actualPath);
     }
 
     @Test
@@ -119,7 +112,7 @@ public class TestArcTo {
         Path2D.Double expectedPath = new Path2D.Double();
         expectedPath.moveTo(X0, Y0);
 
-        assertPath(expectedPath, actualPath);
+        GeometryTestUtils.assertPath(expectedPath, actualPath);
     }
 
     // this test is mostly used to trigger inclusion of some
@@ -139,67 +132,15 @@ public class TestArcTo {
     }
 
     private static ArcTo createArcTo(double a) {
-        RowType row = RowType.Factory.newInstance();
-        row.setIX(0L);
-        row.setDel(false);
-
-        CellType xCell = CellType.Factory.newInstance();
-        xCell.setN("X");
-        xCell.setV(Double.toString(X));
-
-        CellType yCell = CellType.Factory.newInstance();
-        yCell.setN("Y");
-        yCell.setV(Double.toString(Y));
-
-
-        CellType aCell = CellType.Factory.newInstance();
-        aCell.setN("A");
-        aCell.setV(Double.toString(a));
-
-        CellType[] cells = new CellType[] { xCell , yCell, aCell };
-        row.setCellArray(cells);
-
+        RowType row = GeometryTestUtils.createRow(
+                0L,
+                new HashMap<String, Object>() {{
+                    put("X", X);
+                    put("Y", Y);
+                    put("A", a);
+                }}
+        );
         return new ArcTo(row);
     }
 
-    private static void assertPath(Path2D expected, Path2D actual) {
-        PathIterator expectedIterator = expected.getPathIterator(null);
-        PathIterator actualIterator = actual.getPathIterator(null);
-
-        double[] expectedCoordinates = new double[6];
-        double[] actualCoordinates = new double[6];
-        while (!expectedIterator.isDone() && !actualIterator.isDone()) {
-            int expectedSegmentType = expectedIterator.currentSegment(expectedCoordinates);
-            int actualSegmentType = actualIterator.currentSegment(actualCoordinates);
-
-            assertEquals(expectedSegmentType, actualSegmentType);
-            assertCoordinates(expectedCoordinates, actualCoordinates);
-
-            expectedIterator.next();
-            actualIterator.next();
-        }
-
-        if (!expectedIterator.isDone() || !actualIterator.isDone()) {
-            Assertions.fail("Path iterators have different number of segments");
-        }
-    }
-
-    private static void assertCoordinates(double[] expected, double[] actual) {
-        if (expected.length != actual.length) {
-            Assertions.fail(String.format(
-                    LocaleUtil.getUserLocale(),
-                    "Given coordinates arrays have different length: expected=%s, actual=%s",
-                    Arrays.toString(expected), Arrays.toString(actual)));
-        }
-        for (int i = 0; i < expected.length; i++) {
-            double e = expected[i];
-            double a = actual[i];
-
-            if (Math.abs(e - a) > EPS) {
-                Assertions.fail(String.format(
-                        LocaleUtil.getUserLocale(),
-                        "expected <%f> but found <%f>", e, a));
-            }
-        }
-    }
 }
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xdgf/usermodel/section/geometry/TestEllipticalArcTo.java b/poi-ooxml/src/test/java/org/apache/poi/xdgf/usermodel/section/geometry/TestEllipticalArcTo.java
new file mode 100644
index 0000000000..5348d9ae61
--- /dev/null
+++ b/poi-ooxml/src/test/java/org/apache/poi/xdgf/usermodel/section/geometry/TestEllipticalArcTo.java
@@ -0,0 +1,93 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.xdgf.usermodel.section.geometry;
+
+import com.microsoft.schemas.office.visio.x2012.main.RowType;
+import org.junit.jupiter.api.Test;
+
+import java.awt.geom.Arc2D;
+import java.awt.geom.Path2D;
+import java.util.HashMap;
+
+public class TestEllipticalArcTo {
+
+    private static final double X0 = 0.0;
+    private static final double Y0 = 0.0;
+    private static final double R = 100.0;
+    private static final double X = R;
+    private static final double Y = R;
+    // Rotation angle does not affect the calculation
+    private static final double C = 0.0;
+    // Draw a circular arc, it does not matter for this test which type of arc we draw
+    private static final double D = 1.0;
+
+    @Test
+    public void shouldAddArcToPathWhenControlPointIsNotColinearWithBeginAndEnd() {
+        double a = R / 2.0;
+        double b = R - Math.sqrt(R * R - a * a);
+        EllipticalArcTo ellipticalArcTo = createEllipticalArcTo(a, b);
+
+        Path2D.Double actualPath = new Path2D.Double();
+        actualPath.moveTo(X0, Y0);
+
+        // Shape isn't used while creating an elliptical arc
+        ellipticalArcTo.addToPath(actualPath, null);
+
+        Path2D.Double expectedPath = new Path2D.Double();
+        expectedPath.moveTo(X0, Y0);
+        Arc2D arc = new Arc2D.Double(-R, Y0, R * 2, R * 2, 90, -90, Arc2D.OPEN);
+        expectedPath.append(arc, false);
+
+        GeometryTestUtils.assertPath(expectedPath, actualPath);
+    }
+
+    @Test
+    public void shouldAddLineToPathWhenControlPointIsColinearWithBeginAndEnd() {
+        // We artificially set control point that is obviously colinear with begin and end.
+        // However, when you draw a very small arc, it might happen that all three points are colinear to each other
+        EllipticalArcTo ellipticalArcTo = createEllipticalArcTo(50.0, 50.0);
+
+        Path2D.Double actualPath = new Path2D.Double();
+        actualPath.moveTo(X0, Y0);
+
+        // Shape isn't used while creating an elliptical arc
+        ellipticalArcTo.addToPath(actualPath, null);
+
+        Path2D.Double expectedPath = new Path2D.Double();
+        expectedPath.moveTo(X0, Y0);
+        expectedPath.lineTo(X, Y);
+
+        GeometryTestUtils.assertPath(expectedPath, actualPath);
+    }
+
+    private static EllipticalArcTo createEllipticalArcTo(double a, double b) {
+        RowType row = GeometryTestUtils.createRow(
+                0L,
+                new HashMap<String, Object>() {{
+                    put("X", X);
+                    put("Y", Y);
+                    put("A", a);
+                    put("B", b);
+                    put("C", C);
+                    put("D", D);
+                }}
+        );
+        return new EllipticalArcTo(row);
+    }
+
+}