Fixed zone filling crash & thermal stubs inconsistency

Fixes: lp:1737557
* https://bugs.launchpad.net/kicad/+bug/1737557
Fixes: lp:1737542
* https://bugs.launchpad.net/kicad/+bug/1737542
Fixes: lp:1737541
* https://bugs.launchpad.net/kicad/+bug/1737541

common/gal/opengl/opengl_gal.cpp

@@ -789,7 +789,7 @@ void OPENGL_GAL::drawTriangulatedPolyset( const SHAPE_POLY_SET& aPolySet )
         {
             auto triPoly = aPolySet.TriangulatedPolygon( j );
 
-            for ( int i = 0; i < triPoly->m_triangleCount; i++ )
+            for ( int i = 0; i < triPoly->TriangleCount(); i++ )
             {
                 VECTOR2I a, b, c;
                 triPoly->GetTriangle( i ,a,b,c);

common/geometry/shape_poly_set.cpp

@@ -1875,25 +1875,105 @@ SHAPE_POLY_SET &SHAPE_POLY_SET::operator=(const SHAPE_POLY_SET & aOther)
 }
 
 
-typedef std::map<p2t::Point*, int>  P2T_MAP;
-typedef std::vector<p2t::Point*>    P2T_VEC;
 
-static void convert( const SHAPE_LINE_CHAIN& outl,
+
-        P2T_VEC& buffer,
+
-        P2T_MAP& pointMap,
+class SHAPE_POLY_SET::TRIANGULATION_CONTEXT
-        SHAPE_POLY_SET::TRIANGULATED_POLYGON& aPoly )
 {
-    buffer.clear();
+public:
 
-    for( int i = 0; i < outl.PointCount(); i++ )
+    TRIANGULATION_CONTEXT( TRIANGULATED_POLYGON* aResultPoly ) :
+        m_triPoly( aResultPoly )
+        {
+        }
+
+    void AddOutline( const SHAPE_LINE_CHAIN& outl, bool aIsHole = false )
     {
-        const auto& p = outl.CPoint( i );
+        m_points.reserve( outl.PointCount() );
-        auto p2 = new p2t::Point( p.x, p.y );
+        m_points.clear();
-        pointMap[p2] = aPoly.AddVertex( p );
-        buffer.push_back( p2 );
-    }
-}
 
+        for( int i = 0; i < outl.PointCount(); i++ )
+        {
+            m_points.push_back( addPoint( outl.CPoint( i ) ) );
+        }
+
+        if ( aIsHole )
+            m_cdt->AddHole( m_points );
+        else
+        {
+            m_cdt.reset( new p2t::CDT( m_points ) );
+        }
+    }
+
+    void Triangulate()
+    {
+        m_cdt->Triangulate();
+
+        m_triPoly->AllocateTriangles( m_cdt->GetTriangles().size() );
+
+        int i = 0;
+
+        for( auto tri : m_cdt->GetTriangles() )
+        {
+            TRIANGULATED_POLYGON::TRI t;
+
+            t.a = tri->GetPoint( 0 )->id;
+            t.b = tri->GetPoint( 1 )->id;
+            t.c = tri->GetPoint( 2 )->id;
+
+            m_triPoly->SetTriangle(i, t);
+            i++;
+        }
+
+        for( auto p : m_uniquePoints )
+            delete p;
+    }
+
+private:
+
+	class comparePoints
+	{
+	public:
+	    bool operator()( p2t::Point* a, p2t::Point* b ) const
+	    {
+		if (a->x < b->x)
+		    return true;
+
+		if( a->x == b->x )
+		    return ( a->y > b->y );
+
+		return false;
+	    }
+	};
+
+
+    p2t::Point* addPoint( const VECTOR2I& aP )
+    {
+        p2t::Point check( aP.x, aP.y );
+        auto it = m_uniquePoints.find( &check );
+
+        if( it != m_uniquePoints.end() )
+        {
+            return *it;
+        }
+        else
+        {
+            auto lastId = m_triPoly->VertexCount();
+            auto p = new p2t::Point( aP.x, aP.y, lastId );
+            m_triPoly->AddVertex( aP );
+            m_uniquePoints.insert ( p );
+            return p;
+        }
+    }
+
+    typedef std::set<p2t::Point*, comparePoints>  P2T_SET;
+    typedef std::vector<p2t::Point*>    P2T_VEC;
+
+    P2T_VEC m_points;
+    P2T_SET m_uniquePoints;
+    TRIANGULATED_POLYGON *m_triPoly;
+    std::unique_ptr<p2t::CDT> m_cdt;
+};
 
 SHAPE_POLY_SET::TRIANGULATED_POLYGON::~TRIANGULATED_POLYGON()
 {
@@ -1940,49 +2020,21 @@ static int totalVertexCount( const SHAPE_POLY_SET::POLYGON& aPoly )
 void SHAPE_POLY_SET::triangulateSingle( const POLYGON& aPoly,
         SHAPE_POLY_SET::TRIANGULATED_POLYGON& aResult )
 {
-    assert( aPoly.size() >= 1 );
+    if( aPoly.size() == 0 )
+        return;
 
-    P2T_MAP pointMap;
+    TRIANGULATION_CONTEXT ctx ( &aResult );
-    P2T_VEC outline;
 
     aResult.AllocateVertices( totalVertexCount( aPoly ) );
-
+    ctx.AddOutline( aPoly[0], false );
-    convert( aPoly[0], outline, pointMap, aResult );
-
-    std::unique_ptr<p2t::CDT> cdt( new p2t::CDT( outline ) );
-
     for( unsigned i = 1; i < aPoly.size(); i++ )
     {
-        std::vector<p2t::Point*> hole;
+        ctx.AddOutline( aPoly[i], true ); // add holes
-
-        convert( aPoly[i], hole, pointMap, aResult );
-
-        cdt->AddHole( hole );
     }
 
-    cdt->Triangulate();
+    ctx.Triangulate();
-
-    aResult.AllocateTriangles( cdt->GetTriangles().size() );
-
-    int i = 0;
-
-    for( auto tri : cdt->GetTriangles() )
-    {
-        TRIANGULATED_POLYGON::TRI t;
-
-        t.a = pointMap[ tri->GetPoint( 0 ) ];
-        t.b = pointMap[ tri->GetPoint( 1 ) ];
-        t.c = pointMap[ tri->GetPoint( 2 ) ];
-
-        aResult.m_triangles[ i ] = t;
-        i++;
-    }
-
-    for( auto iter = pointMap.begin(); iter!=pointMap.end(); ++iter )
-        delete iter->first;
 }
 
-
 bool SHAPE_POLY_SET::IsTriangulationUpToDate() const
 {
     if( !m_triangulationValid )
@@ -2020,10 +2072,19 @@ void SHAPE_POLY_SET::CacheTriangulation()
         return;
 
     SHAPE_POLY_SET tmpSet = *this;
-    tmpSet.Unfracture( PM_FAST );
+
+    if( !tmpSet.HasHoles() )
+	tmpSet.Unfracture( PM_FAST );
 
     m_triangulatedPolys.clear();
 
+    if ( tmpSet.HasTouchingHoles() )
+    {
+        // temporary workaround for overlapping hole vertices that poly2tri doesn't handle
+        m_triangulationValid = false;
+        return;
+    }
+
     for( int i = 0; i < tmpSet.OutlineCount(); i++ )
     {
         m_triangulatedPolys.push_back( std::make_unique<TRIANGULATED_POLYGON>() );
@@ -2061,3 +2122,38 @@ MD5_HASH SHAPE_POLY_SET::checksum() const
 
     return hash;
 }
+
+bool SHAPE_POLY_SET::HasTouchingHoles() const
+{
+    for( int i = 0; i < OutlineCount(); i++ )
+    {
+        if( hasTouchingHoles( CPolygon( i ) ) )
+        {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+bool SHAPE_POLY_SET::hasTouchingHoles( const POLYGON& aPoly ) const
+{
+    std::vector< VECTOR2I > pts;
+
+    for ( const auto& lc : aPoly )
+    {
+        for( int i = 0; i < lc.PointCount(); i++ )
+        {
+            const auto p = lc.CPoint( i );
+
+            if ( std::find( pts.begin(), pts.end(), p) != pts.end() )
+            {
+                return true;
+            }
+
+            pts.push_back( p );
+        }
+    }
+
+    return false;
+}

include/geometry/shape_poly_set.h

@@ -60,10 +60,11 @@ class SHAPE_POLY_SET : public SHAPE
         ///> the remaining (if any), are the holes
         typedef std::vector<SHAPE_LINE_CHAIN> POLYGON;
 
-        struct TRIANGULATED_POLYGON
+        class TRIANGULATION_CONTEXT;
-        {
-            ~TRIANGULATED_POLYGON();
 
+        class TRIANGULATED_POLYGON
+        {
+        public:
             struct TRI
             {
                 TRI()
@@ -73,6 +74,8 @@ class SHAPE_POLY_SET : public SHAPE
                 int a, b, c;
             };
 
+            ~TRIANGULATED_POLYGON();
+
             void Clear();
 
             void AllocateVertices( int aSize );
@@ -86,12 +89,29 @@ class SHAPE_POLY_SET : public SHAPE
                 c = m_vertices[ tri->c ];
             }
 
+            void SetTriangle( int aIndex, const TRI& aTri )
+            {
+                m_triangles[aIndex] = aTri;
+            }
+
             int AddVertex( const VECTOR2I& aP )
             {
                 m_vertices[ m_vertexCount ] = aP;
                 return (m_vertexCount++);
             }
 
+            int VertexCount() const
+            {
+                return m_vertexCount;
+            }
+
+            int TriangleCount() const
+            {
+                return m_triangleCount;
+            }
+
+        private:
+
             TRI* m_triangles = nullptr;
             VECTOR2I* m_vertices = nullptr;
             int m_vertexCount = 0;
@@ -818,6 +838,10 @@ class SHAPE_POLY_SET : public SHAPE
         ///> Returns true if the polygon set has any holes.
         bool HasHoles() const;
 
+        ///> Returns true if the polygon set has any holes tha share a vertex.
+        bool HasTouchingHoles() const;
+
+
         ///> Simplifies the polyset (merges overlapping polys, eliminates degeneracy/self-intersections)
         ///> For aFastMode meaning, see function booleanOp
         void Simplify( POLYGON_MODE aFastMode );
@@ -1113,6 +1137,9 @@ class SHAPE_POLY_SET : public SHAPE
         POLYGON chamferFilletPolygon( CORNER_MODE aMode, unsigned int aDistance,
                                       int aIndex, int aSegments = -1 );
 
+        ///> Returns true if the polygon set has any holes that touch share a vertex.
+        bool hasTouchingHoles( const POLYGON& aPoly ) const;
+
         typedef std::vector<POLYGON> POLYSET;
 
         POLYSET m_polys;

pcbnew/CMakeLists.txt

@@ -281,7 +281,6 @@ set( PCBNEW_CLASS_SRCS
     tr_modif.cpp
     undo_redo.cpp
     zone_filler.cpp
-    zones_convert_to_polygons_aux_functions.cpp
     zones_by_polygon.cpp
     zones_by_polygon_fill_functions.cpp
     zones_functions_for_undo_redo.cpp

pcbnew/class_zone.cpp

@@ -1346,3 +1346,37 @@ bool ZONE_CONTAINER::BuildSmoothedPoly( SHAPE_POLY_SET& aSmoothedPoly ) const
 
     return true;
 };
+
+/* Function TransformOutlinesShapeWithClearanceToPolygon
+ * Convert the zone filled areas polygons to polygons
+ * inflated (optional) by max( aClearanceValue, the zone clearance)
+ * and copy them in aCornerBuffer
+ * param aClearanceValue = the clearance around polygons
+ * param aAddClearance = true to add a clearance area to the polygon
+ *                      false to create the outline polygon.
+ */
+void ZONE_CONTAINER::TransformOutlinesShapeWithClearanceToPolygon(
+        SHAPE_POLY_SET& aCornerBuffer, int aMinClearanceValue, bool aUseNetClearance ) const
+{
+    // Creates the zone outline polygon (with holes if any)
+    SHAPE_POLY_SET polybuffer;
+    BuildSmoothedPoly( polybuffer );
+
+    // add clearance to outline
+    int clearance = aMinClearanceValue;
+
+    if( aUseNetClearance && IsOnCopperLayer() )
+    {
+        clearance = GetClearance();
+        if( aMinClearanceValue > clearance )
+            clearance = aMinClearanceValue;
+    }
+
+    // Calculate the polygon with clearance
+    // holes are linked to the main outline, so only one polygon is created.
+    if( clearance )
+        polybuffer.Inflate( clearance, 16 );
+
+    polybuffer.Fracture( SHAPE_POLY_SET::PM_FAST );
+    aCornerBuffer.Append( polybuffer );
+}

pcbnew/connectivity_algo.cpp

@@ -526,11 +526,11 @@ void CN_CONNECTIVITY_ALGO::searchConnections( bool aIncludeZones )
         {
             #ifdef USE_OPENMP
                 #pragma omp master
+                if (m_progressReporter)
+                {
+                    m_progressReporter->KeepRefreshing( true );
+                }
             #endif
-            if (m_progressReporter)
-            {
-                m_progressReporter->KeepRefreshing();
-            }
 
             #ifdef USE_OPENMP
                 #pragma omp for schedule(dynamic)

pcbnew/zone_filler.cpp

@@ -51,13 +51,6 @@
 #include <omp.h>
 #endif /* USE_OPENMP */
 
-extern void BuildUnconnectedThermalStubsPolygonList( SHAPE_POLY_SET& aCornerBuffer,
-        const BOARD* aPcb,
-        const ZONE_CONTAINER* aZone,
-        double aArcCorrection,
-        double aRoundPadThermalRotation );
-
-
 extern void CreateThermalReliefPadPolygon( SHAPE_POLY_SET& aCornerBuffer,
         const D_PAD& aPad,
         int aThermalGap,
@@ -85,14 +78,9 @@ ZONE_FILLER::~ZONE_FILLER()
 
 void ZONE_FILLER::SetProgressReporter( PROGRESS_REPORTER* aReporter )
 {
-// TODO OSX currently does not handle the reporter well
-// https://lists.launchpad.net/kicad-developers/msg32306.html
-#ifndef __WXMAC__
     m_progressReporter = aReporter;
-#endif
 }
 
-
 void ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones )
 {
     std::vector<CN_ZONE_ISOLATED_ISLAND_LIST> toFill;
@@ -133,11 +121,11 @@ void ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones )
     {
         #ifdef USE_OPENMP
             #pragma omp master
+            if( m_progressReporter )
+            {
+                m_progressReporter->KeepRefreshing( true );
+            }
         #endif
-        if( m_progressReporter )
-        {
-            m_progressReporter->KeepRefreshing();
-        }
 
         #ifdef USE_OPENMP
             #pragma omp for schedule(dynamic)
@@ -195,11 +183,11 @@ void ZONE_FILLER::Fill( std::vector<ZONE_CONTAINER*> aZones )
     {
         #ifdef USE_OPENMP
             #pragma omp master
+            if( m_progressReporter )
+            {
+                m_progressReporter->KeepRefreshing( true );
+            }
         #endif
-        if( m_progressReporter )
-        {
-            m_progressReporter->KeepRefreshing();
-        }
 
         #ifdef USE_OPENMP
             #pragma omp for schedule(dynamic)
@@ -685,9 +673,12 @@ void ZONE_FILLER::computeRawFilledAreas( const ZONE_CONTAINER* aZone,
     // Test thermal stubs connections and add polygons to remove unconnected stubs.
     // (this is a refinement for thermal relief shapes)
     if( aZone->GetNetCode() > 0 )
-        BuildUnconnectedThermalStubsPolygonList( thermalHoles, m_board, aZone,
+    {
+        buildUnconnectedThermalStubsPolygonList( thermalHoles, aZone, aFinalPolys,
                 correctionFactor, s_thermalRot );
 
+    }
+
     // remove copper areas corresponding to not connected stubs
     if( !thermalHoles.IsEmpty() )
     {
@@ -922,3 +913,178 @@ bool ZONE_FILLER::fillPolygonWithHorizontalSegments( const SHAPE_LINE_CHAIN& aPo
 
     return success;
 }
+
+
+/**
+ * Function buildUnconnectedThermalStubsPolygonList
+ * Creates a set of polygons corresponding to stubs created by thermal shapes on pads
+ * which are not connected to a zone (dangling bridges)
+ * @param aCornerBuffer = a SHAPE_POLY_SET where to store polygons
+ * @param aZone = a pointer to the ZONE_CONTAINER  to examine.
+ * @param aArcCorrection = arc correction factor.
+ * @param aRoundPadThermalRotation = the rotation in 1.0 degree for thermal stubs in round pads
+ */
+
+void ZONE_FILLER::buildUnconnectedThermalStubsPolygonList( SHAPE_POLY_SET& aCornerBuffer,
+                                              const ZONE_CONTAINER*       aZone,
+                                              const SHAPE_POLY_SET&       aRawFilledArea,
+                                              double                aArcCorrection,
+                                              double                aRoundPadThermalRotation ) const
+{
+    SHAPE_LINE_CHAIN spokes;
+    BOX2I itemBB;
+    VECTOR2I ptTest[4];
+    auto zoneBB = aRawFilledArea.BBox();
+
+
+    int      zone_clearance = aZone->GetZoneClearance();
+
+    int      biggest_clearance = m_board->GetDesignSettings().GetBiggestClearanceValue();
+    biggest_clearance = std::max( biggest_clearance, zone_clearance );
+    zoneBB.Inflate( biggest_clearance );
+
+    // half size of the pen used to draw/plot zones outlines
+    int pen_radius = aZone->GetMinThickness() / 2;
+
+    for( auto module : m_board->Modules() )
+    {
+        for( auto pad : module->Pads() )
+        {
+            // Rejects non-standard pads with tht-only thermal reliefs
+            if( aZone->GetPadConnection( pad ) == PAD_ZONE_CONN_THT_THERMAL
+             && pad->GetAttribute() != PAD_ATTRIB_STANDARD )
+                continue;
+
+            if( aZone->GetPadConnection( pad ) != PAD_ZONE_CONN_THERMAL
+             && aZone->GetPadConnection( pad ) != PAD_ZONE_CONN_THT_THERMAL )
+                continue;
+
+            if( !pad->IsOnLayer( aZone->GetLayer() ) )
+                continue;
+
+            if( pad->GetNetCode() != aZone->GetNetCode() )
+                continue;
+
+            // Calculate thermal bridge half width
+            int thermalBridgeWidth = aZone->GetThermalReliefCopperBridge( pad )
+                                     - aZone->GetMinThickness();
+            if( thermalBridgeWidth <= 0 )
+                continue;
+
+            // we need the thermal bridge half width
+            // with a small extra size to be sure we create a stub
+            // slightly larger than the actual stub
+            thermalBridgeWidth = ( thermalBridgeWidth + 4 ) / 2;
+
+            int thermalReliefGap = aZone->GetThermalReliefGap( pad );
+
+            itemBB = pad->GetBoundingBox();
+            itemBB.Inflate( thermalReliefGap );
+            if( !( itemBB.Intersects( zoneBB ) ) )
+                continue;
+
+            // Thermal bridges are like a segment from a starting point inside the pad
+            // to an ending point outside the pad
+
+            // calculate the ending point of the thermal pad, outside the pad
+            VECTOR2I endpoint;
+            endpoint.x = ( pad->GetSize().x / 2 ) + thermalReliefGap;
+            endpoint.y = ( pad->GetSize().y / 2 ) + thermalReliefGap;
+
+            // Calculate the starting point of the thermal stub
+            // inside the pad
+            VECTOR2I startpoint;
+            int copperThickness = aZone->GetThermalReliefCopperBridge( pad )
+                                  - aZone->GetMinThickness();
+
+            if( copperThickness < 0 )
+                copperThickness = 0;
+
+            // Leave a small extra size to the copper area inside to pad
+            copperThickness += KiROUND( IU_PER_MM * 0.04 );
+
+            startpoint.x = std::min( pad->GetSize().x, copperThickness );
+            startpoint.y = std::min( pad->GetSize().y, copperThickness );
+
+            startpoint.x /= 2;
+            startpoint.y /= 2;
+
+            // This is a CIRCLE pad tweak
+            // for circle pads, the thermal stubs orientation is 45 deg
+            double fAngle = pad->GetOrientation();
+            if( pad->GetShape() == PAD_SHAPE_CIRCLE )
+            {
+                endpoint.x     = KiROUND( endpoint.x * aArcCorrection );
+                endpoint.y     = endpoint.x;
+                fAngle = aRoundPadThermalRotation;
+            }
+
+            // contour line width has to be taken into calculation to avoid "thermal stub bleed"
+            endpoint.x += pen_radius;
+            endpoint.y += pen_radius;
+            // compute north, south, west and east points for zone connection.
+            ptTest[0] = VECTOR2I( 0, endpoint.y );       // lower point
+            ptTest[1] = VECTOR2I( 0, -endpoint.y );      // upper point
+            ptTest[2] = VECTOR2I( endpoint.x, 0 );       // right point
+            ptTest[3] = VECTOR2I( -endpoint.x, 0 );      // left point
+
+            // Test all sides
+            for( int i = 0; i < 4; i++ )
+            {
+                // rotate point
+                RotatePoint( ptTest[i], fAngle );
+
+                // translate point
+                ptTest[i] += pad->ShapePos();
+
+                if( aRawFilledArea.Contains( ptTest[i] ) )
+                    continue;
+
+                spokes.Clear();
+
+                // polygons are rectangles with width of copper bridge value
+                switch( i )
+                {
+                case 0:       // lower stub
+                    spokes.Append( -thermalBridgeWidth, endpoint.y );
+                    spokes.Append( +thermalBridgeWidth, endpoint.y );
+                    spokes.Append( +thermalBridgeWidth, startpoint.y );
+                    spokes.Append( -thermalBridgeWidth, startpoint.y );
+                    break;
+
+                case 1:       // upper stub
+                    spokes.Append( -thermalBridgeWidth, -endpoint.y );
+                    spokes.Append( +thermalBridgeWidth, -endpoint.y );
+                    spokes.Append( +thermalBridgeWidth, -startpoint.y );
+                    spokes.Append( -thermalBridgeWidth, -startpoint.y );
+                    break;
+
+                case 2:       // right stub
+                    spokes.Append( endpoint.x, -thermalBridgeWidth );
+                    spokes.Append( endpoint.x, thermalBridgeWidth );
+                    spokes.Append( +startpoint.x, thermalBridgeWidth );
+                    spokes.Append( +startpoint.x, -thermalBridgeWidth );
+                    break;
+
+                case 3:       // left stub
+                    spokes.Append( -endpoint.x, -thermalBridgeWidth );
+                    spokes.Append( -endpoint.x, thermalBridgeWidth );
+                    spokes.Append( -startpoint.x, thermalBridgeWidth );
+                    spokes.Append( -startpoint.x, -thermalBridgeWidth );
+                    break;
+                }
+
+                aCornerBuffer.NewOutline();
+
+                // add computed polygon to list
+                for( unsigned ic = 0; ic < spokes.PointCount(); ic++ )
+                {
+                    auto cpos = spokes.CPoint( i );
+                    RotatePoint( cpos, fAngle );                               // Rotate according to module orientation
+                    cpos += pad->ShapePos();                              // Shift origin to position
+                    aCornerBuffer.Append( cpos );
+                }
+            }
+        }
+    }
+}

pcbnew/zone_filler.h

@@ -82,6 +82,22 @@ private:
             const SHAPE_POLY_SET& aFilledPolys,
             ZONE_SEGMENT_FILL& aFillSegs ) const;
 
+    /**
+     * Function buildUnconnectedThermalStubsPolygonList
+     * Creates a set of polygons corresponding to stubs created by thermal shapes on pads
+     * which are not connected to a zone (dangling bridges)
+     * @param aCornerBuffer = a SHAPE_POLY_SET where to store polygons
+     * @param aPcb = the board.
+     * @param aZone = a pointer to the ZONE_CONTAINER  to examine.
+     * @param aArcCorrection = a pointer to the ZONE_CONTAINER  to examine.
+     * @param aRoundPadThermalRotation = the rotation in 1.0 degree for thermal stubs in round pads
+     */
+    void buildUnconnectedThermalStubsPolygonList( SHAPE_POLY_SET& aCornerBuffer,
+            const ZONE_CONTAINER* aZone,
+            const SHAPE_POLY_SET&       aRawFilledArea,
+            double aArcCorrection,
+            double aRoundPadThermalRotation ) const;
+
     /**
      * Build the filled solid areas polygons from zone outlines (stored in m_Poly)
      * The solid areas can be more than one on copper layers, and do not have holes