[GH-3255] Clip line segments to the raster in pixel space - #3328
Open
Eliaaazzz wants to merge 1 commit into
Open
[GH-3255] Clip line segments to the raster in pixel space#3328Eliaaazzz wants to merge 1 commit into
Eliaaazzz wants to merge 1 commit into
Conversation
Segment clipping dropped every segment whose endpoints were both outside the raster, so a line crossing the whole raster burned nothing. On a 6x6 unit raster, LINESTRING (-1 2.5, 7 2.5) crosses all six columns and left the band empty. The clip now runs in pixel space, where the window edges are the integers 0, width and height. In world space the edges are grid lines that only round-trip approximately, and a clipped endpoint landing a rounding step off a grid line selects the neighbouring cell: LINESTRING (-1 3.0000000000000004, 1 3.0) is strictly above y = 3 inside the raster and belongs to row 2, but its clipped endpoint rounded onto y = 3 and burned row 3. An interpolated coordinate that lands on a grid line now resolves its true side with the robust orientation predicate and steps one ulp off the line, so only that case leaves plain double arithmetic. Three further points: - Segment endpoints are ordered before clipping. The two directions reach a clip edge at complementary parameters that are equal exactly but round apart as doubles, which moved a cell at an exact grid corner. - Differences that would overflow to infinity for two large opposite-sign endpoints are taken on halved coordinates, which is exact for normal doubles. Whether the clipped piece has length is read from the clipped endpoints, because the parameters collapse together when the window falls inside one rounding step of the segment. - A segment that was degenerate to begin with still marks its cell, while a nondegenerate segment that clipping collapses to a tangent point, such as a corner touch, burns nothing. Tests cross-check the clipper against an independent exact-rational clip of the same segment against the same window, so a segment grazing a grid line is resolved rather than dropped.
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
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_AsRaster segment clipping drops crossings and loses grid-line sidedness #3255What changes were proposed in this PR?
Rasterization.clipSegmentToRasterBoundsis replaced by a clip that runs in pixel space. Both reproducers from the issue now produce the expected band, and the four other numerical points in the acceptance criteria are covered.Both endpoints outside
The old clipper returned
nullas soon as neither endpoint was inside the extent, which dropped every segment that crosses the raster from one side to the other. On the issue's 6x6 unit raster,LINESTRING (-1 2.5, 7 2.5)left the band empty; it now burns all six cells of row 3. The parametric clip below that early return was already able to handle the case, it was simply never reached.Grid-line sidedness
Clipping moved to pixel space, where the window edges are the integers
0,widthandheight. In world space the edges are grid lines that only round-trip approximately through(coordinate - upperLeft) / scale, so a clipped endpoint can land a rounding step off the line it belongs to.The coordinate pinned to a window edge is now exact by construction, and only the interpolated one can round onto a grid line. When it does, its true side is resolved with
CGAlgorithmsDD.orientationIndexagainst the lattice point and the coordinate steps one ulp off the line. Solving the clip for the interpolated coordinate leaves exactly the cross product that predicate returns, divided by the delta along the clipped axis, so the sign follows directly.LINESTRING (-1 3.0000000000000004, 1 3.0)is strictly abovey = 3inside the raster except at its final endpoint. Its clipped endpoint rounded ontoy = 3and burned row 3; it now burns row 2. The mirrored case below the line still burns row 3, and the same rounding on the x axis is covered too.Direction independence
Segment endpoints are ordered before clipping. The two directions reach a clip edge at complementary parameters that are equal in exact arithmetic but round apart as doubles, and interpolating with them puts a coordinate on either side of an exact grid corner. My own oracle test caught this on
LINESTRING (-2 0.5, 8 5.5), which passes exactly through the pixel lattice point (5, 2).Overflow and collapsed parameters
A difference that would overflow to infinity for two large opposite-sign endpoints is taken on halved coordinates, which is exact for normal doubles, so the parameters are the ones the direct form would have produced. That path is entered only when the direct difference is infinite.
Whether the clipped piece has length is read from the clipped endpoints rather than from the parameters. The parameters answer it correctly for a single-point touch such as a corner, but they collapse together for a genuine crossing whose endpoints are far enough apart that the whole window falls inside one rounding step.
LINESTRING (-1E308 2.5, 1E308 2.5)burns row 3 rather than nothing, in both directions and on both axes.Degenerate versus collapsed
A segment that was degenerate to begin with still marks the cell holding it. A nondegenerate segment that clipping collapses to a tangent point burns nothing:
LINESTRING (5 7, 7 5)meets the raster only at the corner (6, 6).Allocation
The ordinary finite path is plain doubles with no per-segment allocation. The single scratch array for clipped endpoints is allocated once per geometry, not per segment.
CGAlgorithmsDD.orientationIndexis reached only by an interpolated coordinate that landed exactly on a grid line, and it is already the predicate the traversal uses.How was this patch tested?
Six new tests in
RasterizationTest:testLineCrossingRasterWithBothEndpointsOutsidecovers the issue's first reproducer plus vertical and diagonal crossings, each in both vertex orders.testClippedEndpointKeepsGridLineSidecovers the second reproducer, its mirror below the grid line, and the same rounding on the x axis.testLineTouchingRasterAtASinglePointBurnsNothingcovers the corner tangency.testLineWithExtremeCoordinatesCrossesTheRastercovers horizontal and vertical+/-1e308crossings.testDegenerateLineStringBurnsItsOwnCellcovers the degenerate input.testLineClipAgreesWithExactRationalOraclecross-checks fifteen segments againstSegmentClipOracle, a new test-only exact-rational clip of the same segment against the same window. It runs entirely inBigIntegerrationals with no rounding anywhere, so a segment grazing a grid line is resolved rather than excluded, and it shares no arithmetic with the production clipper. For each segment it asserts that the band is empty exactly when the exact intersection has no length, that the cells holding the exact clipped endpoints are burned, and that reversing the vertices produces an identical band.That oracle test is what caught the direction dependence and the collapsed-parameter omission described above, both of which were in my first draft of the clipper.
mvn -pl common teston Windows with JDK 17: 1350 tests, the same 13 failures as on unmodifiedmaster(RasterBandEditorsTest.testClip, 5RasterEditorsTest.testResample*, 7RasterOutputTest.testAsMatrix*). They fail identically before and after, so they are pre-existing on this platform and unrelated. Everything else passes, including the GH-3118 coastline fixtures with their pinned 1738/1842 counts and the GH-3120 and GH-3251 endpoint-cell tests.I could not run
spark/commonlocally: it fails to compile on the generated OSM PBF protobuf sources (package proto4 does not exist) before reaching any raster code. Leaving those suites to CI.One note on scope. The clip window is now the raster's own pixel bounds rather than the snapped
geomExtent. The two give the same burned cells, because a segment lies inside its geometry's snapped extent andburnCellalready discards anything outside the raster, and the traversal step boundwidth + height + 2assumed the raster bounds anyway. It also means the window edges are exact integers, which is what makes the sidedness argument hold.rasterizeLineStringno longer takesgeomExtent.Did this PR include necessary documentation updates?
🤖 Generated with Claude Code
https://claude.ai/code/session_01P6hpqAnXfeaWRLkL1VQDMd