Hello Sergey
Thank you for your time in review.
Here are some findings that might help in our discussion:
. On Validating- scanlineStride, width, height with checks
. When a user creates a BandedSampleModel object, the values of-
scanlineStride, width and height are checked for non-negative values in the
constructors.
. The only additional check that is present in ComponentSampleModel 's
getBufferSize method is the check for- (Integer.MAX_VALUE -1).
. I don’t understand how this check helps. Does it avoid
OutOfMemoryErrors during DataBuffer allocation? I'm not sure.
. Hence, I didn't add additional validation to the fix proposed.
. On Specification & throws clause:
. I looked into the specification of createDataBuffer method in-
BandedSampleModel & ComponentSampleModel
. Banded sample model mentions-
* @throws IllegalArgumentException if {@code dataType} is not
* one of the supported data types
*/
. So if we wish to take up additional validation for our values-
scanlineStride, width and height, we should update this spec as well. Kindly
let me know your thoughts.
. Surprisingly, ComponentSampleModel does not mention any throws clause
in its spec for createDataBuffer method.
But it throws quite a few IllegalArgumentExceptions in its
getBufferSize invoked from createDataBuffer.
. On ComponentSampleModel's Buffer Size Calculation
. In addition to above points, the logic to calculate buffer size within
ComponentSampleModel's getBufferSize() is incorrect.
. The documentation of the class says that it can be used to store images
that are- Band interleaved, Pixel interleaved & Scanline interleaved.
. For each of the above types- the values of scanline stride, pixel
stride, number of banks would vary.
. But ComponentSampleModel 's getBufferSize seems to have a default logic
that doesn't seem to consider these scenarios.
. There is one open bug- https://bugs.openjdk.java.net/browse/JDK-6604105.
Thank you for your review & this discussion
Have a good day
Prahalad N.
-----Original Message-----
From: Sergey Bylokhov
Sent: Saturday, January 06, 2018 2:13 PM
To: Prahalad Kumar Narayanan; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-8194489] Incorrect size
computation at BandedSampleModel.createDataBuffer
Hi, Prahalad.
Not an expert here, but have a small question about implementation of this
method in the parent class(ComponentSampleModel).
Currently the parent class has a similar implementation of createDataBuffer(),
but instead of "scanlineStride * height" it uses "getBufferSize()". And this
"getBufferSize" has a number of additional checks, should not we do something
similar here as well(see JDK-7058602)?
On 04/01/2018 21:30, Prahalad Kumar Narayanan wrote:
> Request your time to review the fix for the bug:
> JDK-8194489 Incorrect size computation at BandedSampleModel .
> createDataBuffer
>
> Root Cause:
> . The method BandedSampleModel . createDataBuffer does not consider number of
> banks and band offsets while computing the required memory size.
> . As a result, ArrayIndexOutOfBounds exception is thrown when setting pixel
> values on banded sample models having -
> . Multiple bands of image data stored in multiple banks of DataBuffer
> with band offsets
> . Multiple bands of image data stored in single bank of
> DataBuffer
>
> Solution:
> . Appropriate logic has been added to createDataBuffer method.
>
> Other Info:
> . All Jtreg test-cases in java/awt/image were run with the build including
> the fix.
> . No regressions were noticed.
>
> Kindly review the changes at your convenience & share your feedback.
> Link: http://cr.openjdk.java.net/~pnarayanan/8194489/webrev.00/
>
> Thank you for your time in review
> Have a good day
>
> Prahalad N.
>
--
Best regards, Sergey.
