Eliaaazzz opened a new pull request, #3328:
URL: https://github.com/apache/sedona/pull/3328

   ## Did you read the Contributor Guide?
   
   - Yes, I have read the [Contributor 
Rules](https://sedona.apache.org/latest/community/rule/) and [Contributor 
Development Guide](https://sedona.apache.org/latest/community/develop/)
   
   ## Is this PR related to a ticket?
   
   - Yes, and the PR name follows the format `[GH-XXX] my subject`. Closes #3255
   
   ## 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.com/claude-code)
   
   https://claude.ai/code/session_01P6hpqAnXfeaWRLkL1VQDMd
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to