Conversation
…e raster grid rasterizePoint walked the geometry extent by repeatedly adding scaleX / scaleY to a world coordinate while counting the pixel index separately. When the pixel size has no exact double representation (for example 1/3600 degree, the Copernicus GLO-30 grid) the running coordinate can fall a fraction of an ULP short of the extent's far edge and take one more step. The index then lands outside the grid, and because the JAI banded raster's setSample does no bounds check the write goes past the data buffer: a point on a 256x256 tile's bottom-right corner fails with "Index 65536 out of bounds for length 65536", and a point on a row boundary with "Index -246". The same drift moves the tested cell edges a few ULPs off the grid lines, so the cell tested and the cell written can disagree. Iterate over integer column and row ranges instead, clamped to the grid, and build each cell envelope from its index on the reference raster's grid. The envelope and the written index can no longer drift apart, and a cropped output resolves boundary contacts the same way as a full-extent one. The geometry is prepared once for the per-cell tests. A polygon that only grazes the raster clips to a point, a line or a GeometryCollection, which rasterizePolygon cast to Polygon (ClassCastException). Rasterize such clips through rasterizeGeometry, keeping only their polygonal parts unless allTouched is set, since a point or line left behind by the clip covers no pixel centre. In fillPolygon, clamp fill ranges to the grid and discard an unpaired scanline intercept that lands past it instead of writing out of bounds.
… raster edges On a 1/3600 degree grid the raster's right edge converts to a pixel coordinate a rounding error past the grid, so a segment that ends on or crosses it makes the cell traversal visit one column outside the raster, which burnCell skips. The test checks that the edge cells are still burned, comparing each geometry (lines in both directions and an allTouched polygon) against the same geometry rasterized on a larger raster with the same origin and cropped back.
…sits on a grid line rasterizeGeomExtent snaps the geometry envelope to pixel edges through a BigDecimal round trip, and the allTouched expansion then compared the envelope against the snapped value with ==. The round trip can land a few ULPs away from the grid line as computed in double arithmetic, so a geometry lying exactly on a row or column boundary was not recognized as touching the neighbouring row or column, and that row or column was never rasterized. Compare within a few ULPs instead, scaled by the largest operand involved so that a grid line near zero is judged against the raster origin. The widened extent is only the search window: each cell is still tested against the geometry, so a point just inside one row does not burn the row next to it.
This branch has not been deployed
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 allTouched rasterization drops the row or column across a grid line when a geometry lies exactly on it #3415What changes were proposed in this PR?
Stacked on #3416. Until that merges, this PR also shows its commit; the changes for this issue are the last two commits. I'll rebase once #3416 lands.
rasterizeGeomExtent, the fourallTouchedexpansion checks compared the geometry envelope with the snapped grid line using==. The snap goes through a BigDecimal round trip and can land a few ULPs off the grid line computed in double arithmetic (59.99916666666667 vs 59.99916666666666), so the extent was not widened and the touched row or column across the line was never visited. The checks now useisOnGridLine, which allows 8 ULPs of the largest operand (raster origin included, so a grid line near zero is judged against the origin's magnitude). The widened extent is only the search window; each cell is still tested against the geometry, so a point strictly inside one row does not burn its neighbour.burnCell.How was this patch tested?
RasterizationTestcases: a point on a row line, a column line and a corner, full-extent and cropped, on a tile away from the origin; the Rasterization throws ArrayIndexOutOfBoundsException for points on raster edges and ClassCastException for polygons that only graze the raster #3414 point on the row 0/1 line, which now burns(10,0)and(10,1); and a point just inside a row that must burn only that row. The two grid-line tests fail on [GH-3414] Rasterize points by grid index and bound writes to the raster grid #3416 alone ([(10,0)]vs[(10,0), (10,1)],[(10,36)]vs[(10,35), (10,36)]) and pass with this change: 23 run, 0 failures.org.apache.sedona.common.raster: 504 run, 0 failures. SparkrasteralgebraTestandrasterIOTest: 192 passed, 0 failed.allTouched = true. Touched cells dropped (full extent): master 109 / 94 / 180, [GH-3414] Rasterize points by grid index and bound writes to the raster grid #3416 alone 65 / 67 / 132, with this PR 0 / 0 / 0; cropped output is also 0 / 0 / 0.Did this PR include necessary documentation updates?