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

[Jim Graham]

Ah, yes, now I see it.  I was combing through the various classes in 
java/awt/image that *are* public to do some research while I was reading 
the code review and somehow thought that the classes being modified were 
in the same directory/package.

I now see that all of the modified constructors are in sun/awt which is 
fine.

Apologies for the diversion...

			...jim

On 3/29/16 1:24 PM, Sergey Bylokhov wrote:
> On 29.03.16 23:19, Jim Graham wrote:
>> This sounded like such a great idea that I don't think we realized the
>> one obvious reason we can't do this...
>
> We should take into account that this classes in sun/awt/image and is
> not accessible on old jdk when the securitymanager is enabled, and will
> be completely encapsulated in jdk9.
>
>>
>> You are talking about changing the method signatures on existing public
>> constructors.  While it might be nice to have had them typed from the
>> get-go, I don't think we can change them now.  We might be able to add
>> new overloaded typed constructors (or typed factories?), but we can't
>> just delete or modify the existing constructors without creating a
>> compatibility issue...
>>
>>              ...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.
>>>>
>>>
>>>
>
>