From 0d70812641824054c5d90ddadf6a1fb2529abb65 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Tue, 7 Jul 2026 22:11:34 +0300 Subject: [PATCH] Make CGAL circle-segments 0-default deflection-driven (rework #8368) Address maintainer request on #8368: instead of a deflection floor on top of a fixed CircleSegments count, use one mode or the other. When CircleSegments == 0 (the new default) the CGAL kernel derives the conic segment count from MesherLinearDeflection, matching the deflection based meshing OpenCascade already does and fixing #8051. When CircleSegments is non zero it is used directly as a fixed, radius independent count. CircleSegments is only read by the CGAL kernel; OpenCascade meshes by deflection and never reads it, so the new default has no effect there. Update the setting description and the ifcconvert / geometry-settings docs. Co-Authored-By: Claude Opus 4.8 --- src/ifcgeom/ConversionSettings.h | 4 +- src/ifcgeom/kernels/cgal/CgalKernel.cpp | 40 +++++++++++-------- .../docs/ifcconvert/usage.rst | 8 +++- .../docs/ifcopenshell/geometry_settings.rst | 4 +- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/ifcgeom/ConversionSettings.h b/src/ifcgeom/ConversionSettings.h index febd789f6b..c022bfdd1c 100644 --- a/src/ifcgeom/ConversionSettings.h +++ b/src/ifcgeom/ConversionSettings.h @@ -361,8 +361,8 @@ namespace ifcopenshell { struct CircleSegments : public SettingBase { static constexpr const char* const name = "circle-segments"; - static constexpr const char* const description = "Number of segments to approximate full circles in CGAL kernel."; - static constexpr int defaultvalue = 16; + static constexpr const char* const description = "Number of segments to approximate full circles in the CGAL kernel. When 0 (the default) the segment count is derived from mesher-linear-deflection instead, so curves stay within the deflection tolerance regardless of radius."; + static constexpr int defaultvalue = 0; }; struct CgalSmoothAngleDegrees : public SettingBase { diff --git a/src/ifcgeom/kernels/cgal/CgalKernel.cpp b/src/ifcgeom/kernels/cgal/CgalKernel.cpp index be109dc407..f2d8a6c6b7 100644 --- a/src/ifcgeom/kernels/cgal/CgalKernel.cpp +++ b/src/ifcgeom/kernels/cgal/CgalKernel.cpp @@ -431,22 +431,30 @@ namespace { b += 2 * M_PI; } const double span = std::fabs(a - b); - int num_segments = (int)std::ceil(span / (2 * M_PI) * settings_.get().get()); - // CircleSegments allocates segments as a fraction of the *full* circle and is - // radius-agnostic. A large-radius arc spanning a small angle therefore collapses - // to a single chord (issue #8051: curved curtain-wall mullions became straight in - // the CGAL kernels while OpenCascade, which meshes by deflection, kept them curved). - // Enforce a deflection-based floor so the chord deviation stays within the mesher's - // linear deflection, matching OpenCascade behaviour. - const double radius = conic_radius(t); - const double deflection = settings_.get().get(); - if (deflection > 0. && radius > deflection) { - const double max_segment_angle = 2.0 * std::acos(1.0 - deflection / radius); - if (max_segment_angle > 0.) { - const int num_segments_deflection = (int)std::ceil(span / max_segment_angle); - if (num_segments_deflection > num_segments) { - num_segments = num_segments_deflection; - } + // CircleSegments controls how conics (circles, ellipses, arcs) are approximated + // in the CGAL kernel. Two modes, one or the other: + // - CircleSegments == 0 (the default): the segment count is derived from + // MesherLinearDeflection, so the chord deviation stays within the mesher's + // linear deflection regardless of radius. This matches the deflection based + // meshing the OpenCascade kernel already does and fixes issue #8051, where + // large radius arcs (curved curtain wall mullions) collapsed to straight chords + // because a fixed segment count is radius agnostic. + // - CircleSegments > 0: it is used directly as the number of segments for a full + // circle, giving deterministic, radius independent output. + int num_segments; + const int circle_segments = settings_.get().get(); + if (circle_segments > 0) { + num_segments = (int)std::ceil(span / (2 * M_PI) * circle_segments); + } else { + const double radius = conic_radius(t); + const double deflection = settings_.get().get(); + if (deflection > 0. && radius > deflection) { + const double max_segment_angle = 2.0 * std::acos(1.0 - deflection / radius); + num_segments = (int)std::ceil(span / max_segment_angle); + } else { + // Radius within the deflection tolerance (or no deflection set): a chord per + // quarter turn already keeps the deviation within tolerance. + num_segments = (int)std::ceil(span / (M_PI / 2.)); } } if (num_segments < 1) { diff --git a/src/ifcopenshell-python/docs/ifcconvert/usage.rst b/src/ifcopenshell-python/docs/ifcconvert/usage.rst index 3c6846d25b..6910e7e6fe 100644 --- a/src/ifcopenshell-python/docs/ifcconvert/usage.rst +++ b/src/ifcopenshell-python/docs/ifcconvert/usage.rst @@ -311,8 +311,12 @@ CLI Manual output. --force-space-transparency arg Overrides transparency of spaces in geometry output. - --circle-segments arg (= 16) Number of segments to approximate full - circles in CGAL kernel. + --circle-segments arg (= 0) Number of segments to approximate full + circles in the CGAL kernel. When 0 (the + default) the segment count is derived from + mesher-linear-deflection instead, so curves + stay within the deflection tolerance + regardless of radius. --cgal-smooth-angle-degrees arg (= -1) Angle in degrees under which adjacent facets will have averaged vertex diff --git a/src/ifcopenshell-python/docs/ifcopenshell/geometry_settings.rst b/src/ifcopenshell-python/docs/ifcopenshell/geometry_settings.rst index f73c7edc28..6862088c6c 100644 --- a/src/ifcopenshell-python/docs/ifcopenshell/geometry_settings.rst +++ b/src/ifcopenshell-python/docs/ifcopenshell/geometry_settings.rst @@ -228,10 +228,10 @@ circle-segments +------+-----------------------+---------+ | Type | IfcConvert Option | Default | +======+=======================+=========+ -| INT | ``--circle-segments`` | 16 | +| INT | ``--circle-segments`` | 0 | +------+-----------------------+---------+ -Number of segments to approximate full circles in CGAL kernel. +Number of segments to approximate full circles in the CGAL kernel. When 0 (the default) the segment count is derived from mesher-linear-deflection instead, so curves stay within the deflection tolerance regardless of radius. context-identifiers ^^^^^^^^^^^^^^^^^^^