refactor out EXIF rotation nonsense into its self-contained class

6 files changed, 163 insertions, 109 deletions

diff --git a/pdf-over-commons/pom.xml b/pdf-over-commons/pom.xml
index c198773f..385c3136 100644
--- a/pdf-over-commons/pom.xml
+++ b/pdf-over-commons/pom.xml
@@ -22,6 +22,10 @@
 			<version>4.23</version>
 			<scope>compile</scope>
 		</dependency>
+		<dependency>
+			<groupId>com.drewnoakes</groupId>
+			<artifactId>metadata-extractor</artifactId>
+		</dependency>
 	</dependencies>
 
 	<build>
diff --git a/pdf-over-commons/src/main/java/at/asit/pdfover/Util.java b/pdf-over-commons/src/main/java/at/asit/pdfover/Util.java
new file mode 100644
index 00000000..ff33fb5e
--- /dev/null
+++ b/pdf-over-commons/src/main/java/at/asit/pdfover/Util.java
@@ -0,0 +1,147 @@
+package at.asit.pdfover;
+
+import java.awt.Graphics2D;
+import java.awt.image.BufferedImage;
+import java.io.File;
+import java.io.IOException;
+import java.util.Iterator;
+
+import javax.imageio.ImageIO;
+import javax.imageio.ImageReadParam;
+import javax.imageio.ImageReader;
+import javax.imageio.stream.ImageInputStream;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.drew.imaging.ImageMetadataReader;
+import com.drew.imaging.ImageProcessingException;
+import com.drew.metadata.Metadata;
+import com.drew.metadata.MetadataException;
+import com.drew.metadata.exif.ExifDirectoryBase;
+import com.drew.metadata.exif.ExifIFD0Directory;
+
+class EXIFRotation {
+    private static final Logger log = LoggerFactory.getLogger(EXIFRotation.class);
+    /**
+     * rotate by this times Math.PI / 2
+     */
+    final int rotationInQuarters;
+    /**
+     * whether you should mirror (left-right) the image AFTER rotation
+     */
+    final boolean shouldMirrorLR;
+
+    private EXIFRotation(int rotateQuarters, boolean mirrorLR) {
+        this.rotationInQuarters = rotateQuarters;
+        this.shouldMirrorLR = mirrorLR;
+    }
+
+    public static final EXIFRotation NONE = new EXIFRotation(0, false);
+
+    private static final EXIFRotation[] rotationForIndex = {
+        /* invalid (0) */ NONE,
+        /* 1 */ NONE,
+        /* 2 */ new EXIFRotation(0, true),
+        /* 3 */ new EXIFRotation(2, false),
+        /* 4 */ new EXIFRotation(2, true),
+        /* 5 */ new EXIFRotation(1, true),
+        /* 6 */ new EXIFRotation(1, false),
+        /* 7 */ new EXIFRotation(3, true),
+        /* 8 */ new EXIFRotation(3, false)
+    };
+
+    static EXIFRotation For(File file) throws IOException
+    {
+        try
+        {
+            Metadata metadata = ImageMetadataReader.readMetadata(file);
+            if (metadata == null)
+                return NONE;
+            ExifIFD0Directory metaDir = metadata.getFirstDirectoryOfType(ExifIFD0Directory.class);
+            if (metaDir == null)
+                return NONE;
+            int orientation = metaDir.getInt(ExifDirectoryBase.TAG_ORIENTATION);
+            if (rotationForIndex.length <= orientation)
+            {
+                log.warn("Invalid orientation {} in EXIF metadata for {}", orientation, file.getName());
+                return NONE;
+            }
+            return rotationForIndex[orientation];
+        } catch (ImageProcessingException | MetadataException e) {
+            log.error("Failed to read EXIF metadata for {}", file.getName(), e);
+            return NONE;
+        }
+    }
+}
+
+public final class Util {
+    
+    /**
+     * ImageIO.read, except it honors EXIF rotation metadata
+     * (which the default, for some reason, does not)
+     */
+    public static final BufferedImage readImageWithEXIFRotation(File input) throws IOException
+    {
+        if (input == null)
+            throw new IllegalArgumentException("input == null");
+        if (!input.canRead())
+            throw new IllegalArgumentException("cannot read input");
+        
+        ImageInputStream stream = ImageIO.createImageInputStream(input);
+        if (stream == null)
+            throw new RuntimeException("Failed to create ImageInputStream for some reason?");
+        
+        Iterator<ImageReader> iter = ImageIO.getImageReaders(stream);
+        if (!iter.hasNext())
+        {
+            stream.close();
+            return null;
+        }
+
+        ImageReader reader = iter.next();
+        boolean isJPEG = reader.getFormatName().equals("JPEG");
+        ImageReadParam param = reader.getDefaultReadParam();
+        reader.setInput(stream, true, false);
+        BufferedImage image;
+        try {
+            image = reader.read(0, param);
+        } finally {
+            reader.dispose();
+            stream.close();
+        }
+
+        if (!isJPEG)
+            return image;
+
+        EXIFRotation rotation = EXIFRotation.For(input);
+        if (rotation.rotationInQuarters > 0)
+        {
+            boolean isSideways = ((rotation.rotationInQuarters % 2) == 1);
+            int sourceWidth = image.getWidth();
+            int sourceHeight = image.getHeight();
+            int targetWidth = isSideways ? sourceHeight : sourceWidth;
+            int targetHeight = isSideways ? sourceWidth : sourceHeight;
+            
+            BufferedImage result = new BufferedImage(targetWidth, targetHeight, image.getType());
+            Graphics2D g = result.createGraphics();
+            g.translate((targetWidth - sourceWidth)/2, (targetHeight - sourceHeight)/2);
+            g.rotate(rotation.rotationInQuarters * Math.PI / 2, sourceWidth/2, sourceHeight/2);
+            g.drawRenderedImage(image, null);
+            g.dispose();
+            image = result;
+        }
+
+        if (rotation.shouldMirrorLR)
+        {
+            int width = image.getWidth();
+            int height = image.getHeight();
+            BufferedImage result = new BufferedImage(width, height, image.getType());
+            Graphics2D g = result.createGraphics();
+            g.drawImage(image, width, 0, -width, height, null);
+            g.dispose();
+            image = result;
+        }
+        return image;
+    }
+}
diff --git a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/composites/configuration/SimpleConfigurationComposite.java b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/composites/configuration/SimpleConfigurationComposite.java
index 4fb4874e..cd8544d8 100644
--- a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/composites/configuration/SimpleConfigurationComposite.java
+++ b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/composites/configuration/SimpleConfigurationComposite.java
@@ -21,8 +21,6 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Locale;
 
-import javax.imageio.ImageIO;
-
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.dnd.DND;
 import org.eclipse.swt.dnd.DropTarget;
@@ -54,6 +52,7 @@ import org.eclipse.swt.widgets.Text;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import at.asit.pdfover.Util;
 import at.asit.pdfover.commons.Constants;
 import at.asit.pdfover.commons.Messages;
 import at.asit.pdfover.commons.Profile;
@@ -468,8 +467,7 @@ public class SimpleConfigurationComposite extends ConfigurationCompositeBase {
 			try {
 				File imgFile = new File(image);
 				this.logo = new Image(this.getDisplay(),
-						ImageConverter.convertToSWT(Emblem.fixImage(
-								ImageIO.read(imgFile), imgFile)));
+						ImageConverter.convertToSWT(Util.readImageWithEXIFRotation(imgFile)));
 			} catch (IOException e) {
 				log.error("Error reading image", e);
 			}
diff --git a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/utils/SignaturePlaceholderCache.java b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/utils/SignaturePlaceholderCache.java
index 18009895..92efea0a 100644
--- a/pdf-over-gui/src/main/java/at/asit/pdfover/gui/utils/SignaturePlaceholderCache.java
+++ b/pdf-over-gui/src/main/java/at/asit/pdfover/gui/utils/SignaturePlaceholderCache.java
@@ -33,6 +33,7 @@ import org.eclipse.swt.graphics.ImageData;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import at.asit.pdfover.Util;
 import at.asit.pdfover.commons.Constants;
 import at.asit.pdfover.signator.Emblem;
 import at.asit.pdfover.signer.pdfas.PdfAs4SignatureParameter;
@@ -52,7 +53,7 @@ public class SignaturePlaceholderCache {
 	}
 
 	private static Image loadImage(String fileDir, String fileName, String fileExt) throws IOException {
-		return ImageIO.read(new File(fileDir, fileName + "." + fileExt));
+		return Util.readImageWithEXIFRotation(new File(fileDir, fileName + "." + fileExt));
 	}
 
 	/**
diff --git a/pdf-over-signator/pom.xml b/pdf-over-signator/pom.xml
index e1b94137..1e5e345b 100644
--- a/pdf-over-signator/pom.xml
+++ b/pdf-over-signator/pom.xml
@@ -22,10 +22,6 @@
 			<artifactId>commons-codec</artifactId>
 		</dependency>
 		<dependency>
-			<groupId>com.drewnoakes</groupId>
-			<artifactId>metadata-extractor</artifactId>
-		</dependency>
-		<dependency>
 			<groupId>javax.activation</groupId>
 			<artifactId>activation</artifactId>
 		</dependency>
@@ -34,5 +30,11 @@
 			<artifactId>jaxb-runtime</artifactId>
 			<version>2.4.0-b180830.0438</version>
 		</dependency>
+		<dependency>
+			<groupId>at.a-sit</groupId>
+			<artifactId>pdf-over-commons</artifactId>
+			<version>${project.parent.version}</version>
+			<scope>compile</scope>
+		</dependency>
 	</dependencies>
 </project>
diff --git a/pdf-over-signator/src/main/java/at/asit/pdfover/signator/Emblem.java b/pdf-over-signator/src/main/java/at/asit/pdfover/signator/Emblem.java
index 0a130f79..939a27b2 100644
--- a/pdf-over-signator/src/main/java/at/asit/pdfover/signator/Emblem.java
+++ b/pdf-over-signator/src/main/java/at/asit/pdfover/signator/Emblem.java
@@ -15,6 +15,7 @@
  */
 package at.asit.pdfover.signator;
 
+import at.asit.pdfover.Util;
 // Imports
 import java.awt.Graphics2D;
 import java.awt.image.BufferedImage;
@@ -26,25 +27,14 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.file.Files;
 import java.nio.file.Paths;
-import java.util.Iterator;
 import java.util.Properties;
 
 import javax.imageio.ImageIO;
-import javax.imageio.ImageReader;
-import javax.imageio.stream.ImageInputStream;
 
 import org.apache.commons.codec.digest.DigestUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.drew.imaging.ImageMetadataReader;
-import com.drew.imaging.ImageProcessingException;
-import com.drew.metadata.Metadata;
-import com.drew.metadata.MetadataException;
-import com.drew.metadata.exif.ExifDirectoryBase;
-import com.drew.metadata.exif.ExifIFD0Directory;
-
-// TODO: what's the benefit of all of this caching business, especially since we seem to load it from disk anyway to hash it?
 /**
  *
  */
@@ -79,93 +69,6 @@ public class Emblem {
 		return DigestUtils.md5Hex(is);
 	}
 
-	/**
-	 * Correctly rotate JPEG image by EXIF header
-	 * @param img the image
-	 * @param imgFile the image file
-	 * @return the fixed image
-	 */
-	public static BufferedImage fixImage(BufferedImage img, File imgFile) {
-		int oheight = img.getHeight();
-		int owidth = img.getWidth();
-
-		// Read EXIF metadata for jpeg
-		ImageInputStream iis = null;
-		try {
-			iis = ImageIO.createImageInputStream(imgFile);
-			Iterator<ImageReader> imageReaders = ImageIO.getImageReaders(iis);
-
-			while (imageReaders.hasNext()) {
-				ImageReader reader = imageReaders.next();
-				log.debug(reader.getFormatName());
-				if (reader.getFormatName().equals("JPEG")) {
-					try {
-						Metadata metadata = ImageMetadataReader.readMetadata(imgFile);
-						ExifIFD0Directory metaDirectory = (metadata != null) ? metadata.getFirstDirectoryOfType(ExifIFD0Directory.class) : null;
-						if (metaDirectory != null) {
-							int orientation = metaDirectory.getInt(
-									ExifDirectoryBase.TAG_ORIENTATION);
-							if (orientation > 2) {
-								// rotate
-								double rotation = 0;
-								int height = owidth;
-								int width = oheight;
-								switch ((orientation + 1) / 2) {
-									case 2:
-										rotation = Math.PI;
-										height = oheight;
-										width = owidth;
-										break;
-									case 3:
-										rotation = Math.PI / 2;
-										break;
-									case 4:
-										rotation = 3 * Math.PI / 2;
-										break;
-								}
-								log.debug("EXIF orientation " + orientation + ", rotating " + rotation + "rad");
-								BufferedImage result = new BufferedImage(width, height, img.getType());
-								Graphics2D g = result.createGraphics();
-								g.translate((width - owidth) / 2, (height - oheight) / 2);
-								g.rotate(rotation, owidth / 2, oheight / 2);
-								g.drawRenderedImage(img, null);
-								g.dispose();
-								img = result;
-								owidth = width;
-								oheight = height;
-							}
-							if (((orientation < 5) && (orientation % 2 == 0)) ||
-									(orientation > 5) && (orientation % 2 == 1)) {
-								// flip
-								log.debug("flipping image");
-								BufferedImage result = new BufferedImage(owidth, oheight, img.getType());
-								Graphics2D g = result.createGraphics();
-								g.drawImage(img, owidth, 0, -owidth, oheight, null);
-								g.dispose();
-								img = result;
-							}
-						}
-					} catch (ImageProcessingException e) {
-						log.error("Error reading emblem metadata", e);
-					} catch (MetadataException e) {
-						log.error("Error reading emblem metadata", e);
-					} catch (NullPointerException e) {
-						log.error("Error reading emblem metadata", e);
-					}
-				}
-			}
-		} catch (IOException e) {
-			log.error("Error reading image" , e);
-		} finally {
-			try {
-				if (iis != null)
-					iis.close();
-			} catch (IOException e) {
-				log.debug("Error closing stream", e);
-			}
-		}
-		return img;
-	}
 
 	private static BufferedImage scaleImage(BufferedImage img, int maxWidth, int maxHeight) {
 		int oheight = img.getHeight();
@@ -231,9 +134,8 @@ public class Emblem {
 			emblemProps.setProperty(this.hshProp, emblemHsh);
 			File imgFile = new File(emblemImg);
 
-			BufferedImage img = ImageIO.read(imgFile);
-			img = fixImage(img, imgFile);
 			img = scaleImage(img, this.maxWidth, this.maxHeight);
+			BufferedImage img = Util.readImageWithEXIFRotation(imgFile);
 
 			File file = new File(this.fileDir, this.imgFileName + "." + this.imgFileExt);
 			ImageIO.write(img, this.imgFileExt, file); // ignore returned boolean