Imaging-162

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

@@ -58,6 +58,7 @@ public class ImageBuilder {
     private final int width;
     private final int height;
     private final boolean hasAlpha;
+    private final boolean readBottomUp;
 
     /**
      * Construct an ImageBuilder instance
@@ -68,6 +69,26 @@ public class ImageBuilder {
      * requirements for the ImageBuilder or resulting BufferedImage.
      */
     public ImageBuilder(final int width, final int height, final boolean hasAlpha) {
+        this(true, width, height, hasAlpha);
+    }
+
+    /**
+     * Construct an ImageBuilder instance.
+     * <p>
+     * Usually image data is stored bottom-up, that means that the last row in the
+     * image data is the first row of pixels in the image as displayed.
+     * Sometimes image data are stored top-down: The first row in the image data
+     * is also the first row of pixels in the image as displayed.
+     * With this constructor you can define the direction.
+     * @param readBottomUp {@code true}: the image data are stored bottom-up,
+     * {@code false} the image data are stored top-down
+     * @param width the width of the image to be built
+     * @param height the height of the image to be built
+     * @param hasAlpha indicates whether the image has an alpha channel
+     * (the selection of alpha channel does not change the memory
+     * requirements for the ImageBuilder or resulting BufferedImage.
+     */
+    public ImageBuilder(final boolean readBottomUp, final int width, final int height, final boolean hasAlpha) {
         if (width <= 0) {
             throw new RasterFormatException("zero or negative width value");
         }
@@ -79,8 +100,18 @@ public ImageBuilder(final int width, final int height, final boolean hasAlpha) {
         this.width = width;
         this.height = height;
         this.hasAlpha = hasAlpha;
+        this.readBottomUp = readBottomUp;
     }
-
+
+    /**
+     * {@code true}: the image data are stored bottom-up,
+     * {@code false} the image data are stored top-down
+     * @return 
+     */
+    public boolean readBottomUp() {
+        return this.readBottomUp;
+    }
+
     /**
      * Get the width of the ImageBuilder pixel field
      * @return a positive integer

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

@@ -13,6 +13,8 @@
  * 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.
+ *
+ * Changed 2015 by Michael Gross, [email protected]
  */
 package org.apache.commons.imaging.formats.bmp;
 
@@ -33,6 +35,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 +93,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

@@ -13,6 +13,8 @@
  * 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.
+ *
+ * Changed 2015 by Michael Gross, [email protected]
  */
 package org.apache.commons.imaging.formats.bmp;
 
@@ -401,6 +403,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,10 +717,11 @@ 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;
-        final ImageBuilder imageBuilder = new ImageBuilder(width, height, true);
+        final ImageBuilder imageBuilder = new ImageBuilder(bhi.isBottomUpBitmap, width, height, true);
         pixelParser.processImage(imageBuilder);
 
         return imageBuilder.getBufferedImage();

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

@@ -84,7 +84,13 @@ private int processByteOfData(final int[] rgbs, final int repeat, int x, final i
 
         return pixelsWritten;
     }
-
+
+    // TODO: Reading RLE-encoded bitmaps where a negative height is given in the
+    // bitmap header won't work as expected. You'll get always a positive height
+    // from class BitmapHeaderInfo but the image data will appear bottom -> top
+    // when displayed.
+    // See PixelParserSimple.processImage() where we fixed this for non-RLE-encoded
+    // bitmaps.
     @Override
     public void processImage(final ImageBuilder imageBuilder)
             throws ImageReadException, IOException {

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

@@ -13,6 +13,8 @@
  * 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.
+ *
+ * Changed 2015 by Michael Gross, [email protected]
  */
 package org.apache.commons.imaging.formats.bmp;
 
@@ -32,14 +34,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 (imageBuilder.readBottomUp() == 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);
+                }
+                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);
+                    imageBuilder.setRGB(x, y, rgb);
+                    // db.setElem(y * bhi.width + x, rgb);
+                }
+                newline();
             }
-            newline();
         }
     }
 }

src/test/data/images/info.txt

@@ -13,8 +13,14 @@ 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.
 
+Changed 2015 by Michael Gross, [email protected]
 
-
+Adding any image file to this folder means that this file will used immediately
+in various tests. These tests use certain functions of this library for all
+image files in this folder.
+For example, consider an bmp image file which shall bring up an exception.
+Adding this file here will cause some existing tests to fail while the same tests
+pass for all other bmp images in this folder.
 
 
 bmp/1/Oregon Scientific DS6639 - DSC_0307 - small.bmp
@@ -37,7 +43,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,38 @@
+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.
+
+Added 2015 by Michael Gross, [email protected]
+
+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

@@ -13,6 +13,8 @@
  * 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.
+ *
+ * Changed 2015 by Michael Gross, [email protected]
  */
 
 package org.apache.commons.imaging;
@@ -24,13 +26,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,101 @@
+/*
+ * 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.
+ *
+ * Added 2015 by Michael Gross, [email protected]
+ * Changed 2015 by Michael Gross, [email protected]
+ */
+
+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));
+    }
+}