[WIP] Spherical polygon clipping #368
Open
Conversation
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
Implement spherical orientation predicate, point-on-arc test, and great circle arc intersection in the UnitSpherical module: - `spherical_orient(a, b, c)`: determines if point c is left/right of great circle arc a→b using sign((a × b) · c) - `point_on_spherical_arc(p, a, b)`: checks if point p lies on arc - `spherical_arc_intersection(a1, b1, a2, b2)`: finds intersection of two great circle arcs, handling cross/hinge/overlap/disjoint All use robust_cross_product for numerical stability. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…rdinates
Refactor `PolyNode` to be parameterized by point type, allowing it to hold either `Tuple{T,T}` for planar coordinates or `UnitSphericalPoint{T}` for spherical coordinates. This avoids expensive repeated conversions between geographic and unit spherical coordinates during spherical polygon clipping operations.
Changes:
- Add type parameter `P` to `PolyNode{T, P}` struct
- Add type aliases `PlanarPolyNode{T}` and `SphericalPolyNode{T}` for ergonomics
- Add helper function `point_type` to get the appropriate point type for a manifold
- Update copy constructor to preserve type parameter
- Update `TracingError` to include point type parameter
- Update all `PolyNode{T}` usages in clipping code to use `PlanarPolyNode{T}`
- Change default `fracs` value to use `zero(T)` instead of literal `0.`
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add `to_unit_spherical_points` function to convert GeoInterface rings to vectors of `UnitSphericalPoint`. This utility is used in spherical polygon clipping to convert geographic coordinates to unit spherical coordinates once, avoiding repeated conversions during intersection operations. The function uses `UnitSphereFromGeographic` which is a no-op for already-converted points, making it safe to call on any ring type. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Implement `_point_filled_curve_orientation(::Spherical, ...)` using an S2-inspired algorithm with reference point and XOR logic. The algorithm: 1. Chooses a reference point O (north pole, or equator if test point is near a pole) 2. Determines if O is inside the polygon using signed area 3. Counts edge crossings from O to the test point P 4. Returns: origin_inside XOR (crossings is odd) This correctly handles all spherical polygons including those spanning more than a hemisphere. All 9 spherical point-in-polygon tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Implement _point_filled_curve_orientation(::Spherical, ...) using ray-crossing algorithm from Google S2 geometry. Uses origin_inside XOR isodd(crossings) to handle hemisphere-spanning polygons correctly. Also skip planar extent check for Spherical manifold since Cartesian extents don't work on the sphere. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add Spherical() dispatch for: - Predicates.orient: delegates to UnitSpherical.spherical_orient - _intersection_point: delegates to UnitSpherical.spherical_arc_intersection - intersection_points(m::Manifold, ...): convenience method Also fix FosterHormannClipping constructor ambiguity for spherical manifolds. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Verify the spherical clipping pipeline runs without error. Currently, intersection point detection uses proper spherical geometry, but the polygon tracing algorithm still uses planar assumptions. Full spherical clipping is a work in progress. Tests verify: - Pipeline runs without throwing - intersection_points with Spherical() works correctly - Spherical area calculation works on results 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add manifold dispatch to internal clipping functions: - `_find_non_cross_orientation`: pass manifold to point-in-polygon - `_flag_ent_exit!`: pass manifold to point-in-polygon - `_get_side`: add Spherical version using spherical_orient - `_midpoint`: add helper with slerp for Spherical manifold - `_is_collinear`: add helper with spherical_orient for Spherical - `_remove_collinear_points!`: use manifold-aware collinearity check This enables the Foster-Hormann clipping algorithm to use proper spherical geometry predicates throughout the pipeline. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- `_intersection_point(::Spherical)`: Return UnitSphericalPoint directly
instead of converting back to geographic coordinates
- `_build_a_list` / `_build_b_list`: Use manifold-based point type
selection with `point_type(manifold, T)` to create SphericalPolyNode
- Add `_get_point_converter` helper for manifold-based input conversion
- `_trace_polynodes`: Infer point type from node list, construct output
polygons with correct coordinate type
- Non-crossing case: Use converted points from node lists
Spherical clipping now works entirely in UnitSphericalPoint coordinates
and returns polygons with UnitSphericalPoint vertices. Planar clipping
continues to use Tuple{T,T} as before.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add manifold dispatch methods to coveredby, disjoint, crosses, overlaps - Add 5-argument manifold versions to common.jl for Feature/Extent handling - Fix `_intersection_points` to use `point_type(manifold, T)` for correct return type - Update spherical intersection tests to check UnitSphericalPoint output 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
3 tasks
- Add _convert_ring_points helper to properly convert ring points using
manifold-appropriate converter (Planar uses tuples, Spherical uses
UnitSphereFromGeographic)
- Fix isempty(polys) branch to use original ring instead of extracting
from node list (which failed for shared edges and identical polygons)
- Add intersection_points(m::Manifold, ...) dispatch
- Add spherical _intersection_point for great circle arc intersections
- Fix _intersection_points to use point_type(manifold, T) for results
- Update tests for PolyNode{T,P} two-parameter signature
- Update spherical tests to convert UnitSphericalPoint to geographic
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
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.
semi inspired by s2 and other things. but mostly AI so far so keeping this a draft.
The idea is if you ask for clipping on spherical it will convert to UnitSpherical and follow this pipeline. we desperately need
best_manifold(geom)for this to work on regular ops though...