[GH-3245] Fix RS_AsCOG ArrayIndexOutOfBoundsException for byte-band rasters - #3259
Merged
jiayuasu merged 6 commits intoAug 18, 2026
Merged
Conversation
…band rasters imageio-ext's TIFFDeflater passes (offset, height * scanlineStride) verbatim to Deflater.setInput(). When TIFFImageWriter's 8-bit optimized path hands it a raster sharing a source tile wider than the output tile (e.g. a JAI overview image with a single 512x512 tile written as 256x256 tiles), the range overruns the backing array on the last row of tiles. The same path also compressed the inter-row slack of wide source tiles, corrupting overview pixels. The bug is still present in imageio-ext 2.1.0. Work around it in CogWriter by retiling byte-band coverages to the output tile grid before handing them to GeoTiffWriter, so every raster the writer reads starts at offset 0 of a tile-sized buffer. The copy happens lazily one tile at a time via javax.media.jai.TiledImage; pixel data and the written TIFF are unchanged.
jiayuasu
marked this pull request as draft
August 16, 2026 05:15
…OG retiling Address three review findings on the previous commit: - Validate the backing layout, not only declared tile dimensions. A legal byte sample model with 256x256 tiles but a wider scanline stride bypassed the retiling guard and still overran in Deflater.setInput. The guard now requires a ComponentSampleModel sized to the tile, with scanlineStride == tileWidth * pixelStride and a zero first-band offset. - Replace javax.media.jai.TiledImage with a non-caching PlanarImage view. TiledImage strongly retains every realized tile, keeping a second uncompressed copy of the raster alive for the duration of the write. The new RetilingImage materializes each tile on demand as a tight raster and caches nothing. - Make the full-resolution regression test exercise the real reader path. The synthetic TiledImage-based test passed even without the fix; the test now writes a 512-tiled GeoTIFF and reads it back through fromGeoTiff, which reproduces the crash without the fix. A new wide-stride test covers the layout validation.
…ation A DataBufferByte can carry a positive array offset independent of its sample model. Two layers ignore it: the TIFF writer's direct-buffer path reads the backing array from position 0, and WritableRaster.setRect's bulk-copy shortcut drops the offset when copying out of an offset-backed raster. An offset-backed byte source therefore round-tripped with every pixel shifted, both when the tight-layout check skipped retiling and when retiling copied via setRect. Since the offset lives on each tile's DataBuffer, no up-front layout check can prove an image safe. Drop the tight-layout fast path and rewrap every byte-band image: the writer only ever sees freshly materialized tiles with tight, zero-offset buffers. The per-tile copy now reads source tiles directly and goes through SampleModel/DataBuffer accessors, which honor the offset, instead of setRect or PlanarImage.getData cobbling. New regression tests cover an offset-backed single-tile source (fails with a whole-image pixel shift without this fix) and a 128px-tiled source that spans several source tiles per output tile.
Normalizing every byte image through per-sample accessor copies roughly tripled warmed 2048x2048 single-band COG writes (22-42 ms to 82-104 ms). Recover the previous performance while keeping the correctness guarantees: - Zero-copy fast path: when the source tile grid coincides with the output grid, each source tile whose concrete raster is verifiably writer-safe (full-size, tight stride, packed band offsets, single-bank zero-offset DataBufferByte) is returned as-is. The check runs per tile against the actual raster and DataBuffer, so the offset and stride fixes are preserved; unsafe tiles still get copied. - Bulk row copies: the per-tile copy now uses row-wise System.arraycopy for standard byte component layouts (per-band for pixelStride 1 including banded, whole-row for identically packed interleaved pixels), applying each side's DataBuffer bank offsets explicitly. Ineligible layout pairs fall back to the per-sample accessor loop. Warmed 2048x2048 single-band benchmark: untiled source 81-88 ms to 30-38 ms, 256-tiled source 84-98 ms to 32-61 ms, matching the pre-regression range. New regression test covers the zero-copy path with a 256-tiled source read through fromGeoTiff.
Round-trip tests alone cannot catch a lost optimization: they pass whether or not tiles are reused or copied in bulk. Add deterministic structural assertions instead of timing: - testRetilingViewReturnsEligibleTilesByIdentity asserts that an aligned tight source tile is returned by identity, while wide-stride and offset-backed tiles are materialized into tight, zero-offset rasters with values preserved. - testBulkRowCopyEligibility asserts copyRowsDirectly accepts grayscale (including offset-backed sources), identically packed interleaved RGB, and banded multiband layouts with correct values, and refuses mismatched pixel strides. RetilingImage and copyRowsDirectly become package-private for these tests. Both tests were verified to fail when either optimization is removed. Also reword the alignTileLayoutForByteBands comment: every byte image is wrapped, but verified-safe tiles pass through unchanged and only unsafe tiles are materialized.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Did you read the Contributor Guide?
Is this PR related to a ticket?
[GH-XXX] my subject. Closes RS_AsCOG throws ArrayIndexOutOfBoundsException for 1024x1024 byte-band rasters #3245What changes were proposed in this PR?
RS_AsCOGthrewArrayIndexOutOfBoundsExceptioninDeflater.setInputfor byte-band rasters whose backing scanline was wider than the requested COG tile. imageio-ext's optimized 8-bit TIFF path passes a tile's backing byte array, sample-model offset, and full scanline stride directly to the compressor; for these layouts, the computed range can overrun the array. The same path could also compress inter-row padding and silently corrupt pixels.A separate variant occurred when a legal
DataBufferByteused a positive array offset. The TIFF path and common raster bulk-copy shortcuts ignore that buffer offset, shifting every output pixel even when the declared sample-model layout is otherwise tight.The fix wraps every byte-band image in a package-private, non-caching
ByteBandRetilingImagebefore writing:DataBufferoffsets. Other layouts use offset-awareSampleModelaccessors.getTile; bulk raster operations that can drop buffer offsets are avoided.Pixel values, georeferencing, sample dimensions, and metadata are preserved. Non-byte images are unchanged, and the extracted implementation does not add a public API.
How was this patch tested?
End-to-end COG regression coverage verifies:
DataBufferByte-offset sources;Focused
ByteBandRetilingImageTestcoverage verifies:The full
commonmodule suite passed 1,299 tests before the final two test-only boundary fixtures were added; the final focused COG suite passes 41 tests with 0 failures. The 512-tiled regression also passes in a 64 MB test JVM.Did this PR include necessary documentation updates?