From b831e41013b95b11a392b3e0190c558a694c296a Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Wed, 25 Sep 2019 13:03:12 +0200 Subject: [PATCH 1/8] Use a bbox in Side-of_triangle_mesh until a request is inside, only then use the tree. Make it faster when working with disjoint meshes. --- .../include/CGAL/Side_of_triangle_mesh.h | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h index 7b5b1254f43..10e75779b99 100644 --- a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h +++ b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h @@ -28,6 +28,7 @@ #include #include +#include #include #include @@ -104,6 +105,8 @@ class Side_of_triangle_mesh typename GeomTraits::Construct_vector_3 vector_functor; const AABB_tree_* tree_ptr; bool own_tree; + CGAL::Bbox_3 box; + mutable bool tree_built; public: @@ -135,6 +138,8 @@ public: tree_ptr = new AABB_tree(faces(tmesh).first, faces(tmesh).second, tmesh, vpmap); + box = Polygon_mesh_processing::bbox(tmesh, parameters::vertex_point_map(vpmap)); + tree_built = false; } /** @@ -157,6 +162,8 @@ public: tree_ptr = new AABB_tree(faces(tmesh).first, faces(tmesh).second, tmesh); + box = Polygon_mesh_processing::bbox(tmesh); + tree_built = false; } /** @@ -195,8 +202,21 @@ public: */ Bounded_side operator()(const Point& point) const { + bool is_outside = tree_built; + if(!is_outside){ + is_outside = (point.x() < box.xmin() + || point.x() > box.xmax() + || point.y() < box.ymin() + || point.y() > box.ymax() + || point.z() < box.zmin() + || point.z() > box.zmax()); + if(is_outside) + return CGAL::ON_UNBOUNDED_SIDE; + } + + tree_built = true; return internal::Point_inside_vertical_ray_cast()( - point, *tree_ptr, ray_functor, vector_functor); + point, *tree_ptr, ray_functor, vector_functor); } }; From c18ad651d69a149b4d46afbd874b2dc5ad1a98e3 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Tue, 1 Oct 2019 10:19:54 +0200 Subject: [PATCH 2/8] Fix last constro --- Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h index 10e75779b99..4e21aff044d 100644 --- a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h +++ b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h @@ -182,6 +182,7 @@ public: , tree_ptr(&tree) , own_tree(false) { + tree_built = false; } ~Side_of_triangle_mesh() From e42c87b54f3f2aabc172ba3edb0db2c794a1288d Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Wed, 2 Oct 2019 09:35:41 +0200 Subject: [PATCH 3/8] add the missing box --- Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h index 4e21aff044d..82b078df652 100644 --- a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h +++ b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h @@ -183,6 +183,7 @@ public: , own_tree(false) { tree_built = false; + box = tree.bbox(); } ~Side_of_triangle_mesh() From 130d4a5e24d13f146112e33776a83f20f0ad9c36 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Wed, 2 Oct 2019 09:56:07 +0200 Subject: [PATCH 4/8] remove useless bool --- .../include/CGAL/Side_of_triangle_mesh.h | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h index 82b078df652..ec11f7482cf 100644 --- a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h +++ b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h @@ -204,18 +204,15 @@ public: */ Bounded_side operator()(const Point& point) const { - bool is_outside = tree_built; - if(!is_outside){ - is_outside = (point.x() < box.xmin() - || point.x() > box.xmax() - || point.y() < box.ymin() - || point.y() > box.ymax() - || point.z() < box.zmin() - || point.z() > box.zmax()); - if(is_outside) + if(!tree_built){ + if(point.x() < box.xmin() + || point.x() > box.xmax() + || point.y() < box.ymin() + || point.y() > box.ymax() + || point.z() < box.zmin() + || point.z() > box.zmax()) return CGAL::ON_UNBOUNDED_SIDE; } - tree_built = true; return internal::Point_inside_vertical_ray_cast()( point, *tree_ptr, ray_functor, vector_functor); From fe746764cf3dab58a70afa7945c4ab26a0489d45 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Wed, 2 Oct 2019 15:20:18 +0200 Subject: [PATCH 5/8] remove the bool. --- .../include/CGAL/Side_of_triangle_mesh.h | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h index ec11f7482cf..de8b78f0834 100644 --- a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h +++ b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h @@ -106,7 +106,6 @@ class Side_of_triangle_mesh const AABB_tree_* tree_ptr; bool own_tree; CGAL::Bbox_3 box; - mutable bool tree_built; public: @@ -139,7 +138,6 @@ public: faces(tmesh).second, tmesh, vpmap); box = Polygon_mesh_processing::bbox(tmesh, parameters::vertex_point_map(vpmap)); - tree_built = false; } /** @@ -163,7 +161,6 @@ public: faces(tmesh).second, tmesh); box = Polygon_mesh_processing::bbox(tmesh); - tree_built = false; } /** @@ -182,7 +179,6 @@ public: , tree_ptr(&tree) , own_tree(false) { - tree_built = false; box = tree.bbox(); } @@ -204,18 +200,20 @@ public: */ Bounded_side operator()(const Point& point) const { - if(!tree_built){ - if(point.x() < box.xmin() - || point.x() > box.xmax() - || point.y() < box.ymin() - || point.y() > box.ymax() - || point.z() < box.zmin() - || point.z() > box.zmax()) - return CGAL::ON_UNBOUNDED_SIDE; + if(point.x() < box.xmin() + || point.x() > box.xmax() + || point.y() < box.ymin() + || point.y() > box.ymax() + || point.z() < box.zmin() + || point.z() > box.zmax()) + { + return CGAL::ON_UNBOUNDED_SIDE; + } + else + { + return internal::Point_inside_vertical_ray_cast()( + point, *tree_ptr, ray_functor, vector_functor); } - tree_built = true; - return internal::Point_inside_vertical_ray_cast()( - point, *tree_ptr, ray_functor, vector_functor); } }; From acec5469feecc78f1556597e93c0736ee513cd7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Thu, 10 Oct 2019 07:57:26 +0200 Subject: [PATCH 6/8] lazily build the tree --- .../include/CGAL/Side_of_triangle_mesh.h | 58 ++++++++++++------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h index de8b78f0834..b0923db9cda 100644 --- a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h +++ b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h @@ -76,7 +76,7 @@ namespace CGAL { */ template + typename VertexPointMap__> struct AABB_tree_default { typedef CGAL::AABB_face_graph_triangle_primitive Primitive; + VertexPointMap__> Primitive; typedef CGAL::AABB_traits Traits; typedef CGAL::AABB_tree type; }; typedef typename Default::Lazy_get + VertexPointMap_> >::type AABB_tree_; + typedef typename Default::Get::const_type>::type + VertexPointMap; typedef typename GeomTraits::Point_3 Point; //members typename GeomTraits::Construct_ray_3 ray_functor; typename GeomTraits::Construct_vector_3 vector_functor; - const AABB_tree_* tree_ptr; + mutable const AABB_tree_* tree_ptr; + const TriangleMesh* tm_ptr; + boost::optional opt_vpm; bool own_tree; CGAL::Bbox_3 box; +#ifdef CGAL_HAS_THREADS + mutable CGAL_MUTEX tree_mutex; +#endif public: @@ -129,14 +138,13 @@ public: const GeomTraits& gt=GeomTraits()) : ray_functor(gt.construct_ray_3_object()) , vector_functor(gt.construct_vector_3_object()) + , tree_ptr(nullptr) + , tm_ptr(&tmesh) + , opt_vpm(vpmap) , own_tree(true) { CGAL_assertion(CGAL::is_triangle_mesh(tmesh)); CGAL_assertion(CGAL::is_closed(tmesh)); - - tree_ptr = new AABB_tree(faces(tmesh).first, - faces(tmesh).second, - tmesh, vpmap); box = Polygon_mesh_processing::bbox(tmesh, parameters::vertex_point_map(vpmap)); } @@ -150,18 +158,8 @@ public: */ Side_of_triangle_mesh(const TriangleMesh& tmesh, const GeomTraits& gt=GeomTraits()) - : ray_functor(gt.construct_ray_3_object()) - , vector_functor(gt.construct_vector_3_object()) - , own_tree(true) - { - CGAL_assertion(CGAL::is_triangle_mesh(tmesh)); - CGAL_assertion(CGAL::is_closed(tmesh)); - - tree_ptr = new AABB_tree(faces(tmesh).first, - faces(tmesh).second, - tmesh); - box = Polygon_mesh_processing::bbox(tmesh); - } + : Side_of_triangle_mesh(tmesh, get(vertex_point, tmesh), gt) + {} /** * Constructor that takes a pre-built \cgal `AABB_tree` @@ -184,7 +182,7 @@ public: ~Side_of_triangle_mesh() { - if (own_tree) + if (own_tree && tree_ptr!=nullptr) delete tree_ptr; } @@ -211,6 +209,22 @@ public: } else { + // Lazily build the tree only when needed + if (tree_ptr==nullptr) + { +#ifdef CGAL_HAS_THREADS + CGAL_SCOPED_LOCK(tree_mutex); +#endif + CGAL_assertion(tm_ptr != nullptr && opt_vpm!=boost::none); + if (tree_ptr==nullptr) + { + tree_ptr = new AABB_tree(faces(*tm_ptr).first, + faces(*tm_ptr).second, + *tm_ptr, *opt_vpm); + const_cast(tree_ptr)->build(); + } + } + return internal::Point_inside_vertical_ray_cast()( point, *tree_ptr, ray_functor, vector_functor); } From 9221290bef3b1e6c2773678a2497bd93f98942b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Fri, 11 Oct 2019 08:07:02 +0200 Subject: [PATCH 7/8] add missing const --- .../include/CGAL/AABB_face_graph_triangle_primitive.h | 2 +- .../CGAL/AABB_halfedge_graph_segment_primitive.h | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/AABB_tree/include/CGAL/AABB_face_graph_triangle_primitive.h b/AABB_tree/include/CGAL/AABB_face_graph_triangle_primitive.h index 7663d0d78c7..563f9fa665b 100644 --- a/AABB_tree/include/CGAL/AABB_face_graph_triangle_primitive.h +++ b/AABB_tree/include/CGAL/AABB_face_graph_triangle_primitive.h @@ -47,7 +47,7 @@ namespace CGAL { *\tparam FaceGraph is a model of the face graph concept. *\tparam VertexPointPMap is a property map with `boost::graph_traits::%vertex_descriptor` * as key type and a \cgal Kernel `Point_3` as value type. - * The default is `typename boost::property_map< FaceGraph,vertex_point_t>::%type`. + * The default is `typename boost::property_map< FaceGraph,vertex_point_t>::%const_type`. *\tparam OneFaceGraphPerTree is either `CGAL::Tag_true` or `CGAL::Tag_false`. * In the former case, we guarantee that all the primitives will be from a * common `FaceGraph` and some data will be factorized so that the size of diff --git a/AABB_tree/include/CGAL/AABB_halfedge_graph_segment_primitive.h b/AABB_tree/include/CGAL/AABB_halfedge_graph_segment_primitive.h index 1eb27965e6c..870fea421af 100644 --- a/AABB_tree/include/CGAL/AABB_halfedge_graph_segment_primitive.h +++ b/AABB_tree/include/CGAL/AABB_halfedge_graph_segment_primitive.h @@ -58,7 +58,7 @@ namespace CGAL { * \tparam HalfedgeGraph is a model of the halfedge graph concept. * as key type and a \cgal Kernel `Point_3` as value type. * \tparam VertexPointPMap is a property map with `boost::graph_traits::%vertex_descriptor`. - * The default is `typename boost::property_map< HalfedgeGraph,vertex_point_t>::%type`. + * The default is `typename boost::property_map< HalfedgeGraph,vertex_point_t>::%const_type`. * \tparam OneHalfedgeGraphPerTree is either `CGAL::Tag_true` or `CGAL::Tag_false`. * In the former case, we guarantee that all the primitives will be from a * common `HalfedgeGraph` and some data will be factorized so that the size of @@ -86,17 +86,17 @@ class AABB_halfedge_graph_segment_primitive HalfedgeGraph, typename Default::Get::type >::type >, + vertex_point_t>::const_type >::type >, Source_point_from_edge_descriptor_map< HalfedgeGraph, typename Default::Get::type >::type >, + vertex_point_t>::const_type >::type >, OneHalfedgeGraphPerTree, CacheDatum > #endif { - typedef typename Default::Get::type >::type VertexPointPMap_; + typedef typename Default::Get::const_type >::type VertexPointPMap_; typedef typename boost::graph_traits::edge_descriptor ED; typedef typename boost::mpl::if_ >::type Id_; @@ -127,7 +127,7 @@ public: /*! The point type. */ - typedef boost::property_traits< boost::property_map< HalfedgeGraph, vertex_point_t>::type >::value_type Point; + typedef boost::property_traits< boost::property_map< HalfedgeGraph, vertex_point_t>::const_type >::value_type Point; /*! Geometric data type. */ From 7a233375b23c36a65daaa8cf2c2c65a30b5e18f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Fri, 11 Oct 2019 08:07:21 +0200 Subject: [PATCH 8/8] add missing const if non-const was provided, the same map should have been provided to the tree --- Mesh_3/include/CGAL/Polyhedral_mesh_domain_3.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Mesh_3/include/CGAL/Polyhedral_mesh_domain_3.h b/Mesh_3/include/CGAL/Polyhedral_mesh_domain_3.h index 6b013766433..136d8cb3813 100644 --- a/Mesh_3/include/CGAL/Polyhedral_mesh_domain_3.h +++ b/Mesh_3/include/CGAL/Polyhedral_mesh_domain_3.h @@ -215,7 +215,7 @@ public: struct Primitive_type { //setting OneFaceGraphPerTree to false transforms the id type into //std::pair. - typedef AABB_face_graph_triangle_primitive::type, CGAL::Tag_false> type; + typedef AABB_face_graph_triangle_primitive::const_type, CGAL::Tag_false> type; static typename IGT_::Triangle_3 datum(const typename type::Id primitive_id) {