-
Notifications
You must be signed in to change notification settings - Fork 5
Tree based acceleration for polygon clipping / boolean ops #274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
asinghvi17
wants to merge
11
commits into
main
Choose a base branch
from
as/trees
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+657
−103
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
This was referenced Mar 4, 2025
6a29d2a
to
bbd5cac
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
rafaqz
reviewed
Mar 8, 2025
476cf2f
to
be0dd9c
Compare
This was referenced Mar 17, 2025
asinghvi17
added a commit
that referenced
this pull request
Apr 3, 2025
This was factored out of the "dev branch" #259 and contains the subset of changes that apply to GeometryOpsCore, for easier review. Child PRs: #271 (TGGeometry) -> #275 (AdaptivePredicates) -> #273 (clipping algorithm type) -> #274 (trees) - Use [StableTasks.jl](https://github.com/JuliaFolds2/StableTasks.jl) in apply and applyreduce - its type-stable tasks save us some allocations! - Remove `Base.@assume_effects` on the low level functions, which caused issues on Julia v1.11 and was probably incorrect anyway - Add an algorithm interface with an abstract supertype `Algorithm{M <: Manifold}`, as discussed in #247. Also adds an abstract Operator supertype and some discussion in code comments, but no implementation or interface surface there yet. - Split out `types.jl` into a directory `types` with a bunch of files in it, for ease of readability / docs / use. - (out of context change): refactor CI a bit for cleanliness. TODOs for later (not this PR): - [ ] Add a `format` method that takes in an incompletely specified algorithm and some geometry as input, and returns a completely specified algorithm. What does this mean? Imagine I call `GO.intersection(FosterHormannClipping(), geom1, geom2)`. That `FosterHormannClipping()` should get expanded to `FosterHormannClipping(AutoAlgorithm(), AutoAccelerator())`. Then, `format` will take `format(alg, args...)` and: - get the `crstrait` of the two geometries, scan for incompatibilities, assign the correct manifold to the algorithm (maybe warn or emit debug info) - if no geometries available, get the manifold via `best_manifold(::Algorithm)`. - maybe inflate the accelerator by checking `npoint` and later preparations to see what's most efficient, maybe not - depends on what we want!
71b1953
to
a801c2c
Compare
6431ff7
to
9cf5055
Compare
Merged
91b37a9
to
b6b236b
Compare
eef41bd
to
f6d526f
Compare
with optional acceleration via trees and extent thinning
Co-authored-by: Rafael Schouten <rafschouten@gmail.com>
e.g. remove redundant getpoint definitions, etc
do_query -> depth_first_search, etc. Otherwise all is the same.
this probably isn't the best test suite but at least there is something..
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.
Spun out of #259
Parent: #273 (algorithm passthrough in clipping)
Child PRs: none so far, you've hit the leaf node!
Can use STRtrees (expensive to construct, efficient to query) or Natural trees (from
tg
, cheap to construct, ~same query speed for our usecases)There are three methods here:
This algorithm should be easily portable to s2 when that becomes available - and the structure allows e.g. manifold passthrough as well.

This PR also adds a LoopStateMachine module that basically just defines an
Action
wrapper struct, and a macro that can take care of that action wrapper struct.With this macro, a function running within a loop can return
Break()
orContinue()
which indicates to the caller that it should break or continue.E.g.:
TODOs:
tg
benchmarks here