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

[Nikita Gubarkov]

On Thu, 3 Apr 2025 07:32:24 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>>> as of now, we simply don't have generic native loops that support pixel gaps
>> 
>> That was kinda what I was actually implementing in Vulkan when I faced this issue. I just accepted any pixel-interleaved surface with 4-byte stride and assumed that I can blit it directly. Just because the stride is 4 bytes, I know I can copy the whole scanline in one go into a matching Vulkan format, like B8G8R8A8_UNORM, and I can handle any component order (including missing alpha channel) with simple swizzling.
>
>>I just accepted any pixel-interleaved surface with 4-byte stride and assumed that I can blit it directly. 
> 
> So you implemented logic similar to [this one](https://github.com/openjdk/jdk/blob/bd74922157230c866802b4c5269da81e872525aa/src/java.desktop/share/native/libawt/java2d/loops/LoopMacros.h#L847).
> 
> I haven’t found yet whether we have logic to fall back to a "generic Java loop" if the gap is not stored in the array and that memcpy fails. If we’re missing this, we should either:
> 
> - Add the fallback,
> - Implement another version of the loop that handles the tail, or
> - Update the current ISOSCALE_BLIT.
> 
> A similar approach could be implemented for Vulkan as well.

I get the idea. Actually, my Vulkan blit routines know the component order (needed for swizzling anyway), so instead of copying `scanlineStride` or `width * pixelStride`, I can copy `(width - 1) * pixelStride + maxBandOffset + 1` (I don't like breaking the alignment at the end, but anyway).
However, supporting the gap at the end of the pixel seems like a lot of burden to me - not all current code seems to consider this possibility and for the future code it's too easy to get wrong.

Some side thoughts: I think we rely on predefined formats and loops too much. As I already mentioned, `SurfaceDataRasInfo` doesn't have info about its bands and doesn't even have a total size. We rely on specific loops registered for specific surface types, which only works well when we created those surfaces by ourselves. Example:
I profiled Intellij IDEA to see which blit loops are often used there and noticed that icons are loaded as 4-byte RGBA images. See the issue? That's a `TYPE_CUSTOM` image with some generic `SurfaceType` - it doesn't have native raster ops initialized, so it can only go through a generic software loop! That's how I came to https://github.com/openjdk/jdk/pull/24378. So what I was trying to do with that Vulkan work is to make it more generic - register loops for more generic surface types and dynamically inspect their properties to see whether we can actually do an optimal blit. And given that 99% of the time those are 3/4-bytes per pixel RGBA/ARGB/BGR/etc., it's very easy to generalize, but we don't seem to have enough flexibility for that.

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

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