Add separating-axis early-out to ConvexConvexSutherlandHodgman for disjoint inputs#411
Draft
asinghvi17 wants to merge 3 commits into
Draft
Add separating-axis early-out to ConvexConvexSutherlandHodgman for disjoint inputs#411asinghvi17 wants to merge 3 commits into
asinghvi17 wants to merge 3 commits into
Conversation
…sjoint inputs For each edge of either convex polygon, the edge's line (planar) or great circle (spherical) is a candidate separating axis; if all vertices of the other polygon lie strictly outside it, the polygons are disjoint and the empty result is returned before any intermediate point buffers are allocated. Touching and near-touching pairs fall through to the full clip, so results are unchanged. Disjoint spherical pairs go from ~413ns/24 allocations to ~43ns/6 allocations. Fixes #408 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add a `SeparatingAxisCheck` enum (`CheckSeparatingAxis`, the default, and
`SkipSeparatingAxisCheck`) carried as a second type parameter on
`ConvexConvexSutherlandHodgman`, so the disjoint early-out can be disabled
at the type level with no runtime branch:
ConvexConvexSutherlandHodgman(Spherical(), SkipSeparatingAxisCheck)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Name the toggle by intent (checking for disjoint inputs) rather than by mechanism (the separating-axis test), and drop the redundant "Check" from the enum values: `CheckDisjoint` (default) and `SkipDisjointCheck`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
Author
|
This may not actually help in the use case I care about, so I'm going to leave it as a draft for now until I can establish whether it helps. |
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.
Fixes #408.
intersection(ConvexConvexSutherlandHodgman(...), a, b)previously ran the full Sutherland-Hodgman clip — allocating intermediate point buffers — even when the two convex polygons are disjoint, which is the dominant case in workloads like ConservativeRegridding.jl (~80% of spatial-tree candidate pairs).Change
Adds a separating-axis early-out before any buffers are built, for both manifolds:
robust_cross_product, hoisted out of the vertex loop) defines a candidate separating plane. If every vertex of the other polygon lies strictly on the negative side (semantics exactly matchingspherical_orient < 0, same tolerance), the polygons are disjoint and the empty degenerate polygon is returned immediately.Predicates.orient.The check is only a sufficient condition: touching, shared-edge, and exactly-antipodal configurations (where vertices lie on the candidate great circles) fall through to the full clip, so results never change — only speed. Degenerate edges (e.g. the ring's closing point) are skipped, since
robust_cross_product(a, a)returns an arbitrary orthogonal vector that must not be used as a candidate separating plane.Type-level toggle
The early-out is selected at the type level via a new
DisjointCheckenum carried as a second type parameter, defaulting to on:Since the enum value is a type parameter, the branch is resolved at compile time. Existing dispatch on
ConvexConvexSutherlandHodgman{Planar}etc. still works via partial instantiation, and the constructor validates the parameter is aDisjointCheckvalue.Benchmarks (Chairmarks, min time, 1°×1° cells)
The remaining 6 allocations on the disjoint path are the returned degenerate polygon itself. For the ~80%-disjoint regridding mix described in the issue this is roughly a 3× overall speedup of the clipping step; workloads where pairs almost always overlap can opt out with
SkipDisjointCheck.Tests
DisjointChecktoggle: default parameter value, invalid-parameterArgumentError, and identical results with the check disabled on both manifolds.🤖 Generated with Claude Code