On Wed, 2 Apr 2025 12:41:04 GMT, Nikita Gubarkov <ngubar...@openjdk.org> wrote:
>>> why we can't use the first component instead of the last one
>>
>> We definitely cannot do that because of the band-interleaved case: imagine
>> if there are like 10000 bytes of red samples followed by another 10000 bytes
>> of green samples - with `pixelStride=1` and `scanlineStride=100` if we take
>> the first component offset (0), our calculated size would only fit the red
>> channel.
>>
>> In your calculations using "starting offset + pointer to the last pixel +
>> pixelStride", you assume only pixel-interleaved variant, but this
>> calculation would break with other interleaving modes. Actually, for
>> pixel-interleaved layout there is a simple commonly-used formula, like:
>> `size = startingOffset + scanlineStride * height`. But the problem is that
>> we don't really know the `startingOffset`, we only know offsets of the
>> individual components, which leads to the other issue...
>>
>>> the size is not necessarily aligned to pixelStride because scanlineStride
>>> may include a gap larger than pixelStride at the end.
>>
>> Good point, and because of the way `pixelStride` and `scanlineStride` are
>> formulated (offsets between same-band samples of the adjacent
>> pixels/scanlines), they don't tell us anything about the alignment. And
>> moreover, in the case of unused components, where our `bandOffsets` don't
>> fully fill the `pixelStride`, there is an ambiguity:
>> Consider `bandOffsets = {1, 2, 3}` and `pixelStride=4`, is that a `xRGB`
>> layout with `startingOffset=0`, or `RGBx` with `startingOffset=1`? The
>> answer is - we don't have enough info as far as I'm concerned...
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24111#discussion_r2024798193
