Closed
Conversation
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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- `_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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
They need at least 2 rows
Now that julia 1.12.4 is out we should begood
Add guard condition to check `start_chain_edge != unknown` before accessing `a_list[start_chain_idx]`. When processing overlapping edge chains, the code could encounter an unmatched end chain without a corresponding start, leaving `start_chain_idx` at its initial value of 0 and causing a BoundsError on 1-based indexed arrays. This fixes 779 failing spherical polygon intersection cases. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The fixed tolerance of 1e-14 was appropriate for Float64 but impossible
to satisfy for Float32 inputs (Float32 epsilon is ~1.2e-7). GeoJSON
stores coordinates as Float32, which propagate through to
UnitSphericalPoint{Float32} and fail the unit length assertion.
Changed to use `16 * eps(T)` which adapts to the input type:
- Float64: ~3.5e-15 (still tight enough to catch errors)
- Float32: ~1.9e-6 (appropriate for Float32 precision limits)
This fixes 938 AssertionError cases in spherical polygon intersection.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Polygons with duplicate consecutive vertices (especially at geographic poles) create zero-length edges that cause infinite loops in the polygon clipping tracing algorithm. All longitudes map to the same 3D point at the poles, so coordinates like (180, 90) appearing twice create degenerate edges. Changes: - Add `_vertex_tolerance` function with larger tolerance for spherical coordinates due to floating point errors from transformations - Track `last_point` and `last_vertex_edge_idx` to detect and skip duplicate consecutive vertices in `on_each_a` - Skip zero-length edges in `on_each_maybe_intersect` for both A and B - Add guard for `last_vertex_edge_idx != i` to prevent modifying the wrong vertex when duplicates were skipped - Snap β to zero when very close to avoid near-duplicate points that confuse the tracing algorithm This fixes 5 TracingError cases in spherical polygon intersection. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When an intersection occurs at the END of an A edge (α ≈ 1), the next vertex hasn't been added to a_list yet. Previously we just skipped these, expecting them to be caught at α ≈ 0 of the next edge. But when a vertex of polygon A lies ON an edge of polygon B (not at an endpoint), spherical_arc_intersection only returns the interior crossing, not the vertex-on-edge intersection. Fix: Track pending intersections at α ≈ 1 and apply them when the next vertex is added. Also handle wrap-around for the closing edge. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Tests the case where a vertex of polygon A lies ON an edge of polygon B (but not at an endpoint). This exercises the α≈1 intersection handling where we must track pending intersections and apply them to the next vertex. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Member
Author
|
Closing because this is way too much AI and too little sense for me - might resurrect in future |
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.
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...Also contains a bunch of improvements to UnitSpherical and some plotting stuff for that.
Fixes #363