diff --git a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/ColorSpaces.java b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/ColorSpaces.java index c5fcb337..d1de336b 100644 --- a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/ColorSpaces.java +++ b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/ColorSpaces.java @@ -70,13 +70,10 @@ import java.util.Properties; * @version $Id: ColorSpaces.java,v 1.0 24.01.11 17.51 haraldk Exp$ */ public final class ColorSpaces { + final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.color.debug")); - private final static boolean DEBUG = "true".equalsIgnoreCase(System.getProperty("com.twelvemonkeys.imageio.color.debug")); - - // OpenJDK 7 seems to handle non-perceptual rendering intents gracefully, so we don't need to fiddle with the profiles. - // However, the later Oracle distribute JDK seems to include the color management code that has the known bugs... - private final static boolean JDK_HANDLES_RENDERING_INTENTS = - SystemUtil.isClassAvailable("java.lang.invoke.CallSite") && !SystemUtil.isClassAvailable("sun.java2d.cmm.kcms.CMM"); + /** We need special ICC profile handling for KCMS vs LCMS. Delegate to specific strategy. */ + private final static ICCProfileSanitizer profileCleaner = ICCProfileSanitizer.Factory.get(); // NOTE: java.awt.color.ColorSpace.CS_* uses 1000-1004, we'll use 5000+ to not interfere with future additions @@ -86,9 +83,6 @@ public final class ColorSpaces { /** A best-effort "generic" CMYK color space. Either read from disk or built-in. */ public static final int CS_GENERIC_CMYK = 5001; - /** 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}; - // Weak references to hold the color spaces while cached private static WeakReference adobeRGB1998 = new WeakReference(null); private static WeakReference genericCMYK = new WeakReference(null); @@ -129,52 +123,16 @@ public final class ColorSpaces { return cs; } - // NOTE: The intent change breaks JDK7: Seems to be a bug in ICC_Profile.getData/setData, - // as calling it with unchanged header data, still breaks when creating new ICC_ColorSpace... - // However, we simply skip that, as JDK7 handles the rendering intents already. - if (!JDK_HANDLES_RENDERING_INTENTS) { - // Fix profile before lookup/create - profile.setData(ICC_Profile.icSigHead, profileHeader); - } + // Fix profile before lookup/create + profileCleaner.fixProfile(profile, profileHeader); } - - // Special handling to detect problematic Corbis RGB ICC Profile. - // This makes sure tags that are expected to be of type 'XYZ ' really have this expected type. - // Should leave other ICC profiles unchanged. - if (!JDK_HANDLES_RENDERING_INTENTS && fixProfileXYZTag(profile, ICC_Profile.icSigMediaWhitePointTag)) { - fixProfileXYZTag(profile, ICC_Profile.icSigRedColorantTag); - fixProfileXYZTag(profile, ICC_Profile.icSigGreenColorantTag); - fixProfileXYZTag(profile, ICC_Profile.icSigBlueColorantTag); + else { + profileCleaner.fixProfile(profile, null); } return getCachedOrCreateCS(profile, profileHeader); } - /** - * Fixes problematic 'XYZ ' tags in Corbis RGB profile. - * - * @return {@code true} if found and fixed, otherwise {@code false} for short-circuiting - * to avoid unnecessary array copying. - */ - private static boolean fixProfileXYZTag(final ICC_Profile profile, final int tagSignature) { - // TODO: This blows up on OpenJDK... Bug? - byte[] data = profile.getData(tagSignature); - - // 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)) { - data[0] = 'X'; - data[1] = 'Y'; - data[2] = 'Z'; - data[3] = ' '; - - profile.setData(tagSignature, data); - - return true; - } - - return false; - } - private static ICC_ColorSpace getInternalCS(final int profileCSType, final byte[] profileHeader) { if (profileCSType == ColorSpace.TYPE_RGB && Arrays.equals(profileHeader, sRGB.header)) { return (ICC_ColorSpace) ColorSpace.getInstance(ColorSpace.CS_sRGB); @@ -414,6 +372,7 @@ public final class ColorSpaces { private static Properties loadProfiles(final Platform.OperatingSystem os) { Properties systemDefaults; + try { systemDefaults = SystemUtil.loadProperties(ColorSpaces.class, "com/twelvemonkeys/imageio/color/icc_profiles_" + os.id()); } @@ -422,6 +381,7 @@ public final class ColorSpaces { "Warning: Could not load system default ICC profile locations from %s, will use bundled fallback profiles.\n", ignore.getMessage() ); + if (DEBUG) { ignore.printStackTrace(); } diff --git a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/ICCProfileSanitizer.java b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/ICCProfileSanitizer.java new file mode 100644 index 00000000..cf2dc8ac --- /dev/null +++ b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/ICCProfileSanitizer.java @@ -0,0 +1,52 @@ +package com.twelvemonkeys.imageio.color; + +import com.twelvemonkeys.lang.SystemUtil; + +import java.awt.color.ICC_Profile; + +/** + * ProfileCleaner. + * + * @author Harald Kuhr + * @author last modified by $Author: harald.kuhr$ + * @version $Id: ProfileCleaner.java,v 1.0 06/01/15 harald.kuhr Exp$ + */ +interface ICCProfileSanitizer { + void fixProfile(ICC_Profile profile, byte[] profileHeader); + + static class Factory { + static ICCProfileSanitizer get() { + // Strategy pattern: + // - KCMSSanitizerStrategy - Current behaviour, default for Java 1.6 and Oracle JRE 1.7 + // - LCMSSanitizerStrategy - New behaviour, default for OpenJDK 1.7 and all java 1.8 + // (unless KCMS switch -Dsun.java2d.cmm=sun.java2d.cmm.kcms.KcmsServiceProvider present) + // TODO: Allow user-specific strategy selection, should heuristics not work..? + // -Dcom.twelvemonkeys.imageio.color.ICCProfileSanitizer=com.foo.bar.FooCMSSanitizer + + ICCProfileSanitizer instance; + + // Explicit System properties + if ("sun.java2d.cmm.kcms.KcmsServiceProvider".equals(System.getProperty("sun.java2d.cmm")) && SystemUtil.isClassAvailable("sun.java2d.cmm.kcms.CMM")) { + instance = new KCMSSanitizerStrategy(); + } + else if ("sun.java2d.cmm.lcms.LcmsServiceProvider".equals(System.getProperty("sun.java2d.cmm")) && SystemUtil.isClassAvailable("sun.java2d.cmm.lcms.LCMS")) { + instance = new LCMSSanitizerStrategy(); + } + // Default for Java 1.8+ or OpenJDK 1.7+ (no KCMS available) + else if (SystemUtil.isClassAvailable("java.util.stream.Stream") + || SystemUtil.isClassAvailable("java.lang.invoke.CallSite") && !SystemUtil.isClassAvailable("sun.java2d.cmm.kcms.CMM")) { + instance = new LCMSSanitizerStrategy(); + } + // Default for all Java versions <= 1.7 + else { + instance = new KCMSSanitizerStrategy(); + } + + if (ColorSpaces.DEBUG) { + System.out.println("ICC ProfileCleaner instance: " + instance.getClass().getName()); + } + + return instance; + } + } +} diff --git a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/KCMSSanitizerStrategy.java b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/KCMSSanitizerStrategy.java new file mode 100644 index 00000000..a4e018d7 --- /dev/null +++ b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/KCMSSanitizerStrategy.java @@ -0,0 +1,61 @@ +package com.twelvemonkeys.imageio.color; + +import com.twelvemonkeys.lang.Validate; + +import java.awt.color.ICC_Profile; +import java.util.Arrays; + +/** + * KCMSProfileCleaner. + * + * @author Harald Kuhr + * @author last modified by $Author: harald.kuhr$ + * @version $Id: KCMSProfileCleaner.java,v 1.0 06/01/15 harald.kuhr Exp$ + */ +final class KCMSSanitizerStrategy implements ICCProfileSanitizer { + + /** 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}; + + public void fixProfile(final ICC_Profile profile, byte[] profileHeader) { + Validate.notNull(profile, "profile"); + + if (profileHeader != null) { + profile.setData(ICC_Profile.icSigHead, profileHeader); + } + + // Special handling to detect problematic Corbis RGB ICC Profile for KCMS. + // This makes sure tags that are expected to be of type 'XYZ ' really have this expected type. + // Should leave other ICC profiles unchanged. + if (fixProfileXYZTag(profile, ICC_Profile.icSigMediaWhitePointTag)) { + fixProfileXYZTag(profile, ICC_Profile.icSigRedColorantTag); + fixProfileXYZTag(profile, ICC_Profile.icSigGreenColorantTag); + fixProfileXYZTag(profile, ICC_Profile.icSigBlueColorantTag); + } + } + + /** + * Fixes problematic 'XYZ ' tags in Corbis RGB profile. + * + * @return {@code true} if found and fixed, otherwise {@code false} for short-circuiting + * to avoid unnecessary array copying. + */ + private static boolean fixProfileXYZTag(final ICC_Profile profile, final int tagSignature) { + byte[] data = profile.getData(tagSignature); + + // 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)) { + data[0] = 'X'; + data[1] = 'Y'; + data[2] = 'Z'; + data[3] = ' '; + + profile.setData(tagSignature, data); + + return true; + } + + return false; + } + +} diff --git a/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/LCMSSanitizerStrategy.java b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/LCMSSanitizerStrategy.java new file mode 100644 index 00000000..d39a2ab1 --- /dev/null +++ b/imageio/imageio-core/src/main/java/com/twelvemonkeys/imageio/color/LCMSSanitizerStrategy.java @@ -0,0 +1,19 @@ +package com.twelvemonkeys.imageio.color; + +import com.twelvemonkeys.lang.Validate; + +import java.awt.color.ICC_Profile; + +/** + * LCMSProfileCleaner. + * + * @author Harald Kuhr + * @author last modified by $Author: harald.kuhr$ + * @version $Id: LCMSProfileCleaner.java,v 1.0 06/01/15 harald.kuhr Exp$ + */ +final class LCMSSanitizerStrategy implements ICCProfileSanitizer { + public void fixProfile(final ICC_Profile profile, byte[] profileHeader) { + Validate.notNull(profile, "profile"); + // Let LCMS handle things internally for now + } +} diff --git a/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/color/ColorSpacesTest.java b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/color/ColorSpacesTest.java index e1b9fafb..02290b25 100644 --- a/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/color/ColorSpacesTest.java +++ b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/color/ColorSpacesTest.java @@ -33,8 +33,6 @@ import org.junit.Test; import java.awt.color.ColorSpace; import java.awt.color.ICC_ColorSpace; import java.awt.color.ICC_Profile; -import java.io.IOException; -import java.util.Arrays; import static org.junit.Assert.*; @@ -47,8 +45,6 @@ import static org.junit.Assert.*; */ public class ColorSpacesTest { - private static final byte[] XYZ = new byte[] {'X', 'Y', 'Z', ' '}; - @Test public void testCreateColorSpaceFromKnownProfileReturnsInternalCS_sRGB() { ICC_Profile profile = ICC_Profile.getInstance(ColorSpace.CS_sRGB); @@ -189,19 +185,4 @@ public class ColorSpacesTest { public void testIsCS_sRGBNull() { ColorSpaces.isCS_sRGB(null); } - - @Test - public void testCorbisRGBSpecialHandling() throws IOException { - ICC_Profile corbisRGB = ICC_Profile.getInstance(getClass().getResourceAsStream("/profiles/Corbis RGB.icc")); - ICC_ColorSpace colorSpace = ColorSpaces.createColorSpace(corbisRGB); - - assertNotNull(colorSpace); - - // Make sure all known affected tags have type 'XYZ ' - ICC_Profile profile = colorSpace.getProfile(); - assertArrayEquals(XYZ, Arrays.copyOfRange(profile.getData(ICC_Profile.icSigMediaWhitePointTag), 0, 4)); - assertArrayEquals(XYZ, Arrays.copyOfRange(profile.getData(ICC_Profile.icSigRedColorantTag), 0, 4)); - assertArrayEquals(XYZ, Arrays.copyOfRange(profile.getData(ICC_Profile.icSigGreenColorantTag), 0, 4)); - assertArrayEquals(XYZ, Arrays.copyOfRange(profile.getData(ICC_Profile.icSigBlueColorantTag), 0, 4)); - } } diff --git a/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/color/KCMSSanitizerStrategyTest.java b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/color/KCMSSanitizerStrategyTest.java new file mode 100644 index 00000000..a9345dda --- /dev/null +++ b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/color/KCMSSanitizerStrategyTest.java @@ -0,0 +1,54 @@ +package com.twelvemonkeys.imageio.color; + +import org.junit.Test; + +import java.awt.color.ColorSpace; +import java.awt.color.ICC_ColorSpace; +import java.awt.color.ICC_Profile; +import java.io.IOException; +import java.util.Arrays; + +import static org.junit.Assert.assertArrayEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +public class KCMSSanitizerStrategyTest { + private static final byte[] XYZ = new byte[] {'X', 'Y', 'Z', ' '}; + + @Test(expected = IllegalArgumentException.class) + public void testFixProfileNullProfile() throws Exception { + new KCMSSanitizerStrategy().fixProfile(null, null); + } + + @Test + public void testFixProfileNullHeader() throws Exception { + new KCMSSanitizerStrategy().fixProfile(((ICC_ColorSpace) ColorSpace.getInstance(ColorSpace.CS_sRGB)).getProfile(), null); + } + + @Test + public void testFixProfileUpdateHeader() throws Exception { + ICC_Profile profile = mock(ICC_Profile.class); + byte[] header = new byte[0]; + + // Can't test that the values are actually changed, as the LCMS-backed implementation + // of ICC_Profile does not change based on this invocation. + new KCMSSanitizerStrategy().fixProfile(profile, header); + + // Verify that the method was invoked + verify(profile).setData(ICC_Profile.icSigHead, header); + } + + @Test + public void testFixProfileCorbisRGB() throws IOException { + // TODO: Consider re-writing this using mocks, to avoid dependencies on the CMS implementation + ICC_Profile corbisRGB = ICC_Profile.getInstance(getClass().getResourceAsStream("/profiles/Corbis RGB.icc")); + + new KCMSSanitizerStrategy().fixProfile(corbisRGB, null); + + // Make sure all known affected tags have type 'XYZ ' + assertArrayEquals(XYZ, Arrays.copyOfRange(corbisRGB.getData(ICC_Profile.icSigMediaWhitePointTag), 0, 4)); + assertArrayEquals(XYZ, Arrays.copyOfRange(corbisRGB.getData(ICC_Profile.icSigRedColorantTag), 0, 4)); + assertArrayEquals(XYZ, Arrays.copyOfRange(corbisRGB.getData(ICC_Profile.icSigGreenColorantTag), 0, 4)); + assertArrayEquals(XYZ, Arrays.copyOfRange(corbisRGB.getData(ICC_Profile.icSigBlueColorantTag), 0, 4)); + } +} \ No newline at end of file diff --git a/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/color/LCMSSanitizerStrategyTest.java b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/color/LCMSSanitizerStrategyTest.java new file mode 100644 index 00000000..ae51647c --- /dev/null +++ b/imageio/imageio-core/src/test/java/com/twelvemonkeys/imageio/color/LCMSSanitizerStrategyTest.java @@ -0,0 +1,32 @@ +package com.twelvemonkeys.imageio.color; + +import org.junit.Test; + +import java.awt.color.ICC_Profile; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +public class LCMSSanitizerStrategyTest { + + @Test(expected = IllegalArgumentException.class) + public void testFixProfileNullProfile() throws Exception { + new LCMSSanitizerStrategy().fixProfile(null, null); + } + + @Test + public void testFixProfileNoHeader() throws Exception { + ICC_Profile profile = mock(ICC_Profile.class); + new LCMSSanitizerStrategy().fixProfile(profile, null); + + verifyNoMoreInteractions(profile); + } + + @Test + public void testFixProfile() throws Exception { + ICC_Profile profile = mock(ICC_Profile.class); + new LCMSSanitizerStrategy().fixProfile(profile, new byte[0]); + + verifyNoMoreInteractions(profile); + } +} \ No newline at end of file