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

created holes for tetrahedralize(::Mesh, ...) #18

Merged
merged 2 commits into from
May 26, 2021

Conversation

chakravala
Copy link

see issue #17

@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2020

Codecov Report

Merging #18 (b4f6630) into master (9b94238) will increase coverage by 7.30%.
The diff coverage is 90.73%.

❗ Current head b4f6630 differs from pull request most recent head 2d96bd0. Consider uploading reports for the commit 2d96bd0 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
+ Coverage   80.19%   87.50%   +7.30%     
==========================================
  Files           4        6       +2     
  Lines         101      368     +267     
==========================================
+ Hits           81      322     +241     
- Misses         20       46      +26     
Impacted Files Coverage Δ
src/TetGen.jl 100.00% <ø> (ø)
src/rawtetgenio.jl 89.65% <89.65%> (ø)
src/jltetgenio.jl 78.50% <92.85%> (ø)
src/cpptetgenio.jl 94.73% <94.73%> (ø)
src/api.jl 100.00% <100.00%> (+25.00%) ⬆️
src/meshes.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b94238...2d96bd0. Read the comment docs.

@SimonDanisch
Copy link
Member

Thank you!
Do you have some code handy to test this?

@chakravala

This comment was marked as abuse.

@SimonDanisch SimonDanisch requested a review from j-fu May 25, 2021 15:53
@SimonDanisch
Copy link
Member

Yeah hard to tell if it works without a test ;) I don't really know tetgen very well... Tagging @j-fu for a review!

@chakravala

This comment was marked as abuse.

@j-fu
Copy link
Member

j-fu commented May 25, 2021

Essentially, this repo contains two APIs for TetGen, one which is based on GeometryBasics.Mesh which AFAIU does not convey all possibilities of TetGen. I added another API (after your PR) which is essentially array based (very much like that for Triangulate.jl). It is not hard for me to provide a hole example for this one, and from there you can see how to make a hole example for the mesh based API.

@SimonDanisch
Copy link
Member

GeometryBasics.Mesh which AFAIU does not convey all possibilities of TetGen

Which cases are those?

@chakravala

This comment was marked as abuse.

@chakravala

This comment was marked as abuse.

@j-fu
Copy link
Member

j-fu commented May 25, 2021

Hi, just pushed an example how to serve the TetGen API:

function cubewithhole(;vol=1)

One needs to create subregion encircled by facets an put a "holepoint" into it.
See also http://wias-berlin.de/software/tetgen/1.5/doc/manual/manual006.html#ff_poly
and https://juliageometry.github.io/TetGen.jl/stable/#TetGen.RawTetGenIO

@j-fu
Copy link
Member

j-fu commented May 25, 2021

GeometryBasics.Mesh which AFAIU does not convey all possibilities of TetGen

Which cases are those?
May I miss something due to insufficient knowledge how to work with the mesh struct so please correct me on that:
What comes into my mind in addition to holes:

  • different cell region markers: instead of a hole point I can place a "region point" and spread its marker (and a maximal volume) to a whole subregion, creating some integer "cell meta" on output, very much like boundary facet markers
  • local refinement via callback (but this of course wouldn't go into the Mesh API I think it already now could be used together with it)

@chakravala

This comment was marked as abuse.

@j-fu
Copy link
Member

j-fu commented May 26, 2021

Will add a test example after merge.

@SimonDanisch SimonDanisch merged commit 7980c7f into JuliaGeometry:master May 26, 2021
@SimonDanisch
Copy link
Member

Alright, thanks :)

@chakravala

This comment has been minimized.

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.

4 participants