From ae18495c5696b548badfe8d70a3b3cab24f338e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Tue, 13 Feb 2024 16:51:54 +0100 Subject: [PATCH] simplify implementation of no data case to make it work with non MSVC compilers surprisingly tests are broken --- Orthtree/include/CGAL/Orthtree.h | 55 +++----- .../test_octree_copy_move_constructors.cpp | 23 ++-- .../include/CGAL/Property_container.h | 130 +----------------- 3 files changed, 35 insertions(+), 173 deletions(-) diff --git a/Orthtree/include/CGAL/Orthtree.h b/Orthtree/include/CGAL/Orthtree.h index 0f1d9db0be0..03a18ac0c4f 100644 --- a/Orthtree/include/CGAL/Orthtree.h +++ b/Orthtree/include/CGAL/Orthtree.h @@ -72,26 +72,26 @@ BOOST_MPL_HAS_XXX_TRAIT_DEF(Node_data) */ template class Orthtree { -private: - template + template struct Node_data_selector {}; template - struct Node_data_selector> { + struct Node_data_selector { using type = typename A::Node_data; }; template - struct Node_data_selector> { + struct Node_data_selector { using type = void; }; + static inline constexpr bool has_data = internal::has_Node_data::value; + public: /// \name Template Types /// @{ using Traits = GeomTraits; ///< Geometry traits /// @} - using Has_data = boost::mpl::bool_::value>; /// \name Traits Types /// @{ @@ -104,7 +104,7 @@ public: using Adjacency = typename Traits::Adjacency; ///< Adjacency type. using Node_index = typename Traits::Node_index; ///< Index of a given node in the tree; the root always has index 0. - using Node_data = typename Node_data_selector::type; + using Node_data = typename Node_data_selector::type; /// @} @@ -178,7 +178,7 @@ private: // data members : Traits m_traits; /* the tree traits */ Node_property_container m_node_properties; - Property_array m_node_contents; + std::conditional_t, void*> m_node_contents; Property_array m_node_depths; Property_array m_node_coordinates; Property_array> m_node_parents; @@ -190,17 +190,13 @@ private: // data members : Cartesian_ranges cartesian_range; /* a helper to easily iterate over coordinates of points */ - template - void init_data(); - - template<> - void init_data>() { - data(root()) = m_traits.construct_root_node_contents_object()(); + template> + auto init_node_contents(std::true_type) -> std::enable_if_t + { + return Property_array(m_node_properties.template get_or_add_property("contents").first); } - template<> - void init_data>() { - } + void* init_node_contents(std::false_type) { return nullptr; } public: @@ -223,7 +219,7 @@ public: */ explicit Orthtree(Traits traits) : m_traits(traits), - m_node_contents(m_node_properties.template get_or_add_property("contents").first), + m_node_contents(init_node_contents(std::bool_constant())), m_node_depths(m_node_properties.template get_or_add_property("depths", 0).first), m_node_coordinates(m_node_properties.template get_or_add_property("coordinates").first), m_node_parents(m_node_properties.template get_or_add_property>("parents").first), @@ -244,7 +240,8 @@ public: // save orthtree attributes m_side_per_depth.push_back(size); - init_data(); + if constexpr (has_data) + m_traits.construct_root_node_contents_object()(); } /// @} @@ -253,7 +250,7 @@ public: Orthtree(const Orthtree& other) : m_traits(other.m_traits), m_node_properties(other.m_node_properties), - m_node_contents(m_node_properties.template get_property("contents")), + m_node_contents(init_node_contents(std::bool_constant())), m_node_depths(m_node_properties.template get_property("depths")), m_node_coordinates(m_node_properties.template get_property("coordinates")), m_node_parents(m_node_properties.template get_property>("parents")), @@ -264,7 +261,7 @@ public: Orthtree(Orthtree&& other) : m_traits(other.m_traits), m_node_properties(std::move(other.m_node_properties)), - m_node_contents(m_node_properties.template get_property("contents")), + m_node_contents(init_node_contents(std::bool_constant())), m_node_depths(m_node_properties.template get_property("depths")), m_node_coordinates(m_node_properties.template get_property("coordinates")), m_node_parents(m_node_properties.template get_property>("parents")), @@ -729,7 +726,7 @@ public: \brief retrieves a reference to the Node_data associated with the node specified by `n`. */ template - auto data(Node_index n) -> std::enable_if_t::value, Dummy&>{ + auto data(Node_index n) -> std::enable_if_t{ return m_node_contents[n]; } @@ -737,7 +734,7 @@ public: \brief retrieves a const reference to the Node_data associated with the node specified by `n`. */ template - auto data(Node_index n) const -> std::enable_if_t::value, const Dummy&> { + auto data(Node_index n) const -> std::enable_if_t { return m_node_contents[n]; } @@ -965,7 +962,8 @@ public: Point center = barycenter(n); // Add the node's contents to its children - distribute_node_contents(n, center); + if constexpr (has_data) + m_traits.distribute_node_contents_object()(n, *this, center); } /*! @@ -1173,17 +1171,6 @@ private: // functions : return output; } - template - void distribute_node_contents(Node_index n, const Point& center); - - template<> - void distribute_node_contents>(Node_index n, const Point& center) { - m_traits.distribute_node_contents_object()(n, *this, center); - } - - template<> - void distribute_node_contents>(Node_index n, const Point& center) {} - public: /// \cond SKIP_IN_MANUAL diff --git a/Orthtree/test/Orthtree/test_octree_copy_move_constructors.cpp b/Orthtree/test/Orthtree/test_octree_copy_move_constructors.cpp index 3716036ede2..467ad272ff4 100644 --- a/Orthtree/test/Orthtree/test_octree_copy_move_constructors.cpp +++ b/Orthtree/test/Orthtree/test_octree_copy_move_constructors.cpp @@ -48,17 +48,18 @@ int test(Tree &tree) return EXIT_SUCCESS; } -void main() { - std::size_t nb_pts = 100; - Point_set points; - CGAL::Random_points_in_cube_3 generator; - points.reserve(nb_pts); - for (std::size_t i = 0; i < nb_pts; ++i) - points.insert(*(generator++)); +int main() +{ + std::size_t nb_pts = 100; + Point_set points; + CGAL::Random_points_in_cube_3 generator; + points.reserve(nb_pts); + for (std::size_t i = 0; i < nb_pts; ++i) + points.insert(*(generator++)); - Octree base({ points, points.point_map() }); - test(base); + Octree base({ points, points.point_map() }); + test(base); - Octree_without_data base2({}); - test(base2); + Octree_without_data base2({}); + test(base2); } diff --git a/Property_map/include/CGAL/Property_container.h b/Property_map/include/CGAL/Property_container.h index 16a44ff1dc2..739f652b318 100644 --- a/Property_map/include/CGAL/Property_container.h +++ b/Property_map/include/CGAL/Property_container.h @@ -191,43 +191,6 @@ public: }; -template -class Property_array : public Property_array_base { -public: - Property_array() {} - - virtual std::shared_ptr> empty_clone(const std::vector& active_indices) { - return std::make_shared>(); - } - - virtual std::shared_ptr> clone(const std::vector& active_indices) { - return std::make_shared>(); - } - - virtual void copy(const Property_array_base& other) {} - - // desactived as MSVC 2017 as an issue with that but it is not currently used. -#if 0 - virtual void move(Property_array_base&& other) = 0; -#endif - - virtual void append(const Property_array_base& other) {} - - virtual void reserve(std::size_t n) {} - - virtual void shrink_to_fit() {} - - virtual void swap(Index a, Index b) {} - - virtual void reset(Index i) {} - - virtual const std::type_info& type() const { - return typeid(void); - } - - virtual void transfer_from(const Property_array_base& other_base, Index other_index, Index this_index) {} -}; - // todo: property maps/array handles should go in their own file // todo: add const/read-only handle @@ -280,56 +243,6 @@ public: }; -// void specialization -template -class Property_array_handle { - - std::reference_wrapper> m_array; - std::vector m_dummy; - -public: - - // Necessary for use as a boost::property_type - using key_type = Index; - using value_type = void; - using reference = void; - using const_reference = void; - using category = boost::lvalue_property_map_tag; - - using iterator = typename std::vector::iterator; - using const_iterator = typename std::vector::const_iterator; - - Property_array_handle(Property_array& array) : m_array(array) {} - - [[nodiscard]] std::size_t size() const { return 0; } - - [[nodiscard]] std::size_t capacity() const { return 0; } - - Property_array& array() const { return m_array.get(); } - - // todo: This might not be needed, if the other operator[] is made const - const_reference operator[](Index i) const { } - - reference operator[](Index i) { } - - // todo: maybe these can be const, in an lvalue property map? - iterator begin() { return m_dummy.begin(); } - - iterator end() { return m_dummy.end(); } - - const_iterator begin() const { return m_dummy.begin(); } - - const_iterator end() const { return m_dummy.end(); } - - bool operator==(const Property_array_handle& other) const { return true; } - - bool operator!=(const Property_array_handle& other) const { return false; } - - inline friend reference get(Property_array_handle p, const Index& i) {} - - //inline friend void put(Property_array_handle p, const Index& i, const T& v) {} -}; - template class Property_container { @@ -405,46 +318,7 @@ public: template std::pair>, bool> - get_or_add_property(const std::string& name) { - auto range = m_properties.equal_range(name); - for (auto it = range.first; it != range.second; it++) { - Property_array* typed_array_ptr = dynamic_cast*>(it->second.get()); - if (typed_array_ptr != nullptr) - return { {*typed_array_ptr}, false }; - } - - auto it = m_properties.emplace( - name, - std::make_shared>( - m_active_indices, - T() - ) - ); - - return { {*dynamic_cast*>(it->second.get())}, true }; - } - - template<> - std::pair>, bool> - get_or_add_property(const std::string& name) { - auto range = m_properties.equal_range(name); - for (auto it = range.first; it != range.second; it++) { - Property_array* typed_array_ptr = dynamic_cast*>(it->second.get()); - if (typed_array_ptr != nullptr) - return { {*typed_array_ptr}, false }; - } - - auto it = m_properties.emplace( - name, - std::make_shared>() - ); - - return { {*dynamic_cast*>(it->second.get())}, true }; - } - - template - std::pair>, bool> - get_or_add_property(const std::string& name, const T default_value) { + get_or_add_property(const std::string& name, const T default_value = T()) { auto range = m_properties.equal_range(name); for (auto it = range.first; it != range.second; it++) { Property_array* typed_array_ptr = dynamic_cast*>(it->second.get()); @@ -460,7 +334,7 @@ public: ) ); - return { {*dynamic_cast*>(it->second.get())}, true }; + return {{*dynamic_cast*>(it->second.get())}, true}; } template