Imaging-162

src/main/java/org/apache/commons/imaging/common/ImageBuilder.java

@@ -80,7 +80,7 @@ public ImageBuilder(final int width, final int height, final boolean hasAlpha) {
         this.height = height;
         this.hasAlpha = hasAlpha;
     }
-
+    
     /**
      * Get the width of the ImageBuilder pixel field
      * @return a positive integer

src/main/java/org/apache/commons/imaging/formats/bmp/BmpHeaderInfo.java

@@ -33,6 +33,12 @@ class BmpHeaderInfo {
     public final int bitmapHeaderSize;
     public final int width;
     public final int height;
+    /**
+     * According to specification the height can be negative.
+     * <br>Positive height: Bitmap is stored bottom-up (0, 0 is in the bottom left corner), then this field is true
+     * <br>Negative height: Bitmap is stored top-down (0, 0 is in the top left corner), then this field is false
+     */
+    public final boolean isBottomUpBitmap;
     public final int planes;
     public final int bitsPerPixel;
     public final int compression;
@@ -85,7 +91,8 @@ public BmpHeaderInfo(final byte identifier1, final byte identifier2, final int f
 
         this.bitmapHeaderSize = bitmapHeaderSize;
         this.width = width;
-        this.height = height;
+        this.height = (height >= 0) ? height : height * -1;
+        this.isBottomUpBitmap = height >= 0;
         this.planes = planes;
         this.bitsPerPixel = bitsPerPixel;
         this.compression = compression;

src/main/java/org/apache/commons/imaging/formats/bmp/BmpImageParser.java

@@ -401,6 +401,7 @@ private ImageContents readImageContents(final InputStream is,
             // this.debugNumber("ExtraBitsPerPixel", ExtraBitsPerPixel, 4);
             debugNumber("bhi.Width", bhi.width, 4);
             debugNumber("bhi.Height", bhi.height, 4);
+            debugNumber("bhi.isBottomUpBitmap: " + bhi.isBottomUpBitmap, 0, 0);
             debugNumber("ImageLineLength", imageLineLength, 4);
             // this.debugNumber("imageDataSize", imageDataSize, 4);
             debugNumber("PixelCount", pixelCount, 4);
@@ -714,6 +715,7 @@ public BufferedImage getBufferedImage(final InputStream inputStream, Map<String,
             System.out.println("height: " + height);
             System.out.println("width*height: " + width * height);
             System.out.println("width*height*4: " + width * height * 4);
+            System.out.println("image data are stored bottom-up (true) or top-down (false): " + bhi.isBottomUpBitmap);
         }
 
         final PixelParser pixelParser = ic.pixelParser;

src/main/java/org/apache/commons/imaging/formats/bmp/PixelParserRle.java

@@ -84,7 +84,7 @@ private int processByteOfData(final int[] rgbs, final int repeat, int x, final i
 
         return pixelsWritten;
     }
-
+    
     @Override
     public void processImage(final ImageBuilder imageBuilder)
             throws ImageReadException, IOException {

src/main/java/org/apache/commons/imaging/formats/bmp/PixelParserSimple.java

@@ -32,14 +32,29 @@ public PixelParserSimple(final BmpHeaderInfo bhi, final byte[] colorTable, final
 
     @Override
     public void processImage(final ImageBuilder imageBuilder) throws ImageReadException, IOException {
-        for (int y = bhi.height - 1; y >= 0; y--) {
-            for (int x = 0; x < bhi.width; x++) {
-                final int rgb = getNextRGB();
+        // image data are stored bottom-up
+        if (bhi.isBottomUpBitmap == true) {
+            for (int y = bhi.height - 1; y >= 0; y--) {
+                for (int x = 0; x < bhi.width; x++) {
+                    final int rgb = getNextRGB();
 
-                imageBuilder.setRGB(x, y, rgb);
-                // db.setElem(y * bhi.width + x, rgb);
+                    imageBuilder.setRGB(x, y, rgb);
+                    // db.setElem(y * bhi.width + x, rgb);
+                }
+                newline();
+            }
+        }
+        // image data are stored top-down
+        else {
+            for (int y = 0; y < bhi.height; y++) {
+                for (int x = 0; x < bhi.width; x++) {
+                    final int rgb = getNextRGB();
+
+                    imageBuilder.setRGB(x, y, rgb);
+                    // db.setElem(y * bhi.width + x, rgb);
+                }
+                newline();
             }
-            newline();
         }
     }
 }

src/test/data/images/info.txt

@@ -14,9 +14,6 @@ See the License for the specific language governing permissions and
 limitations under the License.
 
 
-
-
-
 bmp/1/Oregon Scientific DS6639 - DSC_0307 - small.bmp
     Contributed by Charles Matthew Chen.
     These photos are from my cameras - a Canon SD 750, a Nikon D50.  
@@ -37,7 +34,7 @@ bmp/4/rle4deltaXY.asm
 bmp/4/rle4deltaXY.bmp
 	Contributed by Damjan Jovanovic.
 	4 bit RLE compression with an encoded-mode delta-Y
-	
+
 dcx/1/Oregon Scientific DS6639 - DSC_0307 - small.dcx
 	Charles Matthew Chen's reference photo, converted by Damjan Jovanovic.
 

src/test/data/specificTests/info.txt

@@ -0,0 +1,36 @@
+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.
+
+Adding a file to this folder has no effect to any existing test. If you want to
+use a just added file you need to add test which uses this file.
+
+For example, this folder can be used to keep images which cause an exception.
+
+bmp/1/monochrome-negative-height.bmp
+    Monochrome 4x8 bitmap with negative height value.
+    This image was taken from http://issues.apache.org/jira/browse/IMAGING-162.
+    It was attached there by Myroslav Golub.
+
+bmp/1/monochrome-positive-height.bmp
+    Monochrome 4x8 bitmap with positive height value.
+    monochrome-positive-height.xfc is the original file created with "The Gimp 2.8.10".
+    monochrome-positive-height.bmp itself was created by exporting the xfc image to bmp
+    using the options shown in monochrome-positive-height_TheGimpOptions.png.
+
+bmp/2/monochrome-positive-height-rle.bmp
+    Monochrome 4x8 bitmap with positive height value.
+    monochrome-positive-height-rle.xfc is the original file created with "The Gimp 2.8.10".
+    monochrome-positive-height-rle.bmp itself was created by exporting the xfc image to bmp
+    using the options shown in monochrome-positive-height-rle_TheGimpOptions.png.

src/test/java/org/apache/commons/imaging/ImagingTestConstants.java

@@ -24,13 +24,13 @@
 public interface ImagingTestConstants {
 
     static final File PHIL_HARVEY_TEST_IMAGE_FOLDER = new File(
-            FilenameUtils
-                    .separatorsToSystem("src\\test\\data\\images\\exif\\philHarvey\\"));
+            FilenameUtils.separatorsToSystem("src\\test\\data\\images\\exif\\philHarvey\\")
+    );
 
     static final File SOURCE_FOLDER = new File("src");
     static final File TEST_SOURCE_FOLDER = new File(SOURCE_FOLDER, "test");
-    static final File TEST_DATA_SOURCE_FOLDER = new File(TEST_SOURCE_FOLDER,
-            "data");
-    static final File TEST_IMAGE_FOLDER = new File(TEST_DATA_SOURCE_FOLDER,
-            "images");
+    static final File TEST_DATA_SOURCE_FOLDER = new File(TEST_SOURCE_FOLDER, "data");
+    static final File TEST_IMAGE_FOLDER = new File(TEST_DATA_SOURCE_FOLDER, "images");
+    // folder for files fo specific tests, see info.txt in the named folder
+    static final File TEST_SPECIFIC_FOLDER = new File(TEST_DATA_SOURCE_FOLDER, "specificTests");
 }

src/test/java/org/apache/commons/imaging/formats/bmp/specific/BmpReadHeightRleTest.java

@@ -0,0 +1,98 @@
+/*
+ * 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.commons.imaging.formats.bmp.specific;
+
+import org.apache.commons.imaging.formats.bmp.*;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertEquals;
+
+import java.awt.image.BufferedImage;
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.ImagingConstants;
+import org.apache.commons.imaging.test.util.BufferedImageTools;
+import org.junit.Test;
+
+/**
+ * This test class contains tests specific for reading an image with
+ * negative and positive height.
+ * These are tests for image files where the image data are RLE-compressed.
+ */
+public class BmpReadHeightRleTest extends BmpBaseTest {
+    /**
+     * Get image info for a bmp with positive height.
+     * Expected result: All information about the size of the image are positive numbers.
+     * @throws ImageReadException
+     * @throws IOException 
+     */
+    @Test
+    public void testImageInfoPositiveHeight() throws ImageReadException, IOException {
+        // set to true to print image data to STDOUT
+        final boolean debugMode = false;
+        final File imageFile = new File(TEST_SPECIFIC_FOLDER, "bmp/2/monochrome-positive-height-rle.bmp");
+        final Map<String, Object> params = new HashMap<String, Object>();
+        if (debugMode) params.put(ImagingConstants.PARAM_KEY_VERBOSE, true);
+        final ImageInfo imageInfo = Imaging.getImageInfo(imageFile, params);
+
+        assertNotNull(imageInfo);
+        assertEquals(8, imageInfo.getHeight());
+        assertEquals(72, imageInfo.getPhysicalHeightDpi());
+        assertEquals(0.11111111f, imageInfo.getPhysicalHeightInch(), 0.1f);
+    }
+
+    /**
+     * Get a buffered image for a bmp with positive height.
+     * Expected: The test image has white pixels in each corner except bottom left (there is a black one).
+     * This test is to prove that changes to enable reading bmp images with negative height doesn't break
+     * the ability to read bmp images with positive height.
+     * @throws ImageReadException
+     * @throws IOException 
+     */
+    @Test
+    public void testBufferedImagePositiveHeight() throws Exception {
+        // set to true to print image data to STDOUT
+        final boolean debugMode = false;
+        final File imageFile = new File(TEST_SPECIFIC_FOLDER, "bmp/2/monochrome-positive-height-rle.bmp");
+        final BufferedImage bufImage = Imaging.getBufferedImage(imageFile);
+
+        if (debugMode) BufferedImageTools.debugBufferedImageAsTable(bufImage, "positive height rle");
+        assertEquals(8, bufImage.getHeight());
+        assertEquals(4, bufImage.getWidth());
+        // the image is monochrome and has 4 black pixels, the remaning pixel are white
+        // the black pixels a placed such that we can make sure the picture is read as expected
+        // -16777216 => -2^24 => black (4 pixels); -1 => white (other pixels)
+        // top left - white
+        assertEquals(-1, bufImage.getRGB(0, 0));
+        // top right - white
+        assertEquals(-1, bufImage.getRGB(3, 0));
+        // bottom left - black
+        assertEquals(-16777216, bufImage.getRGB(0, 7));
+        // bottom right - white
+        assertEquals(-1, bufImage.getRGB(3, 7));
+        //other black pixels - just for fun
+        assertEquals(-16777216, bufImage.getRGB(1, 6));
+        assertEquals(-16777216, bufImage.getRGB(2, 5));
+        assertEquals(-16777216, bufImage.getRGB(3, 4));
+    }
+}