Regroup both GH policies within a single class

Simpler API (users don't have to explicit the vertex cost map) + clearer
that they must go together.

Surface_mesh_simplification/doc/Surface_mesh_simplification/CGAL/Surface_mesh_simplification/Policies/Edge_collapse/GarlandHeckbert_cost.h

@@ -1,62 +0,0 @@
-namespace CGAL {
-namespace Surface_mesh_simplification {
-
-/*!
-\ingroup PkgSurfaceMeshSimplificationRef
-
-This class is a type accessor for the quadric type.
-*/
-template <typename GeomTraits>
-struct GarlandHeckbert_cost_matrix
-{
-  /// The type of a quadric in the Garland-Heckbert algorithm
-  typedef typename Eigen::Matrix<typename GeomTraits::FT, 4, 4>           type;
-};
-
-/*!
-\ingroup PkgSurfaceMeshSimplificationRef
-
-The class `GarlandHeckbert_cost` provides a model for the `GetCost` concept.
-It computes the collapse cost following the Garland-Heckbert strategy
-(Section \ref SurfaceMeshSimplificationGarlandHeckbertStrategy).
-
-It must be used in conjonction with the Garland-Heckbert placement policy,
-`CGAL::Surface_mesh_simplification::GarlandHeckbert_placement<TriangleMesh>`
-
-\tparam VertexCostMap must be a model of `ReadWritePropertyMap` with value type `GarlandHeckbert_cost_matrix`.
-
-\cgalModels `GetCost`
-
-\sa `CGAL::Surface_mesh_simplification::GarlandHeckbert_placement<VertexCostMap>`
-
-*/
-template<class VertexCostMap>
-class GarlandHeckbert_cost
-{
-public:
-  /// \name Creation
-  /// @{
-
-  /*!
-  Initializes the policy with the given <I>garland heckbert state</I> object.
-  Garland&Heckbert strategy requires a shared state object between cost and placementr policies.
-  */
-  GarlandHeckbert_cost(const VertexCostMap&);
-  /// @}
-
-
-  /// \name Operations
-  /// @{
-
-  /*!
-  Returns the cost of collapsing the edge (represented by its profile) considering
-  the new `placement` computed for it.
-  */
-  boost::optional<typename Edge_profile::FT>
-  operator()(const Edge_profile& profile,
-             const boost::optional<typename Edge_profile::Point>& placement) const;
-  /// @}
-};
-
-} // namespace Surface_mesh_simplification
-} // namespace CGAL

Surface_mesh_simplification/doc/Surface_mesh_simplification/CGAL/Surface_mesh_simplification/Policies/Edge_collapse/GarlandHeckbert_placement.h

@@ -1,48 +0,0 @@
-namespace CGAL {
-namespace Surface_mesh_simplification {
-
-/*!
-\ingroup PkgSurfaceMeshSimplificationRef
-
-The class `GarlandHeckbert_placement` provides a model for the `GetPlacement` concept.
-It computes the placement, that is, the new position for the remaining vertex after
-a halfedge collapse, following the Garland-Heckbert strategy
-(Section \ref SurfaceMeshSimplificationGarlandHeckbertStrategy).
-
-It must be used in conjonction with the Garland-Heckbert cost policy,
-`CGAL::Surface_mesh_simplification::GarlandHeckbert_cost<TriangleMesh>`
-
-\tparam VertexCostMap must be a model of `ReadWritePropertyMap` with value type `GarlandHeckbert_cost_matrix`.
-
-\cgalModels `GetPlacement`
-
-\sa `CGAL::Surface_mesh_simplification::GarlandHeckbert_cost<VertexCostMap>`
-
-*/
-template<class VertexCostMap>
-class GarlandHeckbert_placement
-{
-public:
-  /// \name Creation
-  /// @{
-
-  /*!
-  Initializes the policy with the given <I>garland heckbert state</I> object.
-  Garland&Heckbert strategy requires a shared state object between cost, placement, and visitor policies.
-  */
-  GarlandHeckbert_placement(const VertexCostMap&);
-  /// @}
-
-  /// \name Operations
-  /// @{
-
-  /*!
-  Returns the new position for the remaining vertex after collapsing the edge
-  (represented by its profile).
-  */
-  boost::optional<typename Edge_profile::Point> operator()(const Edge_profile& profile) const;
-  /// @}
-};
-
-} // namespace Surface_mesh_simplification
-} // namespace CGAL

Surface_mesh_simplification/doc/Surface_mesh_simplification/Concepts/GetCost.h

@@ -15,7 +15,7 @@ or can be intentionally returned to prevent the edge from being collapsed.
 
 \cgalHasModel `CGAL::Surface_mesh_simplification::Edge_length_cost<TriangleMesh>`
 \cgalHasModel `CGAL::Surface_mesh_simplification::LindstromTurk_cost<TriangleMesh>`
-\cgalHasModel `CGAL::Surface_mesh_simplification::GarlandHeckbert_cost<VertexCostMap>`
+\cgalHasModel `CGAL::Surface_mesh_simplification::GarlandHeckbert_policies<TriangleMesh, GeomTraits>`
 
 */
 class GetCost

Surface_mesh_simplification/doc/Surface_mesh_simplification/Concepts/GetPlacement.h

@@ -16,7 +16,7 @@ must be kept in place, not moved to a new position.
 
 \cgalHasModel `CGAL::Surface_mesh_simplification::Midpoint_placement<TriangleMesh>`
 \cgalHasModel `CGAL::Surface_mesh_simplification::LindstromTurk_placement<TriangleMesh>`
-\cgalHasModel `CGAL::Surface_mesh_simplification::GarlandHeckbert_placement<VertexCostMap>`
+\cgalHasModel `CGAL::Surface_mesh_simplification::GarlandHeckbert_policies<TriangleMesh, GeomTraits>`
 \cgalHasModel `CGAL::Surface_mesh_simplification::Bounded_normal_change_placement<Placement>`
 \cgalHasModel `CGAL::Surface_mesh_simplification::Constrained_placement<Placement>`
 

Surface_mesh_simplification/doc/Surface_mesh_simplification/PackageDescription.txt

@@ -40,8 +40,7 @@
 - `CGAL::Surface_mesh_simplification::Midpoint_placement<TriangleMesh>`
 - `CGAL::Surface_mesh_simplification::LindstromTurk_cost<TriangleMesh>`
 - `CGAL::Surface_mesh_simplification::LindstromTurk_placement<TriangleMesh>`
-- `CGAL::Surface_mesh_simplification::GarlandHeckbert_cost<VertexCostMap>`
+- `CGAL::Surface_mesh_simplification::GarlandHeckbert_policies<TriangleMesh, GeomTraits>`
-- `CGAL::Surface_mesh_simplification::GarlandHeckbert_placement<VertexCostMap>`
 - `CGAL::Surface_mesh_simplification::Bounded_normal_change_placement<Placement>`
 - `CGAL::Surface_mesh_simplification::Constrained_placement<Placement, TriangleMesh>`
 */

Surface_mesh_simplification/doc/Surface_mesh_simplification/Surface_mesh_simplification.txt

@@ -365,15 +365,12 @@ steps of the simplification algorithm.
 
 \subsection Surface_mesh_simplificationExampleWithGarlandHeckbertUsingAPolyhedron Example with Garland-Heckbert using a Polyhedron
 
-Garland-Heckbert simplification is implemented with the following two policy classes:
+Garland-Heckbert simplification is implemented with a single class,
-
+`Surface_mesh_simplification::GarlandHeckbert_policies`, that regroups both a cost and
-- `Surface_mesh_simplification::GarlandHeckbert_cost` is used as the `GetCost` policy,
+a placement policy class: this is because either of these policies require the other one to function properly.
-- `Surface_mesh_simplification::GarlandHeckbert_placement` is used as the `GetPlacement` policy.
+Note that it is still possible to wrap either policy, for example it is possible to use
-
+behavior modifiers such as `Surface_mesh_simplification::Bounded_normal_change_placement`
-Either of this policy require the other one to function properly. Note that it is still possible
+with the Garland-Heckbert placement policy.
-to wrap either policy, for example it is possible to use `Surface_mesh_simplification::Bounded_normal_change_placement`
-with the Garland-Heckbert placement policy, to reduce the odds of self-intersections in the final
-mesh.
 
 \cgalExample{Surface_mesh_simplification/edge_collapse_garland_heckbert.cpp}
 */

Surface_mesh_simplification/examples/Surface_mesh_simplification/edge_collapse_garland_heckbert.cpp

@@ -4,8 +4,7 @@
 #include <CGAL/Surface_mesh_simplification/edge_collapse.h>
 #include <CGAL/Surface_mesh_simplification/Policies/Edge_collapse/Count_ratio_stop_predicate.h>
 #include <CGAL/Surface_mesh_simplification/Policies/Edge_collapse/Bounded_normal_change_placement.h>
-#include <CGAL/Surface_mesh_simplification/Policies/Edge_collapse/GarlandHeckbert_cost.h>
+#include <CGAL/Surface_mesh_simplification/Policies/Edge_collapse/GarlandHeckbert_policies.h>
-#include <CGAL/Surface_mesh_simplification/Policies/Edge_collapse/GarlandHeckbert_placement.h>
 
 #include <chrono>
 #include <iostream>
@@ -51,20 +50,18 @@ int main(int argc, char** argv)
 
   // Garland&Heckbert simplification maintains an error matrix at each vertex,
   // which must be accessible for the cost and placement evaluations.
-  typedef typename SMS::GarlandHeckbert_cost_matrix<Kernel>::type               Cost_matrix;
-  typedef CGAL::dynamic_vertex_property_t<Cost_matrix>                          Cost_property;
-  typedef typename boost::property_map<Surface_mesh, Cost_property>::type       Vertex_cost_map;
-  Vertex_cost_map vcm = get(Cost_property(), surface_mesh);
 
-  typedef SMS::GarlandHeckbert_cost<Vertex_cost_map>                            GH_cost;
+  typedef typename SMS::GarlandHeckbert_policies<Surface_mesh, Kernel>          GH_policies;
-  typedef SMS::GarlandHeckbert_placement<Vertex_cost_map>                       GH_placement;
+  typedef typename GH_policies::Get_cost                                        GH_cost;
+  typedef typename GH_policies::Get_placement                                   GH_placement;
   typedef SMS::Bounded_normal_change_placement<GH_placement>                    Bounded_GH_placement;
 
-  GH_cost cost(vcm);
+  GH_policies gh_policies(surface_mesh);
-  GH_placement gh_placement(vcm);
+  const GH_cost& gh_cost = gh_policies.get_cost();
+  const GH_placement& gh_placement = gh_policies.get_placement();
   Bounded_GH_placement placement(gh_placement);
 
-  int r = SMS::edge_collapse(surface_mesh, stop, CGAL::parameters::get_cost(cost)
+  int r = SMS::edge_collapse(surface_mesh, stop, CGAL::parameters::get_cost(gh_cost)
                                                                   .get_placement(placement));
 
   std::chrono::steady_clock::time_point end_time = std::chrono::steady_clock::now();

Surface_mesh_simplification/include/CGAL/Surface_mesh_simplification/Policies/Edge_collapse/GarlandHeckbert_placement.h

@@ -1,82 +0,0 @@
-// Copyright (c) 2019  GeometryFactory (France). All rights reserved.
-//
-// This file is part of CGAL (www.cgal.org).
-// You can redistribute it and/or modify it under the terms of the GNU
-// General Public License as published by the Free Software Foundation,
-// either version 3 of the License, or (at your option) any later version.
-//
-// Licensees holding a valid commercial license may use this file in
-// accordance with the commercial license agreement provided with the software.
-//
-// This file is provided AS IS with NO WARRANTY OF ANY KIND, INCLUDING THE
-// WARRANTY OF DESIGN, MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE.
-//
-// $URL$
-// $Id$
-// SPDX-License-Identifier: GPL-3.0+
-//
-// Author(s)     : Baskin Burak Senbaslar
-//
-
-#ifndef CGAL_SURFACE_MESH_SIMPLIFICATION_POLICIES_EDGE_COLLAPSE_GARLANDHECKBERT_PLACEMENT_H
-#define CGAL_SURFACE_MESH_SIMPLIFICATION_POLICIES_EDGE_COLLAPSE_GARLANDHECKBERT_PLACEMENT_H
-
-#include <CGAL/license/Surface_mesh_simplification.h>
-
-#include <CGAL/Surface_mesh_simplification/internal/Common.h>
-#include <CGAL/Surface_mesh_simplification/Policies/Edge_collapse/internal/GarlandHeckbert_core.h>
-
-#include <boost/optional/optional.hpp>
-
-namespace CGAL {
-namespace Surface_mesh_simplification {
-
-template<typename VCM_>
-class GarlandHeckbert_placement
-{
-public:
-  typedef VCM_                                                                             Vertex_cost_map;
-
-  GarlandHeckbert_placement(const Vertex_cost_map& cost_matrices)
-    : m_cost_matrices(cost_matrices)
-  { }
-
-  template <typename Profile>
-  boost::optional<typename Profile::Point>
-  operator()(const Profile& profile) const
-  {
-    typedef typename Profile::Triangle_mesh                                                Triangle_mesh;
-    typedef typename Profile::Vertex_point_map                                             Vertex_point_map;
-    typedef typename Profile::Geom_traits                                                  Geom_traits;
-    typedef internal::GarlandHeckbert_core<Triangle_mesh, Vertex_point_map, Geom_traits>   GH_core;
-
-    typedef typename GH_core::Matrix4x4                                                    Matrix4x4;
-    typedef typename GH_core::Col4                                                         Col4;
-
-    CGAL_precondition(get(m_cost_matrices, profile.v0()) != Matrix4x4());
-    CGAL_precondition(get(m_cost_matrices, profile.v1()) != Matrix4x4());
-
-    // the combined matrix has already been computed in the evaluation of the cost...
-    const Matrix4x4 combinedMatrix = GH_core::combine_matrices(
-                                       get(m_cost_matrices, profile.v0()),
-                                       get(m_cost_matrices, profile.v1()));
-
-    const Col4 p0 = GH_core::point_to_homogenous_column(profile.p0());
-    const Col4 p1 = GH_core::point_to_homogenous_column(profile.p1());
-    const Col4 opt = GH_core::optimal_point(combinedMatrix, p0, p1);
-
-    boost::optional<typename Profile::Point> pt = typename Profile::Point(opt(0) / opt(3),
-                                                                          opt(1) / opt(3),
-                                                                          opt(2) / opt(3));
-
-    return pt;
-  }
-
-private:
-  const Vertex_cost_map& m_cost_matrices;
-};
-
-} // namespace Surface_mesh_simplification
-} // namespace CGAL
-
-#endif // CGAL_SURFACE_MESH_SIMPLIFICATION_POLICIES_EDGE_COLLAPSE_GARLANDHECKBERT_PLACEMENT_H

Surface_mesh_simplification/include/CGAL/Surface_mesh_simplification/Policies/Edge_collapse/GarlandHeckbert_policies.h

@@ -18,9 +18,8 @@
 // Author(s)     : Baskin Burak Senbaslar,
 //                 Mael Rouxel-Labbé
 //
-
+#ifndef CGAL_SURFACE_MESH_SIMPLIFICATION_POLICIES_GARLANDHECKBERT_POLICIES_H
-#ifndef CGAL_SURFACE_MESH_SIMPLIFICATION_POLICIES_EDGE_COLLAPSE_GARLANDHECKBERT_COST_H
+#define CGAL_SURFACE_MESH_SIMPLIFICATION_POLICIES_GARLANDHECKBERT_POLICIES_H
-#define CGAL_SURFACE_MESH_SIMPLIFICATION_POLICIES_EDGE_COLLAPSE_GARLANDHECKBERT_COST_H
 
 #include <CGAL/license/Surface_mesh_simplification.h>
 
@@ -29,32 +28,30 @@
 
 #include <CGAL/tags.h>
 
+#include <Eigen/Dense>
+
 #include <boost/optional/optional.hpp>
 
 #include <utility>
 
 namespace CGAL {
 namespace Surface_mesh_simplification {
+namespace internal {
 
-template <typename GT_>
+template <typename VCM_, typename FT_>
-struct GarlandHeckbert_cost_matrix
-{
-  typedef typename Eigen::Matrix<typename GT_::FT, 4, 4>           type;
-};
-
-template <typename VCM_>
 class GarlandHeckbert_cost
 {
 public:
   typedef VCM_                                                     Vertex_cost_map;
-  typedef typename boost::property_traits<VCM_>::value_type        Cost_matrix; // @tmp name
+  typedef FT_                                                      FT;
 
-  // Tells the edge collapse main function that we need to call "initialize"
+  // Tells the main function of `Edge_collapse` that these policies must call "initialize"
   // and "update" functions. A bit awkward, but still better than abusing visitors.
   typedef CGAL::Tag_true                                           Update_tag;
 
-  GarlandHeckbert_cost(Vertex_cost_map& vcm,
+  GarlandHeckbert_cost() { }
-                       const double discontinuity_multiplier = 100.) // abusing FT(double)
+  GarlandHeckbert_cost(Vertex_cost_map vcm,
+                       const FT discontinuity_multiplier = FT(100)) // abusing FT(double)
     :
       m_cost_matrices(vcm),
       m_discontinuity_multiplier(discontinuity_multiplier)
@@ -112,10 +109,91 @@ public:
 
 private:
   Vertex_cost_map m_cost_matrices;
-  const double m_discontinuity_multiplier;
+  FT m_discontinuity_multiplier;
+};
+
+template<typename VCM_>
+class GarlandHeckbert_placement
+{
+public:
+  typedef VCM_                                                                             Vertex_cost_map;
+
+  GarlandHeckbert_placement() { }
+  GarlandHeckbert_placement(Vertex_cost_map cost_matrices)
+    : m_cost_matrices(cost_matrices)
+  { }
+
+  template <typename Profile>
+  boost::optional<typename Profile::Point>
+  operator()(const Profile& profile) const
+  {
+    typedef typename Profile::Triangle_mesh                                                Triangle_mesh;
+    typedef typename Profile::Vertex_point_map                                             Vertex_point_map;
+    typedef typename Profile::Geom_traits                                                  Geom_traits;
+    typedef internal::GarlandHeckbert_core<Triangle_mesh, Vertex_point_map, Geom_traits>   GH_core;
+
+    typedef typename GH_core::Matrix4x4                                                    Matrix4x4;
+    typedef typename GH_core::Col4                                                         Col4;
+
+    CGAL_precondition(get(m_cost_matrices, profile.v0()) != Matrix4x4());
+    CGAL_precondition(get(m_cost_matrices, profile.v1()) != Matrix4x4());
+
+    // the combined matrix has already been computed in the evaluation of the cost...
+    const Matrix4x4 combinedMatrix = GH_core::combine_matrices(
+                                       get(m_cost_matrices, profile.v0()),
+                                       get(m_cost_matrices, profile.v1()));
+
+    const Col4 p0 = GH_core::point_to_homogenous_column(profile.p0());
+    const Col4 p1 = GH_core::point_to_homogenous_column(profile.p1());
+    const Col4 opt = GH_core::optimal_point(combinedMatrix, p0, p1);
+
+    boost::optional<typename Profile::Point> pt = typename Profile::Point(opt(0) / opt(3),
+                                                                          opt(1) / opt(3),
+                                                                          opt(2) / opt(3));
+
+    return pt;
+  }
+
+private:
+  Vertex_cost_map m_cost_matrices;
+};
+
+} // namespace internal
+
+template <typename TriangleMesh, typename GeomTraits>
+class GarlandHeckbert_policies
+{
+public:
+  typedef TriangleMesh                                                          Triangle_mesh;
+  typedef GeomTraits                                                            Geom_traits;
+  typedef typename Geom_traits::FT                                              FT;
+
+  typedef typename Eigen::Matrix<FT, 4, 4>                                      Cost_matrix;
+  typedef CGAL::dynamic_vertex_property_t<Cost_matrix>                          Cost_property;
+  typedef typename boost::property_map<TriangleMesh, Cost_property>::type       Vertex_cost_map;
+
+  typedef internal::GarlandHeckbert_cost<Vertex_cost_map, FT>                   Get_cost;
+  typedef internal::GarlandHeckbert_placement<Vertex_cost_map>                  Get_placement;
+
+  GarlandHeckbert_policies(TriangleMesh& tmesh,
+                           const FT discontinuity_multiplier = FT(100))
+  {
+    Vertex_cost_map vcm = get(Cost_property(), tmesh);
+    get_cost_ = Get_cost(vcm);
+    get_placement_ = Get_placement(vcm);
+  }
+
+  Get_cost& get_cost() { return get_cost_; }
+  const Get_cost& get_cost() const { return get_cost_; }
+  Get_placement& get_placement() { return get_placement_; }
+  const Get_placement& get_placement() const { return get_placement_; }
+
+private:
+  Get_cost get_cost_;
+  Get_placement get_placement_;
 };
 
 } // namespace Surface_mesh_simplification
 } // namespace CGAL
 
-#endif // CGAL_SURFACE_MESH_SIMPLIFICATION_POLICIES_EDGE_COLLAPSE_GARLANDHECKBERT_COST_H
+#endif // CGAL_SURFACE_MESH_SIMPLIFICATION_POLICIES_GARLANDHECKBERT_POLICIES_H