Add ClipByRect2D with Sutherland-Hodgman and Liang-Barsky algorithms#709
Draft
peterstace wants to merge 32 commits into
Draft
Add ClipByRect2D with Sutherland-Hodgman and Liang-Barsky algorithms#709peterstace wants to merge 32 commits into
peterstace wants to merge 32 commits into
Conversation
Detailed description of the algorithm for clipping polygons against axis-aligned rectangles, in preparation for a pure Go ClipByRect implementation.
Document test cases for all geometry types (Point, MultiPoint, LineString, MultiLineString, Polygon, MultiPolygon, GeometryCollection) covering spatial relationships, boundary interactions, degenerate rectangles, numerical edge cases, and coordinate dimension preservation.
Implement clipGeometryCollectionByRect and add GC1-GC14 test cases.
When the polygon contains the entire clipping rectangle, the S-H output is entirely boundary edges with no interior arcs. Previously this case constructed a fresh rect with Z=0, M=0, discarding the correctly interpolated values from S-H clipping. Now uses the clipped exterior ring directly.
Introduce mutableSequence as the mutable counterpart of Sequence, using the same flat []float64 storage. The S-H polygon clipping code now works with mutableSequence throughout instead of converting Sequence → []Coordinates → Sequence. This avoids the space overhead of unpacking every coordinate into a padded struct. mutableSequence behaves like a "fat slice" — all methods use value receivers and mutating operations return the updated value.
This reverts commit 76a48c6.
Force input to DimXY at the public entry point and drop all Z/M interpolation from both clipping algorithms. The previous behaviour synthesised Z/M values at rectangle corners (where the clipped boundary must traverse around the rect) which are not derivable from the input data — better to commit to 2D semantics than to fabricate. Both algorithms now operate on []XY internally. The Sutherland-Hodgman polygon clipper drops its ctype/stride fields and all dimension-aware loops; the Liang-Barsky line clipper drops interpolateSeqCoord. New helpers introduced: - Sequence.asXYs() for the conversion at the input boundary - xysToSeq for converting back at the output boundary - lerpXY in alg_linear_interpolation.go Tests updated: cases with XYZ/XYM/XYZM inputs now expect XY outputs, and new CD5-CD8 cases pin the Z/M-dropped contract across Point, Polygon, MultiPoint, and GeometryCollection.
After rebasing onto master, the per-package test helpers geomFromWKT and expectGeomEq no longer exist - they were replaced by test.FromWKT and test.ExactEquals in the internal/test package.
- Rename min/max to lo/hi in alg_clip_by_rect_liang_barsky.go to avoid
shadowing the Go 1.21+ predeclared builtins (predeclared linter).
- Reformat alg_sutherland_hodgman_test.go to satisfy gofumpt: multi-line
composite literals must have their opening { and closing } on their own
lines.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a native geom.ClipByRect2D operation for clipping geometries against an axis-aligned rectangle (Envelope), using Sutherland–Hodgman for polygons and Liang–Barsky for line strings, and includes extensive unit tests and algorithm documentation.
Changes:
- Introduces
ClipByRect2Ddispatcher and helpers for building XY sequences. - Implements polygon clipping/topology resolution (Sutherland–Hodgman-based) and line clipping (Liang–Barsky-based), with 2D-only outputs.
- Adds exhaustive D4-symmetry-based unit tests plus new documentation and a changelog entry.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| geom/type_sequence.go | Adds internal Sequence.asXYs() helper to extract XYs efficiently for clipping algorithms. |
| geom/alg_sutherland_hodgman_test.go | Adds comprehensive ClipByRect2D test inventory across geometry types and degenerate rectangles. |
| geom/alg_linear_interpolation.go | Adds lerpXY helper used by line clipping. |
| geom/alg_clip_by_rect.go | Adds public ClipByRect2D entrypoint and type-based dispatch + XY sequence builder. |
| geom/alg_clip_by_rect_sutherland_hodgman.go | Implements polygon clipping and topology resolution logic for rectangle clipping. |
| geom/alg_clip_by_rect_liang_barsky.go | Implements line string / multi line string clipping via Liang–Barsky. |
| docs/sutherland_hodgman.md | Adds detailed Sutherland–Hodgman documentation and test case catalog (needs alignment with 2D-only behavior). |
| docs/clip_polygon_algorithm.md | Adds polygon clipping/topology-resolution documentation (needs alignment with 2D-only behavior). |
| CHANGELOG.md | Documents the new ClipByRect2D API under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ## Motivation | ||
|
|
||
| ClipByRect computes the intersection of a geometry with an axis-aligned |
Comment on lines
+815
to
+822
| ### Coordinate Dimension Preservation | ||
|
|
||
| | # | Case | Notes | | ||
| | --- | --------------------- | ----------------------------------------------------------- | | ||
| | CD1 | XY geometry clipped | Output has XY coordinates | | ||
| | CD2 | XYZ geometry clipped | Output has XYZ coordinates, Z interpolated at intersections | | ||
| | CD3 | XYM geometry clipped | Output has XYM coordinates, M interpolated at intersections | | ||
| | CD4 | XYZM geometry clipped | Output has XYZM coordinates, Z and M interpolated | |
Comment on lines
+55
to
+67
| For a vertical clipping edge x = k, the intersection of segment A→B is | ||
| computed as: | ||
|
|
||
| ``` | ||
| t = (k - A.x) / (B.x - A.x) | ||
| result = interpolateCoords(A, B, t) | ||
| result.x = k // set exactly to avoid float imprecision | ||
| ``` | ||
|
|
||
| Horizontal edges (y = k) are analogous. The `interpolateCoords` function | ||
| handles Z and M dimensions using a numerically robust lerp. The explicit | ||
| assignment of the boundary coordinate ensures that later boundary detection | ||
| (which uses `==`) is reliable. |
Comment on lines
+230
to
+232
| For Z/M coordinates, corner values are linearly interpolated between the Z/M | ||
| values at the start and end of the boundary path, proportional to the boundary | ||
| distance. |
Comment on lines
+33
to
+35
| ca := lerpXY(a, b, tMin) | ||
| cb := lerpXY(a, b, tMax) | ||
|
|
Comment on lines
+195
to
+200
| var endPoints []endpoint // sorted by param | ||
| startByParam := make(map[float64]int) | ||
| for i, a := range arcs { | ||
| endPoints = append(endPoints, endpoint{a.endParam, i}) | ||
| startByParam[a.startParam] = i | ||
| } |
Comment on lines
+227
to
+253
| // TODO: Investigate whether this fallback is reachable and | ||
| // whether it produces correct results. | ||
| // | ||
| // This branch is reached when every start param has ccwDist | ||
| // == 0 from endParam — i.e., every arc's start is at the | ||
| // exact same boundary parameter as the current arc's end. | ||
| // The d > 0 filter above rejected them all. | ||
| // | ||
| // For valid clipped polygons this shouldn't happen: an | ||
| // arc's end and the next arc's start should be at distinct | ||
| // boundary positions (separated by at least one boundary | ||
| // edge). Two arcs sharing the same boundary parameter would | ||
| // mean two ring transitions at the exact same point, which | ||
| // would imply a degenerate or self-touching input. | ||
| // | ||
| // If this is truly unreachable, it should be replaced with | ||
| // a panic. If it is reachable, the behaviour of picking an | ||
| // arbitrary arc at the same parameter needs validation — | ||
| // it's unclear whether the walking algorithm produces | ||
| // correct output in this case. | ||
| for _, sp := range startParams { | ||
| if sp == endParam { | ||
| return sp, startByParam[sp] | ||
| } | ||
| } | ||
| } | ||
| return best, startByParam[best] |
The project targets Go 1.18 (per go.mod), which doesn't have the slices or cmp standard library packages (added in Go 1.21). Switch to the equivalent sort.Slice and sort.Float64s calls.
Match alg_clip_by_rect_sutherland_hodgman_test.go to its corresponding implementation file alg_clip_by_rect_sutherland_hodgman.go.
lerp already returns the endpoints exactly at t=0 and t=1, so the guards in lerpXY were dead weight. The doc comment's bit-identical-endpoints contract still holds.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The removed sentence described bit-identical endpoint behaviour, which is a property of the underlying lerp helper rather than the lerpXY wrapper. Keeping the wrapper's doc focused on what the function does avoids documenting an invariant that lives one layer below.
The natural fall-through already handles n == 0: the loop bound n-1 is -1 (Length returns int), so the segment loop is skipped, chains stays empty, and the function returns emptyLine via the existing post-loop check.
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.
Description
Adds
ClipByRect2D, a new function that clips a geometry to a 2D axis-aligned rectangle defined by anEnvelope.The result is always 2D: any Z or M values on the input are discarded. This avoids the ambiguity of synthesising Z/M values at rectangle corners and edge crossings introduced by the clip.
The branch also adds detailed algorithm documentation under
docs/:docs/clip_polygon_algorithm.md— overview of polygon clipping.docs/sutherland_hodgman.md— exhaustive case analysis and rules.Check List
Have you:
Added unit tests? —
geom/alg_sutherland_hodgman_test.gocovers an exhaustive case inventory across all geometry types (Point, MultiPoint, LineString, MultiLineString, Polygon, MultiPolygon, GeometryCollection).Add cmprefimpl tests? (if appropriate?) — Not added.
internal/rawgeosexposesGEOSClipByRect_ralready, but GEOS'sClipByRectdoes not strip Z/M, so a direct comparison test would need a 2D-projection shim on the GEOS side. Worth doing as a follow-up.Updated release notes? (if appropriate?) —
CHANGELOG.mdentry added.Updated the README.md (if new functionality is added?) — Not yet. README enumerates operations under categories like
Transformations; this could be listed there or under a newClippingcategory. Happy to add it before merge.Related Issue
No linked issue.
Benchmark Results
N/A — new functionality with no prior baseline to compare against.