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

Jim Graham james.graham at oracle.com
Tue Mar 22 22:28:02 UTC 2016


Hi Ajit,

Most of your if statements are spaced wrong.  There should be a space 
between "if" and the parentheses.  I'll review more later, but I noticed 
that issue in the first couple of files I looked at...

		...jim

On 3/15/16 7:05 AM, Ajit Ghaisas wrote:
> 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.
>>>
>>
>>



More information about the 2d-dev mailing list