From d1cb941f06abdbf9f1c725649bd6bb5eb444fb5c Mon Sep 17 00:00:00 2001 From: Harald Kuhr Date: Sat, 11 Jul 2015 13:07:20 +0200 Subject: [PATCH] TMI-150: Fix for 24 bit images with bitmask. --- .../imageio/plugins/bmp/BitmapDescriptor.java | 14 +++++ .../imageio/plugins/bmp/BitmapIndexed.java | 59 ++++++++---------- .../imageio/plugins/bmp/BitmapMask.java | 8 +-- .../imageio/plugins/bmp/BitmapRGB.java | 34 ++++++++++ .../imageio/plugins/bmp/DIBImageReader.java | 51 ++++++++++----- .../plugins/bmp/ICOImageReaderTest.java | 4 +- .../src/test/resources/ico/rgb24bitmask.ico | Bin 0 -> 3262 bytes 7 files changed, 117 insertions(+), 53 deletions(-) create mode 100644 imageio/imageio-bmp/src/test/resources/ico/rgb24bitmask.ico diff --git a/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapDescriptor.java b/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapDescriptor.java index 3587d662..a5a00f6a 100755 --- a/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapDescriptor.java +++ b/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapDescriptor.java @@ -43,6 +43,7 @@ abstract class BitmapDescriptor { protected final DIBHeader header; protected BufferedImage image; + protected BitmapMask mask; public BitmapDescriptor(final DirectoryEntry pEntry, final DIBHeader pHeader) { Validate.notNull(pEntry, "entry"); @@ -69,4 +70,17 @@ abstract class BitmapDescriptor { protected final int getBitCount() { return entry.getBitCount() != 0 ? entry.getBitCount() : header.getBitCount(); } + + @Override + public String toString() { + return getClass().getSimpleName() + "[" + entry + ", " + header + "]"; + } + + public final void setMask(final BitmapMask mask) { + this.mask = mask; + } + + public final boolean hasMask() { + return header.getHeight() == getHeight() * 2; + } } diff --git a/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapIndexed.java b/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapIndexed.java index c5d33293..cb2c413a 100755 --- a/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapIndexed.java +++ b/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapIndexed.java @@ -28,8 +28,6 @@ package com.twelvemonkeys.imageio.plugins.bmp; -import com.twelvemonkeys.image.InverseColorMapIndexColorModel; - import java.awt.image.BufferedImage; import java.awt.image.DataBuffer; import java.awt.image.IndexColorModel; @@ -46,8 +44,6 @@ class BitmapIndexed extends BitmapDescriptor { protected final int[] bits; protected final int[] colors; - private BitmapMask mask; - public BitmapIndexed(final DirectoryEntry pEntry, final DIBHeader pHeader) { super(pEntry, pHeader); bits = new int[getWidth() * getHeight()]; @@ -65,7 +61,7 @@ class BitmapIndexed extends BitmapDescriptor { // This is slightly obscure, and should probably be moved.. Hashtable properties = null; if (entry instanceof DirectoryEntry.CUREntry) { - properties = new Hashtable(1); + properties = new Hashtable<>(1); properties.put("cursor_hotspot", ((DirectoryEntry.CUREntry) this.entry).getHotspot()); } @@ -89,8 +85,6 @@ class BitmapIndexed extends BitmapDescriptor { raster.setSamples(0, 0, getWidth(), getHeight(), 0, bits); - //System.out.println("Image: " + image); - return image; } @@ -100,40 +94,40 @@ class BitmapIndexed extends BitmapDescriptor { IndexColorModel createColorModel() { // NOTE: This is a hack to make room for transparent pixel for mask int bits = getBitCount(); - + int colors = this.colors.length; - int trans = -1; + int transparent = -1; // Try to avoid USHORT transfertype, as it results in BufferedImage TYPE_CUSTOM // NOTE: This code assumes icons are small, and is NOT optimized for performance... if (colors > (1 << getBitCount())) { - int index = findTransIndexMaybeRemap(this.colors, this.bits); + int index = findTransparentIndexMaybeRemap(this.colors, this.bits); if (index == -1) { // No duplicate found, increase bitcount bits++; - trans = this.colors.length - 1; + transparent = this.colors.length - 1; } else { - // Found a duplicate, use it as trans - trans = index; + // Found a duplicate, use it as transparent + transparent = index; colors--; } } // NOTE: Setting hasAlpha to true, makes things work on 1.2 - return new InverseColorMapIndexColorModel( - bits, colors, this.colors, 0, true, trans, + return new IndexColorModel( + bits, colors, this.colors, 0, true, transparent, bits <= 8 ? DataBuffer.TYPE_BYTE : DataBuffer.TYPE_USHORT ); } - private static int findTransIndexMaybeRemap(final int[] pColors, final int[] pBits) { + private static int findTransparentIndexMaybeRemap(final int[] colors, final int[] bits) { // Look for unused colors, to use as transparent - final boolean[] used = new boolean[pColors.length - 1]; - for (int pBit : pBits) { - if (!used[pBit]) { - used[pBit] = true; + boolean[] used = new boolean[colors.length - 1]; + for (int bit : bits) { + if (!used[bit]) { + used[bit] = true; } } @@ -144,38 +138,35 @@ class BitmapIndexed extends BitmapDescriptor { } // Try to find duplicates in colormap, and remap - int trans = -1; + int transparent = -1; int duplicate = -1; - for (int i = 0; trans == -1 && i < pColors.length - 1; i++) { - for (int j = i + 1; j < pColors.length - 1; j++) { - if (pColors[i] == pColors[j]) { - trans = j; + for (int i = 0; transparent == -1 && i < colors.length - 1; i++) { + for (int j = i + 1; j < colors.length - 1; j++) { + if (colors[i] == colors[j]) { + transparent = j; duplicate = i; break; } } } - if (trans != -1) { + if (transparent != -1) { // Remap duplicate - for (int i = 0; i < pBits.length; i++) { - if (pBits[i] == trans) { - pBits[i] = duplicate; + for (int i = 0; i < bits.length; i++) { + if (bits[i] == transparent) { + bits[i] = duplicate; } } } - return trans; + return transparent; } public BufferedImage getImage() { if (image == null) { image = createImageIndexed(); } + return image; } - - public void setMask(final BitmapMask pMask) { - mask = pMask; - } } diff --git a/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapMask.java b/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapMask.java index c4831680..5d619cf6 100755 --- a/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapMask.java +++ b/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapMask.java @@ -38,19 +38,19 @@ import java.awt.image.BufferedImage; * @version $Id: BitmapMask.java,v 1.0 25.feb.2006 00:29:44 haku Exp$ */ class BitmapMask extends BitmapDescriptor { - protected final BitmapIndexed mask; + protected final BitmapIndexed bitMask; public BitmapMask(final DirectoryEntry pParent, final DIBHeader pHeader) { super(pParent, pHeader); - mask = new BitmapIndexed(pParent, pHeader); + bitMask = new BitmapIndexed(pParent, pHeader); } boolean isTransparent(final int pX, final int pY) { // NOTE: 1: Fully transparent, 0: Opaque... - return mask.bits[pX + pY * getWidth()] != 0; + return bitMask.bits[pX + pY * getWidth()] != 0; } public BufferedImage getImage() { - return mask.getImage(); + return bitMask.getImage(); } } diff --git a/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapRGB.java b/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapRGB.java index 11dc17d4..05f43600 100755 --- a/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapRGB.java +++ b/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/BitmapRGB.java @@ -28,7 +28,9 @@ package com.twelvemonkeys.imageio.plugins.bmp; +import java.awt.*; import java.awt.image.BufferedImage; +import java.awt.image.WritableRaster; /** * Describes an RGB/true color bitmap structure (16, 24 and 32 bits per pixel). @@ -43,6 +45,38 @@ class BitmapRGB extends BitmapDescriptor { } public BufferedImage getImage() { + // Test is mask != null rather than hasMask(), as 32 bit (w/alpha) + // might still have bitmask, but we don't read or use it. + if (mask != null) { + image = createMaskedImage(); + mask = null; + } + return image; } + + private BufferedImage createMaskedImage() { + BufferedImage masked = new BufferedImage(getWidth(), getHeight(), BufferedImage.TYPE_4BYTE_ABGR); + + Graphics2D graphics = masked.createGraphics(); + try { + graphics.drawImage(image, 0, 0, null); + } + finally { + graphics.dispose(); + } + + WritableRaster alphaRaster = masked.getAlphaRaster(); + + byte[] trans = {0x0}; + for (int y = 0; y < getHeight(); y++) { + for (int x = 0; x < getWidth(); x++) { + if (mask.isTransparent(x, y)) { + alphaRaster.setDataElements(x, y, trans); + } + } + } + + return masked; + } } diff --git a/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/DIBImageReader.java b/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/DIBImageReader.java index bd8087ff..eb5207b1 100644 --- a/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/DIBImageReader.java +++ b/imageio/imageio-bmp/src/main/java/com/twelvemonkeys/imageio/plugins/bmp/DIBImageReader.java @@ -66,14 +66,15 @@ import java.util.List; // TODO: Decide whether DirectoryEntry or DIBHeader should be primary source for color count/bit count // TODO: Support loading icons from DLLs, see // MSDN -// Known issue: 256x256 PNG encoded icons does not have IndexColorModel even if stated in DirectoryEntry (seem impossible as the PNGs are all true color) +// Known issue: 256x256 PNG encoded icons does not have IndexColorModel even if stated in DirectoryEntry +// (seem impossible as the PNGs are all true color) abstract class DIBImageReader extends ImageReaderBase { // TODO: Consider moving the reading to inner classes (subclasses of BitmapDescriptor) private Directory directory; // TODO: Review these, make sure we don't have a memory leak - private Map headers = new WeakHashMap(); - private Map descriptors = new WeakWeakMap(); + private Map headers = new WeakHashMap<>(); + private Map descriptors = new WeakWeakMap<>(); private ImageReader pngImageReader; @@ -101,7 +102,7 @@ abstract class DIBImageReader extends ImageReaderBase { return getImageTypesPNG(entry); } - List types = new ArrayList(); + List types = new ArrayList<>(); DIBHeader header = getHeader(entry); // Use data from header to create specifier @@ -121,10 +122,13 @@ abstract class DIBImageReader extends ImageReaderBase { specifier = ImageTypeSpecifiers.createFromIndexColorModel(indexed.createColorModel()); break; case 16: + // TODO: May have mask?! specifier = ImageTypeSpecifiers.createFromBufferedImageType(BufferedImage.TYPE_USHORT_555_RGB); break; case 24: - specifier = ImageTypeSpecifiers.createFromBufferedImageType(BufferedImage.TYPE_3BYTE_BGR); + specifier = new BitmapRGB(entry, header).hasMask() + ? ImageTypeSpecifiers.createFromBufferedImageType(BufferedImage.TYPE_4BYTE_ABGR) + : ImageTypeSpecifiers.createFromBufferedImageType(BufferedImage.TYPE_3BYTE_BGR); break; case 32: specifier = ImageTypeSpecifiers.createFromBufferedImageType(BufferedImage.TYPE_INT_ARGB); @@ -184,6 +188,7 @@ abstract class DIBImageReader extends ImageReaderBase { } else { Graphics2D g = destination.createGraphics(); + try { g.setComposite(AlphaComposite.Src); g.drawImage(image, 0, 0, null); @@ -316,6 +321,8 @@ abstract class DIBImageReader extends ImageReaderBase { descriptors.put(pEntry, descriptor); } + System.err.println("descriptor: " + descriptor); + return descriptor.getImage(); } @@ -335,7 +342,7 @@ abstract class DIBImageReader extends ImageReaderBase { } BitmapMask mask = new BitmapMask(pBitmap.entry, pBitmap.header); - readBitmapIndexed1(mask.mask, true); + readBitmapIndexed1(mask.bitMask, true); pBitmap.setMask(mask); } @@ -370,7 +377,7 @@ abstract class DIBImageReader extends ImageReaderBase { } } - // NOTE: If we are reading the mask, we don't abort or progress + // NOTE: If we are reading the mask, we don't abort or report progress if (!pAsMask) { if (abortRequested()) { processReadAborted(); @@ -455,7 +462,7 @@ abstract class DIBImageReader extends ImageReaderBase { short[] pixels = new short[pBitmap.getWidth() * pBitmap.getHeight()]; // TODO: Support TYPE_USHORT_565 and the RGB 444/ARGB 4444 layouts - // Will create TYPE_USHORT_555; + // Will create TYPE_USHORT_555 DirectColorModel cm = new DirectColorModel(16, 0x7C00, 0x03E0, 0x001F); DataBuffer buffer = new DataBufferShort(pixels, pixels.length); WritableRaster raster = Raster.createPackedRaster( @@ -480,6 +487,8 @@ abstract class DIBImageReader extends ImageReaderBase { processImageProgress(100 * y / (float) pBitmap.getHeight()); } + + // TODO: Might be mask!? } private void readBitmap24(final BitmapDescriptor pBitmap) throws IOException { @@ -494,16 +503,19 @@ abstract class DIBImageReader extends ImageReaderBase { cs, nBits, false, false, Transparency.OPAQUE, DataBuffer.TYPE_BYTE ); + int scanlineStride = pBitmap.getWidth() * 3; + // BMP rows are padded to 4 byte boundary + int rowSizeBytes = ((8 * scanlineStride + 31) / 32) * 4; + WritableRaster raster = Raster.createInterleavedRaster( - buffer, pBitmap.getWidth(), pBitmap.getHeight(), pBitmap.getWidth(), 3, bOffs, null + buffer, pBitmap.getWidth(), pBitmap.getHeight(), scanlineStride, 3, bOffs, null ); pBitmap.image = new BufferedImage(cm, raster, cm.isAlphaPremultiplied(), null); - - for (int y = 0; y < pBitmap.getHeight(); y++) { - int offset = (pBitmap.getHeight() - y - 1) * pBitmap.getWidth(); - imageInput.readFully(pixels, offset, pBitmap.getWidth() * 3); - // TODO: Possibly read padding byte here! + for (int y = 0; y < pBitmap.getHeight(); y++) { + int offset = (pBitmap.getHeight() - y - 1) * scanlineStride; + imageInput.readFully(pixels, offset, scanlineStride); + imageInput.skipBytes(rowSizeBytes - scanlineStride); if (abortRequested()) { processReadAborted(); @@ -512,6 +524,14 @@ abstract class DIBImageReader extends ImageReaderBase { processImageProgress(100 * y / (float) pBitmap.getHeight()); } + + // 24 bit icons usually have a bit mask + if (pBitmap.hasMask()) { + BitmapMask mask = new BitmapMask(pBitmap.entry, pBitmap.header); + readBitmapIndexed1(mask.bitMask, true); + + pBitmap.setMask(mask); + } } private void readBitmap32(final BitmapDescriptor pBitmap) throws IOException { @@ -535,6 +555,9 @@ abstract class DIBImageReader extends ImageReaderBase { } processImageProgress(100 * y / (float) pBitmap.getHeight()); } + + // There might be a mask here as well, but we'll ignore it, + // and use the 8 bit alpha channel in the ARGB pixel data } private Directory getDirectory() throws IOException { diff --git a/imageio/imageio-bmp/src/test/java/com/twelvemonkeys/imageio/plugins/bmp/ICOImageReaderTest.java b/imageio/imageio-bmp/src/test/java/com/twelvemonkeys/imageio/plugins/bmp/ICOImageReaderTest.java index b169abe2..ed5a2adc 100755 --- a/imageio/imageio-bmp/src/test/java/com/twelvemonkeys/imageio/plugins/bmp/ICOImageReaderTest.java +++ b/imageio/imageio-bmp/src/test/java/com/twelvemonkeys/imageio/plugins/bmp/ICOImageReaderTest.java @@ -39,7 +39,9 @@ public class ICOImageReaderTest extends ImageReaderAbstractTest new Dimension(16, 16), new Dimension(16, 16), new Dimension(32, 32), new Dimension(32, 32), new Dimension(48, 48), new Dimension(48, 48), new Dimension(256, 256), new Dimension(256, 256), new Dimension(16, 16), new Dimension(32, 32), new Dimension(48, 48), new Dimension(256, 256) - ) + ), + // Problematic icon that reports 24 bit in the descriptor, but has separate 1 bit ''mask (height 2 x icon height)! + new TestData(getClassLoaderResource("/ico/rgb24bitmask.ico"), new Dimension(32, 32)) ); } diff --git a/imageio/imageio-bmp/src/test/resources/ico/rgb24bitmask.ico b/imageio/imageio-bmp/src/test/resources/ico/rgb24bitmask.ico new file mode 100644 index 0000000000000000000000000000000000000000..a2bcda1c9b211d5f7794aa74fa2ed18e547f2fae GIT binary patch literal 3262 zcmbuCYgiLk8po$kAHhUes1~VWMNkw7nOsO9K!K2iglw{G{m^^}H<)q7?N5AVea)5&=~!Yg|F^+jGW`R@sORhdLljTI%4@~@)j49vPAo=Weq zxy&!|(5tl5j)m<$Fe@;XD(gc#Cz;iQ%$gx~;~2k7zhHB*XY_|!#1*z$1;lxk{=z*! z$*LQMn#MWJTBJ$GY0(S1^iGjgv}pOXpZJf0q{97}mAPIOBdim8a={>_Qb(;A_beXt zDjT7ekHRM=0y;(H>`95IY(y| zmf}9A>M^HkfTOs_seFLc4f$lLe5>#KRzKv{4WRXpcs2KUbq~?{hdfh1KKSNUL`t&f zY$}J98o@QAm!z9k@S};(KKj&0dc`zhl-;41^s{OQ*fo#nCD$1xeNfFkhP;nme2sqi z2BZ8s^U%Kpy#}F5ZND}$C{6mOjoO%;XTK(szA1{UdP3hP=bayi%lhD~VkoB?mY+s) z>Y=1Gq_i24onpp)3(G2yvWrmsUQR|XS8<7*l+Ve?@vTz92aYf|Zz5urJFm%bUcTj} z1#4T)f(;qG_PZc92=9E+4HN7Wrlx-*cD0a#bArIxW#DWS#I(6tHsMSY>n0D&P7hlM zv@QWH+Hu&G1lw;2mYuknkp$~-!lDR*wRza7$?!Vdyit6S&LF=CGSr+2xEGg$JU11>)Ut+$gIT2^)l-s&>J zA(puK7QyKT$*GsTEDprphcb84g3|F8BaDc6Xjjy-8U6gTY9u!mwCE+fS8~d>673tQ zF2CU{FR^7e1AoxEgvK(m&zhFNi#|60tDRnw!_?3A5kVchO6q3O4ccvSx9E4b5#wh5 zM083dI%z=LdeAM2=%^#i{hsK#1GIR-sThFw?!(PEN3ghxw^>cFsl{9N;AY%`vwr=2 z{Xf8%`j@XDzO>VEN1Q7r-gD_=?$vQh!nkMA7&Y!cwEaW091T6{C^!qGtyXvi?;$#| zC@$-$3@>Vsgxjj&w?Ah__Rzy0Lf`(vO1J>+9AHNF+ruGW)%L!GPfbD-E9^xPJ{3PL zSE)BPY}-`1A*ii6q&6qCyUjbj3~z-Jag8JoSK<;W#Z8L0%OkpG_@8~wYk4lbG{Wy0 z^zRx8{6QahZG6d&Bu3_mKf$MNH|+g-UT`j|=-w({`E{1hTIp7CHZ8JZrO%!O53451 zLLJ_XN_C!~x@f$eb)fYlw6%}lI?QYQ9c>s8v}*%Sk2nS=(BrFL37-nCON+4QC(>Kg zNRI(hsv+eBa>K|<%LZpjJYBTpFT|t;LuA(ql4A>L;Vtf&yGYA(ZsYI#P6O}M_+0-O zK~dWq;8Us4RPjPlGN)L5B4&|PvSG>fQuuMr^I!w0zjYw4rQQm0{h8?x0^zGa7SN9cB>*xCI<{TUL>(w(#Z!!ySqu;5SvY#0R{Vdr{ zr1CNM@K!@u~uzt+BEWX+yfhZWJB<}qHk0XhAGqZ~uej=>$ng6|Fdb1!DWQ9*L$ zJEC4E3p-dp$9IeG2{n2~&pGyl-SEV>U5_?B`5UxNRMhags%gd1UGhbLk3rit+*4Za zSp&QNku9=QRNOc%$eYF7*cj*KiTn#%xbfLsG=`gB`)2)nWGNvb9-H$Vg0eW$5B+R= zox9uGa6jqvzNUSkJ~sdW literal 0 HcmV?d00001