#400 Better handling of colors, now uses fallback B/W for most bi-level images.

Gray image now uses palette if present.
PaletteInfo header flag is ignored and no longer outputs warning.

imageio/imageio-pcx/src/main/java/com/twelvemonkeys/imageio/plugins/pcx/CGAColorModel.java

@@ -56,30 +56,25 @@ final class CGAColorModel {
             // Configured palette
             byte byte3 = cgaMode[3];
 
-            System.err.printf("background: %d\n", background);
+            boolean colorBurstEnable = (byte3 & 0x80) != 0;
-            System.err.printf("cgaMode: %02x\n", (byte3 & 0xff));
-            System.err.printf("cgaMode: %d\n", (byte3 & 0x80) >> 7);
-            System.err.printf("cgaMode: %d\n", (byte3 & 0x40) >> 6);
-            System.err.printf("cgaMode: %d\n", (byte3 & 0x20) >> 5);
-
-            boolean colorBurstEnable = (byte3 & 0x80) == 0;
             boolean paletteValue = (byte3 & 0x40) != 0;
             boolean intensityValue = (byte3 & 0x20) != 0;
 
+            if (PCXImageReader.DEBUG) {
                 System.err.println("colorBurstEnable: " + colorBurstEnable);
                 System.err.println("paletteValue: " + paletteValue);
                 System.err.println("intensityValue: " + intensityValue);
+            }
 
             // Set up the fixed part of the palette
-            if (colorBurstEnable) {
             if (paletteValue) {
                 if (intensityValue) {
                     cmap[1] = CGA_PALETTE[11];
-                        cmap[2] = CGA_PALETTE[13];
+                    cmap[2] = colorBurstEnable ? CGA_PALETTE[13] : CGA_PALETTE[12];
                     cmap[3] = CGA_PALETTE[15];
                 } else {
                     cmap[1] = CGA_PALETTE[3];
-                        cmap[2] = CGA_PALETTE[5];
+                    cmap[2] = colorBurstEnable ? CGA_PALETTE[5] : CGA_PALETTE[4];
                     cmap[3] = CGA_PALETTE[7];
                 }
             } else {
@@ -93,17 +88,6 @@ final class CGAColorModel {
                     cmap[3] = CGA_PALETTE[6];
                 }
             }
-            } else {
-                if (intensityValue) {
-                    cmap[1] = CGA_PALETTE[11];
-                    cmap[2] = CGA_PALETTE[12];
-                    cmap[3] = CGA_PALETTE[15];
-                } else {
-                    cmap[1] = CGA_PALETTE[4];
-                    cmap[2] = CGA_PALETTE[5];
-                    cmap[3] = CGA_PALETTE[7];
-                }
-            }
         }
 
         return new IndexColorModel(bitsPerPixel, cmap.length, cmap, 0, false, -1, DataBuffer.TYPE_BYTE);

imageio/imageio-pcx/src/main/java/com/twelvemonkeys/imageio/plugins/pcx/PCXHeader.java

@@ -30,11 +30,14 @@ package com.twelvemonkeys.imageio.plugins.pcx;
 
 import javax.imageio.IIOException;
 import javax.imageio.stream.ImageInputStream;
+import java.awt.image.DataBuffer;
 import java.awt.image.IndexColorModel;
 import java.io.IOException;
 import java.util.Arrays;
 
 final class PCXHeader {
+    private static final IndexColorModel MONOCHROME = new IndexColorModel(1, 2, new int[] {0, -1}, 0, false, -1, DataBuffer.TYPE_BYTE);
+
     private int version;
     private int compression;
     private int bitsPerPixel;
@@ -69,14 +72,6 @@ final class PCXHeader {
         return height;
     }
 
-    public int getHdpi() {
-        return hdpi;
-    }
-
-    public int getVdpi() {
-        return vdpi;
-    }
-
     public int getChannels() {
         return channels;
     }
@@ -85,22 +80,63 @@ final class PCXHeader {
         return bytesPerLine;
     }
 
-    public int getPaletteInfo() {
+    public IndexColorModel getEGAPalette() {
-        return paletteInfo;
+        // Test for CGA modes
+        if (isCGAVideoMode4() || isCGAVideoMode5() || isCGAVideoMode6()) {
+            return CGAColorModel.create(palette, bitsPerPixel);
         }
 
-    public IndexColorModel getEGAPalette() {
+        // Test if we should use a default B/W palette
-        // TODO: Figure out when/how to enable CGA palette... The test below isn't good enough.
+        if (bitsPerPixel == 1 && channels == 1 && (version < PCX.VERSION_2_X_WINDOWS || isDummyPalette())) {
-//        if (channels == 1 && (bitsPerPixel == 1  || bitsPerPixel == 2)) {
+            return MONOCHROME;
-//            return CGAColorModel.create(palette, bitsPerPixel);
+        }
-//        }
 
         int bits = channels * bitsPerPixel;
         return new IndexColorModel(bits, Math.min(16, 1 << bits), palette, 0, false);
     }
 
+    private boolean isCGAVideoMode4() {
+        return bitsPerPixel * channels == 2 && width == 320 && hdpi == 320 && height == 200 && vdpi == 200;
+    }
+
+    private boolean isCGAVideoMode5() {
+        return bitsPerPixel == 1 && channels == 1 && width == 320 && hdpi == 320 && height == 200 && vdpi == 200;
+    }
+
+    private boolean isCGAVideoMode6() {
+        return bitsPerPixel == 1 && channels == 1 && width == 640 && hdpi == 640 && height == 200 && vdpi == 200;
+    }
+
+    private boolean isDummyPalette() {
+        return isEmptyPalette() || isPhotoshopPalette();
+    }
+
+    private boolean isEmptyPalette() {
+        // All black
+        for (int i = 0; i < 48; i++) {
+            if (palette[i] != 0) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    private boolean isPhotoshopPalette() {
+        // Written by Photoshop: 15,15,15, 14,14,14, ... 0,0,0
+        for (int i = 0; i < 16; i++) {
+            int off = i * 3;
+
+            if (palette[off] != 15 - i || palette[off + 1] != 15 - i || palette[off + 2] != 15 - i) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     @Override public String toString() {
-        return "PCXHeader{" +
+        return "PCXHeader[" +
                 "version=" + version +
                 ", compression=" + compression +
                 ", bitsPerPixel=" + bitsPerPixel +
@@ -114,7 +150,7 @@ final class PCXHeader {
                 ", hScreenSize=" + hScreenSize +
                 ", vScreenSize=" + vScreenSize +
                 ", palette=" + Arrays.toString(palette) +
-                '}';
+                ']';
     }
 
     public static PCXHeader read(final ImageInputStream imageInput) throws IOException {
@@ -142,7 +178,7 @@ final class PCXHeader {
 
         byte magic = imageInput.readByte();
         if (magic != PCX.MAGIC) {
-            throw new IIOException(String.format("Not a PCX image. Expected PCX magic %02x, read %02x", PCX.MAGIC, magic));
+            throw new IIOException(String.format("Not a PCX image. Expected PCX magic 0x%02x: 0x%02x", PCX.MAGIC, magic));
         }
 
         PCXHeader header = new PCXHeader();
@@ -165,10 +201,10 @@ final class PCXHeader {
 
         imageInput.readUnsignedByte(); // Reserved, should be 0
 
-        header.channels = imageInput.readUnsignedByte();
+        header.channels = imageInput.readUnsignedByte();      // Channels or Bit planes
         header.bytesPerLine = imageInput.readUnsignedShort(); // Must be even!
 
-        header.paletteInfo = imageInput.readUnsignedShort(); // 1 == Color/BW, 2 == Gray
+        header.paletteInfo = imageInput.readUnsignedShort() & 0x2; // 1 == Color/BW, 2 == Gray. Ignored
 
         header.hScreenSize = imageInput.readUnsignedShort();
         header.vScreenSize = imageInput.readUnsignedShort();

imageio/imageio-pcx/src/main/java/com/twelvemonkeys/imageio/plugins/pcx/PCXImageReader.java

@@ -54,7 +54,15 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
+/**
+ * ImageReader for ZSoft PC Paintbrush (PCX) format.
+ *
+ * @see <a href="http://www.drdobbs.com/pcx-graphics/184402396">PCX Graphics</a>
+ */
 public final class PCXImageReader extends ImageReaderBase {
+
+    final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.plugins.pcx.debug"));
+
     /** 8 bit ImageTypeSpecifer used for reading bitplane images. */
     private static final ImageTypeSpecifier GRAYSCALE = ImageTypeSpecifiers.createGrayscale(8, DataBuffer.TYPE_BYTE);
 
@@ -93,7 +101,7 @@ public final class PCXImageReader extends ImageReaderBase {
     public Iterator<ImageTypeSpecifier> getImageTypes(final int imageIndex) throws IOException {
         ImageTypeSpecifier rawType = getRawImageType(imageIndex);
 
-        List<ImageTypeSpecifier> specifiers = new ArrayList<ImageTypeSpecifier>();
+        List<ImageTypeSpecifier> specifiers = new ArrayList<>();
 
         // TODO: Implement
         specifiers.add(rawType);
@@ -107,27 +115,30 @@ public final class PCXImageReader extends ImageReaderBase {
         readHeader();
 
         int channels = header.getChannels();
-        int paletteInfo = header.getPaletteInfo();
-        ColorSpace cs = paletteInfo == PCX.PALETTEINFO_GRAY ? ColorSpace.getInstance(ColorSpace.CS_GRAY) : ColorSpace.getInstance(ColorSpace.CS_sRGB);
 
         switch (header.getBitsPerPixel()) {
             case 1:
             case 2:
             case 4:
+                // TODO: If there's a VGA palette here, use it?
+
                 return ImageTypeSpecifiers.createFromIndexColorModel(header.getEGAPalette());
             case 8:
+                if (channels == 1) {
                     // We may have IndexColorModel here for 1 channel images
-                if (channels == 1 && paletteInfo != PCX.PALETTEINFO_GRAY) {
                     IndexColorModel palette = getVGAPalette();
-                    if (palette == null) {
-                        throw new IIOException("Expected VGA palette not found");
-                    }
 
+                    if (palette != null) {
                         return ImageTypeSpecifiers.createFromIndexColorModel(palette);
                     }
+                    else {
+                        // PCX Gray has 1 channel and no palette
+                        return ImageTypeSpecifiers.createGrayscale(8, DataBuffer.TYPE_BYTE);
+                    }
+                }
 
-                // PCX has 1 or 3 channels for 8 bit gray or 24 bit RGB, will be validated by ImageTypeSpecifier
+                // PCX RGB has channels for 24 bit RGB, will be validated by ImageTypeSpecifier
-                return ImageTypeSpecifiers.createBanded(cs, createIndices(channels, 1), createIndices(channels, 0), DataBuffer.TYPE_BYTE, false, false);
+                return ImageTypeSpecifiers.createBanded(ColorSpace.getInstance(ColorSpace.CS_sRGB), createIndices(channels, 1), createIndices(channels, 0), DataBuffer.TYPE_BYTE, channels == 4, false);
             case 24:
                 // Some sources says this is possible...
                 return ImageTypeSpecifiers.createFromBufferedImageType(BufferedImage.TYPE_3BYTE_BGR);
@@ -154,10 +165,6 @@ public final class PCXImageReader extends ImageReaderBase {
         Iterator<ImageTypeSpecifier> imageTypes = getImageTypes(imageIndex);
         ImageTypeSpecifier rawType = getRawImageType(imageIndex);
 
-        if (header.getPaletteInfo() != PCX.PALETTEINFO_COLOR && header.getPaletteInfo() != PCX.PALETTEINFO_GRAY) {
-            processWarningOccurred(String.format("Unsupported color mode: %d, colors may look incorrect", header.getPaletteInfo()));
-        }
-
         int width = getWidth(imageIndex);
         int height = getHeight(imageIndex);
 
@@ -351,7 +358,11 @@ public final class PCXImageReader extends ImageReaderBase {
         if (header == null) {
             imageInput.setByteOrder(ByteOrder.LITTLE_ENDIAN);
             header = PCXHeader.read(imageInput);
-//            System.err.println("header: " + header);
+
+            if (DEBUG) {
+                System.err.println("header: " + header);
+            }
+
             imageInput.flushBefore(imageInput.getStreamPosition());
         }
 
@@ -372,29 +383,27 @@ public final class PCXImageReader extends ImageReaderBase {
             // Mark palette as read, to avoid further attempts
             readPalette = true;
 
-            // Wee can't simply skip to an offset, as the RLE compression makes the file size unpredictable
+            if (header.getVersion() >= PCX.VERSION_3 || header.getVersion() == PCX.VERSION_2_8_PALETTE) {
+                // We can't simply skip to an offset, as the RLE compression makes the file size unpredictable
                 skipToEOF(imageInput);
 
-            // Seek backwards from EOF
+                int paletteSize = 256 * 3; // 256 * 3 for RGB
-            long paletteStart = imageInput.getStreamPosition() - 769;
-            if (paletteStart <= imageInput.getFlushedPosition()) {
-                return null;
-            }
 
+                // Seek backwards from EOF
+                long paletteStart = imageInput.getStreamPosition() - paletteSize - 1;
+                if (paletteStart > imageInput.getFlushedPosition()) {
                     imageInput.seek(paletteStart);
 
                     byte val = imageInput.readByte();
 
                     if (val == PCX.VGA_PALETTE_MAGIC) {
-                byte[] palette = new byte[768]; // 256 * 3 for RGB
+                        byte[] palette = new byte[paletteSize];
                         imageInput.readFully(palette);
 
                         vgaPalette = new IndexColorModel(8, 256, palette, 0, false);
-
-                return vgaPalette;
                     }
-
+                }
-            return null;
+            }
         }
 
         return vgaPalette;
@@ -414,7 +423,7 @@ public final class PCXImageReader extends ImageReaderBase {
             long pos = stream.getStreamPosition();
 
             // ...skip 1k blocks until we're passed EOF...
-            while (stream.skipBytes(1024l) > 0) {
+            while (stream.skipBytes(1024L) > 0) {
                 if (stream.read() == -1) {
                     break;
                 }

imageio/imageio-pcx/src/main/java/com/twelvemonkeys/imageio/plugins/pcx/PCXImageReaderSpi.java

@@ -87,7 +87,7 @@ public final class PCXImageReaderSpi extends ImageReaderSpiBase {
     }
 
     @Override public String getDescription(final Locale locale) {
-        return "PC Paintbrush (PCX) image reader";
+        return "ZSoft PC Paintbrush (PCX) image reader";
     }
 }
 

imageio/imageio-pcx/src/test/java/com/twelvemonkeys/imageio/plugins/pcx/PCXImageReaderTest.java

@@ -31,13 +31,20 @@ package com.twelvemonkeys.imageio.plugins.pcx;
 import com.twelvemonkeys.imageio.util.ImageReaderAbstractTest;
 import org.junit.Test;
 
+import javax.imageio.ImageIO;
+import javax.imageio.ImageReadParam;
 import javax.imageio.spi.ImageReaderSpi;
+import javax.imageio.stream.ImageInputStream;
 import java.awt.*;
+import java.awt.image.BufferedImage;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
 /**
  * PCXImageReaderTest
  *
@@ -50,7 +57,6 @@ public class PCXImageReaderTest extends ImageReaderAbstractTest<PCXImageReader>
     protected List<TestData> getTestData() {
         return Arrays.asList(
                 new TestData(getClassLoaderResource("/pcx/MARBLES.PCX"), new Dimension(1419, 1001)), // RLE encoded RGB
-//                new TestData(getClassLoaderResource("/pcx/GMARBLES.PCX"), new Dimension(1419, 1001)) // RLE encoded gray (seems to be damaged, missing the last few scan lines)
                 new TestData(getClassLoaderResource("/pcx/lena.pcx"), new Dimension(512, 512)), // RLE encoded RGB
                 new TestData(getClassLoaderResource("/pcx/lena2.pcx"), new Dimension(512, 512)), // RLE encoded, 256 color indexed (8 bps/1 channel)
                 new TestData(getClassLoaderResource("/pcx/lena3.pcx"), new Dimension(512, 512)), // RLE encoded, 16 color indexed (4 bps/1 channel)
@@ -62,7 +68,7 @@ public class PCXImageReaderTest extends ImageReaderAbstractTest<PCXImageReader>
                 new TestData(getClassLoaderResource("/pcx/lena9.pcx"), new Dimension(512, 512)), // RLE encoded, 2 color indexed (1 bps/1 channel)
                 new TestData(getClassLoaderResource("/pcx/lena10.pcx"), new Dimension(512, 512)), // RLE encoded, 16 color indexed (4 bps/1 channel) (uses only 8 colors)
                 new TestData(getClassLoaderResource("/pcx/DARKSTAR.PCX"), new Dimension(88, 52)), // RLE encoded monochrome (1 bps/1 channel)
-                // TODO: Get correct colors for CGA mode, see cga-pcx.txt (however, the text seems to be in error, the bits are not as described)
+                // See cga-pcx.txt, however, the text seems to be in error, the bits can not not as described
                 new TestData(getClassLoaderResource("/pcx/CGA_BW.PCX"), new Dimension(640, 200)), // RLE encoded indexed (CGA mode)
                 new TestData(getClassLoaderResource("/pcx/CGA_FSD.PCX"), new Dimension(320, 200)), // RLE encoded indexed (CGA mode)
                 new TestData(getClassLoaderResource("/pcx/CGA_RGBI.PCX"), new Dimension(320, 200)), // RLE encoded indexed (CGA mode)
@@ -102,6 +108,29 @@ public class PCXImageReaderTest extends ImageReaderAbstractTest<PCXImageReader>
         );
     }
 
+    @Test
+    public void testReadGray() throws IOException {
+        // Seems like the last scan lines have been overwritten by an unnecessary 768 byte palette + 1 byte magic...
+        try (ImageInputStream input = ImageIO.createImageInputStream(getClassLoaderResource("/pcx/GMARBLES.PCX"))) {
+            PCXImageReader reader = createReader();
+            reader.setInput(input);
+
+            assertEquals(1, reader.getNumImages(true));
+            assertEquals(1419, reader.getWidth(0));
+            assertEquals(1001, reader.getHeight(0));
+
+            ImageReadParam param = reader.getDefaultReadParam();
+            param.setSourceRegion(new Rectangle(1419, 1000)); // Ignore the last garbled line
+
+            BufferedImage image = reader.read(0, param);
+
+            assertNotNull(image);
+            assertEquals(BufferedImage.TYPE_BYTE_INDEXED, image.getType());
+            assertEquals(1419, image.getWidth());
+            assertEquals(1000, image.getHeight());
+        }
+    }
+
     @Test
     public void testReadWithSourceRegionParamEqualImage() throws IOException {
         TestData data = getTestData().get(1);