Hey,
Attached is a patch to fix the ColorConvertOp class, as well as fixlets
to the ComponentColorModel (get rid of that null pointer exception in
the constructor!) and the gnu.java.awt.color.PyccConverter (throw
UnsupportedOperationExceptions instead of returning null; it would end
up causing a NullPointerException anyways)
Good to commit?
Thanks,
Francis
Index: gnu/java/awt/color/PyccConverter.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/awt/color/PyccConverter.java,v
retrieving revision 1.3
diff -u -r1.3 PyccConverter.java
--- gnu/java/awt/color/PyccConverter.java 2 Jul 2005 20:32:11 -0000 1.3
+++ gnu/java/awt/color/PyccConverter.java 18 Aug 2006 20:08:48 -0000
@@ -37,7 +37,6 @@
package gnu.java.awt.color;
-
/**
* PyccConverter - conversion routines for the PhotoYCC colorspace
*
@@ -52,21 +51,21 @@
{
public float[] toRGB(float[] in)
{
- return null;
+ throw new UnsupportedOperationException();
}
public float[] fromRGB(float[] in)
{
- return null;
+ throw new UnsupportedOperationException();
}
public float[] toCIEXYZ(float[] in)
{
- return null;
+ throw new UnsupportedOperationException();
}
public float[] fromCIEXYZ(float[] in)
{
- return null;
+ throw new UnsupportedOperationException();
}
}
Index: java/awt/image/ComponentColorModel.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/image/ComponentColorModel.java,v
retrieving revision 1.8
diff -u -r1.8 ComponentColorModel.java
--- java/awt/image/ComponentColorModel.java 2 Jul 2005 20:32:30 -0000 1.8
+++ java/awt/image/ComponentColorModel.java 18 Aug 2006 20:08:48 -0000
@@ -42,9 +42,11 @@
import java.awt.Point;
import java.awt.color.ColorSpace;
+import java.util.Arrays;
public class ComponentColorModel extends ColorModel
{
+ // Find sum of all elements of the array.
private static int sum(int[] values)
{
int sum = 0;
@@ -52,6 +54,22 @@
sum += values[i];
return sum;
}
+
+ // Create an appropriate array of bits, given a colorspace (ie, number of
+ // bands), size of the storage data type, and presence of an alpha band.
+ private static int[] findBits(ColorSpace colorSpace, int transferType,
+ boolean hasAlpha)
+ {
+ int[] bits;
+ if (hasAlpha)
+ bits = new int[colorSpace.getNumComponents()+1];
+ else
+ bits = new int[colorSpace.getNumComponents()];
+
+ Arrays.fill(bits, DataBuffer.getDataTypeSize(transferType));
+
+ return bits;
+ }
public ComponentColorModel(ColorSpace colorSpace, int[] bits,
boolean hasAlpha,
@@ -84,8 +102,8 @@
boolean isAlphaPremultiplied,
int transparency, int transferType)
{
- this(colorSpace, null, hasAlpha, isAlphaPremultiplied,
- transparency, transferType);
+ this(colorSpace, findBits(colorSpace, transferType, hasAlpha), hasAlpha,
+ isAlphaPremultiplied, transparency, transferType);
}
public int getRed(int pixel)
Index: java/awt/image/ColorConvertOp.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/image/ColorConvertOp.java,v
retrieving revision 1.5
diff -u -r1.5 ColorConvertOp.java
--- java/awt/image/ColorConvertOp.java 26 Jul 2006 18:14:04 -0000 1.5
+++ java/awt/image/ColorConvertOp.java 18 Aug 2006 20:08:48 -0000
@@ -39,6 +39,7 @@
package java.awt.image;
import java.awt.Graphics2D;
+import java.awt.Point;
import java.awt.RenderingHints;
import java.awt.color.ColorSpace;
import java.awt.color.ICC_ColorSpace;
@@ -47,9 +48,9 @@
import java.awt.geom.Rectangle2D;
/**
- * ColorConvertOp is a filter for converting an image from one colorspace to
- * another colorspace. The filter can convert the image through a sequence
- * of colorspaces or just from source to destination.
+ * ColorConvertOp is a filter for converting images or rasters between
+ * colorspaces, either through a sequence of colorspaces or just from source to
+ * destination.
*
* Color conversion is done on the color components without alpha. Thus
* if a BufferedImage has alpha premultiplied, this is divided out before
@@ -63,24 +64,22 @@
*/
public class ColorConvertOp implements BufferedImageOp, RasterOp
{
- private ColorSpace srccs;
- private ColorSpace dstcs;
private RenderingHints hints;
- private ICC_Profile[] profiles;
+ private ICC_Profile[] profiles = null;
private ColorSpace[] spaces;
- private boolean rasterValid;
/**
- * Convert BufferedImage through a ColorSpace.
+ * Convert a BufferedImage through a ColorSpace.
*
- * This filter version is only valid for BufferedImages. The source image
- * is converted to cspace. If the destination is not null, it is then
- * converted to the destination colorspace. Normally this filter will only
- * be used with a null destination.
+ * Objects created with this constructor can be used to convert
+ * BufferedImage's to a destination ColorSpace. Attempts to convert Rasters
+ * with this constructor will result in an IllegalArgumentException when the
+ * filter(Raster, WritableRaster) method is called.
*
* @param cspace The target color space.
- * @param hints Rendering hints to use in conversion, or null.
+ * @param hints Rendering hints to use in conversion, if any (may be null)
+ * @throws NullPointerException if the ColorSpace is null.
*/
public ColorConvertOp(ColorSpace cspace, RenderingHints hints)
{
@@ -88,9 +87,27 @@
throw new NullPointerException();
spaces = new ColorSpace[]{cspace};
this.hints = hints;
- rasterValid = false;
}
+ /**
+ * Convert from a source colorspace to a destination colorspace.
+ *
+ * This constructor takes two ColorSpace arguments as the source and
+ * destination color spaces. It is usually used with the
+ * filter(Raster, WritableRaster) method, in which case the source colorspace
+ * is assumed to correspond to the source Raster, and the destination
+ * colorspace with the destination Raster.
+ *
+ * If used with BufferedImages that do not match the source or destination
+ * colorspaces specified here, there is an implicit conversion from the
+ * source image to the source ColorSpace, or the destination ColorSpace to
+ * the destination image.
+ *
+ * @param srcCspace The source ColorSpace.
+ * @param dstCspace The destination ColorSpace.
+ * @param hints Rendering hints to use in conversion, if any (may be null).
+ * @throws NullPointerException if any ColorSpace is null.
+ */
public ColorConvertOp(ColorSpace srcCspace, ColorSpace dstCspace,
RenderingHints hints)
{
@@ -101,61 +118,77 @@
}
/**
- * Convert from a source image destination image color space.
+ * Convert from a source colorspace to a destinatino colorspace.
*
* This constructor builds a ColorConvertOp from an array of ICC_Profiles.
- * The source image will be converted through the sequence of color spaces
+ * The source will be converted through the sequence of color spaces
* defined by the profiles. If the sequence of profiles doesn't give a
- * well-defined conversion, throws IllegalArgumentException.
+ * well-defined conversion, an IllegalArgumentException is thrown.
*
- * NOTE: Sun's docs don't clearly define what a well-defined conversion is
- * - or perhaps someone smarter can come along and sort it out.
- *
- * For BufferedImages, when the first and last profiles match the
- * requirements of the source and destination color space respectively, the
- * corresponding conversion is unnecessary. TODO: code this up. I don't
- * yet understand how you determine this.
+ * If used with BufferedImages that do not match the source or destination
+ * colorspaces specified here, there is an implicit conversion from the
+ * source image to the source ColorSpace, or the destination ColorSpace to
+ * the destination image.
*
* For Rasters, the first and last profiles must have the same number of
* bands as the source and destination Rasters, respectively. If this is
* not the case, or there fewer than 2 profiles, an IllegalArgumentException
* will be thrown.
*
- * @param profiles
- * @param hints
+ * @param profiles An array of ICC_Profile's to convert through.
+ * @param hints Rendering hints to use in conversion, if any (may be null).
+ * @throws NullPointerException if the profile array is null.
+ * @throws IllegalArgumentException if the array is not a well-defined
+ * conversion.
*/
public ColorConvertOp(ICC_Profile[] profiles, RenderingHints hints)
{
if (profiles == null)
throw new NullPointerException();
+
this.hints = hints;
this.profiles = profiles;
- // TODO: Determine if this is well-defined.
+
// Create colorspace array with space for src and dest colorspace
+ // Note that the ICC_ColorSpace constructor will throw an
+ // IllegalArgumentException if the profile is invalid; thus we check
+ // for a "well defined conversion"
spaces = new ColorSpace[profiles.length];
for (int i = 0; i < profiles.length; i++)
spaces[i] = new ICC_ColorSpace(profiles[i]);
}
- /** Convert from source image color space to destination image color space.
+ /**
+ * Convert from source color space to destination color space.
*
* Only valid for BufferedImage objects, this Op converts from the source
- * color space to the destination color space. The destination can't be
- * null for this operation.
+ * image's color space to the destination image's color space.
+ *
+ * The destination in the filter(BufferedImage, BufferedImage) method cannot
+ * be null for this operation, and it also cannot be used with the
+ * filter(Raster, WritableRaster) method.
*
- * @param hints Rendering hints to use during conversion, or null.
+ * @param hints Rendering hints to use in conversion, if any (may be null).
*/
public ColorConvertOp(RenderingHints hints)
{
- this.hints = hints;
- srccs = null;
- dstcs = null;
- rasterValid = false;
+ this.hints = hints;
+ spaces = new ColorSpace[0];
}
- /* (non-Javadoc)
- * @see java.awt.image.BufferedImageOp#filter(java.awt.image.BufferedImage,
- java.awt.image.BufferedImage)
+ /**
+ * Converts the source image using the conversion path specified in the
+ * constructor. The resulting image is stored in the destination image if one
+ * is provided; otherwise a new BufferedImage is created and returned.
+ *
+ * The source and destination BufferedImage (if one is supplied) must have
+ * the same dimensions.
+ *
+ * @param src The source image.
+ * @param dst The destination image.
+ * @throws IllegalArgumentException if the rasters and/or color spaces are
+ * incompatible.
+ * @return The transformed image.
*/
public final BufferedImage filter(BufferedImage src, BufferedImage dst)
{
@@ -163,127 +196,230 @@
// For now we just suck it up and create intermediate buffers.
if (dst == null && spaces.length == 0)
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("Not enough color space information "
+ + "to complete conversion.");
+
+ if (dst != null
+ && (src.getHeight() != dst.getHeight() || src.getWidth() != dst.getWidth()))
+ throw new IllegalArgumentException("Source and destination images have "
+ + "different dimensions");
// Make sure input isn't premultiplied by alpha
if (src.isAlphaPremultiplied())
- {
- BufferedImage tmp = createCompatibleDestImage(src, src.getColorModel());
- copyimage(src, tmp);
- tmp.coerceData(false);
- src = tmp;
- }
+ {
+ BufferedImage tmp = createCompatibleDestImage(src, src.getColorModel());
+ copyimage(src, tmp);
+ tmp.coerceData(false);
+ src = tmp;
+ }
- ColorModel scm = src.getColorModel();
+ // Convert through defined intermediate conversions
+ BufferedImage tmp;
for (int i = 0; i < spaces.length; i++)
- {
- BufferedImage tmp = createCompatibleDestImage(src, scm);
- copyimage(src, tmp);
- src = tmp;
- }
+ {
+ if (src.getColorModel().getColorSpace().getType() != spaces[i].getType())
+ {
+ tmp = createCompatibleDestImage(src,
+ createCompatibleColorModel(src,
+ spaces[i]));
+ copyimage(src, tmp);
+ src = tmp;
+ }
+ }
- // Intermediate conversions leave result in src
+ // No implicit conversion to destination type needed; return result from the
+ // last intermediate conversions (which was left in src)
if (dst == null)
- return src;
-
- // Apply final conversion
- copyimage(src, dst);
-
+ dst = src;
+
+ // Implicit conversion to destination image's color space
+ else
+ copyimage(src, dst);
+
return dst;
}
- /* (non-Javadoc)
- * @see java.awt.image.BufferedImageOp#createCompatibleDestImage(java.awt.image.BufferedImage, java.awt.image.ColorModel)
+ /**
+ * Converts the source raster using the conversion path specified in the
+ * constructor. The resulting raster is stored in the destination raster if
+ * one is provided; otherwise a new WritableRaster is created and returned.
+ *
+ * This operation is not valid with every constructor of this class; see
+ * the constructors for details. Further, the source raster must have the
+ * same number of bands as the source ColorSpace, and the destination raster
+ * must have the same number of bands as the destination ColorSpace.
+ *
+ * The source and destination raster (if one is supplied) must also have the
+ * same dimensions.
+ *
+ * @param src The source raster.
+ * @param dest The destination raster.
+ * @throws IllegalArgumentException if the rasters and/or color spaces are
+ * incompatible.
+ * @return The transformed raster.
+ */
+ public final WritableRaster filter(Raster src, WritableRaster dest)
+ {
+ // Various checks to ensure that the rasters and color spaces are compatible
+ if (spaces.length < 2)
+ throw new IllegalArgumentException("Not enough information about " +
+ "source and destination colorspaces.");
+
+ if (spaces[0].getNumComponents() != src.getNumBands()
+ || (dest != null && spaces[spaces.length - 1].getNumComponents() != dest.getNumBands()))
+ throw new IllegalArgumentException("Source or destination raster " +
+ "contains the wrong number of bands.");
+
+ if (dest != null
+ && (src.getHeight() != dest.getHeight() || src.getWidth() != dest.getWidth()))
+ throw new IllegalArgumentException("Source and destination rasters " +
+ "have different dimensions");
+
+ // Need to iterate through each color space.
+ // spaces[0] corresponds to the ColorSpace of the source raster, and
+ // spaces[spaces.length - 1] corresponds to the ColorSpace of the
+ // destination, with any number (or zero) of intermediate conversions.
+
+ for (int i = 0; i < spaces.length - 2; i++)
+ {
+ WritableRaster tmp = createCompatibleDestRaster(src, spaces[i + 1],
+ false);
+ copyraster(src, spaces[i], tmp, spaces[i + 1]);
+ src = tmp;
+ }
+
+ // The last conversion is done outside of the loop so that we can
+ // use the dest raster supplied, instead of creating our own temp raster
+ if (dest == null)
+ dest = createCompatibleDestRaster(src, spaces[spaces.length - 1], false);
+ copyraster(src, spaces[spaces.length - 2], dest, spaces[spaces.length - 1]);
+
+ return dest;
+ }
+
+ /**
+ * Creates an empty BufferedImage with the size equal to the source and the
+ * correct number of bands for the conversion defined in this Op. The newly
+ * created image is created with the specified ColorModel, or if no ColorModel
+ * is supplied, an appropriate one is chosen.
+ *
+ * @param src The source image.
+ * @param dstCM A color model for the destination image (may be null).
+ * @throws IllegalArgumentException if an appropriate colormodel cannot be
+ * chosen with the information given.
+ * @return The new compatible destination image.
*/
public BufferedImage createCompatibleDestImage(BufferedImage src,
- ColorModel dstCM)
+ ColorModel dstCM)
{
- // FIXME: set properties to those in src
+ if (dstCM == null && spaces.length == 0)
+ throw new IllegalArgumentException("Don't know the destination " +
+ "colormodel");
+
+ if (dstCM == null)
+ {
+ dstCM = createCompatibleColorModel(src, spaces[spaces.length - 1]);
+ }
+
return new BufferedImage(dstCM,
- src.getRaster().createCompatibleWritableRaster(),
- src.isPremultiplied,
- null);
+ createCompatibleDestRaster(src.getRaster(),
+ dstCM.getColorSpace(),
+ src.getColorModel().hasAlpha),
+ src.isPremultiplied, null);
}
- public final ICC_Profile[] getICC_Profiles()
- {
- return profiles;
- }
-
- /** Return the rendering hints for this op. */
- public final RenderingHints getRenderingHints()
+ /**
+ * Creates a new WritableRaster with the size equal to the source and the
+ * correct number of bands.
+ *
+ * Note, the new Raster will always use a BYTE storage size, regardless of
+ * the color model or defined destination; this is for compatibility with
+ * the reference implementation.
+ *
+ * @param src The source Raster.
+ * @throws IllegalArgumentException if there isn't enough colorspace
+ * information to create a compatible Raster.
+ * @return The new compatible destination raster.
+ */
+ public WritableRaster createCompatibleDestRaster(Raster src)
{
- return hints;
+ if (spaces.length < 2)
+ throw new IllegalArgumentException("Not enough destination colorspace " +
+ "information");
+
+ // Create a new raster with the last ColorSpace in the conversion
+ // chain, and with no alpha (implied)
+ return createCompatibleDestRaster(src, spaces[spaces.length-1], false);
}
- /* (non-Javadoc)
- * @see java.awt.image.RasterOp#filter(java.awt.image.Raster, java.awt.image.WritableRaster)
+ /**
+ * Returns the array of ICC_Profiles used to create this Op, or null if the
+ * Op was created using ColorSpace arguments.
+ *
+ * @return The array of ICC_Profiles, or null.
*/
- public final WritableRaster filter(Raster src, WritableRaster dest)
+ public final ICC_Profile[] getICC_Profiles()
{
- if (!rasterValid)
- throw new IllegalArgumentException();
-
- // Need to iterate through each color space - there must be at least 2
- for (int i = 1; i < spaces.length - 1; i++)
- {
- // FIXME: this is wrong. tmp needs to have the same number of bands as
- // spaces[i] has.
- WritableRaster tmp = createCompatibleDestRaster(src);
- copyraster(src, spaces[i - 1], tmp, spaces[i]);
- src = tmp;
- }
-
- // FIXME: this is wrong. dst needs to have the same number of bands as
- // spaces[i] has.
- if (dest == null)
- dest = createCompatibleDestRaster(src);
- copyraster(src, spaces[spaces.length - 2],
- dest, spaces[spaces.length - 1]);
-
- return dest;
+ return profiles;
}
- /* (non-Javadoc)
- * @see java.awt.image.RasterOp#createCompatibleDestRaster(java.awt.image.Raster)
+ /**
+ * Returns the rendering hints for this op.
+ *
+ * @return The rendering hints for this Op, or null.
*/
- public WritableRaster createCompatibleDestRaster(Raster src)
+ public final RenderingHints getRenderingHints()
{
- return src.createCompatibleWritableRaster();
+ return hints;
}
- /** Return corresponding destination point for source point.
+ /**
+ * Returns the corresponding destination point for a source point.
+ * Because this is not a geometric operation, the destination and source
+ * points will be identical.
*
- * LookupOp will return the value of src unchanged.
* @param src The source point.
- * @param dst The destination point.
- * @see java.awt.image.RasterOp#getPoint2D(java.awt.geom.Point2D, java.awt.geom.Point2D)
+ * @param dst The transformed destination point.
+ * @return The transformed destination point.
*/
public final Point2D getPoint2D(Point2D src, Point2D dst)
{
- if (dst == null) return (Point2D)src.clone();
+ if (dst == null)
+ return (Point2D)src.clone();
+
dst.setLocation(src);
return dst;
}
- /* (non-Javadoc)
- * @see java.awt.image.BufferedImageOp#getBounds2D(java.awt.image.BufferedImage)
+ /**
+ * Returns the corresponding destination boundary of a source boundary.
+ * Because this is not a geometric operation, the destination and source
+ * boundaries will be identical.
+ *
+ * @param src The source boundary.
+ * @return The boundaries of the destination.
*/
public final Rectangle2D getBounds2D(BufferedImage src)
{
return src.getRaster().getBounds();
}
- /* (non-Javadoc)
- * @see java.awt.image.RasterOp#getBounds2D(java.awt.image.Raster)
+ /**
+ * Returns the corresponding destination boundary of a source boundary.
+ * Because this is not a geometric operation, the destination and source
+ * boundaries will be identical.
+ *
+ * @param src The source boundary.
+ * @return The boundaries of the destination.
*/
public final Rectangle2D getBounds2D(Raster src)
{
return src.getBounds();
}
-
- // According to Sven de Marothy, we need to copy the src into the dest
- // using Graphics2D, in order to use the rendering hints.
+
+ // Copy a source image to a destination image, respecting their colorspaces
+ // and performing colorspace conversions if necessary. This is done
+ // using Graphics2D in order to use the rendering hints.
private void copyimage(BufferedImage src, BufferedImage dst)
{
Graphics2D gg = dst.createGraphics();
@@ -297,13 +433,15 @@
gg.dispose();
}
- private void copyraster(Raster src, ColorSpace scs, WritableRaster dst,
- ColorSpace dcs)
+ // Copy a source raster to a destination raster, performing a colorspace
+ // conversion.
+ private void copyraster(Raster src, ColorSpace scs, WritableRaster dst, ColorSpace dcs)
{
float[] sbuf = new float[src.getNumBands()];
- if (hints.get(RenderingHints.KEY_COLOR_RENDERING) ==
- RenderingHints.VALUE_COLOR_RENDER_QUALITY)
+ if (hints != null
+ && hints.get(RenderingHints.KEY_COLOR_RENDERING) ==
+ RenderingHints.VALUE_COLOR_RENDER_QUALITY)
{
// use cie for accuracy
for (int y = src.getMinY(); y < src.getHeight() + src.getMinY(); y++)
@@ -321,4 +459,41 @@
}
}
+ // This method creates a compatible color model, given a source image and
+ // a colorspace. The choice of ComponentColorModel and DataBuffer.TYPE_BYTE
+ // is based on Mauve testing of the reference implementation.
+ private ColorModel createCompatibleColorModel(BufferedImage img, ColorSpace cs)
+ {
+ return new ComponentColorModel(cs,
+ img.getColorModel().hasAlpha(),
+ img.isAlphaPremultiplied(),
+ img.getColorModel().getTransparency(),
+ DataBuffer.TYPE_BYTE);
+ }
+
+ // This method creates a compatible Raster, given a source raster and
+ // colorspace.
+ private WritableRaster createCompatibleDestRaster(Raster src, ColorSpace cs, boolean hasAlpha)
+ {
+ // The use of a PixelInterleavedSampleModel (and it's parameters) and
+ // a DataBufferByte were determined using mauve tests, based on the
+ // reference implementation
+
+ int numComponents = cs.getNumComponents();
+ if (hasAlpha)
+ numComponents++;
+
+ int[] offsets = new int[numComponents];
+ for (int i = 0; i < offsets.length; i++)
+ offsets[i] = i;
+
+ return new WritableRaster(new PixelInterleavedSampleModel(DataBuffer.TYPE_BYTE,
+ src.getWidth(),
+ src.getHeight(),
+ numComponents,
+ numComponents * src.getWidth(),
+ offsets),
+ new DataBufferByte(src.getWidth() * src.getHeight() * numComponents, 1),
+ new Point(src.getMinX(), src.getMinY()));
+ }
}
