RFR: 8352407: PixelInterleavedSampleModel with unused components throws RasterFormatException: Incorrect pixel stride [v2]

[Nikita Gubarkov]

On Wed, 2 Apr 2025 06:26:13 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> Nikita Gubarkov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   The previous approach was wrong for non-pixel interleaving.
>>   
>>   Just align the buffer size to the pixel stride instead, should be better.
>
> src/java.desktop/share/classes/java/awt/image/ComponentSampleModel.java line 285:
> 
>> 283:          // Align to the pixel stride.
>> 284:          size = (size + pixelStride - 1) / pixelStride * pixelStride;
>> 285: 
> 
> Some thoughts I’d like to mention to ensure I understand the problem:
> 
> This method should return the number of bytes required to store the image with given parameters—width, height, pixel stride, and scanline. It seems that the calculation could follow this approach:
> 
> **starting offset + pointer to the last pixel + pixelStride**
> 
> Is that a correct assumption?
> 
> I have doubts that the logic in this method actually calculates the size properly, especially this line:
> 
> 
> int val = pixelStride * (width - 1);
> size += val;
> 
> It seems like it should be:
> 
> int val = pixelStride + pixelStride * (width - 1);
> 
> or simply:
> 
> int val = pixelStride * width;
> 
> Or am I missing something?

As I understand, an important thing here is that this calculation needs to be correct for any kind of pixel/scanline/band interleaving.
What if our channels are stored separately (complete image of only R followed by complete image of G and so on) - your formula would only work if we change "**pointer to the last pixel**" -> "**pointer to the last band of the last pixel**". But then it would be wrong for the pixel-interleaving case (RGBA for each pixel stored together), because adding `pixelStride=4` to the "**last band of the last pixel**" would actually be 3 bytes more than needed.

The idea of the current approach, as I understand it is that we find the last band of the first pixel, be it 3 for pixel-interleaved RGBA, or 30000 for band-interleaved image, and then add 1 to it, meaning the size sufficient to fit that last band:

int size = maxBandOff + 1;

At this point we can already fit the first pixel, so what's left is to fit the remaining first scanline (hence `width - 1`):

int val = pixelStride * (width - 1);
size += val;

And the remaining rows (hence `height - 1`):

val = scanlineStride * (height - 1);
size += val;


### This gets us to our subject:

As current calculation only considers used bands by finding `maxBandOff`, it doesn't really care when we have pixel interleaving and `pixelStride > numBands`, it will happily cut any unused space at the end of the last pixel, thus causing various problems when we just want to, say, make an RGBx (`pixelStride=4`, `bandOffsets = {0, 1, 2}`) raster.

In my approach I just fixed that space which was cut by the previous calculation by aligning the size up to `pixelStride`. Not that I find this particularly elegant, but it seems to work and not cause regressions in non-pixel interleaved variants.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/24111#discussion_r2024265636