feat!: (fix + chore) streamline nSegments usage (#3419)

The generation of display vertices for segments was buggy and very unstable, I have put this onto more solid grounds, which helps displaying (not only) `AnnulusBounds` correctly.

ITk Petal Before:

![Screenshot 2024-07-18 at 17 01 59](https://github.com/user-attachments/assets/930b93f2-8df7-4e80-a44b-3ec7a20a84ac)

ITk Petal After:

![Screenshot 2024-07-19 at 12 07 32](https://github.com/user-attachments/assets/bc4edd4b-cc22-4b30-bad8-6032d42854da)

When checking the code, I see that there was a pretty big inconsistency in how the nSegments have been handled, so I re-worked that:

* `nSegments` (full 2 * PI segments) -> changed and renamed to `quaterSegments`

This guarantees that the number of segments for a full circle is a multiplier of 4 and hence the phi extrema points at `(-pi,-0.5*pi,0,0.5*pi,pi)` are consistently added. This will lead to a correct `x` and `y` estimation for the `Extent` of the `Polyhedron`.

This also guarantees that e.g. overlapping/attaching surfaces with the same number of segments will have vertices at the same positions, which will be better for the displaying.

* inconsitent use of `unsigned int` (SurfaceBounds) and `std::size_t` -> changed to `unsigned int everywhere

* Introduces tests for the vertex generation (which were missing)


Co-authored-by: Paul Gessinger <1058585+paulgessinger@users.noreply.github.com>

Core/include/Acts/Detector/LayerStructureBuilder.hpp

@@ -91,8 +91,9 @@ class LayerStructureBuilder : public IInternalStructureBuilder {
     /// Minimum number of surfaces to build an internal structure
     /// - otherwise the tryAll options is used
     unsigned int nMinimalSurfaces = 4u;
-    /// Polyhedron approximations
-    unsigned int nSegments = 1u;
+    /// Polyhedron approximations: number of segments to be used
+    /// to approximate a quarter of a circle
+    unsigned int quarterSegments = 1u;
     /// Extra information, mainly for screen output
     std::string auxiliary = "";
   };

Core/include/Acts/Geometry/Polyhedron.hpp

@@ -39,6 +39,7 @@ struct Polyhedron {
   /// @param facesIn List of lists of indices for faces.
   /// @param triangularMeshIn List of lists of indices for a triangular mesh
   /// @param isExact A dedicated flag if this is exact or not
+  ///
   /// @note This creates copies of the input vectors
   Polyhedron(const std::vector<Vector3>& verticesIn,
              const std::vector<FaceType>& facesIn,

Core/include/Acts/Surfaces/AnnulusBounds.hpp

@@ -133,19 +133,20 @@ class AnnulusBounds : public DiscBounds {
   std::vector<Vector2> corners() const;
 
   /// This method returns the xy coordinates of the four corners of the
-  /// bounds in module coordinates (in x/y)
+  /// bounds in module coordinates (in x/y), and if quarterSegments is bigger or
+  /// equal to 0, the curved part of the segment is included and approximated
+  /// by the corresponding number of segments.
+  ///
   /// Starting from the upper right (max R, pos locX) and proceeding clock-wise
   /// i.e. (max R; pos locX), (min R; pos locX), (min R; neg loc X), (max R: neg
   /// locX)
   ///
-  /// @param lseg the number of segments used to approximate
-  /// and eventually curved line
-  ///
-  /// @note that that if @c lseg > 0, the extrema points are given,
-  ///  which may slightly alter the number of segments returned
+  /// @param quarterSegments the number of segments used to approximate
+  /// a quarter of a circle
   ///
   /// @return vector for vertices in 2D
-  std::vector<Vector2> vertices(unsigned int lseg) const override;
+  std::vector<Vector2> vertices(
+      unsigned int quarterSegments = 2u) const override;
 
   /// This method returns inner radius
   double rMin() const final;

Core/include/Acts/Surfaces/ConeSurface.hpp

@@ -200,15 +200,16 @@ class ConeSurface : public RegularSurface {
   /// Return a Polyhedron for the surfaces
   ///
   /// @param gctx The current geometry context object, e.g. alignment
-  /// @param lseg Number of segments along curved lines, it represents
-  /// the full 2*M_PI coverange, if lseg is set to 1 only the extrema
-  /// are given
-  /// @note that a surface transform can invalidate the extrema
-  /// in the transformed space
+  /// @param quarterSegments Number of segments used to approximate a quarter
+  ///
+  /// @note The phi extrema points at (-pi, -1/2 pi, 0, 1/2 pi) that fall within
+  /// the surface will be inserted to guarantee an appropriate extent
+  /// measurement in x and y
   ///
   /// @return A list of vertices and a face/facett description of it
-  Polyhedron polyhedronRepresentation(const GeometryContext& gctx,
-                                      std::size_t lseg) const override;
+  Polyhedron polyhedronRepresentation(
+      const GeometryContext& gctx,
+      unsigned int quarterSegments = 2u) const override;
 
   /// Return properly formatted class name for screen output
   std::string name() const override;

Core/include/Acts/Surfaces/ConvexPolygonBounds.hpp

@@ -114,13 +114,13 @@ class ConvexPolygonBounds : public ConvexPolygonBoundsBase {
 
   /// Return the vertices
   ///
-  /// @param lseg the number of segments used to approximate
+  /// @param ignoredSegments the number of segments used to approximate
   /// and eventually curved line
   ///
   /// @note the number of segments is ignored in this representation
   ///
   /// @return vector for vertices in 2D
-  std::vector<Vector2> vertices(unsigned int lseg = 1) const final;
+  std::vector<Vector2> vertices(unsigned int ignoredSegments = 0u) const final;
 
   /// Return a rectangle bounds object that encloses this polygon.
   /// @return The rectangular bounds

Core/include/Acts/Surfaces/ConvexPolygonBounds.ipp

@@ -111,7 +111,7 @@ bool Acts::ConvexPolygonBounds<N>::inside(
 
 template <int N>
 std::vector<Acts::Vector2> Acts::ConvexPolygonBounds<N>::vertices(
-    unsigned int /*lseg*/) const {
+    unsigned int /*ignoredSegments*/) const {
   return {m_vertices.begin(), m_vertices.end()};
 }
 

Core/include/Acts/Surfaces/CylinderBounds.hpp

@@ -112,12 +112,18 @@ class CylinderBounds : public SurfaceBounds {
   /// Returns true for full phi coverage
   bool coversFullAzimuth() const;
 
-  /// Create the bows/circles on either side of the cylinder
+  /// Create the bow/circle vertices on either side of the cylinder
   ///
-  /// @param trans is the global transform
-  /// @param lseg  are the numbero if phi segments
-  std::vector<Vector3> createCircles(const Transform3 trans,
-                                     std::size_t lseg) const;
+  /// @param transform is the global transform
+  /// @param quarterSegments is the number of segments to approximate a quarter
+  /// of a circle. In order to symmetrize fully closed and sectoral cylinders,
+  /// also in the first case the two end points are given (albeit they overlap)
+  /// in -pi / pi
+  ///
+  /// @return a singlevector containing the vertices from one side and then
+  /// from the other side consecutively
+  std::vector<Vector3> circleVertices(const Transform3 transform,
+                                      unsigned int quarterSegments) const;
 
   /// Output Method for std::ostream
   std::ostream& toStream(std::ostream& sl) const final;

Core/include/Acts/Surfaces/CylinderSurface.hpp

@@ -214,14 +214,20 @@ class CylinderSurface : public RegularSurface {
 
   /// Return a Polyhedron for a cylinder
   ///
+  /// This method represents the cylinder as a polyhedron with a given number
+  /// of segments to represent a quarter of a full circle. The polyedron will
+  /// consist of the vertices of the cylinder on both sides, and faces between
+  /// them, both as rectangular faces and as triangular faces.
+  ///
   /// @param gctx The current geometry context object, e.g. alignment
-  /// @param lseg Number of segments along curved lines, it represents
-  /// the full 2*M_PI coverange, if lseg is set to 1 only the extrema
-  /// are given
+  /// @param quarterSegments The number of segments to approximate a quarter of the
+  /// full circle; it's chosen to be 1, only the extrema points (-pi, -0.5pi,
+  /// 0., 0.5pi) are inserted to capture the correct extent in the x-y plane
   ///
   /// @return A list of vertices and a face/facett description of it
-  Polyhedron polyhedronRepresentation(const GeometryContext& gctx,
-                                      std::size_t lseg) const override;
+  Polyhedron polyhedronRepresentation(
+      const GeometryContext& gctx,
+      unsigned int quarterSegments = 2u) const override;
 
   /// Calculate the derivative of path length at the geometry constraint or
   /// point-of-closest-approach w.r.t. alignment parameters of the surface (i.e.

Core/include/Acts/Surfaces/DiamondBounds.hpp

@@ -90,15 +90,13 @@ class DiamondBounds : public PlanarBounds {
   bool inside(const Vector2& lposition,
               const BoundaryTolerance& boundaryTolerance) const final;
 
-  /// Return the vertices
+  /// Return the vertices that describe this shape
   ///
-  /// @param lseg the number of segments used to approximate
-  /// and eventually curved line
-  ///
-  /// @note the number of segments is ignored for this representation
+  /// @param ignoredSegments is an ignored parameter only used for
+  /// curved bound segments
   ///
   /// @return vector for vertices in 2D
-  std::vector<Vector2> vertices(unsigned int lseg = 1) const final;
+  std::vector<Vector2> vertices(unsigned int ignoredSegments = 0u) const final;
 
   // Bounding box representation
   const RectangleBounds& boundingBox() const final;

Core/include/Acts/Surfaces/DiscBounds.hpp

@@ -29,14 +29,13 @@ class DiscBounds : public SurfaceBounds {
 
   /// Return the vertices
   ///
-  /// @param lseg the number of segments used to approximate
-  /// and eventually curved line, the number refers to full 2*PI
-  ///
-  /// @note that the extremas are given, which may slightly alter the
-  /// number of segments returned
+  /// @param quarterSegments The number of segments used to describe a quarter
+  /// of a circle, if it is 1, then only the extrema points in phi are inserted
+  /// next to the segment corners
   ///
   /// @return vector for vertices in 2D
-  virtual std::vector<Vector2> vertices(unsigned int lseg) const = 0;
+  virtual std::vector<Vector2> vertices(
+      unsigned int quarterSegments = 2u) const = 0;
 
   /// Returns a reference radius for binning
   virtual double binningValueR() const = 0;

Core/include/Acts/Surfaces/DiscSurface.hpp

@@ -312,13 +312,12 @@ class DiscSurface : public RegularSurface {
   /// Return a Polyhedron for the surfaces
   ///
   /// @param gctx The current geometry context object, e.g. alignment
-  /// @param lseg Number of segments along curved lines, it represents
-  /// the full 2*M_PI coverange, if lseg is set to 1 only the extrema
-  /// are given
+  /// @param quarterSegments Number of segments used to describe the
+  /// quarter of a full circle
   ///
   /// @return A list of vertices and a face/facett description of it
-  Polyhedron polyhedronRepresentation(const GeometryContext& gctx,
-                                      std::size_t lseg) const override;
+  Polyhedron polyhedronRepresentation(
+      const GeometryContext& gctx, unsigned int quarterSegments) const override;
 
   /// Calculate the derivative of bound track parameters local position w.r.t.
   /// position in local 3D Cartesian coordinates

Core/include/Acts/Surfaces/DiscTrapezoidBounds.hpp

@@ -123,13 +123,11 @@ class DiscTrapezoidBounds : public DiscBounds {
   /// This method returns the xy coordinates of the four corners of the
   /// bounds in module coorindates (in xy)
   ///
-  /// @param lseg the number of segments used to approximate
-  /// and eventually curved line
-  ///
-  /// @note that the number of segments are ignored for this surface
+  /// @param ignoredSegments is an ignored parameter only used for
+  /// curved bound segments
   ///
   /// @return vector for vertices in 2D
-  std::vector<Vector2> vertices(unsigned int lseg) const final;
+  std::vector<Vector2> vertices(unsigned int ignoredSegments = 0u) const final;
 
  private:
   std::array<double, eSize> m_values;

Core/include/Acts/Surfaces/EllipseBounds.hpp

@@ -92,14 +92,13 @@ class EllipseBounds : public PlanarBounds {
 
   /// Return the vertices
   ///
-  /// @param lseg the number of segments used to approximate
-  /// and eventually curved line, here it refers to the full 2PI Ellipse
-  ///
-  /// @note the number of segments to may be altered by also providing
-  /// the extremas in all direction
+  /// @param quarterSegments is the number of segments to approximate a quarter
+  /// of a circle. In order to symmetrize fully closed and sectoral cylinders,
+  /// also in the first case the two end points are given (albeit they overlap)
+  /// in -pi / pi
   ///
   /// @return vector for vertices in 2D
-  std::vector<Vector2> vertices(unsigned int lseg) const final;
+  std::vector<Vector2> vertices(unsigned int quarterSegments) const final;
 
   // Bounding box representation
   const RectangleBounds& boundingBox() const final;

Core/include/Acts/Surfaces/PerigeeSurface.hpp

@@ -76,11 +76,11 @@ class PerigeeSurface : public LineSurface {
   /// Return a Polyhedron for the surfaces
   ///
   /// @param gctx The current geometry context object, e.g. alignment
-  /// @param lseg is ignored for a perigee @note ignored
+  /// @param ingoreSegments is an ignored parameter
   ///
   /// @return A list of vertices and a face/facett description of it
   Polyhedron polyhedronRepresentation(const GeometryContext& gctx,
-                                      std::size_t lseg) const final;
+                                      unsigned int ingoreSegments) const final;
 
  protected:
   /// Output Method for std::ostream

Core/include/Acts/Surfaces/PlanarBounds.hpp

@@ -26,14 +26,15 @@ class PlanarBounds : public SurfaceBounds {
  public:
   /// Return the vertices
   ///
-  /// @param lseg the number of segments used to approximate
-  /// and eventually curved line
+  /// @param quarterSegments is the number of segments used to describe curved
+  /// segments in a quarter of the phi range. If it is 1, then only the extrema
+  /// points in phi are inserted next to the segment corners.
   ///
-  /// @note that the extremas are given, which may slightly alter the
-  /// number of segments returned
+  /// @note for planar bounds without curved segments @c quarterSegments is ignored
   ///
   /// @return vector for vertices in 2D
-  virtual std::vector<Vector2> vertices(unsigned int lseg = 1) const = 0;
+  virtual std::vector<Vector2> vertices(
+      unsigned int quarterSegments = 2u) const = 0;
 
   /// Bounding box parameters
   ///

Core/include/Acts/Surfaces/PlaneSurface.hpp

@@ -198,13 +198,15 @@ class PlaneSurface : public RegularSurface {
   /// Return a Polyhedron for the surfaces
   ///
   /// @param gctx The current geometry context object, e.g. alignment
-  /// @param lseg Number of segments along curved lines, it represents
-  /// the full 2*M_PI coverange, if lseg is set to 1 only the extrema
-  /// are given
+  /// @param quarterSegments is the number of segments used to describe curved
+  /// segments in a quarter of the phi range. If it is 1, then only the extrema
+  /// points in phi are inserted next to the segment corners.
+  ///
+  /// @note for planar surfaces without curved segments @c quarterSegments is ignored
   ///
   /// @return A list of vertices and a face/facett description of it
-  Polyhedron polyhedronRepresentation(const GeometryContext& gctx,
-                                      std::size_t lseg) const override;
+  Polyhedron polyhedronRepresentation(
+      const GeometryContext& gctx, unsigned int quarterSegments) const override;
 
   /// Return properly formatted class name for screen output
   std::string name() const override;

Core/include/Acts/Surfaces/RectangleBounds.hpp

@@ -86,13 +86,12 @@ class RectangleBounds : public PlanarBounds {
 
   /// Return the vertices
   ///
-  /// @param lseg the number of segments used to approximate
-  /// and eventually curved line
-  ///
+  /// @param quarterSegments is the number of segments used to describe curved
+  /// segments in a quarter of the phi range.
   /// @note the number of segments is ignored in this representation
   ///
   /// @return vector for vertices in 2D
-  std::vector<Vector2> vertices(unsigned int lseg = 1) const final;
+  std::vector<Vector2> vertices(unsigned int quarterSegments = 0u) const final;
 
   // Bounding box representation
   const RectangleBounds& boundingBox() const final;

Core/include/Acts/Surfaces/StrawSurface.hpp

@@ -92,13 +92,13 @@ class StrawSurface : public LineSurface {
   /// Return a Polyhedron for the surfaces
   ///
   /// @param gctx The current geometry context object, e.g. alignment
-  /// @param lseg Number of segments along curved lines, it represents
-  /// the full 2*M_PI coverange, if lseg is set to 1 only the extrema
-  /// are given @note if lseg is set to 1 then only the straw is created
+  /// @param quarterSegments is the number of segments used to describe curved
+  /// segments in a quarter of the phi range. If it is 1, then only the extrema
+  /// points in phi are inserted next to the segment corners.
   ///
   /// @return A list of vertices and a face/facett description of it
   Polyhedron polyhedronRepresentation(const GeometryContext& gctx,
-                                      std::size_t lseg) const final;
+                                      unsigned int quarterSegments) const final;
 };
 
 inline Surface::SurfaceType StrawSurface::type() const {

Core/include/Acts/Surfaces/Surface.hpp

@@ -418,20 +418,21 @@ class Surface : public virtual GeometryObject,
   /// Return properly formatted class name
   virtual std::string name() const = 0;
 
-  /// Return a Polyhedron for this object
+  /// Return a Polyhedron for surface objects
   ///
   /// @param gctx The current geometry context object, e.g. alignment
-  /// @param lseg Number of segments along curved lines, if the lseg
-  /// is set to one, only the corners and the extrema are given,
-  /// otherwise it represents the number of segments for a full 2*M_PI
-  /// circle and is scaled to the relevant sector
+  /// @param quarterSegments The number of segemtns to approximate a 0.5*pi sector,
+  /// which represents a quarter of the full circle
+  ///
+  /// @note In order to symmetrize the code between sectoral and closed cylinders
+  /// in case of closed cylinders, both (-pi, pi) are given as separate vertices
   ///
   /// @note An internal surface transform can invalidate the extrema
   /// in the transformed space
   ///
   /// @return A list of vertices and a face/facett description of it
-  virtual Polyhedron polyhedronRepresentation(const GeometryContext& gctx,
-                                              std::size_t lseg) const = 0;
+  virtual Polyhedron polyhedronRepresentation(
+      const GeometryContext& gctx, unsigned int quarterSegments = 2u) const = 0;
 
   /// The derivative of bound track parameters w.r.t. alignment
   /// parameters of its reference surface (i.e. local frame origin in

Core/include/Acts/Surfaces/SurfaceConcept.hpp

@@ -87,7 +87,7 @@ concept SurfaceConcept = requires(S s, const S cs, S s2, const S cs2,
   { cs.name() } -> std::same_as<std::string>;
 
   {
-    cs.polyhedronRepresentation(gctx, std::declval<std::size_t>())
+    cs.polyhedronRepresentation(gctx, std::declval<unsigned int>())
   } -> std::same_as<Polyhedron>;
 
   {

Core/include/Acts/Surfaces/TrapezoidBounds.hpp

@@ -108,13 +108,13 @@ class TrapezoidBounds : public PlanarBounds {
 
   /// Return the vertices
   ///
-  /// @param lseg the number of segments used to approximate
-  /// and eventually curved line
+  /// @param ignoredSegments is and ignored parameter used to describe
+  /// the number of segments to approximate curved sectors.
   ///
   /// @note the number of segments is ignored in this representation
   ///
   /// @return vector for vertices in 2D
-  std::vector<Vector2> vertices(unsigned int lseg = 1) const final;
+  std::vector<Vector2> vertices(unsigned int ignoredSegments = 0u) const final;
 
   // Bounding box representation
   const RectangleBounds& boundingBox() const final;