Thanks Jim and Phil. Yes. It was a mistake to omit spaces in 'if' conditions. I have corrected the spacing issues for my changes now.
I have corrected the indentation of the lines similar to those mentioned by Phil. I have also removed the redundant first condition check on line 890 in Raster.java mentioned as a review comment. Please review updated webrev : http://cr.openjdk.java.net/~arapte/ajit/6353518/webrev.03/ Regards, Ajit -----Original Message----- From: Phil Race Sent: Wednesday, March 23, 2016 4:40 AM To: Jim Graham Cc: Ajit Ghaisas; Sergey Bylokhov; 2d-dev Subject: Re: [OpenJDK 2D-Dev] [9] Review-request for 6353518: Creation of a WritableRaster with a custom DataBuffer causes erroneous Exception Ajit, There is also some odd indentation in ByteBandedRaster.java which is not yours but 98 public ByteBandedRaster(SampleModel sampleModel, 99 DataBufferByte dataBuffer, 100 Point origin) { This appears to be the result of someone using tabs a long time ago. Since you are touching the method signature lines anyway may be better fixed so we have these aligned public ByteBandedRaster(SampleModel sampleModel, DataBufferByte dataBuffer, Point origin) { [not sure if that will make it through mail as intended]. Here in Raster.java, the first condition now seems to be redundant .. 890 if (dataType == DataBuffer.TYPE_BYTE && 891 dataBuffer instanceof DataBufferByte && -phil. On 03/22/2016 03:28 PM, Jim Graham wrote: > 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-...@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. >>>> >>> >>>
