#490: Fix for "incomplete" paths with implicit line back to start.

imageio/imageio-clippath/src/main/java/com/twelvemonkeys/imageio/path/AdobePathWriter.java

@@ -67,11 +67,12 @@ public final class AdobePathWriter {
      * regardless of image dimensions.
      * </p>
      *
-     * @param path A {@code Shape} instance that has {@link Path2D#WIND_EVEN_ODD WIND_EVEN_ODD} rule
+     * @param path A {@code Shape} instance that has {@link Path2D#WIND_EVEN_ODD WIND_EVEN_ODD} rule,
-     *             and is contained within the rectangle [x=0.0,y=0.0,w=1.0,h=1.0].
+     *             is contained within the rectangle [x=0.0,y=0.0,w=1.0,h=1.0], and is closed.
      * @throws IllegalArgumentException if {@code path} is {@code null},
      *                                  the paths winding rule is not @link Path2D#WIND_EVEN_ODD} or
-     *                                  the paths bounding box is outside [x=0.0,y=0.0,w=1.0,h=1.0].
+     *                                  the paths bounding box is outside [x=0.0,y=0.0,w=1.0,h=1.0] or
+     *                                  the path is not closed.
      */
     public AdobePathWriter(final Shape path) {
         notNull(path, "path");
@@ -128,8 +129,8 @@ public final class AdobePathWriter {
                     break;
 
                 case PathIterator.SEG_CLOSE:
-                    // Replace initial point.
                     AdobePathSegment initial = subpath.get(0);
+
                     if (initial.apx != prev.apx || initial.apy != prev.apy) {
                         // Line back to initial if last anchor point does not equal initial anchor
                         collinear = isCollinear(prev.cppx, prev.cppy, initial.apx, initial.apy, initial.apx, initial.apy);
@@ -137,13 +138,7 @@ public final class AdobePathWriter {
                         prev = new AdobePathSegment(CLOSED_SUBPATH_BEZIER_LINKED, initial.apy, initial.apx, initial.apy, initial.apx, 0, 0);
                     }
 
-                    collinear = isCollinear(prev.cppx, prev.cppy, initial.apx, initial.apy, initial.cplx, initial.cply);
+                    close(initial, prev, subpath, segments);
-                    subpath.set(0, new AdobePathSegment(collinear ? CLOSED_SUBPATH_BEZIER_LINKED : CLOSED_SUBPATH_BEZIER_UNLINKED, prev.cppy, prev.cppx, initial.apy, initial.apx, initial.cply, initial.cplx));
-
-                    // Add to full path
-                    segments.add(new AdobePathSegment(CLOSED_SUBPATH_LENGTH_RECORD, subpath.size()));
-                    segments.addAll(subpath);
-
                     subpath.clear();
 
                     break;
@@ -152,12 +147,31 @@ public final class AdobePathWriter {
             pathIterator.next();
         }
 
-        // TODO: If subpath is not empty at this point, there was no close segment...
+        // If subpath is not empty at this point, there was no close segment...
-        // Either wrap up (if coordinates match), or throw exception (otherwise)
+        // Wrap up if coordinates match, otherwise throw exception
+        if (!subpath.isEmpty()) {
+            AdobePathSegment initial = subpath.get(0);
+
+            if (initial.apx != prev.apx || initial.apy != prev.apy) {
+                throw new IllegalArgumentException("Path must be closed");
+            }
+
+            close(initial, prev, subpath, segments);
+        }
 
         return segments;
     }
 
+    private static void close(AdobePathSegment initial, AdobePathSegment prev, List<AdobePathSegment> subpath, List<AdobePathSegment> segments) {
+        // Replace initial point.
+        boolean collinear = isCollinear(prev.cppx, prev.cppy, initial.apx, initial.apy, initial.cplx, initial.cply);
+        subpath.set(0, new AdobePathSegment(collinear ? CLOSED_SUBPATH_BEZIER_LINKED : CLOSED_SUBPATH_BEZIER_UNLINKED, prev.cppy, prev.cppx, initial.apy, initial.apx, initial.cply, initial.cplx));
+
+        // Add to full path
+        segments.add(new AdobePathSegment(CLOSED_SUBPATH_LENGTH_RECORD, subpath.size()));
+        segments.addAll(subpath);
+    }
+
     private static boolean isCollinear(double x1, double y1, double x2, double y2, double x3, double y3) {
         // Photoshop seems to write as linked if all points are the same....
         return (x1 == x2 && x2 == x3 && y1 == y2 && y2 == y3) ||
@@ -248,5 +262,4 @@ public final class AdobePathWriter {
 
         return bytes.toByteArray();
     }
-
 }

imageio/imageio-clippath/src/main/java/com/twelvemonkeys/imageio/path/Paths.java

@@ -53,6 +53,7 @@ import java.awt.*;
 import java.awt.geom.AffineTransform;
 import java.awt.geom.Path2D;
 import java.awt.image.BufferedImage;
+import java.awt.image.RenderedImage;
 import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -258,7 +259,30 @@ public final class Paths {
         return applyClippingPath(clip, image);
     }
 
-    public static boolean writeClipped(final BufferedImage image, Shape clipPath, final String formatName, final ImageOutputStream output) throws IOException {
+    /**
+     * Writes the image along with a clipping path resource, in the given format to the supplied output.
+     * The image is written to the
+     * {@code ImageOutputStream} starting at the current stream
+     * pointer, overwriting existing stream data from that point
+     * forward, if present.
+     * <p>
+     * Note: As {@link ImageIO#write(RenderedImage, String, ImageOutputStream)}, this method does
+     * <em>not</em> close the output stream.
+     * It is the responsibility of the caller to close the stream, if desired.
+     * </p>
+     *
+     * @param image the image to be written, may not be {@code null}.
+     * @param clipPath the clip path, may not be {@code null}.
+     * @param formatName the informal format name, may not be {@code null}.
+     * @param output the stream to write to, may not be {@code null}.
+     *
+     * @return {@code true} if the image was written,
+     *         otherwise {@code false} (ie. no writer was found for the specified format).
+     *
+     * @exception IllegalArgumentException if any parameter is {@code null}.
+     * @exception IOException if an error occurs during writing.
+     */
+    public static boolean writeClipped(final RenderedImage image, Shape clipPath, final String formatName, final ImageOutputStream output) throws IOException {
         if (image == null) {
             throw new IllegalArgumentException("image == null!");
         }
@@ -269,9 +293,6 @@ public final class Paths {
             throw new IllegalArgumentException("output == null!");
         }
 
-        String format = "JPG".equalsIgnoreCase(formatName) ? "JPEG" : formatName.toUpperCase();
-
-        if ("TIFF".equals(format) || "JPEG".equals(format)) {
         ImageTypeSpecifier type = ImageTypeSpecifier.createFromRenderedImage(image);
         Iterator<ImageWriter> writers = ImageIO.getImageWriters(type, formatName);
 
@@ -284,7 +305,7 @@ public final class Paths {
 
             byte[] pathResource = new AdobePathWriter(clipPath).writePathResource();
 
-                if ("TIFF".equals(format)) {
+            if (metadataFormats.contains("javax_imageio_tiff_image_1.0") || metadataFormats.contains("com_sun_media_imageio_plugins_tiff_image_1.0")) {
                 param.setCompressionMode(ImageWriteParam.MODE_EXPLICIT);
                 param.setCompressionType("Deflate");
 
@@ -305,8 +326,13 @@ public final class Paths {
                 root.appendChild(ifd);
 
                 metadata.mergeTree(metadataFormat, root);
+
+                writer.setOutput(output);
+                writer.write(null, new IIOImage(image, null, metadata), param);
+
+                return true;
             }
-                else if ("JPEG".equals(format)) {
+            else if (metadataFormats.contains("javax_imageio_jpeg_image_1.0")) {
                 String metadataFormat = "javax_imageio_jpeg_image_1.0";
                 IIOMetadataNode root = new IIOMetadataNode(metadataFormat);
 
@@ -329,14 +355,13 @@ public final class Paths {
                 root.appendChild(sequence);
 
                 metadata.mergeTree(metadataFormat, root);
-                }
-                // TODO: Else if PSD... Requires PSD write + new metadata format...
 
                 writer.setOutput(output);
                 writer.write(null, new IIOImage(image, null, metadata), param);
 
                 return true;
             }
+            // TODO: Else if PSD... Requires PSD write + new metadata format...
         }
 
         return false;

imageio/imageio-clippath/src/test/java/com/twelvemonkeys/imageio/path/AdobePathWriterTest.java

@@ -44,7 +44,8 @@ import java.util.Arrays;
 
 import static com.twelvemonkeys.imageio.path.AdobePathSegment.*;
 import static com.twelvemonkeys.imageio.path.PathsTest.assertPathEquals;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
 
 /**
  * AdobePathWriterTest.
@@ -92,6 +93,16 @@ public class AdobePathWriterTest {
         new AdobePathWriter(path);
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreateNotClosed() {
+        GeneralPath path = new GeneralPath(Path2D.WIND_EVEN_ODD);
+        path.moveTo(.5, .5);
+        path.lineTo(1, .5);
+        path.curveTo(1, 1, 1, 1, .5, 1);
+
+        new AdobePathWriter(path).writePath();
+    }
+
     @Test
     public void testCreateClosed() {
         GeneralPath path = new GeneralPath(Path2D.WIND_EVEN_ODD);
@@ -100,9 +111,30 @@ public class AdobePathWriterTest {
         path.curveTo(1, 1, 1, 1, .5, 1);
         path.closePath();
 
-        new AdobePathWriter(path).writePath();
+        byte[] bytes = new AdobePathWriter(path).writePath();
 
-        fail("Test that we have 4 segments");
+        assertEquals(6 * 26, bytes.length);
+
+        int off = 0;
+
+        // Path/initial fill rule: Even-Odd (0)
+        assertArrayEquals(new byte[] {0, PATH_FILL_RULE_RECORD, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, INITIAL_FILL_RULE_RECORD, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+
+        // Rectangle 1: 0, 0, 1, .5
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_LENGTH_RECORD, 0, 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_BEZIER_UNLINKED, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 1, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_BEZIER_UNLINKED, 0, -128, 0, 0, 1, 0, 0, 0, 0, -128, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_BEZIER_UNLINKED, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+
+        // Sanity
+        assertEquals(bytes.length, off);
     }
 
     @Test
@@ -113,9 +145,31 @@ public class AdobePathWriterTest {
         path.curveTo(1, 1, 1, 1, .5, 1);
         path.lineTo(.5, .5);
 
-        new AdobePathWriter(path).writePath(); // TODO: Should we allow this?
+        byte[] bytes = new AdobePathWriter(path).writePath();
+
+        assertEquals(6 * 26, bytes.length);
+
+        int off = 0;
+
+        // Path/initial fill rule: Even-Odd (0)
+        assertArrayEquals(new byte[] {0, PATH_FILL_RULE_RECORD, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, INITIAL_FILL_RULE_RECORD, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+
+        // Rectangle 1: 0, 0, 1, .5
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_LENGTH_RECORD, 0, 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_BEZIER_UNLINKED, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 1, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_BEZIER_UNLINKED, 0, -128, 0, 0, 1, 0, 0, 0, 0, -128, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_BEZIER_UNLINKED, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+
+        // Sanity
+        assertEquals(bytes.length, off);
 
-        fail("Test that we have 4 segments, and that it is equal to the one above");
     }
 
     @Test
@@ -127,9 +181,30 @@ public class AdobePathWriterTest {
         path.lineTo(.5, .5);
         path.closePath();
 
-        new AdobePathWriter(path).writePath();
+        byte[] bytes = new AdobePathWriter(path).writePath();
 
-        fail("Test that we have 4 segments, and that it is equal to the one above");
+        assertEquals(6 * 26, bytes.length);
+
+        int off = 0;
+
+        // Path/initial fill rule: Even-Odd (0)
+        assertArrayEquals(new byte[] {0, PATH_FILL_RULE_RECORD, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, INITIAL_FILL_RULE_RECORD, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+
+        // Rectangle 1: 0, 0, 1, .5
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_LENGTH_RECORD, 0, 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_BEZIER_UNLINKED, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 1, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_BEZIER_UNLINKED, 0, -128, 0, 0, 1, 0, 0, 0, 0, -128, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+        assertArrayEquals(new byte[] {0, CLOSED_SUBPATH_BEZIER_UNLINKED, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0, 0, -128, 0, 0},
+                Arrays.copyOfRange(bytes, off, off += 26));
+
+        // Sanity
+        assertEquals(bytes.length, off);
     }
 
     @Test
@@ -207,7 +282,6 @@ public class AdobePathWriterTest {
         AdobePathWriter pathCreator = new AdobePathWriter(path);
 
         byte[] bytes = pathCreator.writePath();
-//        System.err.println(Arrays.toString(bytes));
 
         assertEquals(12 * 26, bytes.length);
 

imageio/imageio-clippath/src/test/java/com/twelvemonkeys/imageio/path/PathsTest.java

@@ -38,15 +38,18 @@ import org.junit.Test;
 import javax.imageio.ImageIO;
 import javax.imageio.spi.IIORegistry;
 import javax.imageio.stream.ImageInputStream;
+import javax.imageio.stream.ImageOutputStream;
 import java.awt.*;
 import java.awt.geom.GeneralPath;
 import java.awt.geom.Path2D;
 import java.awt.geom.PathIterator;
 import java.awt.image.BufferedImage;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.ObjectInputStream;
 
 import static org.junit.Assert.*;
+import static org.junit.Assume.assumeTrue;
 
 /**
  * PathsTest.
@@ -240,6 +243,9 @@ public class PathsTest {
     }
 
     static void assertPathEquals(final Path2D expectedPath, final Path2D actualPath) {
+        assertNotNull("Expected path is null, check your tests...", expectedPath);
+        assertNotNull(actualPath);
+
         PathIterator expectedIterator = expectedPath.getPathIterator(null);
         PathIterator actualIterator = actualPath.getPathIterator(null);
 
@@ -261,4 +267,37 @@ public class PathsTest {
 
         assertTrue("More points than expected", actualIterator.isDone());
     }
+
+    @Test
+    public void testWriteJPEG() throws IOException {
+        Path2D originalPath = readExpectedPath("/ser/multiple-clips.ser");
+
+        ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+        BufferedImage image = new BufferedImage(2, 2, BufferedImage.TYPE_3BYTE_BGR);
+        try (ImageOutputStream stream = ImageIO.createImageOutputStream(bytes)) {
+            boolean written = Paths.writeClipped(image, originalPath, "JPEG", stream);
+            assertTrue(written);
+        }
+        assertTrue(bytes.size() > 1024); // Actual size may be plugin specific...
+
+        Path2D actualPath = Paths.readPath(new ByteArrayImageInputStream(bytes.toByteArray()));
+        assertPathEquals(originalPath, actualPath);
+    }
+
+    @Test
+    public void testWriteTIFF() throws IOException {
+        Path2D originalPath = readExpectedPath("/ser/grape-path.ser");
+
+        ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+        BufferedImage image = new BufferedImage(2, 2, BufferedImage.TYPE_INT_ARGB);
+        try (ImageOutputStream stream = ImageIO.createImageOutputStream(bytes)) {
+            boolean written = Paths.writeClipped(image, originalPath, "TIFF", stream);
+            assumeTrue(written); // TIFF support is optional
+        }
+
+        assertTrue(bytes.size() > 1024); // Actual size may be plugin specific...
+
+        Path2D actualPath = Paths.readPath(new ByteArrayImageInputStream(bytes.toByteArray()));
+        assertPathEquals(originalPath, actualPath);
+    }
 }