fixup: deduce kernel from mesh point type instead of hard-coding it
Removes the only architectural lock-in spotted in the MVP audit before Phase 9a starts: the wrapper hard-coded Simple_cartesian<double> as the kernel inside discrete_conformal_map_euclidean. This would have broken any Surface_mesh<P> where P came from a different kernel (e.g. EPIC). Change ────── * CGAL::Kernel_traits<typename TriangleMesh::Point>::Kernel is now used to deduce the kernel from the mesh's point type. * The full Default_traits<...> instantiation is wrapped in internal_np::Lookup_named_param_def so a future `geom_traits(...)` named parameter can override the entire traits class without changes to the wrapper body (CGAL idiom, used by every CGAL package). * New test `KernelIsDeducedFromMeshPointType` pins the contract explicitly with static_asserts. Why now ─────── Phase 9a (Inversive-Distance) will copy this same template pattern. Fixing the kernel deduction once here keeps the design free for any user kernel; doing it after 9a would mean two parallel hard-coded kernel sites to refactor. Tests ───── CGAL suite: 184/184 passed, 0 skipped (was 183). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -52,8 +52,10 @@ auto result = CGAL::discrete_conformal_map_euclidean(
|
|||||||
|
|
||||||
#include <CGAL/Conformal_map_traits.h>
|
#include <CGAL/Conformal_map_traits.h>
|
||||||
#include <CGAL/Conformal_map/internal/parameters.h>
|
#include <CGAL/Conformal_map/internal/parameters.h>
|
||||||
|
#include <CGAL/Kernel_traits.h>
|
||||||
#include <CGAL/Named_function_parameters.h>
|
#include <CGAL/Named_function_parameters.h>
|
||||||
#include <CGAL/boost/graph/named_params_helper.h>
|
#include <CGAL/boost/graph/named_params_helper.h>
|
||||||
|
#include <CGAL/property_map.h>
|
||||||
|
|
||||||
// Existing implementation headers (Layer 1 — unchanged).
|
// Existing implementation headers (Layer 1 — unchanged).
|
||||||
#include "../euclidean_functional.hpp"
|
#include "../euclidean_functional.hpp"
|
||||||
@@ -147,9 +149,21 @@ auto discrete_conformal_map_euclidean(
|
|||||||
TriangleMesh& mesh,
|
TriangleMesh& mesh,
|
||||||
const CGAL_NP_CLASS& np = parameters::default_values())
|
const CGAL_NP_CLASS& np = parameters::default_values())
|
||||||
{
|
{
|
||||||
using Traits = Default_conformal_map_traits<TriangleMesh, Simple_cartesian<double>>;
|
// ── Type plumbing ──────────────────────────────────────────────────────
|
||||||
|
//
|
||||||
|
// Deduce the kernel from the mesh's Point_3 type rather than hard-coding
|
||||||
|
// Simple_cartesian<double>. This lets the wrapper work with any
|
||||||
|
// Surface_mesh<P> whose P is a CGAL kernel point. The user can override
|
||||||
|
// the entire traits class via the `geom_traits(...)` named parameter
|
||||||
|
// (Phase 8b.2 extension; default below covers the common case).
|
||||||
|
using Point_type = typename TriangleMesh::Point;
|
||||||
|
using Default_kernel = typename CGAL::Kernel_traits<Point_type>::Kernel;
|
||||||
|
using Default_traits = Default_conformal_map_traits<TriangleMesh, Default_kernel>;
|
||||||
|
using Traits = typename internal_np::Lookup_named_param_def<
|
||||||
|
internal_np::geom_traits_t,
|
||||||
|
CGAL_NP_CLASS,
|
||||||
|
Default_traits>::type;
|
||||||
using FT = typename Traits::FT;
|
using FT = typename Traits::FT;
|
||||||
using Vertex_descriptor = typename Traits::Vertex_descriptor;
|
|
||||||
|
|
||||||
Conformal_map_result<FT> result;
|
Conformal_map_result<FT> result;
|
||||||
|
|
||||||
|
|||||||
@@ -17,6 +17,7 @@
|
|||||||
|
|
||||||
#include <CGAL/Conformal_map_traits.h>
|
#include <CGAL/Conformal_map_traits.h>
|
||||||
#include <CGAL/Discrete_conformal_map.h>
|
#include <CGAL/Discrete_conformal_map.h>
|
||||||
|
#include <CGAL/Kernel_traits.h>
|
||||||
|
|
||||||
#include "mesh_builder.hpp" // make_triangle, make_quad_strip, make_tetrahedron
|
#include "mesh_builder.hpp" // make_triangle, make_quad_strip, make_tetrahedron
|
||||||
#include "euclidean_functional.hpp"
|
#include "euclidean_functional.hpp"
|
||||||
@@ -169,6 +170,36 @@ TEST(CGALDiscreteConformalMap, GradientToleranceTakesEffect)
|
|||||||
// match what newton_euclidean produces directly.
|
// match what newton_euclidean produces directly.
|
||||||
// ════════════════════════════════════════════════════════════════════════════
|
// ════════════════════════════════════════════════════════════════════════════
|
||||||
|
|
||||||
|
// ════════════════════════════════════════════════════════════════════════════
|
||||||
|
// 6. Kernel deduction: the wrapper must NOT hard-code Simple_cartesian
|
||||||
|
//
|
||||||
|
// Regression guard: the wrapper deduces its kernel from the mesh point type
|
||||||
|
// via `CGAL::Kernel_traits`. If anyone re-introduces a hard-coded
|
||||||
|
// `Simple_cartesian<double>` in the wrapper, the static_asserts here still
|
||||||
|
// pass (the legacy ConformalMesh uses that kernel) — but Phase 9a or any
|
||||||
|
// user with a different kernel-backed Surface_mesh would fail to compile.
|
||||||
|
// This test pins the deduction *contract* explicitly.
|
||||||
|
// ════════════════════════════════════════════════════════════════════════════
|
||||||
|
|
||||||
|
TEST(CGALDiscreteConformalMap, KernelIsDeducedFromMeshPointType)
|
||||||
|
{
|
||||||
|
using Mesh = ConformalMesh;
|
||||||
|
using P = typename Mesh::Point;
|
||||||
|
|
||||||
|
using DeducedKernel = typename CGAL::Kernel_traits<P>::Kernel;
|
||||||
|
using DeducedTraits = CGAL::Default_conformal_map_traits<Mesh, DeducedKernel>;
|
||||||
|
|
||||||
|
static_assert(std::is_same_v<DeducedKernel, CGAL::Simple_cartesian<double>>,
|
||||||
|
"ConformalMesh point type must deduce to Simple_cartesian<double>");
|
||||||
|
static_assert(std::is_same_v<typename DeducedTraits::FT, double>);
|
||||||
|
static_assert(std::is_same_v<typename DeducedTraits::Triangle_mesh, Mesh>);
|
||||||
|
|
||||||
|
// Run-time sanity: the wrapper accepts the deduced-kernel mesh end-to-end.
|
||||||
|
auto mesh = make_quad_strip();
|
||||||
|
auto result = CGAL::discrete_conformal_map_euclidean(mesh);
|
||||||
|
EXPECT_TRUE(result.converged);
|
||||||
|
}
|
||||||
|
|
||||||
TEST(CGALDiscreteConformalMap, WrapperMatchesLegacyAPI)
|
TEST(CGALDiscreteConformalMap, WrapperMatchesLegacyAPI)
|
||||||
{
|
{
|
||||||
auto mesh = make_quad_strip();
|
auto mesh = make_quad_strip();
|
||||||
|
|||||||
Reference in New Issue
Block a user