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

[Nikita Gubarkov]

On Wed, 2 Apr 2025 13:09:46 GMT, Nikita Gubarkov <ngubarkov at openjdk.org> wrote:

>> I see a way to deal with this problem though - we don't have to deal with the ambiguity, but just allocate enough memory to be on the safe side no matter the interpretation of the layout - with the example I provided earlier `bandOffsets = {1, 2, 3}` and `pixelStride=4`, just allocate 1 extra byte at the end to prevent out-of-bounds access in any case.
>
>> Or maybe we should modify the code that relies on the current size?
> 
> I have mixed feelings about that. It sounds logical "mathematically", so to speak, but in practice there are assumptions other code often relies on, which are so natural, that breaking them would be like shooting ourselves in the foot.
> 
> This follows the same logic as C struct layout: unused padding is considered an integral part of the struct layout and its size - we can always `memcpy` a struct with its `sizeof` without calculating the position of the last byte which actually holds meaningful data.
> Following this:
> 1. `pixelStride` equals to the size of the pixel - it must always be resident in memory together with any padding it includes
> 2. `scanlineStride` equals to the size of the scanline - it must always be resident in memory together with any padding it includes
> 3. Derived from the latter: `dataSize >= height * scanlineStride` - yes, we could have unused space in the end of the scanline, but it is often assumed that we can copy the whole image with just `height * scanlineStride` without worrying about the last scanline ruining our day.
> 
> I find this logic very natural and convenient, that's why I think we need to fix the size calculation and not its usage.

Please see the related PR: https://github.com/openjdk/jdk/pull/24378
It tries to solve the similar problem of calculating pixel-aligned starting offset from given band offsets, so this discussion also applies there.

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

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