Add CGAL::Surface_mesh support #13653

fdrmrc · 2022-04-28T13:30:57Z

Depends on #13644
This is intended to work in 2d and 3d, and allows to convert a dealii cell to a CGAL::Surface_mesh

luca-heltai · 2022-04-28T16:47:25Z

Only 4864299 refers to this PR.

31c5f13 is included in #13644.

peterrum · 2022-04-29T06:38:37Z

doc/news/changes/minor/20220428MarcoFederLucaHeltai

@@ -0,0 +1,4 @@
+New: Added function CGALWrappers::to_cgal_mesh() to convert a deal.II cell


I would merge the change-log entries so that the major one lists all the features that can be used.

peterrum · 2022-04-29T06:41:51Z

include/deal.II/cgal/surface_mesh.h

+//
+// ---------------------------------------------------------------------
+
+#ifndef dealii_cgal_surface_mesh_h


A general comment (not just related to this library): I think we should consider to not make these folders a top level folder but rather move them into subdirs (here into grid). The same holds also for arborx.

But CGAL does other things in addition to "grid". It also handles point clouds, bounding boxes of trees, etc. Don't you think it makes sense to keep features related to an external library in their own directory when this is the case (e.g, sundials, opencascade)?

But CGAL does other things in addition to "grid". It also handles point clouds, bounding boxes of trees, etc.

I see. It seems we have now too many options to do the same thing ;) Does CGAL work in the distributed setting?

Apart from that I would very much prefer if external libraries where we only provide wrappers (incl. sundials) would not be in the main deal.II include directory but in a 3rd party library. But I guess I am too late for such wishes...

I see your point. If it makes you feel better, there will be also functions like

const Quadrature<spacedim> compute_intersection(Tria::cell_iterator, OtherTria::cell_iterator)

that internally use CGAL. This is not a wrapper, strictly speaking... where would you prefer these functions to be?

(all the types there are native dealii types)

include/deal.II/cgal/surface_mesh.h

source/cgal/surface_mesh.cc

tests/cgal/cgal_surface_mesh_01.cc

fdrmrc · 2022-04-30T00:08:57Z

@peterrum Thanks for the review. I addressed some of your comments.

peterrum

I left some minor comments. Good to go, once the other PR is merged!

source/cgal/surface_mesh.cc

include/deal.II/cgal/surface_mesh.h

luca-heltai · 2022-05-01T21:55:47Z

@peterrum, I think we have addressed all your comments.

luca-heltai · 2022-05-01T21:56:09Z

/rebuild

peterrum

Good to go, once you have rebased!

peterrum · 2022-05-02T06:52:45Z

include/deal.II/cgal/surface_mesh.h

+//
+// ---------------------------------------------------------------------
+
+#ifndef dealii_cgal_surface_mesh_h


peterrum · 2022-05-02T06:58:50Z

@luca-heltai

[ 81%] Building CXX object source/base/CMakeFiles/obj_base_debug.dir/polynomials_rannacher_turek.cc.o
In file included from /home/runner/work/dealii/dealii/bundled/boost-1.70.0/include/boost/thread/tss.hpp:8:0,
                 from /usr/include/CGAL/CORE/MemoryPool.h:44,
                 from /usr/include/CGAL/CORE/Impl.h:56,
                 from /usr/include/CGAL/CORE/extLong.h:41,
                 from /usr/include/CGAL/CORE/CoreDefs.h:41,
                 from /usr/include/CGAL/CORE/CORE.h:39,
                 from /usr/include/CGAL/CORE_coercion_traits.h:33,
                 from /usr/include/CGAL/CORE_Expr.h:29,
                 from /usr/include/CGAL/Exact_predicates_exact_constructions_kernel_with_sqrt.h:33,
                 from /home/runner/work/dealii/dealii/include/deal.II/cgal/utilities.h:24,
                 from /home/runner/work/dealii/dealii/include/deal.II/cgal/surface_mesh.h:25,
                 from /home/runner/work/dealii/dealii/source/cgal/surface_mesh.cc:18:
/home/runner/work/dealii/dealii/bundled/boost-1.70.0/include/boost/thread/detail/config.hpp:14:10: fatal error: boost/thread/detail/thread_safety.hpp: No such file or directory
 #include <boost/thread/detail/thread_safety.hpp>
``

There seems to be an issue with the compilation (includes).

luca-heltai · 2022-05-02T08:01:23Z

This is failing with bundled boost. This header is missing. I'm adding it to our bundled boost.

#include <boost/thread/detail/thread_safety.hpp>

luca-heltai · 2022-05-02T08:02:36Z

Also, @peterrum, shall we rename the functions to be more verbose also here? Something like dealii_cell_to_cgal_surface_mesh anddealii_triangulation_to_cgal_surface_mesh?

peterrum · 2022-05-02T08:10:09Z

Something like dealii_cell_to_cgal_surface_mesh anddealii_triangulation_to_cgal_surface_mesh?

Sounds good 👍 I guess we will have at some stage functions that work in the opposite direction?

fdrmrc · 2022-05-02T08:16:41Z

Sounds good 👍 I guess we will have at some stage functions that work in the opposite direction?

Yes, I'm working on it right now! @peterrum

peterrum

OK from my side once the compilation issue has been sorted out!

luca-heltai · 2022-05-02T22:21:57Z

I need help with the FindCGAL.cmake script. I wasted more hours than I can afford trying to get this working correctly on all three machines I have, and I don't seem to be able to do anything useful at this time.

Here are the issues:

Versions < 5.0.0 of CGAL are not header only
Also header only versions of CGAL maybe have some compiled stuff
While find_package(CGAL) seems to work fine, I am not able to extract information in a consistent way and feed it back into deal.II

I think I need @tamiko's help here.

For users' codes, I do

find_package(CGAL REQUIRED)

target_link_libraries(${target} CGAL::CGAL)
include_directories(${CGAL_INCLUDE_DIRS})

In the FindCGAL.cmake file, however, I am not able to extract information from the CGAL::CGAL target in a consistent way, i.e., this does not seem to work consistently across different systems and cmake versions:

GET_PROPERTY(CGAL_INCLUDE_DIRS TARGET CGAL::CGAL PROPERTY INTERFACE_INCLUDE_DIRECTORIES)

some versions do, and some don't have the INTERFACE_INCLUDE_DIRECTORIES, and some do and don't have GET_PROPERTY(CGAL_LIBRARIES TARGET CGAL::CGAL PROPERTY INTERFACE_LINK_LIBRARIES)...

Any ideas?

luca-heltai · 2022-05-04T05:44:26Z

@peterrum, for the moment I fixed the compilation by requiring a newer version of CGAL. In a followup PR I will update the simplex runner to 20.04 (20.04 ships with CGAL 5). Is there a reason for having it 18.04 in the first place?

peterrum · 2022-05-04T05:55:40Z

It used to be that all linux runners used 18.04. But recently we switch one to be able to test C++20: #13535

luca-heltai · 2022-05-04T06:39:13Z

@peterrum, can we merge this, and address the problem of the runner in a later PR? @fdrmrc is holding back a lot more stuff (there's about 5 more PRs to add), and I don't want those to be held back by the tester here.

peterrum

Let's go ahead. @luca-heltai Could you open an issue to track the described problem.

luca-heltai · 2022-05-04T06:56:33Z

#13668

fdrmrc force-pushed the cgal-surface-mesh branch from 738f207 to 591a201 Compare April 28, 2022 13:41

luca-heltai force-pushed the cgal-surface-mesh branch 2 times, most recently from 5b5530d to 4864299 Compare April 28, 2022 16:45

peterrum reviewed Apr 29, 2022

View reviewed changes

fdrmrc force-pushed the cgal-surface-mesh branch from fe0a87c to 647c05e Compare April 30, 2022 00:04

peterrum approved these changes May 1, 2022

View reviewed changes

luca-heltai force-pushed the cgal-surface-mesh branch from 07cdfb3 to 50ae8d4 Compare May 1, 2022 21:54

luca-heltai added ready for review ready to test labels May 1, 2022

peterrum approved these changes May 2, 2022

View reviewed changes

fdrmrc and others added 4 commits May 2, 2022 13:30

Added CGALWrappers::to_cgal_mesh()

684ffe8

Address some comments

54d50ef

Get ref_cells by reference

0180620

Code review.

222c472

luca-heltai force-pushed the cgal-surface-mesh branch from 50ae8d4 to f56fd40 Compare May 2, 2022 10:31

Renamed function. Added boost header.

aafc7e0

luca-heltai force-pushed the cgal-surface-mesh branch from f56fd40 to aafc7e0 Compare May 2, 2022 10:33

Trying to fix FindCGAL.

6945366

peterrum approved these changes May 2, 2022

View reviewed changes

Make sure we use recent versions of CGAL.

926318c

peterrum approved these changes May 4, 2022

View reviewed changes

peterrum merged commit 73b814f into dealii:master May 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CGAL::Surface_mesh support #13653

Add CGAL::Surface_mesh support #13653

fdrmrc commented Apr 28, 2022

luca-heltai commented Apr 28, 2022

peterrum Apr 29, 2022

peterrum Apr 29, 2022

luca-heltai Apr 29, 2022

peterrum May 1, 2022

luca-heltai May 1, 2022

luca-heltai May 1, 2022

peterrum May 2, 2022

fdrmrc commented Apr 30, 2022

peterrum left a comment

luca-heltai commented May 1, 2022

luca-heltai commented May 1, 2022

peterrum left a comment

peterrum May 2, 2022

peterrum commented May 2, 2022

luca-heltai commented May 2, 2022

luca-heltai commented May 2, 2022 •

edited

peterrum commented May 2, 2022

fdrmrc commented May 2, 2022

peterrum left a comment

luca-heltai commented May 2, 2022

luca-heltai commented May 4, 2022

peterrum commented May 4, 2022

luca-heltai commented May 4, 2022

peterrum left a comment

luca-heltai commented May 4, 2022

		@@ -0,0 +1,4 @@
		New: Added function CGALWrappers::to_cgal_mesh() to convert a deal.II cell

Add CGAL::Surface_mesh support #13653

Add CGAL::Surface_mesh support #13653

Conversation

fdrmrc commented Apr 28, 2022

luca-heltai commented Apr 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fdrmrc commented Apr 30, 2022

peterrum left a comment

Choose a reason for hiding this comment

luca-heltai commented May 1, 2022

luca-heltai commented May 1, 2022

peterrum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterrum commented May 2, 2022

luca-heltai commented May 2, 2022

luca-heltai commented May 2, 2022 • edited

peterrum commented May 2, 2022

fdrmrc commented May 2, 2022

peterrum left a comment

Choose a reason for hiding this comment

luca-heltai commented May 2, 2022

luca-heltai commented May 4, 2022

peterrum commented May 4, 2022

luca-heltai commented May 4, 2022

peterrum left a comment

Choose a reason for hiding this comment

luca-heltai commented May 4, 2022

luca-heltai commented May 2, 2022 •

edited