Hi,

Split this into two patches... this one fixes the BandCombineOp, mostly
stuff to bring it in line with the reference implementation.  Mauve
tests have been committed for this class as well.  Good to commit?

Cheers,
Francis


2006-08-11  Francis Kung  <[EMAIL PROTECTED]>
        * java/awt/image/BandCombineOp.java
        (BandCombineOp): Perform checks on validity of matrix.
        (createCompatibleDestRaster): Add checks and choose raster type
dynamically.
        (filter): Updated to work with new matrix storage.
        (getMatrix): Updated javadoc.
        (getPoint2D): Formatting change.

Index: java/awt/image/BandCombineOp.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/image/BandCombineOp.java,v
retrieving revision 1.4
diff -u -r1.4 BandCombineOp.java
--- java/awt/image/BandCombineOp.java	16 Mar 2006 00:50:23 -0000	1.4
+++ java/awt/image/BandCombineOp.java	11 Aug 2006 21:01:12 -0000
@@ -1,4 +1,5 @@
-/* Copyright (C) 2004  Free Software Foundation
+/* BandCombineOp.java - perform a combination on the bands of a raster
+   Copyright (C) 2004, 2006  Free Software Foundation
 
 This file is part of GNU Classpath.
 
@@ -36,7 +37,6 @@
 
 package java.awt.image;
 
-import java.awt.Point;
 import java.awt.RenderingHints;
 import java.awt.geom.Point2D;
 import java.awt.geom.Rectangle2D;
@@ -65,52 +65,66 @@
    * 
    * @param matrix The matrix to filter pixels with.
    * @param hints Rendering hints to apply.  Ignored.
+   * @throws ArrayIndexOutOfBoundsException if the matrix is invalid
    */
   public BandCombineOp(float[][] matrix, RenderingHints hints)
   {
-    this.matrix = matrix;
+    this.matrix = new float[matrix.length][];
+    int width = matrix[0].length;
+    for (int i = 0; i < matrix.length; i++)
+      {
+        this.matrix[i] = new float[width + 1];
+        for (int j = 0; j < width; j++)
+          this.matrix[i][j] = matrix[i][j];
+
+        // The reference implementation pads the array with a trailing zero...
+        this.matrix[i][width] = 0;
+      }
+
     this.hints = hints;
   }
 
   /**
-   * Filter Raster pixels through a matrix.
-   * 
-   * Applies the Op matrix to source pixes to produce dest pixels.  Each row
-   * of the matrix is multiplied by the src pixel components to produce the
-   * dest pixel.  If matrix is one more than the number of bands in the src,
-   * the last element is implicitly multiplied by 1, i.e. added to the sum
-   * for that dest component.
-   * 
-   * If dest is null, a suitable Raster is created.  This implementation uses
-   * createCompatibleDestRaster.  
+   * Filter Raster pixels through a matrix. Applies the Op matrix to source
+   * pixes to produce dest pixels. Each row of the matrix is multiplied by the
+   * src pixel components to produce the dest pixel. If matrix is one more than
+   * the number of bands in the src, the last element is implicitly multiplied
+   * by 1, i.e. added to the sum for that dest component. If dest is null, a
+   * suitable Raster is created. This implementation uses
+   * createCompatibleDestRaster.
    * 
    * @param src The source Raster.
-   * @param dest  The destination Raster, or null.
-   * @returns The destination Raster or an allocated Raster.
+   * @param dest The destination Raster, or null.
+   * @throws IllegalArgumentException if the destination raster is incompatible
+   *           with the source raster.
+   * @return The filtered Raster.
    * @see java.awt.image.RasterOp#filter(java.awt.image.Raster,
-   *java.awt.image.WritableRaster)
+   *      java.awt.image.WritableRaster)
    */
   public WritableRaster filter(Raster src, WritableRaster dest) {
     if (dest == null)
       dest = createCompatibleDestRaster(src);
-    
+    else if (dest.getNumBands() != src.getNumBands()
+             || dest.getTransferType() != src.getTransferType())
+      throw new IllegalArgumentException("Destination raster is incompatible with source raster");
+
     // Filter the pixels
-    float[] spix = new float[matrix[0].length];
+    float[] spix = new float[matrix[0].length - 1];
     float[] dpix = new float[matrix.length];
     for (int y = src.getMinY(); y < src.getHeight() + src.getMinY(); y++)
       for (int x = src.getMinX(); x < src.getWidth() + src.getMinX(); x++)
-      {
-        // In case matrix rows have implicit translation
-        spix[spix.length - 1] = 1.0f;
-        src.getPixel(x, y, spix);
-        for (int i = 0; i < matrix.length; i++)
         {
-          dpix[i] = 0;
-          for (int j = 0; j < matrix[0].length; j++)
-            dpix[i] += spix[j] * matrix[i][j];
+          // In case matrix rows have implicit translation
+          spix[spix.length - 1] = 1.0f;
+          src.getPixel(x, y, spix);
+          for (int i = 0; i < matrix.length; i++)
+            {
+              dpix[i] = 0;
+              for (int j = 0; j < matrix[0].length - 1; j++)
+                dpix[i] += spix[j] * matrix[i][j];
+            }
+          dest.setPixel(x, y, dpix);
         }
-        dest.setPixel(x, y, dpix);
-      }
 
     return dest;
   }
@@ -125,28 +139,48 @@
 
   /**
    * Creates a new WritableRaster that can be used as the destination for this
-   * Op.  This implementation creates a Banded Raster with data type FLOAT.
-   * @see
-   *java.awt.image.RasterOp#createCompatibleDestRaster(java.awt.image.Raster) 
+   * Op. The number of bands in the source raster must equal the number of rows
+   * in the op matrix, which must also be equal to either the number of columns
+   * or (columns - 1) in the matrix.
+   * 
+   * @param src The source raster.
+   * @return A compatible raster.
+   * @see java.awt.image.RasterOp#createCompatibleDestRaster(java.awt.image.Raster)
+   * @throws IllegalArgumentException if the raster is incompatible with the
+   *           matrix.
    */
   public WritableRaster createCompatibleDestRaster(Raster src)
   {
-    return Raster.createBandedRaster(DataBuffer.TYPE_FLOAT, src.getWidth(),
-        src.getHeight(), matrix.length,
-        new Point(src.getMinX(), src.getMinY()));
+    // Destination raster must have same number of bands as source
+    if (src.getNumBands() != matrix.length)
+      throw new IllegalArgumentException("Number of rows in matrix specifies an "
+                                             + "incompatible number of bands");
+
+    // We use -1 and -2 because we previously padded the rows with a trailing 0
+    if (src.getNumBands() != matrix[0].length - 1
+        && src.getNumBands() != matrix[0].length - 2)
+      throw new IllegalArgumentException("Incompatible number of bands: "
+                                             + "the number of bands in the raster must equal the number of "
+                                             + "columns in the matrix, optionally minus one");
+
+    return src.createCompatibleWritableRaster();
   }
 
-  /** Return corresponding destination point for source point.
+  /**
+   * Return corresponding destination point for source point.  Because this is
+   * not a geometric operation, it simply returns a copy of the source.
    * 
-   * LookupOp will return the value of src unchanged.
    * @param src The source point.
    * @param dst The destination point.
+   * @return dst The destination point.
    * @see java.awt.image.RasterOp#getPoint2D(java.awt.geom.Point2D,
    *java.awt.geom.Point2D) 
    */
   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;
   }
@@ -159,7 +193,11 @@
     return hints;
   }
   
-  /** Return the matrix for this Op. */
+  /**
+   * Return the matrix used in this operation.
+   *
+   * @return The matrix used in this operation.
+   */
   public final float[][] getMatrix()
   {
     return matrix;

Reply via email to