[cp-patches] Re: java.awt.image.SinglePixelPackedSampleModel

[Mark Wielaard]

Hi David,

On Tue, 2004-09-28 at 01:32, David Gilbert wrote:
> David Gilbert wrote:
> > I've attached some tests for the
> > java.awt.image.SinglePixelPackedSampleModel class.  With JDK 1.4 the
> > tests all pass, with a fairly recent CVS version of Classpath running on
> > JamVM 1.2.0 there are 14 failures.  
> 
> And now attached is my stab at a patch for Classpath to resolve the 
> failing tests.

Thanks! I have moved this to classpath-patches since that is were
patches for GNU Classpath are discussed.

Could you write a ChangeLog entry for this?
(http://www.gnu.org/software/classpath/docs/hacking.html#SEC9)

Your new SinglePixelPackedSampleModel.equals() only tests for bitMasks.
Shouldn't you also test for dataType, height, numBands and width?

I would suggest a hashCode() method that just xors (^) all these values
and the content of the bitMasks array.

Thanks for reindenting some stuff so that it looks more like the rest of
out coding style. While you are at it, could you also change things
like:

     for (int yy=y; yy<(y+h); yy++)
       for (int xx=x; xx<(x+w); xx++)

into:

     for (int yy = y; yy < y + h; yy++)
       for (int xx = x; xx < x + w; xx++)

If you want that is (the old code already used the wrong style.
See for the current coding style guide:
http://www.gnu.org/software/classpath/docs/hacking.html#SEC7
But hopefully soon this can be done more automatically.

Thanks,

Mark

> Index: gnu/java/awt/Buffers.java
> ===================================================================
> RCS file: /cvsroot/classpath/classpath/gnu/java/awt/Buffers.java,v
> retrieving revision 1.3
> diff -u -r1.3 Buffers.java
> --- gnu/java/awt/Buffers.java 15 Apr 2004 07:44:09 -0000      1.3
> +++ gnu/java/awt/Buffers.java 27 Sep 2004 23:20:31 -0000
> @@ -170,37 +170,49 @@
>      Object from;
>      if (src instanceof DataBufferByte)
>        {
> -     from = ((DataBufferByte) src).getData();
> -     if (dest == null) dest = new byte[length+destOffset];
> +        from = ((DataBufferByte) src).getData();
> +        if (dest == null) 
> +          dest = new byte[length+destOffset];
> +        else
> +          dest = (byte[]) dest;  // check for correct type
>        }
>      else if (src instanceof DataBufferShort)
>        {
> -     from = ((DataBufferShort) src).getData();
> -     if (dest == null) dest = new short[length+destOffset];
> +        from = ((DataBufferShort) src).getData();
> +        if (dest == null) 
> +          dest = new short[length+destOffset];
> +        else
> +          dest = (short[]) dest;
>        }
>      else if (src instanceof DataBufferUShort)
>        {
> -     from = ((DataBufferUShort) src).getData();
> -     if (dest == null) dest = new short[length+destOffset];
> +        from = ((DataBufferUShort) src).getData();
> +        if (dest == null) 
> +          dest = new short[length+destOffset];
> +        else
> +          dest = (short[]) dest;
>        }
>      else if (src instanceof DataBufferInt)
>        {
> -     from = ((DataBufferInt) src).getData();
> -     if (dest == null) dest = new int[length+destOffset];
> +        from = ((DataBufferInt) src).getData();
> +        if (dest == null) 
> +          dest = new int[length+destOffset];
> +        else
> +          dest = (int[]) dest;
>        }
>      else if (src instanceof DataBufferFloat)
>        {
> -     from = ((DataBufferFloat) src).getData();
> -     if (dest == null) dest = new float[length+destOffset];
> +        from = ((DataBufferFloat) src).getData();
> +        if (dest == null) dest = new float[length+destOffset];
>        }
>      else if (src instanceof DataBufferDouble)
>        {
> -     from = ((DataBufferDouble) src).getData();
> -     if (dest == null) dest = new double[length+destOffset];
> +        from = ((DataBufferDouble) src).getData();
> +        if (dest == null) dest = new double[length+destOffset];
>        }
>      else
>        {
> -     throw new ClassCastException("Unknown data buffer type");
> +        throw new ClassCastException("Unknown data buffer type");
>        }
>      
>      System.arraycopy(from, srcOffset, dest, destOffset, length);
> Index: java/awt/image/SampleModel.java
> ===================================================================
> RCS file: /cvsroot/classpath/classpath/java/awt/image/SampleModel.java,v
> retrieving revision 1.3
> diff -u -r1.3 SampleModel.java
> --- java/awt/image/SampleModel.java   7 Apr 2004 14:38:12 -0000       1.3
> +++ java/awt/image/SampleModel.java   27 Sep 2004 23:20:36 -0000
> @@ -441,26 +441,42 @@
>      int inOffset = 0;
>      for (int yy=y; yy<(y+h); yy++)
>        for (int xx=x; xx<(x+w); xx++)
> -     setSample(xx, yy, b, fArray[inOffset++], data);
> +        setSample(xx, yy, b, fArray[inOffset++], data);
> +  }
>  
> -    }
> -
> -    public void setSamples(int x, int y, int w, int h, int b,
> -                        double[] dArray, DataBuffer data) {
> -      int size = w*h;
> -      int inOffset = 0;
> -      for (int yy=y; yy<(y+h); yy++)
> -     for (int xx=x; xx<(x+w); xx++)
> -       setSample(xx, yy, b, dArray[inOffset++], data);
> -    }
> -
> -    public abstract SampleModel createCompatibleSampleModel(int w, int h);
> -
> -    public abstract SampleModel createSubsetSampleModel(int[] bands);
> -
> -    public abstract DataBuffer createDataBuffer();
> -
> -    public abstract int[] getSampleSize();
> -
> -    public abstract int getSampleSize(int band);
> +  public void setSamples(int x, int y, int w, int h, int b,
> +                        double[] dArray, DataBuffer data) 
> +  {
> +    int size = w*h;
> +    int inOffset = 0;
> +    for (int yy=y; yy<(y+h); yy++)
> +      for (int xx=x; xx<(x+w); xx++)
> +         setSample(xx, yy, b, dArray[inOffset++], data);
> +  }
> +
> +  public abstract SampleModel createCompatibleSampleModel(int w, int h);
> +
> +  public abstract SampleModel createSubsetSampleModel(int[] bands);
> +
> +  public abstract DataBuffer createDataBuffer();
> +
> +  public abstract int[] getSampleSize();
> +
> +  public abstract int getSampleSize(int band);
> +    
> +  public boolean equals(Object obj) 
> +  {
> +    if (obj == this) 
> +      return true;
> +    if (!(obj instanceof SampleModel))
> +      return false;
> +    SampleModel that = (SampleModel) obj;
> +    if (this.dataType != that.dataType)
> +      return false;
> +    if (this.width != that.width)
> +      return false;
> +    if (this.height != that.height)
> +      return false;
> +    return true;
> +  }
>  }
> Index: java/awt/image/SinglePixelPackedSampleModel.java
> ===================================================================
> RCS file: 
> /cvsroot/classpath/classpath/java/awt/image/SinglePixelPackedSampleModel.java,v
> retrieving revision 1.6
> diff -u -r1.6 SinglePixelPackedSampleModel.java
> --- java/awt/image/SinglePixelPackedSampleModel.java  22 Sep 2004 13:03:28 
> -0000      1.6
> +++ java/awt/image/SinglePixelPackedSampleModel.java  27 Sep 2004 23:20:37 
> -0000
> @@ -39,6 +39,8 @@
>  import gnu.java.awt.BitMaskExtent;
>  import gnu.java.awt.Buffers;
>  
> +import java.util.Arrays;
> +
>  /**
>   * @author Rolf W. Rasmussen <address@hidden>
>   */
> @@ -71,6 +73,8 @@
>        }
>      
>      this.scanlineStride = scanlineStride;
> +    if (bitMasks.length == 0)
> +      throw new IllegalArgumentException("Zero length bitMasks array.");
>      this.bitMasks = bitMasks;
>      
>      bitOffsets = new int[numBands];
> @@ -85,6 +89,15 @@
>        }
>    }
>  
> +  /**
> +   * Returns the number of data elements used to store all the samples for 
> +   * a single pixel.  Since this class is used in the situation where all
> +   * samples are packed into a single data element, this method always 
> returns
> +   * <code>1</code>.
> +   * 
> +   * @return The number of data elements (always <code>1</code> for this 
> +   *         class).
> +   */
>    public int getNumDataElements()
>    {
>      return 1;
> @@ -152,6 +165,8 @@
>      // FIXME: Is this the right way to interpret bands?
>      
>      int numBands = bands.length;
> +    if (numBands > this.numBands)
> +      throw new RasterFormatException("Too many bands specified.");
>      
>      int[] bitMasks = new int[numBands];
>  
> @@ -446,4 +461,25 @@
>      result.append("]");
>      return result.toString();
>    }
> +  
> +  /**
> +   * Tests this sample model for equality with an arbitrary object.
> +   * 
> +   * @param obj  the object.
> +   * 
> +   * @return A boolean.
> +   */
> +  public boolean equals(Object obj) 
> +  {
> +    if (obj == this)
> +      return true;
> +    if (!(obj instanceof SinglePixelPackedSampleModel))
> +      return false;
> +    if (!super.equals(obj))
> +      return false;
> +    SinglePixelPackedSampleModel that = (SinglePixelPackedSampleModel) obj;
> +    if (!Arrays.equals(this.bitMasks, that.bitMasks))
> +      return false;
> +    return true;
> +  }
>  }

signature.asc
Description: This is a digitally signed message part