Skip to content

[GH-3255] Clip line segments to the raster in pixel space - #3328

Open
Eliaaazzz wants to merge 1 commit into
apache:masterfrom
Eliaaazzz:fix/GH-3255-segment-clipping
Open

[GH-3255] Clip line segments to the raster in pixel space#3328
Eliaaazzz wants to merge 1 commit into
apache:masterfrom
Eliaaazzz:fix/GH-3255-segment-clipping

Conversation

@Eliaaazzz

Copy link
Copy Markdown
Contributor

Did you read the Contributor Guide?

Is this PR related to a ticket?

What changes were proposed in this PR?

Rasterization.clipSegmentToRasterBounds is 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 null as 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, width and height. 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.orientationIndex against 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 above y = 3 inside the raster except at its final endpoint. Its clipped endpoint rounded onto y = 3 and 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.orientationIndex is 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:

  • testLineCrossingRasterWithBothEndpointsOutside covers the issue's first reproducer plus vertical and diagonal crossings, each in both vertex orders.
  • testClippedEndpointKeepsGridLineSide covers the second reproducer, its mirror below the grid line, and the same rounding on the x axis.
  • testLineTouchingRasterAtASinglePointBurnsNothing covers the corner tangency.
  • testLineWithExtremeCoordinatesCrossesTheRaster covers horizontal and vertical +/-1e308 crossings.
  • testDegenerateLineStringBurnsItsOwnCell covers the degenerate input.
  • testLineClipAgreesWithExactRationalOracle cross-checks fifteen segments against SegmentClipOracle, a new test-only exact-rational clip of the same segment against the same window. It runs entirely in BigInteger rationals 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 test on Windows with JDK 17: 1350 tests, the same 13 failures as on unmodified master (RasterBandEditorsTest.testClip, 5 RasterEditorsTest.testResample*, 7 RasterOutputTest.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/common locally: 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 and burnCell already discards anything outside the raster, and the traversal step bound width + height + 2 assumed the raster bounds anyway. It also means the window edges are exact integers, which is what makes the sidedness argument hold. rasterizeLineString no longer takes geomExtent.

Did this PR include necessary documentation updates?

  • No, this PR does not affect any public API so no need to change the documentation.

🤖 Generated with Claude Code

https://claude.ai/code/session_01P6hpqAnXfeaWRLkL1VQDMd

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.
@Eliaaazzz
Eliaaazzz requested a review from jiayuasu as a code owner September 6, 2026 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RS_AsRaster segment clipping drops crossings and loses grid-line sidedness

1 participant