On Wed, 16 Apr 2025 22:10:53 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> 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. > > There is no reason to change our current implementation of > ComponentSampleModel, since a similar raster can be created manually and > should be properly handled by accelerated pipelines. > > DataBuffer manualBuffer = new DataBufferByte( > scanlineStride * (SIZE - 1) + pixelStride * SIZE > ); > WritableRaster manualRaster = Raster.createWritableRaster(sampleModel, > manualBuffer, null); > BufferedImage manualImage = new BufferedImage(colorModel, manualRaster, > isAlphaPremultiplied, null); In your sample you provided an example, when raster fits whole pixels (`scanline * (height - 1) + pixelStride * width`), which, I agree, is a perfectly fine case and must be supported by accelerated pipelines. But the question is: should partial pixels be handled by our pipelines too (`scanline * (height - 1) + pixelStride * (width - 1) + maxBandOffset + 1` where `maxBandOffset + 1 < pixelStride`), or should whole pixels be enforced? I am for the latter. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24111#discussion_r2060690975
