diff --git a/AABB_tree/benchmark/AABB_tree/CMakeLists.txt b/AABB_tree/benchmark/AABB_tree/CMakeLists.txt index 6199f3c0f0cd..1a0f49a76f40 100644 --- a/AABB_tree/benchmark/AABB_tree/CMakeLists.txt +++ b/AABB_tree/benchmark/AABB_tree/CMakeLists.txt @@ -1,31 +1,11 @@ -# Created by the script cgal_create_CMakeLists -# This is the CMake script for compiling a set of CGAL applications. +# Created by the script cgal_create_cmake_script +# This is the CMake script for compiling a CGAL application. -cmake_minimum_required(VERSION 3.1...3.15) -project( AABB_traits_benchmark) +cmake_minimum_required(VERSION 3.1...3.14) +project( AABB_traits_benchmark ) -# CGAL and its components -find_package( CGAL QUIET) -if ( CGAL_FOUND ) - include( ${CGAL_USE_FILE} ) -else () - message(STATUS "This project requires the CGAL library, and will not be compiled.") - return() - -endif() - - -# Boost and its components -find_package( Boost REQUIRED ) -# include for local directory -if ( NOT Boost_FOUND ) - message(STATUS "This project requires the Boost library, and will not be compiled.") - return() -endif() - -# include for local package -include_directories( BEFORE ../../include ) - -add_executable (test_ test.cpp) +find_package(CGAL REQUIRED QUIET OPTIONAL_COMPONENTS Core ) +create_single_source_cgal_program( "test.cpp" ) +create_single_source_cgal_program( "tree_construction.cpp" ) diff --git a/AABB_tree/benchmark/AABB_tree/tree_construction.cpp b/AABB_tree/benchmark/AABB_tree/tree_construction.cpp new file mode 100644 index 000000000000..3ce2948b0506 --- /dev/null +++ b/AABB_tree/benchmark/AABB_tree/tree_construction.cpp @@ -0,0 +1,124 @@ +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +int longest_axis(const CGAL::Bbox_3& bbox) +{ + const double dx = bbox.xmax() - bbox.xmin(); + const double dy = bbox.ymax() - bbox.ymin(); + const double dz = bbox.zmax() - bbox.zmin(); + return (dx>=dy) ? ((dx>=dz) ? 0 : 2) : ((dy>=dz) ? 1 : 2); +} + +template +struct Split_primitives +{ + Split_primitives(RPM rpm) + : rpm(rpm) + {} + + template + void operator()(PrimitiveIterator first, + PrimitiveIterator beyond, + const CGAL::Bbox_3& bbox) const + { + PrimitiveIterator middle = first + (beyond - first)/2; + typedef typename std::iterator_traits::value_type Primitive; + const int crd=longest_axis(bbox); + const RPM& l_rpm=rpm; + std::nth_element(first, middle, beyond, + [l_rpm, crd](const Primitive& p1, const Primitive& p2){ return get(l_rpm, p1.id())[crd] < get(l_rpm, p2.id())[crd];}); + } + RPM rpm; +}; + +template +struct Compute_bbox { + Compute_bbox(const BBM& bbm) + : bbm(bbm) + {} + + template + CGAL::Bbox_3 operator()(ConstPrimitiveIterator first, + ConstPrimitiveIterator beyond) const + { + CGAL::Bbox_3 bbox = get(bbm, first->id()); + for(++first; first != beyond; ++first) + { + bbox += get(bbm, first->id()); + } + return bbox; + } + BBM bbm; +}; + +template +void run(std::string input) +{ + typedef typename K::Point_3 Point_3; + typedef CGAL::Surface_mesh Mesh; + typedef CGAL::AABB_face_graph_triangle_primitive Primitive; + typedef CGAL::AABB_traits Traits; + typedef CGAL::AABB_tree Tree; + + Mesh tm; + std::ifstream(input) >> tm; + + { + Tree tree(faces(tm).begin(), faces(tm).end(), tm); + CGAL::Timer time; + time.start(); + tree.build(); + time.stop(); + std::cout << " build() time: " << time.time() << "\n"; + } + + { + Tree tree(faces(tm).begin(), faces(tm).end(), tm); + CGAL::Timer time; + time.start(); + + typedef CGAL::Pointer_property_map::type BBM; + typedef CGAL::Pointer_property_map::type RPM; // EPIC on purpose here + + std::vector v_bb; + std::vector v_rp; + + std::size_t nbf = num_faces(tm); + v_bb.resize(nbf); + v_rp.resize(nbf); + BBM bbm = CGAL::make_property_map(v_bb); + RPM rpm = CGAL::make_property_map(v_rp); + + CGAL::Cartesian_converter to_input; + for(typename Mesh::Face_index f : tm.faces()) + { + v_bb[f]=CGAL::Polygon_mesh_processing::face_bbox(f, tm); + v_rp[f]=to_input(tm.point(target(halfedge(f, tm), tm))); + } + + Compute_bbox compute_bbox(bbm); + Split_primitives split_primitives(rpm); + tree.custom_build(compute_bbox, split_primitives); + time.stop(); + std::cout << " custom_build() time: " << time.time() << "\n"; + } +} + +int main(int, char** argv) +{ + std::cout << "Build with Epick\n"; + run(argv[1]); + std::cout << "Build with Epeck\n"; + run(argv[1]); + return EXIT_SUCCESS; +} diff --git a/AABB_tree/doc/AABB_tree/Concepts/AABBGeomTraits.h b/AABB_tree/doc/AABB_tree/Concepts/AABBGeomTraits.h index 2f0d0926c221..6f732e503400 100644 --- a/AABB_tree/doc/AABB_tree/Concepts/AABBGeomTraits.h +++ b/AABB_tree/doc/AABB_tree/Concepts/AABBGeomTraits.h @@ -132,62 +132,62 @@ typedef unspecified_type Equal_3; /// @{ /*! -Returns the intersection detection functor. +returns the intersection detection functor. */ Do_intersect_3 do_intersect_3_object(); /*! -Returns the intersection constructor. +returns the intersection constructor. */ Intersect_3 intersect_3_object(); /*! -Returns the sphere constructor. +returns the sphere constructor. */ Construct_sphere_3 construct_sphere_3_object(); /*! -Returns the closest point constructor. +returns the closest point constructor. */ Construct_projected_point_3 construct_projected_point_3_object(); /*! -Returns the compare distance constructor. +returns the compare distance constructor. */ Compare_distance_3 compare_distance_3_object(); /*! -Returns the closest point constructor. +returns the closest point constructor. */ Has_on_bounded_side_3 has_on_bounded_side_3_object(); /*! -Returns the squared radius functor. +returns the squared radius functor. */ Compute_squared_radius_3 compute_squared_radius_3_object(); /*! -Returns the squared distance functor. +returns the squared distance functor. */ Compute_squared_distance_3 compute_squared_distance_3_object(); /*! -Returns the `Less_x_3` functor. +returns the `Less_x_3` functor. */ Less_x_3 less_x_3_object(); /*! -Returns the `Less_y_3` functor. +returns the `Less_y_3` functor. */ Less_y_3 less_y_3_object(); /*! -Returns the `Less_z_3` functor. +returns the `Less_z_3` functor. */ Less_z_3 less_z_3_object(); /*! -Returns the equal functor. +returns the equal functor. */ Equal_3 equal_3_object(); diff --git a/AABB_tree/doc/AABB_tree/Concepts/AABBPrimitive.h b/AABB_tree/doc/AABB_tree/Concepts/AABBPrimitive.h index 1e6ece8051c9..11195b5808ec 100644 --- a/AABB_tree/doc/AABB_tree/Concepts/AABBPrimitive.h +++ b/AABB_tree/doc/AABB_tree/Concepts/AABBPrimitive.h @@ -56,17 +56,17 @@ typedef unspecified_type Id; /// @{ /*! -Returns the datum (geometric object) represented by the primitive. +returns the datum (geometric object) represented by the primitive. */ Datum_reference datum(); /*! -Returns the corresponding identifier. This identifier is only used as a reference for the objects in the output of the `AABB_tree` methods. +returns the corresponding identifier. This identifier is only used as a reference for the objects in the output of the `AABB_tree` methods. */ Id id(); /*! -Returns a 3D point located on the geometric object represented by the primitive. This function is used to sort the primitives during the AABB tree construction as well as to construct the search KD-tree internal to the AABB tree used to accelerate distance queries. +returns a 3D point located on the geometric object represented by the primitive. This function is used to sort the primitives during the AABB tree construction as well as to construct the search KD-tree internal to the AABB tree used to accelerate distance queries. */ Point_reference reference_point(); diff --git a/AABB_tree/doc/AABB_tree/Concepts/AABBPrimitiveWithSharedData.h b/AABB_tree/doc/AABB_tree/Concepts/AABBPrimitiveWithSharedData.h index fb740edf212e..3c47d53d5529 100644 --- a/AABB_tree/doc/AABB_tree/Concepts/AABBPrimitiveWithSharedData.h +++ b/AABB_tree/doc/AABB_tree/Concepts/AABBPrimitiveWithSharedData.h @@ -66,22 +66,22 @@ typedef unspecified_type Shared_data; /// \name Operations /// @{ /*! -Returns the datum (geometric object) represented by the primitive. +returns the datum (geometric object) represented by the primitive. */ Datum_reference datum(const Shared_data& data); /*! -Returns the corresponding identifier. This identifier is only used as a reference for the objects in the output of the `AABB_tree` methods. +returns the corresponding identifier. This identifier is only used as a reference for the objects in the output of the `AABB_tree` methods. */ Id id(); /*! -Returns a 3D point located on the geometric object represented by the primitive. This function is used to sort the primitives during the AABB tree construction as well as to construct the search KD-tree internal to the AABB tree used to accelerate distance queries. +returns a 3D point located on the geometric object represented by the primitive. This function is used to sort the primitives during the AABB tree construction as well as to construct the search KD-tree internal to the AABB tree used to accelerate distance queries. */ Point_reference reference_point(const Shared_data& data); /*! -A static function responsible for the creation of the shared data of a primitive. +constructs the shared data of a primitive. The parameter pack is such that there exists a constructor `template AABBPrimitiveWithSharedData (T1,T...)`. */ template diff --git a/AABB_tree/doc/AABB_tree/Concepts/AABBRayIntersectionTraits.h b/AABB_tree/doc/AABB_tree/Concepts/AABBRayIntersectionTraits.h index dfe82f27d016..2633242ab96b 100644 --- a/AABB_tree/doc/AABB_tree/Concepts/AABBRayIntersectionTraits.h +++ b/AABB_tree/doc/AABB_tree/Concepts/AABBRayIntersectionTraits.h @@ -44,7 +44,7 @@ class AABBRayIntersectionTraits { typedef unspecified_type Intersection_distance; /*! - Returns the intersection distance functor. + returns the intersection distance functor. */ Intersection_distance intersection_distance_object() const ; }; diff --git a/AABB_tree/doc/AABB_tree/Concepts/AABBTraits.h b/AABB_tree/doc/AABB_tree/Concepts/AABBTraits.h index aacdc45fd367..b1db24dab217 100644 --- a/AABB_tree/doc/AABB_tree/Concepts/AABBTraits.h +++ b/AABB_tree/doc/AABB_tree/Concepts/AABBTraits.h @@ -164,42 +164,42 @@ typedef unspecified_type Equal_3; /// @{ /*! -Returns the primitive splitting functor. +returns the primitive splitting functor. */ Split_primitives split_primitives_object(); /*! -Returns the bounding box constructor. +returns the bounding box constructor. */ Compute_bbox compute_bbox_object(); /*! -Returns the intersection detection functor. +returns the intersection detection functor. */ Do_intersect do_intersect_object(); /*! -Returns the intersection constructor. +returns the intersection constructor. */ Intersection intersection_object(); /*! -Returns the distance comparison functor. +returns the distance comparison functor. */ Compare_distance compare_distance_object(); /*! -Returns the closest point constructor. +returns the closest point constructor. */ Closest_point closest_point_object(); /*! -Returns the squared distance functor. +returns the squared distance functor. */ Squared_distance squared_distance_object(); /*! -Returns the equal functor. +returns the equal functor. */ Equal_3 equal_3_object(); @@ -220,7 +220,7 @@ void set_shared_data(T ... t); {} /*! -Returns the shared data of the primitive constructed after a call to `set_shared_data`. +returns the shared data of the primitive constructed after a call to `set_shared_data`. If no call to `set_shared_data` has been done, `Primitive::Shared_data()` is returned. */ const Primitive::Shared_data& shared_data() const; diff --git a/AABB_tree/doc/AABB_tree/aabb_tree.txt b/AABB_tree/doc/AABB_tree/aabb_tree.txt index 7bb8e36d7dce..3f8b7a9fb944 100644 --- a/AABB_tree/doc/AABB_tree/aabb_tree.txt +++ b/AABB_tree/doc/AABB_tree/aabb_tree.txt @@ -336,7 +336,7 @@ inside the bounding box. The experiments described above are neither exhaustive nor conclusive as we have chosen one specific case where the input primitives are the facets of a triangle surface polyhedron. Nevertheless we now provide -some general observations and advices about how to put the AABB tree +some general observations and advises about how to put the AABB tree to use with satisfactory performances. While the tree construction times and memory occupancy do not fluctuate much in our experiments depending on the input surface triangle mesh, the performance @@ -423,10 +423,11 @@ primitives at the leafs of the tree. The ball radius is then shrunk to the distance between `p` and `q` for all remaining recursive traversals of the tree. Efficiency is achieved through setting the initial ball radius to a small value still guaranteed to intersect the -input primitives. This is achieved by constructing through the -function `AABB_tree::accelerate_distance_queries()` an internal secondary data +input primitives. This is achieved by constructing an internal secondary data structure which provides a good hint to the algorithm at the beginning -of the traversal. +of the traversal (done by default). +Calling `do_not_accelerate_distance_queries()` will disable +the construction and the usage of this internal secondary data structure. \section aabb_tree_history Design and Implementation History @@ -434,10 +435,10 @@ Camille Wormser and Pierre Alliez started working on a data structure for efficient collision detection in 2007. The generic design for implementing both intersection and distance queries, and for generic queries and primitives was developed by Camille Wormser. In 2009, -Pierre Alliez, Stéphane Tayeb and Camille Wormser made the +Pierre Alliez, Stéphane Tayeb and Camille Wormser made the implementation CGAL-compliant, with the help of Laurent Rineau for optimizing the tree construction. The authors wish to thank Andreas -Fabri, Jane Tournois, Mariette Yvinec and Sylvain Lefèbvre for +Fabri, Jane Tournois, Mariette Yvinec and Sylvain Lefèbvre for helpful comments and discussions. */ 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 619b5a027e25..26271b0e5448 100644 --- a/AABB_tree/include/CGAL/AABB_face_graph_triangle_primitive.h +++ b/AABB_tree/include/CGAL/AABB_face_graph_triangle_primitive.h @@ -130,7 +130,7 @@ class AABB_face_graph_triangle_primitive // constructors /*! \tparam Iterator an input iterator with `Id` as value type. - Constructs a primitive. + constructs a primitive. If `VertexPointPMap` is the default of the class, an additional constructor is available with `vppm` set to `get(vertex_point, graph)`. */ @@ -142,7 +142,7 @@ class AABB_face_graph_triangle_primitive {} /*! - Constructs a primitive. + constructs a primitive. If `VertexPointPMap` is the default of the class, an additional constructor is available with `vppm` set to `get(vertex_point, graph)`. */ 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 1010525d5c3a..f5c6e2eac69c 100644 --- a/AABB_tree/include/CGAL/AABB_halfedge_graph_segment_primitive.h +++ b/AABB_tree/include/CGAL/AABB_halfedge_graph_segment_primitive.h @@ -141,7 +141,7 @@ class AABB_halfedge_graph_segment_primitive typedef typename boost::graph_traits::edge_descriptor edge_descriptor; /*! - Constructs a primitive. + constructs a primitive. \tparam Iterator is an input iterator with `Id` as value type. This \ref AABB_tree/AABB_halfedge_graph_edge_example.cpp "example" gives a way to call this constructor using the insert-by-range method of the class `AABB_tree`. @@ -156,7 +156,7 @@ class AABB_halfedge_graph_segment_primitive {} /*! - Constructs a primitive. + constructs a primitive. If `VertexPointPMap` is the default of the class, an additional constructor is available with `vppm` set to `boost::get(vertex_point, graph)`. */ diff --git a/AABB_tree/include/CGAL/AABB_primitive.h b/AABB_tree/include/CGAL/AABB_primitive.h index ec7f057a8e84..8b7713939ae8 100644 --- a/AABB_tree/include/CGAL/AABB_primitive.h +++ b/AABB_tree/include/CGAL/AABB_primitive.h @@ -112,14 +112,14 @@ struct AABB_primitive /// @} /*! - Constructs a primitive and initializes the property maps. + constructs a primitive and initializes the property maps. */ AABB_primitive(Id id, ObjectPropertyMap o_pmap=ObjectPropertyMap(), PointPropertyMap p_pmap=PointPropertyMap()); /*! - Constructs a primitive from an iterator with `Id` as value type + constructs a primitive from an iterator with `Id` as value type and initializes the property maps. */ template diff --git a/AABB_tree/include/CGAL/AABB_segment_primitive.h b/AABB_tree/include/CGAL/AABB_segment_primitive.h index 5ee3e4846fed..34851f55a632 100644 --- a/AABB_tree/include/CGAL/AABB_segment_primitive.h +++ b/AABB_tree/include/CGAL/AABB_segment_primitive.h @@ -88,7 +88,7 @@ class AABB_segment_primitive Tag_false, CacheDatum > Base; public: - ///Constructor from an iterator + ///constructor from an iterator AABB_segment_primitive(Iterator it) : Base(it){} }; diff --git a/AABB_tree/include/CGAL/AABB_tree.h b/AABB_tree/include/CGAL/AABB_tree.h index 0216405563ce..84a45fba1328 100644 --- a/AABB_tree/include/CGAL/AABB_tree.h +++ b/AABB_tree/include/CGAL/AABB_tree.h @@ -39,7 +39,7 @@ namespace CGAL { /// @{ /** - * Class AABB_tree is a static data structure for efficient + * Static data structure for efficient * intersection and distance computations in 3D. It builds a * hierarchy of axis-aligned bounding boxes (an AABB tree) from a set * of 3D geometric objects, and can receive intersection and distance @@ -61,6 +61,8 @@ namespace CGAL { // type of the primitives container typedef std::vector Primitives; + typedef internal::Primitive_helper Helper; + public: typedef AABBTraits AABB_traits; @@ -107,7 +109,7 @@ namespace CGAL { /// \name Creation ///@{ - /// Constructs an empty tree, and initializes the internally stored traits + /// constructs an empty tree, and initializes the internally stored traits /// class using `traits`. AABB_tree(const AABBTraits& traits = AABBTraits()); @@ -116,39 +118,45 @@ namespace CGAL { * @param first iterator over first primitive to insert * @param beyond past-the-end iterator * - * It is equivalent to constructing an empty tree and calling `insert(first,last,t...)`. + * constructs an empty tree followed by a call to `insert(first,last,t...)`. * The tree stays empty if the memory allocation is not successful. */ template AABB_tree(InputIterator first, InputIterator beyond,T&& ...); - /// After one or more calls to `insert()` the internal data - /// structure of the tree must be reconstructed. This procedure - /// has a complexity of \f$O(n log(n))\f$, where \f$n\f$ is the number of - /// primitives of the tree. This procedure is called implicitly - /// at the first call to a query member function. You can call - /// `build()` explicitly to ensure that the next call to - /// query functions will not trigger the reconstruction of the - /// data structure. - /// A call to `AABBTraits::set_shared_data(t...)` - /// is made using the internally stored traits. + /// triggers the (re)construction of the internal tree structure. + /// The internal tree structure is automatically invalidated by the insertion of any primitives + /// after one or more calls to `insert()`. + /// This procedure is called implicitly at the first call to a query member function. + /// An explicit call to `build()` must be made to ensure that the next call to + /// a query function will not trigger the construction of the data structure. + /// A call to `AABBTraits::set_shared_data(t...)` is made using the internally stored traits. + /// This procedure has a complexity of \f$O(n log(n))\f$, where \f$n\f$ is the number of + /// primitives of the tree. template void build(T&& ...); #ifndef DOXYGEN_RUNNING void build(); + + /// triggers the (re)construction of the tree similarly to a call to `build()` + /// but the traits functors `Compute_bbox` and `Split_primitives` are ignored + /// and `compute_bbox` and `split_primitives` are used instead. + template + void custom_build(const ComputeBbox& compute_bbox, + const SplitPrimitives& split_primitives); #endif ///@} /// \name Operations ///@{ - /// Equivalent to calling `clear()` and then `insert(first,last,t...)`. + /// is equivalent to calling `clear()`, `insert(first,last,t...)`, and `build()` template void rebuild(ConstPrimitiveIterator first, ConstPrimitiveIterator beyond,T&& ...); - /// Add a sequence of primitives to the set of primitives of the AABB tree. - /// `%InputIterator` is any iterator and the parameter pack `T` are any types + /// adds a sequence of primitives to the set of primitives of the AABB tree. + /// `%InputIterator` is any iterator and the parameter pack `T` contains any types /// such that `Primitive` has a constructor with the following signature: /// `Primitive(%InputIterator, T...)`. If `Primitive` is a model of the concept /// `AABBPrimitiveWithSharedData`, a call to `AABBTraits::set_shared_data(t...)` @@ -156,30 +164,31 @@ namespace CGAL { template void insert(InputIterator first, InputIterator beyond,T&& ...); - /// Adds a primitive to the set of primitives of the tree. + /// adds a primitive to the set of primitives of the tree. inline void insert(const Primitive& p); - /// Clears and destroys the tree. + /// clears and destroys the tree. ~AABB_tree() { clear(); } - /// Returns a const reference to the internally stored traits class. + /// returns a const reference to the internally stored traits class. const AABBTraits& traits() const{ return m_traits; } - /// Clears the tree. + /// clears the tree and the search tree if it was constructed, + /// and switches on the usage of the search tree to find the hint for the distance queries void clear() { // clear AABB tree clear_nodes(); m_primitives.clear(); clear_search_tree(); - m_default_search_tree_constructed = true; + m_use_default_search_tree = true; } - /// Returns the axis-aligned bounding box of the whole tree. + /// returns the axis-aligned bounding box of the whole tree. /// \pre `!empty()` const Bounding_box bbox() const { CGAL_precondition(!empty()); @@ -190,10 +199,10 @@ namespace CGAL { m_primitives.end()); } - /// Returns the number of primitives in the tree. + /// returns the number of primitives in the tree. size_type size() const { return m_primitives.size(); } - /// Returns \c true, iff the tree contains no primitive. + /// returns \c true, iff the tree contains no primitive. bool empty() const { return m_primitives.empty(); } ///@} @@ -209,47 +218,46 @@ namespace CGAL { set_primitive_data_impl(CGAL::Boolean_tag::value>(),std::forward(t)...); } - bool build_kd_tree() const; + bool build_kd_tree(); template - bool build_kd_tree(ConstPointIterator first, ConstPointIterator beyond) const; + bool build_kd_tree(ConstPointIterator first, ConstPointIterator beyond); public: /// \name Intersection Tests ///@{ - /// Returns `true`, iff the query intersects at least one of - /// the input primitives. \tparam Query must be a type for - /// which `do_intersect` predicates are - /// defined in the traits class `AABBTraits`. + /// returns `true`, iff the query intersects at least one of + /// the input primitives. + /// \tparam Query must be a type for which `Do_intersect` operators are + /// defined in the traits class `AABBTraits`. template bool do_intersect(const Query& query) const; - /// Returns the number of primitives intersected by the - /// query. \tparam Query must be a type for which - /// `do_intersect` predicates are defined - /// in the traits class `AABBTraits`. + /// returns the number of primitives intersected by the + /// query. + /// \tparam Query must be a type for which `Do_intersect` operators are + /// defined in the traits class `AABBTraits`. template size_type number_of_intersected_primitives(const Query& query) const; - /// Outputs to the iterator the list of all intersected primitives - /// ids. This function does not compute the intersection points + /// puts in `out` the ids of all intersected primitives. + /// This function does not compute the intersection points /// and is hence faster than the function `all_intersections()` - /// function below. \tparam Query must be a type for which - /// `do_intersect` predicates are defined - /// in the traits class `AABBTraits`. + /// function below. + /// \tparam Query must be a type for which `Do_intersect` operators are + /// defined in the traits class `AABBTraits`. template OutputIterator all_intersected_primitives(const Query& query, OutputIterator out) const; - /// Returns the intersected primitive id that is encountered first + /// returns the id of the intersected primitive that is encountered first /// in the tree traversal, iff /// the query intersects at least one of the input primitives. No /// particular order is guaranteed over the tree traversal, such /// that, e.g, the primitive returned is not necessarily the - /// closest from the source point of a ray query. \tparam Query - /// must be a type for which - /// `do_intersect` predicates are defined - /// in the traits class `AABBTraits`. + /// closest from the source point of a ray query. + /// \tparam Query must be a type for which `Do_intersect` operators are + /// defined in the traits class `AABBTraits`. template boost::optional any_intersected_primitive(const Query& query) const; ///@} @@ -257,30 +265,29 @@ namespace CGAL { /// \name Intersections ///@{ - /// Outputs the list of all intersections, as objects of + /// puts in `out` all intersections, as objects of /// `Intersection_and_primitive_id::%Type`, /// between the query and the input data to - /// the iterator. `do_intersect()` - /// predicates and intersections must be defined for `Query` - /// in the `AABBTraits` class. + /// the iterator. + /// \tparam Query must be a type for which `Do_intersect` and `Intersection` operators are + /// defined in the traits class `AABBTraits`. template OutputIterator all_intersections(const Query& query, OutputIterator out) const; - /// Returns the intersection that is encountered first + /// returns if any the intersection that is encountered first /// in the tree traversal. No particular /// order is guaranteed over the tree traversal, e.g, the - /// primitive returned is not necessarily the closest from the - /// source point of a ray query. Type `Query` must be a type - /// for which `do_intersect` predicates - /// and intersections are defined in the traits class AABBTraits. + /// primitive returned is not necessarily the closest from the query. + /// \tparam Query must be a type for which `Do_intersect` and `Intersection` operators are + /// defined in the traits class `AABBTraits`. template boost::optional< typename Intersection_and_primitive_id::Type > any_intersection(const Query& query) const; - /// Returns the intersection and primitive id closest to the source point of the ray + /// returns the intersection and primitive id closest to the source point of the ray /// query. /// \tparam Ray must be the same as `AABBTraits::Ray_3` and /// `do_intersect` predicates and intersections for it must be @@ -311,7 +318,7 @@ namespace CGAL { } /// \endcond - /// Returns the primitive id closest to the source point of the ray + /// returns the primitive id closest to the source point of the ray /// query. /// \tparam Ray must be the same as `AABBTraits::Ray_3` and /// `do_intersect` predicates and intersections for it must be @@ -340,31 +347,22 @@ namespace CGAL { /// \name Distance Queries ///@{ - /// Returns the minimum squared distance between the query point - /// and all input primitives. Method - /// `accelerate_distance_queries()` should be called before the - /// first distance query, so that an internal secondary search - /// structure is build, for improving performance. + /// returns the minimum squared distance between the query point + /// and all input primitives. /// \pre `!empty()` FT squared_distance(const Point& query) const; - /// Returns the point in the union of all input primitives which + /// returns the point in the union of all input primitives which /// is closest to the query. In case there are several closest /// points, one arbitrarily chosen closest point is - /// returned. Method `accelerate_distance_queries()` should be - /// called before the first distance query, so that an internal - /// secondary search structure is build, for improving - /// performance. + /// returned. /// \pre `!empty()` Point closest_point(const Point& query) const; - /// Returns a `Point_and_primitive_id` which realizes the + /// returns a `Point_and_primitive_id` which realizes the /// smallest distance between the query point and all input - /// primitives. Method `accelerate_distance_queries()` should be - /// called before the first distance query, so that an internal - /// secondary search structure is build, for improving - /// performance. + /// primitives. /// \pre `!empty()` Point_and_primitive_id closest_point_and_primitive(const Point& query) const; @@ -404,19 +402,18 @@ namespace CGAL { /// one may want to provide a much better hint than a vertex of /// the triangle soup could be. It could be, for example, the /// barycenter of one of the triangles. But, except with the use - /// of an exact constructions kernel, one cannot easily construct + /// of a kernel with exact constructions, one cannot easily construct /// points other than the vertices, that lie exactly on a triangle /// soup. Hence, providing a good hint sometimes means not being /// able to provide it exactly on the primitives. In rare /// occasions, this hint can be returned as the closest point. /// In order to accelerate distance queries significantly, the /// AABB tree builds an internal KD-tree containing a set of - /// potential hints, when the method - /// `accelerate_distance_queries()` is called. This KD-tree - /// provides very good hints that allow the algorithms to run much - /// faster than with a default hint (such as the - /// `reference_point` of the first primitive). The set of - /// potential hints is a sampling of the union of the primitives, + /// potential hints. This KD-tree provides very good hints + /// that allow the algorithms to run much faster than + /// when `do_not_accelerate_distance_queries()` that makes the + /// hint to always be the `reference_point` of the first primitive. + /// The set of potential hints is a sampling of the union of the primitives, /// which is obtained, by default, by calling the method /// `reference_point` of each of the primitives. However, such /// a sampling with one point per primitive may not be the most @@ -424,45 +421,46 @@ namespace CGAL { /// inserting more than one sample on them. Conversely, a sparser /// sampling with less than one point per input primitive is /// relevant in some cases. + /// The internal KD-tree is always used if no call to `do_not_accelerate_distance_queries()` + /// was made since object creation or the last call to `clear()`. It will be built by + /// the first distance query or by a call to `accelerate_distance_queries()`. ///@{ - /// Constructs internal search tree from + /// constructs the internal search tree from /// a point set taken on the internal primitives /// returns `true` iff successful memory allocation - bool accelerate_distance_queries() const; - ///Turns off the lazy construction of the internal search tree. - void do_not_accelerate_distance_queries() const; + bool accelerate_distance_queries(); + /// turns off the usage of the internal search tree and clears it if it was already constructed. + void do_not_accelerate_distance_queries(); - /// Constructs an internal KD-tree containing the specified point + /// constructs an internal KD-tree containing the specified point /// set, to be used as the set of potential hints for accelerating - /// the distance queries. + /// the distance queries. Note that the search tree built in + /// this function will not be invalidated by the insertion of a new + /// primitive, and an explicit call to `accelerate_distance_queries()` + /// is needed to update the search tree. /// \tparam ConstPointIterator is an iterator with /// value type `Point_and_primitive_id`. template - bool accelerate_distance_queries(ConstPointIterator first, ConstPointIterator beyond) const + bool accelerate_distance_queries(ConstPointIterator first, ConstPointIterator beyond) { - #ifdef CGAL_HAS_THREADS - //this ensures that this is done once at a time - CGAL_SCOPED_LOCK(kd_tree_mutex); - #endif - clear_search_tree(); - m_default_search_tree_constructed = false; // not a default kd-tree + m_use_default_search_tree = false; return build_kd_tree(first,beyond); } - /// Returns the minimum squared distance between the query point + /// returns the minimum squared distance between the query point /// and all input primitives. The internal KD-tree is not used. /// \pre `!empty()` FT squared_distance(const Point& query, const Point& hint) const; - /// Returns the point in the union of all input primitives which + /// returns the point in the union of all input primitives which /// is closest to the query. In case there are several closest /// points, one arbitrarily chosen closest point is returned. The /// internal KD-tree is not used. /// \pre `!empty()` Point closest_point(const Point& query, const Point& hint) const; - /// Returns a `Point_and_primitive_id` which realizes the + /// returns a `Point_and_primitive_id` which realizes the /// smallest distance between the query point and all input /// primitives. The internal KD-tree is not used. /// \pre `!empty()` @@ -484,15 +482,23 @@ namespace CGAL { } // clears internal KD tree - void clear_search_tree() const + void clear_search_tree() { +#ifdef CGAL_HAS_THREADS + if ( m_atomic_search_tree_constructed.load(std::memory_order_relaxed) ) +#else if ( m_search_tree_constructed ) +#endif { CGAL_assertion( m_p_search_tree!=nullptr ); delete m_p_search_tree; m_p_search_tree = nullptr; +#ifdef CGAL_HAS_THREADS + m_atomic_search_tree_constructed.store(false, std::memory_order_relaxed); +#else m_search_tree_constructed = false; - } +#endif + } } public: @@ -523,31 +529,43 @@ namespace CGAL { { CGAL_assertion(!empty()); return Point_and_primitive_id( - internal::Primitive_helper::get_reference_point(m_primitives[0],m_traits), m_primitives[0].id() + Helper::get_reference_point(m_primitives[0],m_traits), m_primitives[0].id() ); } public: Point_and_primitive_id best_hint(const Point& query) const { +#ifdef CGAL_HAS_THREADS + bool m_search_tree_constructed = m_atomic_search_tree_constructed.load(std::memory_order_acquire); +#endif + + // lazily build the search tree in case the default should be used + if (m_use_default_search_tree && !m_search_tree_constructed) + { +#ifdef CGAL_HAS_THREADS + CGAL_SCOPED_LOCK(build_mutex); + m_search_tree_constructed = m_atomic_search_tree_constructed.load(std::memory_order_relaxed); + if (!m_search_tree_constructed) +#endif + m_search_tree_constructed = const_cast(this)->build_kd_tree(); + } + if(m_search_tree_constructed) - { return m_p_search_tree->closest_point(query); - } else return this->any_reference_point_and_id(); } //! Returns the datum (geometric object) represented `p`. #ifndef DOXYGEN_RUNNING - typename internal::Primitive_helper::Datum_type + typename Helper::Datum_type #else typename AABBTraits::Primitive::Datum_reference #endif datum(Primitive& p)const { - return internal::Primitive_helper:: - get_datum(p, this->traits()); + return Helper::get_datum(p, this->traits()); } private: @@ -556,21 +574,25 @@ namespace CGAL { // set of input primitives Primitives m_primitives; // single root node - Node* m_p_root_node; + Node* m_p_root_node = nullptr; #ifdef CGAL_HAS_THREADS - mutable CGAL_MUTEX internal_tree_mutex;//mutex used to protect const calls inducing build() - mutable CGAL_MUTEX kd_tree_mutex;//mutex used to protect calls to accelerate_distance_queries + mutable CGAL_MUTEX build_mutex; // mutex used to protect const calls inducing build() and build_kd_tree() #endif const Node* root_node() const { CGAL_assertion(size() > 1); + +#ifdef CGAL_HAS_THREADS + bool m_need_build = m_atomic_need_build.load(std::memory_order_acquire); +#endif if(m_need_build){ - #ifdef CGAL_HAS_THREADS +#ifdef CGAL_HAS_THREADS //this ensures that build() will be called once - CGAL_SCOPED_LOCK(internal_tree_mutex); + CGAL_SCOPED_LOCK(build_mutex); + m_need_build = m_atomic_need_build.load(std::memory_order_relaxed); if(m_need_build) - #endif - const_cast< AABB_tree* >(this)->build(); +#endif + const_cast< AABB_tree* >(this)->build(); } return m_p_root_node; } @@ -581,10 +603,15 @@ namespace CGAL { } // search KD-tree - mutable const Search_tree* m_p_search_tree; - mutable bool m_search_tree_constructed; - mutable bool m_default_search_tree_constructed; // indicates whether the internal kd-tree should be built - bool m_need_build; + const Search_tree* m_p_search_tree = nullptr; + bool m_use_default_search_tree = true; // indicates whether the internal kd-tree should be built +#ifdef CGAL_HAS_THREADS + std::atomic m_atomic_need_build; + std::atomic m_atomic_search_tree_constructed; +#else + bool m_need_build = false; + mutable bool m_search_tree_constructed = false; +#endif private: // Disabled copy constructor & assignment operator @@ -599,12 +626,10 @@ namespace CGAL { template AABB_tree::AABB_tree(const Tr& traits) : m_traits(traits) - , m_primitives() - , m_p_root_node(nullptr) - , m_p_search_tree(nullptr) - , m_search_tree_constructed(false) - , m_default_search_tree_constructed(true) - , m_need_build(false) +#ifdef CGAL_HAS_THREADS + , m_atomic_need_build(false) + , m_atomic_search_tree_constructed(false) +#endif {} template @@ -612,13 +637,10 @@ namespace CGAL { AABB_tree::AABB_tree(ConstPrimitiveIterator first, ConstPrimitiveIterator beyond, T&& ... t) - : m_traits() - , m_primitives() - , m_p_root_node(nullptr) - , m_p_search_tree(nullptr) - , m_search_tree_constructed(false) - , m_default_search_tree_constructed(true) - , m_need_build(false) +#ifdef CGAL_HAS_THREADS + : m_atomic_need_build(false) + , m_atomic_search_tree_constructed(false) +#endif { // Insert each primitive into tree insert(first, beyond,std::forward(t)...); @@ -630,13 +652,19 @@ namespace CGAL { ConstPrimitiveIterator beyond, T&& ... t) { + if (m_use_default_search_tree && first!=beyond) + clear_search_tree(); set_shared_data(std::forward(t)...); while(first != beyond) { m_primitives.push_back(Primitive(first,std::forward(t)...)); ++first; } +#ifdef CGAL_HAS_THREADS + m_atomic_need_build.store(true, std::memory_order_relaxed); +#else m_need_build = true; +#endif } // Clears tree and insert a set of primitives @@ -655,24 +683,40 @@ namespace CGAL { build(); } - template - template - void AABB_tree::build(T&& ... t) - { - set_shared_data(std::forward(t)...); - build(); - } + template + template + void AABB_tree::build(T&& ... t) + { + set_shared_data(std::forward(t)...); + build(); + } template void AABB_tree::insert(const Primitive& p) { + if (m_use_default_search_tree) + clear_search_tree(); m_primitives.push_back(p); +#ifdef CGAL_HAS_THREADS + m_atomic_need_build.store(true, std::memory_order_relaxed); +#else m_need_build = true; +#endif } - // Build the data structure, after calls to insert(..) template void AABB_tree::build() + { + custom_build(m_traits.compute_bbox_object(), + m_traits.split_primitives_object()); + } +#ifndef DOXYGEN_RUNNING + // Build the data structure, after calls to insert(..) + template + template + void AABB_tree::custom_build( + const ComputeBbox& compute_bbox, + const SplitPrimitives& split_primitives) { clear_nodes(); if(m_primitives.size() > 1) { @@ -689,37 +733,31 @@ namespace CGAL { // constructs the tree m_p_root_node->expand(m_primitives.begin(), m_primitives.end(), - m_primitives.size(), m_traits); - } - - - // In case the users has switched on the accelerated distance query - // data structure with the default arguments, then it has to be - // /built/rebuilt. - if(m_default_search_tree_constructed && !empty()){ - build_kd_tree(); + m_primitives.size(), + compute_bbox, + split_primitives, + m_traits); } +#ifdef CGAL_HAS_THREADS + m_atomic_need_build.store(false, std::memory_order_release); // in case build() is triggered by a call to root_node() +#else m_need_build = false; +#endif } +#endif // constructs the search KD tree from given points // to accelerate the distance queries template - bool AABB_tree::build_kd_tree() const + bool AABB_tree::build_kd_tree() { // iterate over primitives to get reference points on them std::vector points; points.reserve(m_primitives.size()); - typename Primitives::const_iterator it; - for(it = m_primitives.begin(); it != m_primitives.end(); ++it) - points.push_back( Point_and_primitive_id( - internal::Primitive_helper::get_reference_point( - *it,m_traits), it->id() ) ); + for(const Primitive& p : m_primitives) + points.push_back( Point_and_primitive_id( Helper::get_reference_point(p, m_traits), p.id() ) ); // clears current KD tree - clear_search_tree(); - bool res = build_kd_tree(points.begin(), points.end()); - m_default_search_tree_constructed = true; - return res; + return build_kd_tree(points.begin(), points.end()); } // constructs the search KD tree from given points @@ -727,13 +765,18 @@ namespace CGAL { template template bool AABB_tree::build_kd_tree(ConstPointIterator first, - ConstPointIterator beyond) const + ConstPointIterator beyond) { + clear_search_tree(); m_p_search_tree = new Search_tree(first, beyond); - m_default_search_tree_constructed = true; + if(m_p_search_tree != nullptr) { +#ifdef CGAL_HAS_THREADS + m_atomic_search_tree_constructed.store(true, std::memory_order_release); // in case build_kd_tree() is triggered by a call to best_hint() +#else m_search_tree_constructed = true; +#endif return true; } else @@ -744,41 +787,20 @@ namespace CGAL { } template - void AABB_tree::do_not_accelerate_distance_queries()const + void AABB_tree::do_not_accelerate_distance_queries() { clear_search_tree(); - m_default_search_tree_constructed = false; + m_use_default_search_tree = false; } // constructs the search KD tree from internal primitives template - bool AABB_tree::accelerate_distance_queries() const + bool AABB_tree::accelerate_distance_queries() { + m_use_default_search_tree = true; if(m_primitives.empty()) return true; - if (m_default_search_tree_constructed) - { - if (!m_need_build) return m_search_tree_constructed; - return true; // default return type, no tree built - } - - if(!m_need_build) // the tree was already built, build the kd-tree - { - #ifdef CGAL_HAS_THREADS - //this ensures that this function will be done once - CGAL_SCOPED_LOCK(kd_tree_mutex); - #endif - if (!m_need_build) - { - // clears current KD tree - clear_search_tree(); - bool res = build_kd_tree(); - m_default_search_tree_constructed = true; - return res; - }; - } - m_default_search_tree_constructed = true; - return m_search_tree_constructed; + return build_kd_tree(); } template diff --git a/AABB_tree/include/CGAL/AABB_triangle_primitive.h b/AABB_tree/include/CGAL/AABB_triangle_primitive.h index c4d45899f4d1..5c1502d60aff 100644 --- a/AABB_tree/include/CGAL/AABB_triangle_primitive.h +++ b/AABB_tree/include/CGAL/AABB_triangle_primitive.h @@ -88,7 +88,7 @@ class AABB_triangle_primitive Tag_false, CacheDatum > Base; public: - ///Constructor from an iterator + ///constructor from an iterator AABB_triangle_primitive(Iterator it) : Base(it){} }; diff --git a/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h b/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h index 3844166ed50e..6fcfef9ede88 100644 --- a/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h +++ b/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h @@ -56,10 +56,12 @@ class AABB_node * * [first,last[ is the range of primitives to be added to the tree. */ - template + template void expand(ConstPrimitiveIterator first, ConstPrimitiveIterator beyond, const std::size_t range, + const ComputeBbox& compute_bbox, + const SplitPrimitives& split_primitives, const AABBTraits&); /** @@ -117,19 +119,20 @@ class AABB_node }; // end class AABB_node - template -template +template void AABB_node::expand(ConstPrimitiveIterator first, ConstPrimitiveIterator beyond, const std::size_t range, + const ComputeBbox& compute_bbox, + const SplitPrimitives& split_primitives, const Tr& traits) { - m_bbox = traits.compute_bbox_object()(first, beyond); + m_bbox = compute_bbox(first, beyond); // sort primitives along longest axis aabb - traits.split_primitives_object()(first, beyond, m_bbox); + split_primitives(first, beyond, m_bbox); switch(range) { @@ -140,14 +143,14 @@ AABB_node::expand(ConstPrimitiveIterator first, case 3: m_p_left_child = &(*first); m_p_right_child = static_cast(this)+1; - right_child().expand(first+1, beyond, 2,traits); + right_child().expand(first+1, beyond, 2, compute_bbox, split_primitives, traits); break; default: const std::size_t new_range = range/2; m_p_left_child = static_cast(this) + 1; m_p_right_child = static_cast(this) + new_range; - left_child().expand(first, first + new_range, new_range,traits); - right_child().expand(first + new_range, beyond, range - new_range,traits); + left_child().expand(first, first + new_range, new_range, compute_bbox, split_primitives, traits); + right_child().expand(first + new_range, beyond, range - new_range, compute_bbox, split_primitives, traits); } } diff --git a/Installation/CHANGES.md b/Installation/CHANGES.md index 3e458a383247..1026c224a7c4 100644 --- a/Installation/CHANGES.md +++ b/Installation/CHANGES.md @@ -3,6 +3,14 @@ Release History [Release 5.1] (https://github.com/CGAL/cgal/releases/tag/releases%2FCGAL-5.1) +### 3D Fast Intersection and Distance Computation +- The introduction of the usage of the search tree by default for all distance queries + in the 5.0 release was actually not lazy contrary to what was announced. The behavior of the + search tree is now the following: it will be used except if an explicit call to `do_not_accelerate_distance_queries()` + is made. The construction of the search tree (once insertion of primitives is done) will be triggered by the first + distance query or by an explicit call to `accelerate_distance_queries()`. +- **Breaking change**: `accelerate_distance_queries()` and `do_not_accelerate_distance_queries()` are not longer `const` functions. + ### Optimal Bounding Box (new package) - This package implements an optimization algorithm that aims to construct a close approximation of the *optimal bounding box* of a mesh or a point set, which is defined as the smallest 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 00f80a82afcf..20ec968d50a6 100644 --- a/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h +++ b/Polygon_mesh_processing/include/CGAL/Side_of_triangle_mesh.h @@ -98,13 +98,15 @@ class Side_of_triangle_mesh //members typename GeomTraits::Construct_ray_3 ray_functor; typename GeomTraits::Construct_vector_3 vector_functor; - 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; + mutable std::atomic atomic_tree_ptr; +#else + mutable const AABB_tree_* tree_ptr; #endif public: @@ -129,10 +131,14 @@ class Side_of_triangle_mesh 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) +#ifdef CGAL_HAS_THREADS + , atomic_tree_ptr(nullptr) +#else + , tree_ptr(nullptr) +#endif { CGAL_assertion(CGAL::is_triangle_mesh(tmesh)); CGAL_assertion(CGAL::is_closed(tmesh)); @@ -165,16 +171,24 @@ class Side_of_triangle_mesh const GeomTraits& gt = GeomTraits()) : ray_functor(gt.construct_ray_3_object()) , vector_functor(gt.construct_vector_3_object()) - , tree_ptr(&tree) , own_tree(false) +#ifdef CGAL_HAS_THREADS + , atomic_tree_ptr(&tree) +#else + , tree_ptr(&tree) +#endif { box = tree.bbox(); } ~Side_of_triangle_mesh() { - if (own_tree && tree_ptr!=nullptr) + if (own_tree) +#ifdef CGAL_HAS_THREADS + delete atomic_tree_ptr.load(); +#else delete tree_ptr; +#endif } public: @@ -200,11 +214,16 @@ class Side_of_triangle_mesh } else { +#ifdef CGAL_HAS_THREADS + AABB_tree_* tree_ptr = + const_cast(atomic_tree_ptr.load(std::memory_order_acquire)); +#endif // Lazily build the tree only when needed if (tree_ptr==nullptr) { #ifdef CGAL_HAS_THREADS CGAL_SCOPED_LOCK(tree_mutex); + tree_ptr = const_cast(atomic_tree_ptr.load(std::memory_order_relaxed)); #endif CGAL_assertion(tm_ptr != nullptr && opt_vpm!=boost::none); if (tree_ptr==nullptr) @@ -213,9 +232,11 @@ class Side_of_triangle_mesh faces(*tm_ptr).second, *tm_ptr, *opt_vpm); const_cast(tree_ptr)->build(); +#ifdef CGAL_HAS_THREADS + atomic_tree_ptr.store(tree_ptr, std::memory_order_release); +#endif } } - return internal::Point_inside_vertical_ray_cast()( point, *tree_ptr, ray_functor, vector_functor); } diff --git a/Surface_mesh_simplification/include/CGAL/Surface_mesh_simplification/Policies/Edge_collapse/Bounded_distance_placement.h b/Surface_mesh_simplification/include/CGAL/Surface_mesh_simplification/Policies/Edge_collapse/Bounded_distance_placement.h index dad4317af92f..59a33fa465ce 100644 --- a/Surface_mesh_simplification/include/CGAL/Surface_mesh_simplification/Policies/Edge_collapse/Bounded_distance_placement.h +++ b/Surface_mesh_simplification/include/CGAL/Surface_mesh_simplification/Policies/Edge_collapse/Bounded_distance_placement.h @@ -59,21 +59,22 @@ class Bounded_distance_placement const Triangle_mesh& tm = profile.surface_mesh(); const Geom_traits& gt = profile.geom_traits(); - std::vector input_triangles; + m_input_triangles.reserve(faces(tm).size()); for(face_descriptor f : faces(profile.surface_mesh())) { halfedge_descriptor h = halfedge(f, tm); CGAL_assertion(!is_border(h, tm)); - input_triangles.push_back(gt.construct_triangle_3_object()( - get(profile.vertex_point_map(), source(h, tm)), - get(profile.vertex_point_map(), target(h, tm)), - get(profile.vertex_point_map(), target(next(h, tm), tm)))); + m_input_triangles.emplace_back(gt.construct_triangle_3_object()( + get(profile.vertex_point_map(), source(h, tm)), + get(profile.vertex_point_map(), target(h, tm)), + get(profile.vertex_point_map(), target(next(h, tm), tm)))); } - m_tree_ptr = new AABB_tree(input_triangles.begin(), input_triangles.end()); + m_tree_ptr = new AABB_tree(m_input_triangles.begin(), m_input_triangles.end()); const_cast(m_tree_ptr)->build(); + const_cast(m_tree_ptr)->accelerate_distance_queries(); } public: @@ -108,8 +109,7 @@ class Bounded_distance_placement const Point& p = *op; - m_tree_ptr->accelerate_distance_queries(); - const Point& cp = m_tree_ptr->best_hint(p).first; // requires accelerate distance query to be called. + const Point& cp = m_tree_ptr->best_hint(p).first; // We could do better by having access to the internal kd-tree // and call search_any_point with a fuzzy_sphere. @@ -130,6 +130,7 @@ class Bounded_distance_placement private: const FT m_sq_threshold_dist; mutable const AABB_tree* m_tree_ptr; + mutable std::vector m_input_triangles; const BasePlacement m_base_placement; }; @@ -166,8 +167,7 @@ class Bounded_distance_placement > const Point& p = *op; - m_tree_ptr->accelerate_distance_queries(); - const Point& cp = m_tree_ptr->best_hint(p).first; // requires accelerate distance query to be called. + const Point& cp = m_tree_ptr->best_hint(p).first; if(CGAL::compare_squared_distance(p, cp, m_sq_threshold_dist) != LARGER || m_tree_ptr->do_intersect(CGAL::Sphere_3(p, m_sq_threshold_dist))) diff --git a/Surface_mesh_simplification/test/Surface_mesh_simplification/test_edge_collapse_bounded_distance.cpp b/Surface_mesh_simplification/test/Surface_mesh_simplification/test_edge_collapse_bounded_distance.cpp index 8aaa51a46518..0764b881fe1f 100644 --- a/Surface_mesh_simplification/test/Surface_mesh_simplification/test_edge_collapse_bounded_distance.cpp +++ b/Surface_mesh_simplification/test/Surface_mesh_simplification/test_edge_collapse_bounded_distance.cpp @@ -62,15 +62,16 @@ int main(int argc, char** argv) Filtered_placement_with_tree placement_small(0.00005*diag, tree, placement_ref); SMS::edge_collapse(small_mesh, stop, CGAL::parameters::get_cost(Cost()).get_placement(placement_small)); - Filtered_placement placement_big(50*diag, placement_ref); // lazily builds the AABB tree + Filtered_placement placement_big(0.005*diag, placement_ref); // lazily builds the AABB tree SMS::edge_collapse(big_mesh, stop, CGAL::parameters::get_cost(Cost()).get_placement(placement_big)); std::cout << "no filtering: " << vertices(ref_mesh).size() << " vertices left" << std::endl; std::cout << "large filtering distance: " << vertices(big_mesh).size() << " vertices left" << std::endl; std::cout << "small filtering distance: " << vertices(small_mesh).size() << " vertices left" << std::endl; - assert(vertices(small_mesh).size() != vertices(ref_mesh).size()); - assert(vertices(big_mesh).size() == vertices(ref_mesh).size()); + assert(vertices(ref_mesh).size() < vertices(small_mesh).size()); + assert(vertices(big_mesh).size() < vertices(small_mesh).size()); + assert(vertices(ref_mesh).size() < vertices(big_mesh).size()); return EXIT_SUCCESS; }