Skip to content

Conversation

@dgleich
Copy link
Contributor

@dgleich dgleich commented Nov 11, 2022

I wanted to get the Delaunay edges out of this tesselator too, so here is one proposed implementation. An alternative would be to communicate the rt structure back to the caller, but that seems more fiddly as there are all the point wise changes.

@robertdj
Copy link
Collaborator

Thanks for opening this PR. Just want to make sure I understand your motivation, as I haven't used the Delaunay capabilities in VoronoiDelaunay.

Does the Delaunay iterator from VoronoiDelaunay satisfy your needs? Or does it have the issues similar to the Voronoi iterator? Off the top of my head I don't see any issues like interaction with the observation window since the lines go between your points.

@dgleich
Copy link
Contributor Author

dgleich commented Nov 13, 2022

The Delaunay triangulation is just on the original point set, so (as far as I know) it will be correct for point set as is (that is, no additional correction needed due to the observation window).

The only possible "issue" is that the Delaunay and Voronoi ought to be dual to each other. (so you can use Delaunay edges to find adjacencies between Voronoi cells... which is the specific use I had). In this case, there may be additional adjacencies that aren't present in the observation window but are present in the "dual". This is a common issue and Wikipedia has a note on it (see the section with the phrase "infinite rays")

So if you actually just wanted adjacencies, it would require dropping a few of the edges on the boundary. But then this wouldn't actually be a triangulation.

@robertdj
Copy link
Collaborator

Sorry about my long absence. That makes sense. Let me take a closer look at your code.

@codecov
Copy link

codecov bot commented Nov 21, 2022

Codecov Report

Merging #33 (0d372fd) into master (327e34e) will increase coverage by 0.33%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
+ Coverage   89.04%   89.38%   +0.33%     
==========================================
  Files           7        7              
  Lines         219      226       +7     
==========================================
+ Hits          195      202       +7     
  Misses         24       24              
Impacted Files Coverage Δ
src/Cells.jl 88.52% <85.71%> (+1.48%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@robertdj
Copy link
Collaborator

robertdj commented Dec 7, 2022

Looks fine! I'll merge it.

(Again, please excuse my long response times.)

@robertdj robertdj merged commit 10dd2b9 into JuliaGeometry:master Dec 7, 2022
@robertdj
Copy link
Collaborator

robertdj commented Dec 7, 2022

@dgleich Ahh. In hindsight I was a bit too hasty. The Project.toml should also be updated. Would you mind checking compatibility with dependencies and bump the version to 0.4.0 in a new PR?

@dgleich
Copy link
Contributor Author

dgleich commented Dec 8, 2022

Yeah, I'll try to get around to it soon, but the next two weeks are ... frustratingly busy :)

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.

2 participants