Skip to content
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

Add Bentley-Ottman Algorithm #1143

Closed
wants to merge 423 commits into from

Conversation

souma4
Copy link

@souma4 souma4 commented Nov 21, 2024

see issue #1126 and PR (#1125 )[https://github.com//pull/1125]

@souma4 souma4 changed the title Jrc add bentley ottman Add Bentley-Ottman Algorithm Nov 21, 2024
@souma4
Copy link
Author

souma4 commented Feb 10, 2025

finally getting back to this. latest commit message is my perspective

@juliohm
Copy link
Member

juliohm commented Feb 11, 2025

Welcome back, Jeffrey! =)

Could you please refactor the PR to only include the Bentley-Ottman algorithm?

Regarding the output format, we could go for a dict-like data structure as suggested. This could be

  • a Dict{K,V} where {K<:Tuple{Int,Int},V<:Int} or
  • a sparse matrix where the entry M[i,j] has an integer code for the intersection type

I believe this dict-like encoding is very efficient and the Julia compiler can optimize it away in downstream algorithms. Just be careful to provide the type of keys and values, the default Dict constructor can lead to Any types.

eliascarv and others added 27 commits February 11, 2025 09:40
* Preserve the CRS with the 'withcrs' function

* Replace LatLon with Mercator in tests

* Apply suggestions from code review

Co-authored-by: Júlio Hoffimann <[email protected]>

---------

Co-authored-by: Júlio Hoffimann <[email protected]>
* [WIP] Add CRS testset

* Add the latest tests
* Refactor the implementation of the 'rand' function

* Add docstring

* Add tests

* Apply suggestions

* Fix tests

* Update documentation

* Fix tests

* Add documentation

* Apply suggestions

* Update docs/src/rand.md

* Add more tests

---------

Co-authored-by: Júlio Hoffimann <[email protected]>
* [WIP] Remove 'Dim' from types

* Apply suggestions

* Fix Dim methods in: geometries.jl, domains.jl

* Apply suggestions

* Apply suggestions

* Apply suggestions

* Remove 'Dim' form methods of files: utils.jl, viewing.jl, traversing.jl, predicates.jl

* Adjust docstrings

* Apply suggestions

* Apply suggestions from code review

* Add assertdim

* Remove 'Dim' from methods in the files: boundary.jl, clamping.jl, measures.jl, orientation.jl, clipping.jl, intersections.jl

* Add more assertdim

* Fix Dim in docs

* More adjustments

* Update MeshesMakieExt

* Update discretization.jl

* Adjust discretization.jl

* Remove Dim from tests

* Update discretization.jl

* Fix code

* Fix Primitives testset

* Fix Polytopes testset

* Fix Domains, SubDomains and Sets testsets

* Refactor sampling/regular.jl

* Remove more Dim uses in tests

* Fix getindex of Grid

* Fix Predicates testset

* Fix Meshes and Predicates testset

* Fix Partitioning and Discretization testset

* Fix Repair and viz

* Apply suggestions

* Fix allocations in 'assertdim'

* Fix 'BezierCurve' docstring

* Replace 'assertdim' with 'checkdim'

---------

Co-authored-by: Júlio Hoffimann <[email protected]>
* Add manifolds.jl

* Cleanup imports

* Add M paramters to all primitives

* Flip C<>M in primitives.jl

* Add M to polytopes.jl

* Add M to multigeoms.jl

* Add M to domains.jl

* Additional fixes

* More fixes

* Fix meshes

* Delay addition of deps

* Add constructors for 'Point'

* Apply suggestions

* Update Primitives

* Update Polytopes

* Update src/primitives/paraboloidsurface.jl

Co-authored-by: Júlio Hoffimann <[email protected]>

* Apply suggestions

* Update 'axis' function

* Update Domains

* Add partial order for Ellipsoid manifold

* Improve docstring for Box

* Fix tests

* Apply suggestions

* Update tests

* Update aliases

* Add more type parameter restrictions

* Update MeshesMakieExt

* Apply suggestions

* Apply suggestions

* Apply suggestions

* Apply suggestions

* Update 'vizsubdom!' fallback

* Update exports

* Fix viz

* Fix docs

* Fix tests

* Fix viz

---------

Co-authored-by: Elias Carvalho <[email protected]>
Co-authored-by: Elias Carvalho <[email protected]>
* Use flat in bridge

* Fix and test
juliohm and others added 24 commits February 11, 2025 09:40
* Implement indexable api for topology

* Update src/topologies.jl

Co-authored-by: Joshua Lampert <[email protected]>

* Update src/topologies.jl

Co-authored-by: Joshua Lampert <[email protected]>

* Apply suggestions to Domain

---------

Co-authored-by: Joshua Lampert <[email protected]>
…ry#1123)

* Use 'TiledIteration.TileIterator' in 'RegularCoarsening'

* Add tests

* Update comments

* Update src/coarsening/regular.jl

Co-authored-by: Júlio Hoffimann <[email protected]>

* Add more tests

* Apply suggestions

---------

Co-authored-by: Júlio Hoffimann <[email protected]>
* Use 'SVector' in 'Polytope' instead of 'NTuple'

* Update constructors

* Fix constructors

* Fix tests
* Remove unnecessary uses of 'collect'

* Fix code

* Remove comments
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 4 to 5.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md)
- [Commits](codecov/codecov-action@v4...v5)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* initial draft for vertices iteration

* added small (temporary) comments for better initial digestion, will remove later

* refactor into `eachvertex`

* fix: typo in the export list

* fix: Multi -> MultiPolytope, no importing of base methods

* revert to simple generators. Rewrite some function in term of the iterator instead.

* add tests, first part

* fix: change test

* typo

Co-authored-by: Júlio Hoffimann <[email protected]>

* fix: use first instead of getindex,

* revive the custom iterator

* add typecheck to avoid stack overflows.

* better tests

* fix tests

* revert Multi constructor

* Update src/geometries/multigeom.jl

* Update src/geometries/multigeom.jl

* Cleanup

* *removed double definition of the `eachvertex` function
*eltype returns the specific point based on manifold and tranformation.
*added missing `IteratorEltype` trait

* * improved allocation testing
* small cleanup

* Update src/geometries/multigeom.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update src/geometries/multigeom.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update test/meshes.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update test/meshes.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update test/meshes.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update test/meshes.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update test/testutils.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update test/polytopes.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update test/polytopes.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update test/polytopes.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update test/multigeoms.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Update test/meshes.jl

Co-authored-by: Elias Carvalho <[email protected]>

* Apply suggestions from code review

* Add more tests

* Update tests

* Simplify tests

* Simplify tests

* Apply suggestions

* Update test/testutils.jl

Co-authored-by: Júlio Hoffimann <[email protected]>

* Apply suggestions

* Fix tests

---------

Co-authored-by: Júlio Hoffimann <[email protected]>
Co-authored-by: Elias Carvalho <[email protected]>
Co-authored-by: Elias Carvalho <[email protected]>
* Avoid allocations in 'vertex' of 'PolyArea'

* Update 'vertextest'

* Update src/geometries/polytopes/polyarea.jl

Co-authored-by: Júlio Hoffimann <[email protected]>

* Update 'vertices' of 'PolyArea'

---------

Co-authored-by: Júlio Hoffimann <[email protected]>
…y of all vertices and the segments related to that point. There are three ways of handling this in my opinion. 1. return the dictionary and build a separate method that reconstructs the polygon 2. return all points, build a new polygon based on that (dubious on this one) 3. build the polygon with new intersections. I lean the first one because if we want to implement this into a clip algorithm, we'd need the dictionary of vertices so we can track which points belong to which polygon. This would enable two utility methods. One that takes the dictionary ofsegments and makes one polygon, then one that takes dictionary of segments + both polygons to reconstruct each polygon
@souma4 souma4 force-pushed the JRC_add_BentleyOttman branch from 0eecb20 to 6928431 Compare February 11, 2025 16:43
…ictionary of all vertices and the segments related to that point. There are three ways of handling this in my opinion. 1. return the dictionary and build a separate method that reconstructs the polygon 2. return all points, build a new polygon based on that (dubious on this one) 3. build the polygon with new intersections. I lean the first one because if we want to implement this into a clip algorithm, we'd need the dictionary of vertices so we can track which points belong to which polygon. This would enable two utility methods. One that takes the dictionary ofsegments and makes one polygon, then one that takes dictionary of segments + both polygons to reconstruct each polygon"

This reverts commit 6928431.
@souma4
Copy link
Author

souma4 commented Feb 11, 2025

Ah. messed up the refactor. Sorry everyone. I'll make a new draft and cherry pick what I need to clean up the commit history

@souma4 souma4 closed this Feb 11, 2025
@juliohm
Copy link
Member

juliohm commented Feb 11, 2025

Git is always tricky. Thanks for closing and starting a fresh one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants