On Thu, 3 Apr 2025 05:27:23 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>>> please add a test case for this scenario >> >> Sure, gonna look into it. >> >>> is it possible for pixelStride to be smaller than maxBandOff? >> >> Surprisingly, yes! Band offsets are arbitrary, even for >> `PixelInterleavedSampleModel`, the only restriction is that `maxBandOff - >> minBandOff <= pixelStride` **(which is wrong btw, there should be < >> instead)** - [see >> here](https://github.com/openjdk/jdk/blob/209e72d311234c8279289172dab2cbb255e4fed9/src/java.desktop/share/classes/java/awt/image/PixelInterleavedSampleModel.java#L94). >> >> That's where the ambiguity I mentioned comes from - the code doesn't assume >> any specific starting offset and we cannot expect band offsets to always be >> in `[0, pixelStride)` range. Therefore, whenever there is an unused padding >> at the beginning or end of the pixel, this creates some wiggle. Let's >> imagine we have some weird non-zero starting offset: >> >> x x x x x R G B x R G B x R G B x x x x x >> >> Knowing that `pixelStride=4` we might split it like this: >> >> x x x x | x R G B | x R G B | x R G B | x x x x x >> >> Or like this: >> >> x x x x x | R G B x | R G B x | R G B x | x x x x >> >> Both could be correct and there doesn't seem to be any other info, which >> could allow us to determine this unambiguously. This "real starting pixel >> offset" is implicitly assumed, when working with [known types in native >> code](https://github.com/openjdk/jdk/blob/209e72d311234c8279289172dab2cbb255e4fed9/src/java.desktop/share/classes/sun/awt/image/BufImgSurfaceData.java#L319), >> but is explicitly stored nowhere. >> >> _This is all theoretical though, I have never seen a pixel-interleaved image >> with a non-zero starting offset._ >> >> ### Regarding the `pixelStride` and `scanlineStride` >> >> I am reading the documentation for >> [SurfaceDataRasInfo](https://github.com/openjdk/jdk/blob/209e72d311234c8279289172dab2cbb255e4fed9/src/java.desktop/share/native/libawt/java2d/SurfaceData.h#L71): >>> Valid memory locations are required at: >> `*(pixeltype *) (((char *)rasBase) + y * scanStride + x * pixelStride)` >> for each x, y pair such that (bounds.x1 <= x < bounds.x2) and (bounds.y1 <= >> y < bounds.y2). >> >> So it doesn't require the end of the last scanline to be resident in memory, >> however `pixeltype` is not defined there, so it's hard to tell whether it's >> assumed that `sizeof(pixeltype) == pixelStride`, or that this question was >> considered at all. I am using a little thought experiment to figure this out: >> >> Imagine that we want to copy image data from a gi... > >> > is it possible for pixelStride to be smaller than maxBandOff? >> >> Surprisingly, yes! Band offsets are arbitrary, even for >> `PixelInterleavedSampleModel`, the only restriction is that `maxBandOff - >> minBandOff <= pixelStride` > > But isn't it essentially the same thing? It just shifts the "pixelStride" > window in one direction or another. > >> Both could be correct and there doesn't seem to be any other info, which >> could allow us to determine this unambiguously. This "real starting pixel >> offset" is implicitly assumed, when working with [known types in native >> code](https://github.com/openjdk/jdk/blob/209e72d311234c8279289172dab2cbb255e4fed9/src/java.desktop/share/classes/sun/awt/image/BufImgSurfaceData.java#L319), >> but is explicitly stored nowhere. > > It depends on where and when the ColorModel/SampleModel/Raster were created. > If the raster was created by our code and we can determine the starting > offsets, and if we can confirm that the gaps are safe to read, then we could > set some flags(JDK-8353542). These flags could be recognized by the pipelines > to enable fast-path optimizations. > > Just one example: > > private static BufferedImage makeCustom3BYTE_BGR() { > ColorSpace cs = ColorSpace.getInstance(ColorSpace.CS_sRGB); > int[] nBits = {8, 8, 8}; > int[] bOffs = {2, 1, 0}; > ColorModel colorModel = new ComponentColorModel(cs, nBits, false, > false, > Transparency.OPAQUE, > DataBuffer.TYPE_BYTE); > > PixelInterleavedSampleModel sm = > new PixelInterleavedSampleModel(DataBuffer.TYPE_BYTE, > 2, 1, 400, 4000, bOffs); > WritableRaster raster = Raster.createWritableRaster(sm, null); > > System.out.println("raster.getDataBuffer().getSize() = " + > raster.getDataBuffer().getSize()); > return new BufferedImage(colorModel, raster, true, null); > } > > BufferedImage bi = new BufferedImage(3000, 3000, TYPE_INT_ARGB); > BufferedImage src = makeCustom3BYTE_BGR(); > Graphics2D g2d = bi.createGraphics(); > g2d.setTransform(new AffineTransform()); > g2d.drawImage(src, 0, 0, null); > g2d.dispose(); > > > The size of the image is "403", meaning it does not include the gap after the > last pixel data. > > Now, if we attempt to blit this into various pipelines, we may encounter > different slow/fast paths: > > - CMM code will bail out since such formats are not suppor... Probably, the changes similar to https://github.com/openjdk/jdk/pull/24378 will help in some cases. However, for the generic case with random rasters, it will be necessary to determine whether the gaps at the end of the image are actually stored in the raster or not. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24111#discussion_r2026271718
