Don't assume an error location for PAD::GetEffectivePolygon().

While ERROR_INSIDE was good for plotting, 3D generation, etc., it's
not good for generating router hulls.

Also reverts part of the change to always use polygons for PNS::SOLIDs.  A single shape in a SHAPE_COMPOUND will be faster (and
more accurate).

Fixes https://gitlab.com/kicad/code/kicad/-/issues/14898

3d-viewer/3d_canvas/create_3Dgraphic_brd_items.cpp

@@ -737,7 +737,7 @@ void BOARD_ADAPTER::buildPadOutlineAsSegments( const PAD* aPad, CONTAINER_2D_BAS
     else
     {
         // For other shapes, add outlines as thick segments in polygon buffer
-        const std::shared_ptr<SHAPE_POLY_SET>& corners = aPad->GetEffectivePolygon();
+        const std::shared_ptr<SHAPE_POLY_SET>& corners = aPad->GetEffectivePolygon( ERROR_INSIDE );
         const SHAPE_LINE_CHAIN&                path = corners->COutline( 0 );
 
         for( int j = 0; j < path.PointCount(); j++ )

3d-viewer/3d_canvas/create_layer_items.cpp

@@ -72,7 +72,7 @@ void buildPadOutlineAsPolygon( const PAD* aPad, SHAPE_POLY_SET& aBuffer, int aWi
     else
     {
         // For other shapes, add outlines as thick segments in polygon buffer
-        const SHAPE_LINE_CHAIN& path = aPad->GetEffectivePolygon()->COutline( 0 );
+        const SHAPE_LINE_CHAIN& path = aPad->GetEffectivePolygon( ERROR_INSIDE )->COutline( 0 );
 
         for( int ii = 0; ii < path.PointCount(); ++ii )
         {

libs/kimath/include/geometry/geometry_utils.h

@@ -40,7 +40,7 @@
  * or inside of the curve?  (Generally speaking filled shape errors go on the inside
  * and knockout errors go on the outside.  This preserves minimum clearances.)
  */
-enum ERROR_LOC { ERROR_OUTSIDE, ERROR_INSIDE };
+enum ERROR_LOC { ERROR_OUTSIDE = 0, ERROR_INSIDE };
 
 /**
  * @return the number of segments to approximate a arc by segments

pcbnew/board.cpp

@@ -1975,7 +1975,7 @@ std::tuple<int, double, double> BOARD::GetTrackLength( const PCB_TRACK& aTrack )
                     VECTOR2I loc;
 
                     // We may not collide even if we passed the bounding-box hit test
-                    if( pad->GetEffectivePolygon()->Collide( trackSeg, 0, nullptr, &loc ) )
+                    if( pad->GetEffectivePolygon( ERROR_INSIDE )->Collide( trackSeg, 0, nullptr, &loc ) )
                     {
                         // Part 1: length of the seg to the intersection with the pad poly
                         if( hitStart )

pcbnew/connectivity/connectivity_data.cpp

@@ -448,7 +448,7 @@ bool CONNECTIVITY_DATA::IsConnectedOnLayer( const BOARD_CONNECTED_ITEM *aItem, i
                     if( zone->IsFilled() )
                     {
                         const SHAPE_POLY_SET*   zoneFill = zone->GetFill( ToLAYER_ID( aLayer ) );
-                        const SHAPE_LINE_CHAIN& padHull = pad->GetEffectivePolygon()->Outline( 0 );
+                        const SHAPE_LINE_CHAIN& padHull = pad->GetEffectivePolygon( ERROR_INSIDE )->Outline( 0 );
 
                         for( const VECTOR2I& pt : zoneFill->COutline( islandIdx ).CPoints() )
                         {

pcbnew/convert_shape_list_to_polygon.cpp

@@ -142,7 +142,8 @@ static bool isCopperOutside( const FOOTPRINT* aFootprint, SHAPE_POLY_SET& aShape
     {
         SHAPE_POLY_SET poly = aShape.CloneDropTriangulation();
 
-        poly.BooleanIntersection( *pad->GetEffectivePolygon(), SHAPE_POLY_SET::PM_FAST );
+        poly.BooleanIntersection( *pad->GetEffectivePolygon( ERROR_INSIDE ),
+                                  SHAPE_POLY_SET::PM_FAST );
 
         if( poly.OutlineCount() == 0 )
         {

pcbnew/exporters/step/step_pcb_model.cpp

@@ -215,7 +215,7 @@ STEP_PCB_MODEL::~STEP_PCB_MODEL()
 
 bool STEP_PCB_MODEL::AddPadShape( const PAD* aPad, const VECTOR2D& aOrigin )
 {
-    const std::shared_ptr<SHAPE_POLY_SET>& pad_shape = aPad->GetEffectivePolygon();
+    const std::shared_ptr<SHAPE_POLY_SET>& pad_shape = aPad->GetEffectivePolygon( ERROR_INSIDE );
     bool success = true;
     VECTOR2I pos = aPad->GetPosition();
 

pcbnew/pad.cpp

@@ -356,12 +356,12 @@ void PAD::SetChamferRectRatio( double aChamferScale )
 }
 
 
-const std::shared_ptr<SHAPE_POLY_SET>& PAD::GetEffectivePolygon() const
+const std::shared_ptr<SHAPE_POLY_SET>& PAD::GetEffectivePolygon( ERROR_LOC aErrorLoc ) const
 {
-    if( m_polyDirty )
-        BuildEffectivePolygon();
+    if( m_polyDirty[ aErrorLoc ] )
+        BuildEffectivePolygon( aErrorLoc );
 
-    return m_effectivePolygon;
+    return m_effectivePolygon[ aErrorLoc ];
 }
 
 
@@ -414,7 +414,7 @@ std::shared_ptr<SHAPE_SEGMENT> PAD::GetEffectiveHoleShape() const
 int PAD::GetBoundingRadius() const
 {
     if( m_polyDirty )
-        BuildEffectivePolygon();
+        BuildEffectivePolygon( ERROR_OUTSIDE );
 
     return m_effectiveBoundingRadius;
 }
@@ -595,41 +595,46 @@ void PAD::BuildEffectiveShapes( PCB_LAYER_ID aLayer ) const
 }
 
 
-void PAD::BuildEffectivePolygon() const
+void PAD::BuildEffectivePolygon( ERROR_LOC aErrorLoc ) const
 {
     std::lock_guard<std::mutex> RAII_lock( m_polyBuildingLock );
 
     // If we had to wait for the lock then we were probably waiting for someone else to
     // finish rebuilding the shapes.  So check to see if they're clean now.
-    if( !m_polyDirty )
+    if( !m_polyDirty[ aErrorLoc ] )
         return;
 
     const BOARD* board = GetBoard();
     int          maxError = board ? board->GetDesignSettings().m_MaxError : ARC_HIGH_DEF;
 
     // Polygon
-    m_effectivePolygon = std::make_shared<SHAPE_POLY_SET>();
-    TransformShapeToPolygon( *m_effectivePolygon, UNDEFINED_LAYER, 0, maxError, ERROR_INSIDE );
+    std::shared_ptr<SHAPE_POLY_SET>& effectivePolygon = m_effectivePolygon[ aErrorLoc ];
+
+    effectivePolygon = std::make_shared<SHAPE_POLY_SET>();
+    TransformShapeToPolygon( *effectivePolygon, UNDEFINED_LAYER, 0, maxError, aErrorLoc );
 
     // Bounding radius
     //
     // PADSTACKS TODO: these will both need to cycle through all layers to get the largest
     // values....
-    m_effectiveBoundingRadius = 0;
-
-    for( int cnt = 0; cnt < m_effectivePolygon->OutlineCount(); ++cnt )
+    if( aErrorLoc == ERROR_OUTSIDE )
     {
-        const SHAPE_LINE_CHAIN& poly = m_effectivePolygon->COutline( cnt );
+        m_effectiveBoundingRadius = 0;
 
-        for( int ii = 0; ii < poly.PointCount(); ++ii )
+        for( int cnt = 0; cnt < effectivePolygon->OutlineCount(); ++cnt )
         {
-            int dist = KiROUND( ( poly.CPoint( ii ) - m_pos ).EuclideanNorm() );
-            m_effectiveBoundingRadius = std::max( m_effectiveBoundingRadius, dist );
+            const SHAPE_LINE_CHAIN& poly = effectivePolygon->COutline( cnt );
+
+            for( int ii = 0; ii < poly.PointCount(); ++ii )
+            {
+                int dist = KiROUND( ( poly.CPoint( ii ) - m_pos ).EuclideanNorm() );
+                m_effectiveBoundingRadius = std::max( m_effectiveBoundingRadius, dist );
+            }
         }
     }
 
     // All done
-    m_polyDirty = false;
+    m_polyDirty[ aErrorLoc ] = false;
 }
 
 
@@ -1086,7 +1091,7 @@ bool PAD::HitTest( const VECTOR2I& aPosition, int aAccuracy ) const
     if( delta.SquaredEuclideanNorm() > SEG::Square( boundingRadius ) )
         return false;
 
-    return GetEffectivePolygon()->Contains( aPosition, -1, aAccuracy );
+    return GetEffectivePolygon( ERROR_INSIDE )->Contains( aPosition, -1, aAccuracy );
 }
 
 
@@ -1109,7 +1114,7 @@ bool PAD::HitTest( const BOX2I& aRect, bool aContained, int aAccuracy ) const
         if( !arect.Intersects( bbox ) )
             return false;
 
-        const std::shared_ptr<SHAPE_POLY_SET>& poly = GetEffectivePolygon();
+        const std::shared_ptr<SHAPE_POLY_SET>& poly = GetEffectivePolygon( ERROR_INSIDE );
 
         int count = poly->TotalVertices();
 

pcbnew/pad.h

@@ -356,13 +356,14 @@ public:
 
     bool IsDirty() const
     {
-        return m_shapesDirty || m_polyDirty;
+        return m_shapesDirty || m_polyDirty[ERROR_INSIDE] || m_polyDirty[ERROR_OUTSIDE];
     }
 
     void SetDirty()
     {
         m_shapesDirty = true;
-        m_polyDirty = true;
+        m_polyDirty[ERROR_INSIDE] = true;
+        m_polyDirty[ERROR_OUTSIDE] = true;
     }
 
     void SetLayerSet( LSET aLayers ) override   { m_layerMask = aLayers; }
@@ -443,7 +444,7 @@ public:
     GetEffectiveShape( PCB_LAYER_ID aLayer = UNDEFINED_LAYER,
                        FLASHING flashPTHPads = FLASHING::DEFAULT ) const override;
 
-    const std::shared_ptr<SHAPE_POLY_SET>& GetEffectivePolygon() const;
+    const std::shared_ptr<SHAPE_POLY_SET>& GetEffectivePolygon( ERROR_LOC aErrorLoc ) const;
 
     /**
      * Return a SHAPE_SEGMENT object representing the pad's hole.
@@ -695,7 +696,7 @@ public:
      * the dirty bit.
      */
     void BuildEffectiveShapes( PCB_LAYER_ID aLayer ) const;
-    void BuildEffectivePolygon() const;
+    void BuildEffectivePolygon( ERROR_LOC aErrorLoc ) const;
 
     virtual void ViewGetLayers( int aLayers[], int& aCount ) const override;
 
@@ -754,9 +755,9 @@ private:
     mutable std::shared_ptr<SHAPE_COMPOUND>   m_effectiveShape;
     mutable std::shared_ptr<SHAPE_SEGMENT>    m_effectiveHoleShape;
 
-    mutable bool                              m_polyDirty;
+    mutable bool                              m_polyDirty[2];
     mutable std::mutex                        m_polyBuildingLock;
-    mutable std::shared_ptr<SHAPE_POLY_SET>   m_effectivePolygon;
+    mutable std::shared_ptr<SHAPE_POLY_SET>   m_effectivePolygon[2];
     mutable int                               m_effectiveBoundingRadius;
 
     int               m_subRatsnest;        // Variable used to handle subnet (block) number in

pcbnew/plot_brditems_plotter.cpp

@@ -260,7 +260,7 @@ void BRDITEMS_PLOTTER::PlotPad( const PAD* aPad, const COLOR4D& aColor, OUTLINE_
     default:
     case PAD_SHAPE::CUSTOM:
     {
-        const std::shared_ptr<SHAPE_POLY_SET>& polygons = aPad->GetEffectivePolygon();
+        const std::shared_ptr<SHAPE_POLY_SET>& polygons = aPad->GetEffectivePolygon( ERROR_INSIDE );
 
         if( polygons->OutlineCount() )
         {

pcbnew/router/pns_kicad_iface.cpp

@@ -1185,23 +1185,26 @@ std::unique_ptr<PNS::SOLID> PNS_KICAD_IFACE_BASE::syncPad( PAD* aPad )
     if( aPad->GetDrillSize().x > 0 )
         solid->SetHole( new PNS::HOLE( aPad->GetEffectiveHoleShape()->Clone() ) );
 
-    std::shared_ptr<SHAPE> shape = aPad->GetEffectiveShape( UNDEFINED_LAYER,
-                                                            FLASHING::ALWAYS_FLASHED );
-    std::shared_ptr<SHAPE_POLY_SET> polygon = aPad->GetEffectivePolygon();
+    // We generate a single SOLID for a pad, so we have to treat it as ALWAYS_FLASHED and then
+    // perform layer-specific flashing tests internally.
+    const std::shared_ptr<SHAPE>& shape = aPad->GetEffectiveShape( UNDEFINED_LAYER,
+                                                                   FLASHING::ALWAYS_FLASHED );
 
-    if( shape->HasIndexableSubshapes() && polygon->OutlineCount() )
+    if( shape->HasIndexableSubshapes() && shape->GetIndexableSubshapeCount() == 1 )
     {
-        solid->SetShape( new SHAPE_SIMPLE( polygon->Outline( 0 ) ) );
+        std::vector<const SHAPE*> subshapes;
+        shape->GetIndexableSubshapes( subshapes );
 
-        // GetEffectivePolygon may produce an approximation of the shape, so we need to account for
-        // this when building hulls around this shape.
-        solid->SetExtraClearance( m_board->GetDesignSettings().m_MaxError );
+        solid->SetShape( subshapes[0]->Clone() );
     }
+    // For anything that's not a single shape we use a polygon.  Multiple shapes have a tendency
+    // to confuse the hull generator. https://gitlab.com/kicad/code/kicad/-/issues/15553
     else
     {
-        // Prefer using the original shape if it's not a compound shape; the hulls for
-        // circular and rectangular pads can be exact.
-        solid->SetShape( shape->Clone() );
+        const std::shared_ptr<SHAPE_POLY_SET>& poly = aPad->GetEffectivePolygon( ERROR_OUTSIDE );
+
+        if( poly->OutlineCount() )
+            solid->SetShape( new SHAPE_SIMPLE( poly->Outline( 0 ) ) );
     }
 
     return solid;

pcbnew/router/pns_solid.cpp

@@ -41,8 +41,6 @@ const SHAPE_LINE_CHAIN SOLID::Hull( int aClearance, int aWalkaroundThickness, in
     if( !m_shape )
         return SHAPE_LINE_CHAIN();
 
-    aClearance += ExtraClearance();
-
     if( m_shape->Type() == SH_COMPOUND )
     {
         SHAPE_COMPOUND* cmpnd = static_cast<SHAPE_COMPOUND*>( m_shape );

pcbnew/router/pns_solid.h

@@ -38,8 +38,7 @@ public:
     SOLID() :
             ITEM( SOLID_T ),
             m_shape( nullptr ),
-            m_hole( nullptr ),
-            m_extraClearance( 0 )
+            m_hole( nullptr )
     {
         m_movable = false;
         m_padToDie = 0;
@@ -66,7 +65,6 @@ public:
         m_padToDie = aSolid.m_padToDie;
         m_orientation = aSolid.m_orientation;
         m_anchorPoints = aSolid.m_anchorPoints;
-        m_extraClearance = aSolid.m_extraClearance;
     }
 
     SOLID& operator=( const SOLID& aB )
@@ -81,7 +79,6 @@ public:
         m_padToDie = aB.m_padToDie;
         m_orientation = aB.m_orientation;
         m_anchorPoints = aB.m_anchorPoints;
-        m_extraClearance = aB.m_extraClearance;
         return *this;
     }
 
@@ -138,9 +135,6 @@ public:
     virtual bool HasHole() const override { return m_hole != nullptr; }
     virtual HOLE *Hole() const override { return m_hole; }
 
-    int ExtraClearance() const { return m_extraClearance; }
-    void SetExtraClearance( int aClearance ) { m_extraClearance = aClearance; }
-
 private:
     VECTOR2I    m_pos;
     SHAPE*      m_shape;
@@ -149,7 +143,6 @@ private:
     EDA_ANGLE   m_orientation;
     HOLE*       m_hole;
     std::vector<VECTOR2I> m_anchorPoints;
-    int         m_extraClearance;
 };
 
 }

pcbnew/router/pns_topology.cpp

@@ -367,7 +367,7 @@ const ITEM_SET TOPOLOGY::AssembleTuningPath( ITEM* aStart, SOLID** aStartPad, SO
     auto clipLineToPad =
             []( SHAPE_LINE_CHAIN& aLine, PAD* aPad, bool aForward = true )
             {
-                const std::shared_ptr<SHAPE_POLY_SET>& shape = aPad->GetEffectivePolygon();
+                const auto& shape = aPad->GetEffectivePolygon( ERROR_INSIDE );
 
                 int start = aForward ? 0 : aLine.PointCount() - 1;
                 int delta = aForward ? 1 : -1;
@@ -413,7 +413,7 @@ const ITEM_SET TOPOLOGY::AssembleTuningPath( ITEM* aStart, SOLID** aStartPad, SO
     auto processPad =
             [&]( const JOINT* aJoint, PAD* aPad )
             {
-                const std::shared_ptr<SHAPE_POLY_SET>& shape = aPad->GetEffectivePolygon();
+                const auto& shape = aPad->GetEffectivePolygon( ERROR_INSIDE );
 
                 for( int idx = 0; idx < initialPath.Size(); idx++ )
                 {

pcbnew/tools/pcb_grid_helper.cpp

@@ -642,7 +642,7 @@ void PCB_GRID_HELPER::computeAnchors( BOARD_ITEM* aItem, const VECTOR2I& aRefPos
 
                 default:
                 {
-                    const std::shared_ptr<SHAPE_POLY_SET>& outline = aPad->GetEffectivePolygon();
+                    const auto& outline = aPad->GetEffectivePolygon( ERROR_INSIDE );
 
                     if( !outline->IsEmpty() )
                     {

pcbnew/tracks_cleaner.cpp

@@ -328,7 +328,8 @@ void TRACKS_CLEANER::deleteTracksInPads()
                 track->TransformShapeToPolygon( poly, track->GetLayer(), 0, ARC_HIGH_DEF,
                                                 ERROR_INSIDE );
 
-                poly.BooleanSubtract( *pad->GetEffectivePolygon(), SHAPE_POLY_SET::PM_FAST );
+                poly.BooleanSubtract( *pad->GetEffectivePolygon( ERROR_INSIDE ),
+                                      SHAPE_POLY_SET::PM_FAST );
 
                 if( poly.IsEmpty() )
                 {

pcbnew/zone_filler.cpp

@@ -125,7 +125,7 @@ bool ZONE_FILLER::Fill( std::vector<ZONE*>& aZones, bool aCheck, wxWindow* aPare
             if( pad->IsDirty() )
             {
                 pad->BuildEffectiveShapes( UNDEFINED_LAYER );
-                pad->BuildEffectivePolygon();
+                pad->BuildEffectivePolygon( ERROR_OUTSIDE );
             }
         }
 
@@ -1864,7 +1864,7 @@ void ZONE_FILLER::buildThermalSpokes( const ZONE* aZone, PCB_LAYER_ID aLayer,
                     seg.B += pad->ShapePos();
 
                     // Make sure seg.A is the origin
-                    if( !pad->GetEffectivePolygon()->Contains( seg.A ) )
+                    if( !pad->GetEffectivePolygon( ERROR_OUTSIDE )->Contains( seg.A ) )
                         seg.Reverse();
 
                     // Trim seg.B to the thermal outline