Add ClipByRect2D with Sutherland-Hodgman and Liang-Barsky algorithms#709
Draft
peterstace wants to merge 36 commits into
Draft
Add ClipByRect2D with Sutherland-Hodgman and Liang-Barsky algorithms#709peterstace wants to merge 36 commits into
peterstace wants to merge 36 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.
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.
…es into clip-by-rect-doc
clipLineStringByRect computed boundary intersections purely by interpolation via lerpXY, so a clipped endpoint could land a few ULPs off the rect edge. Under this package's zero-tolerance equality that leaves line clipping inconsistent with the polygon clipper, which already sets the bound axis exactly. Fold clipSegmentParams into clipSegment, which now tracks which rect edge bound tMin/tMax and snaps that axis onto the exact boundary coordinate before returning the clipped endpoints.
When walkArcs could not find a successor start param, findNextStart fell through to startByParam[best] with best still -1.0. The missing key made the map return index 0, so the walk silently continued from an arbitrary arc — and because the inner loop only terminates when nextIdx == firstIdx, it could spin forever on non-finite input. findNextStart now returns an explicit ok bool: true from the nearest-ahead path and the coincident-param fallback, false only when no successor exists (reachable only via non-finite boundary params, e.g. NaN coordinates). The caller closes the ring as-is when ok is false, giving a bounded result rather than looping.
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.