[OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

[Ajit Ghaisas]

Hi,

   Thanks Sergey and Jim for suggestions.

    I have made the data specific Raster constructors type safe now.  Also, I have modified all Raster creations in Raster.java to support custom DataBuffer classes.

    Please review the changes present in updated webrev : 
    http://cr.openjdk.java.net/~arapte/ajit/6353518/webrev.02/

Regards,
Ajit

-----Original Message-----
From: Jim Graham 
Sent: Friday, March 11, 2016 2:40 AM
To: Sergey Bylokhov; Ajit Ghaisas; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception

Yes, those constructors should be type-safe.  Perhaps that was done to save code in the caller, but that is a small price to pay to avoid errors in the future, and it makes sure there is a backup plan for different kinds of buffers...

			...jim

On 3/10/16 4:48 AM, Sergey Bylokhov wrote:
> Hi, Ajit.
> What about other cases in Raster.java, where the DataBuffer is passed 
> as a parameter? Probably we can simplify the code and find all such 
> issues if we changes the ByteInterleavedRaster/etc. For example:
>
> ByteInterleavedRaster(SampleModel sampleModel,DataBuffer 
> dataBuffer,...) to
>
> ByteInterleavedRaster(SampleModel sampleModel,DataBufferByte
> dataBuffer,...)
>
> Because we throw an exception in such cases anyway:
>
> if (!(dataBuffer instanceof DataBufferByte)) {
>      throw new RasterFormatException("ByteInterleavedRasters must have "
> + "byte DataBuffers");
> }
>
> And the compiler will help us, what everybody think about it?
>
> On 09.03.16 17:38, Ajit Ghaisas wrote:
>> Hi,
>>
>>     Modified the test and added check for MultiPixelPackedSampleModel 
>> condition.
>>
>>     Please review updated webrev :
>>     http://cr.openjdk.java.net/~arapte/ajit/6353518/webrev.01/
>>
>> Regards,
>> Ajit
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Wednesday, March 09, 2016 4:06 PM
>> To: Ajit Ghaisas; awt-dev at openjdk.java.net; Semyon Sadetsky; Ambarish 
>> Rapte; 2d-dev
>> Subject: Re: [9] Review-request for 6353518: Creation of a 
>> WritableRaster with a custom DataBuffer causes erroneous Exception
>>
>> Changes for 2d area.(cc)
>>
>>
>> On 07.03.16 11:20, Ajit Ghaisas wrote:
>>> Hi,
>>>
>>> Bug : https://bugs.openjdk.java.net/browse/JDK-6353518
>>>
>>> Issue : (Text from bug description)
>>>
>>> An attempt to create a WritableRaster via 
>>> Raster.createWritableRaster(SampleModel sm, DataBuffer db, Point
>>> location) using a custom DataBuffer causes an erroneous 
>>> RasterFormatException.
>>> Apparently the reason for this bug is that IntegerComponentRaster 
>>> insists on being passed an instance of DataBufferInt rather than 
>>> just a DataBuffer with a DataType of TYPE_INT.
>>> This is quite annoying since DataBufferInt is declared final and 
>>> thus cannot be extended.
>>>
>>> Fix :
>>>
>>> The last line of Raster.createWritableRaster() method already has a 
>>> way to handle generic case if the input does not match any of the 
>>> cases in switch statements in that method.
>>>
>>> The fact that  " *InterleavedRaster() constructors throw exception 
>>> if DataBuffer passed to them does not match the expected type" was 
>>> ignored earlier.
>>>
>>> This fix adds a check of "DataBuffer type" before creating 
>>> respective
>>> *InterleavedRaster() objects.
>>>
>>> It constructs the SunWritableRaster in case DataBuffer type does not 
>>> match any data specific DataBuffer classes (DataBufferByte, 
>>> DataBufferUShort, DataBufferInt)
>>>
>>> Request to review webrev containing this fix :
>>>
>>> http://cr.openjdk.java.net/~arapte/ajit/6353518/webrev.00/
>>>
>>> Regards,
>>>
>>> Ajit
>>>
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>