Skip to content

Geometry hackathon: Reduce overlap between Surface and SurfaceBounds #4852

Description

@andiwand

Currently there seems to be a significant overlap between Surface and SurfaceBounds. They share the same local coordinate system and only specific SurfaceBounds can be supplied to certain Surfaces.

This also results in various round trips during intersections and boundary checks. We have to go through various virtual functions which are hidden behind a single Surface function call.

One idea I had was to combine SurfaceBounds into Surface as they are tightly coupled anyways.

Activity

  1. changed the title [-]Reduce overlap between `Surface` and `SurfaceBounds`[/-] [+]Geometry hackathon: Reduce overlap between `Surface` and `SurfaceBounds`[/+] on Dec 1, 2025
  2. andiwand commented on Dec 1, 2025

    @andiwand
    ContributorAuthor

    Class overview Surface and SurfaceBounds

    What we have right now as Surface types

    • Surface
      • LineSurface - $(d_0, z_0)$
        • PerigeeSurface - no bounds
        • StrawSurface - only provides a custom polyhedronRepresentation
      • RegularSurface - direction invariant surfaces
        • ConeSurface - $(r \phi, z)$
        • CylinderSurface - $(r \phi, z)$
        • DiscSurface - $(r, \phi)$
        • PlaneSurface - $(x, y)$

    What we have right now as SurfaceBounds types

    • SurfaceBounds
      • ConeBounds - for ConeSurface $(\alpha, z_\texttt{min}, z_\texttt{max}, \phi/2, \phi_\texttt{avg})$
      • CylinderBounds - for CylinderSurface $(r, h_z/2, \phi/2, \phi_\texttt{avg}, z_\texttt{min bevel}, z_\texttt{max bevel})$
      • InfiniteBounds - will only work for surfaces that accept the SurfaceBounds type
      • LineBounds - for LineSurface $(r_\texttt{max}, h_z/2)$
      • DiscBounds - for DiscSurface
        • AnnulusBounds - ☠️ $(r_\texttt{min}, r_\texttt{max}, \phi_\texttt{rel min}, \phi_\texttt{rel max}, \phi_\texttt{avg}, x_\texttt{origin}, y_\texttt{origin})$
        • DiscTrapezoidBounds - $(h_\texttt{x min r}/2, h_\texttt{x max r}/2, r_{min}, r_{max}, \phi_\texttt{avg}, \theta_\texttt{stereo})$
        • RadialBounds - $(r_{min}, r_{max}, \phi/2, \phi_\texttt{avg})$
      • PlanarBounds - for PlaneSurface
        • ConvexPolygonBoundsBase
          • ConvexPolygonBounds<N> - templated degree
          • ConvexPolygonBounds<PolygonDynamic> - template specialized for variable degree
        • DiamondBounds - $(h_\texttt{x neg y}/2, h_\texttt{x zero y}/2, h_\texttt{x pos y}/2, h_\texttt{y neg}/2, h_\texttt{y pos}/2)$
        • EllipseBounds - $(x_\texttt{inner}, y_\texttt{inner}, x_\texttt{outer}, y_\texttt{outer}, \phi/2, \phi_\texttt{avg})$
        • RectangleBounds - $(x_\texttt{min}, y_\texttt{min}, x_\texttt{max}, y_\texttt{max})$
        • TrapezoidBounds - $(h_\texttt{x pos y}/2, h_\texttt{y}/2, \theta_\texttt{rotation})$
  3. andiwand commented on Dec 1, 2025

    @andiwand
    ContributorAuthor

    Case study CylinderSurface

    The following methods strictly assume a standard cylinder shape with a single radius:

    • CylinderSurface::intersect
    • CylinderSurface::localToGlobal
    • CylinderSurface::globalToLocal
    • There are most likely more than that

    Following an CylinderSurface::intersect call can result in a number of non-inlinable function calls:

    graph TD;
        A[CylinderSurface::intersect];
        B1[Surface::transform];
        B[CylinderSurface::intersectionSolver];
        C{infinite bounds?};
        D{full phi coverage?};
        E[Surface::isOnSurface];
        F[CylinderSurface::globalToLocal];
        G[SurfaceBounds::inside];
        H[CylinderSurface::inside];
        I[depending on boundary tolerance also CylinderSurface::boundToCartesianJacobian];
        Z[done];
    
        A --> B1;
        B1 --> B;
        B --> C;
        C -- Yes --> Z;
        C -- No --> D;
        D -- Yes --> Z;
        D -- No --> E;
        E --> F;
        F --> G;
        G --> H;
        H --> I;
        I --> Z
    
    Loading
  4. andiwand commented on Dec 2, 2025

    @andiwand
    ContributorAuthor

    I tried putting the CylinderBounds as a plain member into CylinderSurface which is a small modification and measure the compute performance on the ODD. For Fatras this seems to make a measurable difference while the difference for the CKF seems to be below noise threshold.

    I believe any performance difference here can only attributed to the memory locality. At the same time one can argue that the surface bounds could be allocated in proximity to the surface with the existing design, if done properly during the shared_ptr allocation.

    Image Image
  5. github-actions commented on Jan 1, 2026

    @github-actions
    Contributor

    This issue/PR has been automatically marked as stale because it has not had recent activity. The stale label will be removed if any interaction occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions