-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Partition_2: add Container template parameter to Partition_traits_2 #9351
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,7 +25,9 @@ | |
|
|
||
| namespace CGAL { | ||
|
|
||
| template <class Base_traits, class PointPropertyMap = Identity_property_map<typename Base_traits::Point_2> > | ||
| template <class Base_traits, | ||
| class PointPropertyMap = Identity_property_map<typename Base_traits::Point_2>, | ||
| class Container_ = std::list<typename boost::property_traits<PointPropertyMap>::key_type> > | ||
| class Partition_traits_2; | ||
|
Comment on lines
+28
to
31
|
||
|
|
||
| template <typename BT, typename PM> | ||
|
|
@@ -38,12 +40,12 @@ class Partition_traits_2; | |
| typedef BT type; | ||
| }; | ||
|
|
||
| template <class Base_traits, class PointPropertyMap> | ||
| template <class Base_traits, class PointPropertyMap, class Container_> | ||
| class Partition_traits_2 : public Base_traits | ||
| { | ||
| private: | ||
| typedef Base_traits Kernel; | ||
| typedef Partition_traits_2<Base_traits,PointPropertyMap> Self; | ||
| typedef Partition_traits_2<Base_traits,PointPropertyMap,Container_> Self; | ||
|
|
||
| PointPropertyMap ppmap; | ||
|
|
||
|
|
@@ -62,7 +64,7 @@ class Partition_traits_2 : public Base_traits | |
| typedef typename boost::property_traits<PointPropertyMap>::key_type Point_2; | ||
| typedef typename boost::call_traits<Point_2>::param_type Arg_type; | ||
|
|
||
| typedef ::std::list<Point_2> Container; | ||
| typedef Container_ Container; | ||
| typedef typename Polygon_traits_getter<Base_traits,PointPropertyMap>::type PolygonTraits; | ||
| typedef CGAL::Polygon_2<PolygonTraits, Container> Polygon_2; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| // Regression test for the Container_ template parameter of Partition_traits_2. | ||
| // Verifies that a user-specified container type (std::vector) can be used in | ||
| // place of the default std::list for the output Polygon_2 vertex storage. | ||
| // Fixes: https://github.com/CGAL/cgal/issues/7545 | ||
|
|
||
| #include <CGAL/Simple_cartesian.h> | ||
| #include <CGAL/Partition_traits_2.h> | ||
| #include <CGAL/partition_2.h> | ||
| #include <list> | ||
| #include <vector> | ||
| #include <cassert> | ||
|
|
||
| typedef CGAL::Simple_cartesian<double> K; | ||
| typedef K::Point_2 Point_2; | ||
|
|
||
| // Traits that stores polygon vertices in a std::vector instead of std::list. | ||
| typedef CGAL::Partition_traits_2<K, | ||
| CGAL::Identity_property_map<Point_2>, | ||
| std::vector<Point_2>> VecTraits; | ||
| typedef VecTraits::Polygon_2 VecPolygon_2; | ||
| typedef std::list<VecPolygon_2> VecPolygon_list; | ||
|
|
||
| int main() | ||
| { | ||
| // Build a simple non-convex CCW input polygon (same as the standard | ||
| // Partition_2 test suite polygon — proven to exercise all partitioners). | ||
| VecPolygon_2 polygon; | ||
| polygon.push_back(Point_2(227,423)); | ||
| polygon.push_back(Point_2(123,364)); | ||
| polygon.push_back(Point_2(129,254)); | ||
| polygon.push_back(Point_2(230,285)); | ||
| polygon.push_back(Point_2(231,128)); | ||
| polygon.push_back(Point_2(387,205)); | ||
| polygon.push_back(Point_2(417,331)); | ||
| polygon.push_back(Point_2(319,225)); | ||
| polygon.push_back(Point_2(268,293)); | ||
| polygon.push_back(Point_2(367,399)); | ||
| polygon.push_back(Point_2(298,418)); | ||
| polygon.push_back(Point_2(196,326)); | ||
|
|
||
| // Partition using y_monotone — output polygons use std::vector vertices. | ||
| VecPolygon_list partition_polys; | ||
| CGAL::y_monotone_partition_2(polygon.vertices_begin(), | ||
| polygon.vertices_end(), | ||
| std::back_inserter(partition_polys), | ||
| VecTraits()); | ||
|
|
||
| assert(!partition_polys.empty()); | ||
|
|
||
| // Verify the vertex container is indeed std::vector<Point_2>. | ||
| static_assert( | ||
| std::is_same<VecTraits::Container, std::vector<Point_2>>::value, | ||
| "Container typedef must alias the Container_ template parameter"); | ||
|
|
||
| // Partition using approx_convex. | ||
| partition_polys.clear(); | ||
| CGAL::approx_convex_partition_2(polygon.vertices_begin(), | ||
| polygon.vertices_end(), | ||
| std::back_inserter(partition_polys), | ||
| VecTraits()); | ||
| assert(!partition_polys.empty()); | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the documentation header, the template parameter is named
Container, and the nested typedef is written astypedef Container Container;. This effectively redefines the template parameter name in the same scope and does not document the actual container type alias (it should alias the template parameter type, not itself). Consider renaming the template parameter toContainer_(matching the implementation) and definingtypedef Container_ Container;so the documented API matches the real one and parses cleanly.