#386 Fixed ColorSpaces ICC profile caching bug pt II

This commit is contained in:
Harald Kuhr 2017-11-18 14:20:40 +01:00
parent d677141ab7
commit b68848f58f
3 changed files with 71 additions and 43 deletions

View File

@ -72,6 +72,10 @@ import java.util.Properties;
* @version $Id: ColorSpaces.java,v 1.0 24.01.11 17.51 haraldk Exp$ * @version $Id: ColorSpaces.java,v 1.0 24.01.11 17.51 haraldk Exp$
*/ */
public final class ColorSpaces { public final class ColorSpaces {
// TODO: Consider creating our own ICC profile class, which just wraps the byte array,
// for easier access and manipulation until creating a "real" ICC_Profile/ColorSpace.
// This will also let us work around the issues in the LCMS implementation.
final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.color.debug")); final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.color.debug"));
/** We need special ICC profile handling for KCMS vs LCMS. Delegate to specific strategy. */ /** We need special ICC profile handling for KCMS vs LCMS. Delegate to specific strategy. */
@ -80,9 +84,11 @@ public final class ColorSpaces {
// NOTE: java.awt.color.ColorSpace.CS_* uses 1000-1004, we'll use 5000+ to not interfere with future additions // NOTE: java.awt.color.ColorSpace.CS_* uses 1000-1004, we'll use 5000+ to not interfere with future additions
/** The Adobe RGB 1998 (or compatible) color space. Either read from disk or built-in. */ /** The Adobe RGB 1998 (or compatible) color space. Either read from disk or built-in. */
@SuppressWarnings("WeakerAccess")
public static final int CS_ADOBE_RGB_1998 = 5000; public static final int CS_ADOBE_RGB_1998 = 5000;
/** A best-effort "generic" CMYK color space. Either read from disk or built-in. */ /** A best-effort "generic" CMYK color space. Either read from disk or built-in. */
@SuppressWarnings("WeakerAccess")
public static final int CS_GENERIC_CMYK = 5001; public static final int CS_GENERIC_CMYK = 5001;
// Weak references to hold the color spaces while cached // Weak references to hold the color spaces while cached
@ -122,37 +128,34 @@ public final class ColorSpaces {
} }
private static byte[] getProfileHeaderWithProfileId(final ICC_Profile profile) { private static byte[] getProfileHeaderWithProfileId(final ICC_Profile profile) {
byte[] header = profile.getData(ICC_Profile.icSigHead);
computeProfileIdMD5(profile, header);
return header;
}
private static void computeProfileIdMD5(final ICC_Profile profile, final byte[] header) {
// Clear out preferred CMM, platform & creator, as these does not affect the profile in any way
// - LCMS updates CMM + creator to "lcms" and platform to current platform
// - KCMS keeps the values in the file...
Arrays.fill(header, ICC_Profile.icHdrCmmId, ICC_Profile.icHdrCmmId + 4, (byte) 0);
Arrays.fill(header, ICC_Profile.icHdrPlatform, ICC_Profile.icHdrPlatform + 4, (byte) 0);
// + Clear out rendering intent, as this may be updated by application
Arrays.fill(header, ICC_Profile.icHdrRenderingIntent, ICC_Profile.icHdrRenderingIntent + 4, (byte) 0);
Arrays.fill(header, ICC_Profile.icHdrCreator, ICC_Profile.icHdrCreator + 4, (byte) 0);
// Clear out any existing MD5, as it is no longer correct
Arrays.fill(header, ICC_Profile.icHdrProfileID, ICC_Profile.icHdrProfileID + 16, (byte) 0);
// Get *entire profile data*... :-/ // Get *entire profile data*... :-/
byte[] data = profile.getData(); byte[] data = profile.getData();
// Update with the new header data // Clear out preferred CMM, platform & creator, as these does not affect the profile in any way
System.arraycopy(header, 0, data, 0, header.length); // - LCMS updates CMM + creator to "lcms" and platform to current platform
// - KCMS keeps the values in the file...
Arrays.fill(data, ICC_Profile.icHdrCmmId, ICC_Profile.icHdrCmmId + 4, (byte) 0);
Arrays.fill(data, ICC_Profile.icHdrPlatform, ICC_Profile.icHdrPlatform + 4, (byte) 0);
// + Clear out rendering intent, as this may be updated by application
Arrays.fill(data, ICC_Profile.icHdrRenderingIntent, ICC_Profile.icHdrRenderingIntent + 4, (byte) 0);
Arrays.fill(data, ICC_Profile.icHdrCreator, ICC_Profile.icHdrCreator + 4, (byte) 0);
// Clear out any existing MD5, as it is no longer correct
Arrays.fill(data, ICC_Profile.icHdrProfileID, ICC_Profile.icHdrProfileID + 16, (byte) 0);
// Generate new MD5 and store in header // Generate new MD5 and store in header
byte[] md5 = computeMD5(data);
System.arraycopy(md5, 0, data, ICC_Profile.icHdrProfileID, md5.length);
// ICC profile header is the first 128 bytes
return Arrays.copyOf(data, 128);
}
private static byte[] computeMD5(byte[] data) {
try { try {
byte[] md5 = MessageDigest.getInstance("MD5").digest(data); return MessageDigest.getInstance("MD5").digest(data);
System.arraycopy(md5, 0, header, ICC_Profile.icHdrProfileID, md5.length); }
} catch (NoSuchAlgorithmException e) { catch (NoSuchAlgorithmException e) {
throw new IllegalStateException("Missing MD5 MessageDigest"); throw new IllegalStateException("Missing MD5 MessageDigest");
} }
} }
@ -186,9 +189,7 @@ public final class ColorSpaces {
if (cs == null) { if (cs == null) {
cs = new ICC_ColorSpace(profile); cs = new ICC_ColorSpace(profile);
// Validate the color space, to avoid caching bad color spaces validateColorSpace(cs);
// Will throw IllegalArgumentException or CMMException if the profile is bad
cs.fromRGB(new float[] {1f, 0f, 0f});
cache.put(key, cs); cache.put(key, cs);
} }
@ -197,6 +198,12 @@ public final class ColorSpaces {
} }
} }
private static void validateColorSpace(ICC_ColorSpace cs) {
// Validate the color space, to avoid caching bad color spaces
// Will throw IllegalArgumentException or CMMException if the profile is bad
cs.fromRGB(new float[] {1f, 0f, 0f});
}
/** /**
* Tests whether an ICC color profile is equal to the default sRGB profile. * Tests whether an ICC color profile is equal to the default sRGB profile.
* *
@ -240,7 +247,7 @@ public final class ColorSpaces {
byte[] header = profile.getData(ICC_Profile.icSigHead); byte[] header = profile.getData(ICC_Profile.icSigHead);
return header[ICC_Profile.icHdrRenderingIntent] != 0 || header[ICC_Profile.icHdrRenderingIntent + 1] != 0 return header[ICC_Profile.icHdrRenderingIntent] != 0 || header[ICC_Profile.icHdrRenderingIntent + 1] != 0
|| header[ICC_Profile.icHdrRenderingIntent + 2] != 0 || header[ICC_Profile.icHdrRenderingIntent + 3] != 0; || header[ICC_Profile.icHdrRenderingIntent + 2] != 0 || header[ICC_Profile.icHdrRenderingIntent + 3] > 3;
} }
/** /**
@ -258,7 +265,9 @@ public final class ColorSpaces {
* @throws java.awt.color.CMMException if {@code profile} is invalid. * @throws java.awt.color.CMMException if {@code profile} is invalid.
*/ */
public static ICC_Profile validateProfile(final ICC_Profile profile) { public static ICC_Profile validateProfile(final ICC_Profile profile) {
createColorSpace(profile); // Creating a color space will fail if the profile is bad // Fix profile before validation
profileCleaner.fixProfile(profile);
validateColorSpace(new ICC_ColorSpace(profile));
return profile; return profile;
} }
@ -342,6 +351,7 @@ public final class ColorSpaces {
} }
} }
@SuppressWarnings("SameParameterValue")
private static ICC_Profile readProfileFromClasspathResource(final String profilePath) { private static ICC_Profile readProfileFromClasspathResource(final String profilePath) {
InputStream stream = ColorSpaces.class.getResourceAsStream(profilePath); InputStream stream = ColorSpaces.class.getResourceAsStream(profilePath);
@ -401,6 +411,11 @@ public final class ColorSpaces {
public int hashCode() { public int hashCode() {
return Arrays.hashCode(data); return Arrays.hashCode(data);
} }
@Override
public String toString() {
return getClass().getSimpleName() + "@" + Integer.toHexString(hashCode());
}
} }
// Cache header profile data to avoid excessive array creation/copying in static inner class for on-demand lazy init // Cache header profile data to avoid excessive array creation/copying in static inner class for on-demand lazy init

View File

@ -3,7 +3,6 @@ package com.twelvemonkeys.imageio.color;
import com.twelvemonkeys.lang.Validate; import com.twelvemonkeys.lang.Validate;
import java.awt.color.ICC_Profile; import java.awt.color.ICC_Profile;
import java.util.Arrays;
/** /**
* KCMSProfileCleaner. * KCMSProfileCleaner.
@ -15,7 +14,7 @@ import java.util.Arrays;
final class KCMSSanitizerStrategy implements ICCProfileSanitizer { final class KCMSSanitizerStrategy implements ICCProfileSanitizer {
/** Value used instead of 'XYZ ' in problematic Corbis RGB Profiles */ /** Value used instead of 'XYZ ' in problematic Corbis RGB Profiles */
private static final byte[] CORBIS_RGB_ALTERNATE_XYZ = new byte[] {0x17, (byte) 0xA5, 0x05, (byte) 0xB8}; private static final int CORBIS_RGB_ALTERNATE_XYZ = 0x17 << 24 | 0xA5 << 16 | 0x05 << 8 | 0xB8;
public void fixProfile(final ICC_Profile profile) { public void fixProfile(final ICC_Profile profile) {
Validate.notNull(profile, "profile"); Validate.notNull(profile, "profile");
@ -23,9 +22,8 @@ final class KCMSSanitizerStrategy implements ICCProfileSanitizer {
// Special case for color profiles with rendering intent != 0, see ColorSpaces.isOffendingColorProfile method // Special case for color profiles with rendering intent != 0, see ColorSpaces.isOffendingColorProfile method
// NOTE: Rendering intent is a 4 byte value, legal values are 0-3 (ICC1v42_2006_05_1.pdf, 7.2.15, p. 19) // NOTE: Rendering intent is a 4 byte value, legal values are 0-3 (ICC1v42_2006_05_1.pdf, 7.2.15, p. 19)
byte[] header = profile.getData(ICC_Profile.icSigHead); byte[] header = profile.getData(ICC_Profile.icSigHead);
if (header[ICC_Profile.icHdrRenderingIntent] != 0 || header[ICC_Profile.icHdrRenderingIntent + 1] != 0 if (intFromBigEndian(header, ICC_Profile.icHdrRenderingIntent) != ICC_Profile.icPerceptual) {
|| header[ICC_Profile.icHdrRenderingIntent + 2] != 0 || header[ICC_Profile.icHdrRenderingIntent + 3] != 0) { intToBigEndian(ICC_Profile.icPerceptual, header, ICC_Profile.icHdrRenderingIntent);
Arrays.fill(header, ICC_Profile.icHdrRenderingIntent, ICC_Profile.icHdrRenderingIntent + 4, (byte) 0);
profile.setData(ICC_Profile.icSigHead, header); profile.setData(ICC_Profile.icSigHead, header);
} }
@ -49,12 +47,8 @@ final class KCMSSanitizerStrategy implements ICCProfileSanitizer {
byte[] data = profile.getData(tagSignature); byte[] data = profile.getData(tagSignature);
// The CMM expects 0x64 65 73 63 ('XYZ ') but is 0x17 A5 05 B8..? // The CMM expects 0x64 65 73 63 ('XYZ ') but is 0x17 A5 05 B8..?
if (data != null && Arrays.equals(Arrays.copyOfRange(data, 0, 4), CORBIS_RGB_ALTERNATE_XYZ)) { if (data != null && intFromBigEndian(data, 0) == CORBIS_RGB_ALTERNATE_XYZ) {
data[0] = 'X'; intToBigEndian(ICC_Profile.icSigXYZData, data, 0);
data[1] = 'Y';
data[2] = 'Z';
data[3] = ' ';
profile.setData(tagSignature, data); profile.setData(tagSignature, data);
return true; return true;
@ -63,4 +57,20 @@ final class KCMSSanitizerStrategy implements ICCProfileSanitizer {
return false; return false;
} }
// TODO: Move to some common util
static int intFromBigEndian(final byte[] array, final int index) {
return ((array[index ] & 0xff) << 24) |
((array[index + 1] & 0xff) << 16) |
((array[index + 2] & 0xff) << 8) |
((array[index + 3] & 0xff) );
}
// TODO: Move to some common util
static void intToBigEndian(final int value, final byte[] array, final int index) {
array[index ] = (byte) (value >> 24);
array[index + 1] = (byte) (value >> 16);
array[index + 2] = (byte) (value >> 8);
array[index + 3] = (byte) (value );
}
} }

View File

@ -83,7 +83,10 @@ public class ColorSpacesTest {
private ICC_Profile createBrokenProfile(ICC_Profile internal) { private ICC_Profile createBrokenProfile(ICC_Profile internal) {
byte[] data = internal.getData(); byte[] data = internal.getData();
data[ICC_Profile.icHdrRenderingIntent + 3] = 1; // Intent: 1 == Relative Colormetric data[ICC_Profile.icHdrRenderingIntent] = 1; // Intent: 1 == Relative Colormetric Little Endian
data[ICC_Profile.icHdrRenderingIntent + 1] = 0;
data[ICC_Profile.icHdrRenderingIntent + 2] = 0;
data[ICC_Profile.icHdrRenderingIntent + 3] = 0;
return ICC_Profile.getInstance(data); return ICC_Profile.getInstance(data);
} }