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

Ruppert's Refinement Algorithm. #133

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Islam0mar
Copy link
Contributor

@Islam0mar Islam0mar commented Aug 9, 2023

Current status --20°threshold--:
a
cdt
guitar
island

Issues:

  • Currently refinement is applied on all triangles inside super triangle which isn't efficient. Better update triangles linkage after deleting super/outer/... triangles, so refinement can be applied on the result.
  • Some cases goes into infinite loop (Constrained Sweden.txt, square with crack.txt).

TODO:

  • Cleanup
  • Increase Steiner point number inside addVertex.
  • Remove not referenced vertices.

Ref.s:

@artem-ogre
Copy link
Owner

Great, thank you for the PR, @Islam0mar!
I will look into it as soon as I will have time.

@artem-ogre
Copy link
Owner

artem-ogre commented Aug 21, 2023

Hi @Islam0mar,
I had some time today to start looking at your code. So far it looks really neat! Thanks a lot for your work.

I did some code refactoring and fixing. You can check it out here.

Constrained Sweden.txt refines now and looks absolutely stunning:
cdt_screenshot

I did not yet have time to look at the core parts of the algorithm, so this is still a work in progress.

@Islam0mar
Copy link
Contributor Author

Islam0mar commented Aug 22, 2023

Hi @artem-ogre,

I have merged your branch and did these changes:

  • Add functions to collect erased triangles to run refinement on non-erased triangles only.
  • Fix split condition.
  • some GUI improvements.

Results:
cdt_screenshot2
cdt_screenshot3
cdt_screenshot4
cdt_screenshot1
cdt_screenshot6
cdt_screenshot5

@Islam0mar
Copy link
Contributor Author

@artem-ogre sorry, I didn't notice your recent refactor.

let me know your opinion about collecting erased triangles.

@artem-ogre
Copy link
Owner

artem-ogre commented Sep 7, 2023

Hi @Islam0mar,

Sorry that I could not find time to continue working on it yet. But I remember about this PR all the time :)

Yes, I think we should implement skipping the 'to-be-erased-later' triangles and I have some ideas of how this could be done. However, I think it is best to first focus on the core of the algorithm and get it right. Skipping triangles is an improvement that can always be implemented on top.

Another feature that's really nice to have is not to attempt refining triangles where both edges of the sharp angle are constrained (because it is futile and leads to infinite loop in the refinement).

@starseeker
Copy link

@artem-ogre @Islam0mar I'm interested in this work as a possibly-more-robust-than-poly2tri CDT for triangle generation from trimmed NURBS surfaces... if there's a potential infinite loop case with constrained edges on sharp angles, would you advise avoiding testing such an application for now?

@artem-ogre
Copy link
Owner

artem-ogre commented Oct 6, 2023

@starseeker
Are you interested particularly in Ruppert refinement or generally in triangulating polygons? Triangulating polygons (constrained or conforming Delaunay triangulation) is already possible in CDT. And if there are no self-intersections in the polygon both methods should be robust (constrained triangulation 100% numerically robust).

@artem-ogre
Copy link
Owner

artem-ogre commented Oct 6, 2023

@starseeker
Generally triangles where an acute angle is formed by two constrained edges cannot be refined without violating the constraints. In this PR (as it is now) this indeed leads to an infinite refinement but we plan to add some strategy to terminate the refinement (e.g., not even attempting to refine triangles with constrained acute angle or adding a lower threshold on such triangle's surface area). Also it is already possible to set a limit on the number of vertices inserted during refinement.

@starseeker
Copy link

starseeker commented Oct 6, 2023

@artem-ogre My specific interest is triangulating a data set with bounding polygons, holes and "interior" points not on polygons - the latter serving to force the generated mesh to more closely conform to a NURBS surface from which we're generating the input data. In poly2tri the latter are Steiner points - I wasn't sure if this work was also adding that capability, or whether the inserted points are only generated algorithmically for refinement purposes.

This report has some visual examples of the sort of thing we're trying to do: https://apps.dtic.mil/sti/pdfs/AD1094344.pdf (Figures 12, 13 and 17 for example) Apologies if I'm a little off topic for this particular pull request.

@artem-ogre
Copy link
Owner

@starseeker
CDT is a general purpose Delaunay triangulation library: input does not have to be a polygon and any arbitrary points can be added. One needs to provide the points however. This PR is implementing a triangulation refinement: new points are inserted into a triangulation so that triangles with acute angles are split into triangles with larger angles until some refinement criterion is reached.

@Islam0mar
Copy link
Contributor Author

Hello @artem-ogre,

I have found the error in Constrained Sweden.txt case:

  • according to WHEN AND WHY RUPPERT’S ALGORITHM WORKS, not all bad triangles should be resolved.
  • some new vertices already exists in the input vertices.
  • tryAddVertexToLocator doesn't handle nan and consume lots of memory--this case happens from duplicated vertices--.

Reading through triangle. I used his condition on equidistant to avoid aggressive splitting and it seems to work.

Let me know your opinion about these updates, so we can move forward with this PR.

@artem-ogre
Copy link
Owner

Hi, Islam. Thanks for continuing working on it. I came to similar conclusions earlier and was thinking about skipping triangles when, e.g. acute angle is formed by constraint edges. Regarding inserting duplicates I recently updated CDT to throw an exception when this happens.

I will check out your changes. Are you available for a call or messaging so we can efficiently discuss how this can be finalized? My email is artem.ogre@gmail.com.

@Islam0mar
Copy link
Contributor Author

Sure. I available for a call or messaging, io1131@fayoum.edu.eg

@sampotter
Copy link

How's this going? Anything I can do to help? Having this feature would be huge.

@artem-ogre
Copy link
Owner

@sampotter
Hello, thanks for your interest.

The status is that I can’t yet find the time and energy to integrate this PR. I work on CDT in my spare time, and I’m on parental leave with two small kids (almost no spare time). Integrating this feature is very high on my TODO. I think that Islam’s fork is in quite a good state and can be used at one’s own risk for CDT refinement “here and now”. I will try to do it this July but no promises.

@sampotter
Copy link

sampotter commented Jun 15, 2024

@artem-ogre Ah, I hope you're enjoying your parental leave. :-)

I just forked both CDT and PythonCDT and hooked Islam's changes up for myself and it seems to work well so far. At least, for the simple example I'm working with, I didn't notice any issues, and tried a reasonable spread of parameters while refining.

Is there anything I can do to help with integration? Or should I just wait until you have more time?

@LukeMinnich
Copy link

LukeMinnich commented Jun 17, 2024

hello! FWIW I would also like to use the work in this branch. I'd like to use CDT in a commercial product, so I plan to perform lots of fuzz testing. If @Islam0mar or anyone else could resolve the branch conflicts, I'd be happy to test and share my findings of any issues. After some initial testing, I'm hitting an infinite loop, and I don't know if that's simply because the fork is behind by several commits. In the meantime, I'll read up on the underlying methods and see if I can't troubleshoot any issues on my own.

@sampotter
Copy link

sampotter commented Jun 19, 2024

@LukeMinnich @artem-ogre I'm actually in a pretty similar situation. We need a Delaunay mesher with refinement that has a more permissive license and haven't been able to reach Shewchuk to work out some kind of deal to use Triangle. I'm not sure to what extent we'll be able to share data, but if we're able to get CDT integrated into our usual workflow, we'll be stress testing it on a continuous stream of examples coming from real CAD geometry and should be able to provide useful fuzzing.

@artem-ogre
Copy link
Owner

@LukeMinnich @artem-ogre I'm actually in a pretty similar situation. We need a Delaunay mesher with refinement that has a more permissive license and haven't been able to reach Shewchuk to work out some kind of deal to use Triangle.

CDT exists because we were not able to reach Shewchuk. And we tried hard.

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.

5 participants