Allow drawing self-intersecting polygons, just display a warning

Fixes: lp:1751654
* https://bugs.launchpad.net/kicad/+bug/1751654

common/preview_items/polygon_geom_manager.cpp

@@ -29,7 +29,7 @@
 POLYGON_GEOM_MANAGER::POLYGON_GEOM_MANAGER( CLIENT& aClient ):
     m_client( aClient ),
     m_leaderMode( LEADER_MODE::DIRECT ),
-    m_intersectionsAllowed( false )
+    m_intersectionsAllowed( true )
 {}
 
 
@@ -53,12 +53,8 @@ bool POLYGON_GEOM_MANAGER::AddPoint( const VECTOR2I& aPt )
         m_lockedPoints.Append( aPt );
     }
 
-    // check for self-intersections (line chain needs to be set as closed for proper checks)
-    m_lockedPoints.SetClosed( true );
-    bool selfIntersect = !!m_lockedPoints.SelfIntersecting();
-    m_lockedPoints.SetClosed( false );
-
-    if( !m_intersectionsAllowed && selfIntersect )
+    // check for self-intersections
+    if( !m_intersectionsAllowed && IsSelfIntersecting( false ) )
     {
         m_lockedPoints.Remove( m_lockedPoints.PointCount() - 1 );
         return false;
@@ -81,6 +77,23 @@ void POLYGON_GEOM_MANAGER::SetLeaderMode( LEADER_MODE aMode )
 }
 
 
+bool POLYGON_GEOM_MANAGER::IsSelfIntersecting( bool aIncludeLeaderPts ) const
+{
+    auto pts( m_lockedPoints );
+
+    if( aIncludeLeaderPts )
+    {
+        for( int i = 0; i < m_leaderPts.PointCount(); ++i )
+            pts.Append( m_leaderPts.CPoint( i ) );
+    }
+
+    // line chain needs to be set as closed for proper checks
+    pts.SetClosed( true );
+
+    return !!pts.SelfIntersecting();
+}
+
+
 void POLYGON_GEOM_MANAGER::SetCursorPosition( const VECTOR2I& aPos, LEADER_MODE aModifier )
 {
     updateLeaderPoints( aPos, aModifier );

include/preview_items/polygon_geom_manager.h

@@ -113,6 +113,13 @@ public:
         return m_intersectionsAllowed;
     }
 
+    /**
+     * Checks whether the locked points constitute a self-intersecting outline.
+     * @param aIncludeLeaderPts when true, also the leading points (not placed ones) will be tested.
+     * @return True when the outline is self-intersecting.
+     */
+    bool IsSelfIntersecting( bool aIncludeLeaderPts ) const;
+
     /**
      * Set the current cursor position
      */

pcbnew/tools/drawing_tool.cpp

@@ -1253,7 +1253,10 @@ void DRAWING_TOOL::runPolygonEventLoop( POLYGON_GEOM_MANAGER& polyGeomMgr )
 {
     auto&   controls    = *getViewControls();
     bool    started     = false;
+
     STATUS_TEXT_POPUP status( m_frame );
+    status.SetTextColor( wxColour( 255, 0, 0 ) );
+    status.SetText( _( "Self-intersecting polygons are not allowed" ) );
 
     while( OPT_TOOL_EVENT evt = Wait() )
     {
@@ -1310,15 +1313,6 @@ void DRAWING_TOOL::runPolygonEventLoop( POLYGON_GEOM_MANAGER& polyGeomMgr )
                     controls.CaptureCursor( true );
                 }
             }
-            else if( started )
-            {
-                status.SetTextColor( wxColour( 255, 0, 0 ) );
-                status.SetText( _( "Self-intersecting polygons are not allowed" ) );
-                wxPoint p = wxGetMousePosition() + wxPoint( 20, 20 );
-                status.Move( p );
-                status.Popup( m_frame );
-                status.Expire( 1500 );
-            }
 
         }
         else if( evt->IsAction( &deleteLastPoint ) )
@@ -1342,6 +1336,18 @@ void DRAWING_TOOL::runPolygonEventLoop( POLYGON_GEOM_MANAGER& polyGeomMgr )
             polyGeomMgr.SetCursorPosition( cursorPos, evt->Modifier( MD_CTRL )
                                                       ? POLYGON_GEOM_MANAGER::LEADER_MODE::DEG45
                                                       : POLYGON_GEOM_MANAGER::LEADER_MODE::DIRECT );
+
+            if( polyGeomMgr.IsSelfIntersecting( true ) )
+            {
+                wxPoint p = wxGetMousePosition() + wxPoint( 20, 20 );
+                status.Move( p );
+                status.Popup( m_frame );
+                status.Expire( 1500 );
+            }
+            else
+            {
+                status.Hide();
+            }
         }
     }    // end while
 }

pcbnew/tools/point_editor.cpp

@@ -229,7 +229,10 @@ void POINT_EDITOR::Reset( RESET_REASON aReason )
     m_editPoints.reset();
     m_altConstraint.reset();
     getViewControls()->SetAutoPan( false );
+
     m_statusPopup.reset( new STATUS_TEXT_POPUP( getEditFrame<PCB_BASE_EDIT_FRAME>() ) );
+    m_statusPopup->SetTextColor( wxColour( 255, 0, 0 ) );
+    m_statusPopup->SetText( _( "Self-intersecting polygons are not allowed" ) );
 }
 
 
@@ -473,13 +476,12 @@ void POINT_EDITOR::updateItem() const
 
         case S_POLYGON:
         {
-            SHAPE_POLY_SET originalPoly = segment->GetPolyShape();
             SHAPE_POLY_SET& outline = segment->GetPolyShape();
 
             for( int i = 0; i < outline.TotalVertices(); ++i )
                 outline.Vertex( i ) = m_editPoints->Point( i ).GetPosition();
 
-            validatePolygon( outline, &originalPoly );
+            validatePolygon( outline );
             break;
         }
 
@@ -499,12 +501,11 @@ void POINT_EDITOR::updateItem() const
         ZONE_CONTAINER* zone = static_cast<ZONE_CONTAINER*>( item );
         zone->ClearFilledPolysList();
         SHAPE_POLY_SET& outline = *zone->Outline();
-        SHAPE_POLY_SET originalPoly( outline );
 
         for( int i = 0; i < outline.TotalVertices(); ++i )
             outline.Vertex( i ) = m_editPoints->Point( i ).GetPosition();
 
-        validatePolygon( outline, &originalPoly );
+        validatePolygon( outline );
         zone->Hatch();
         break;
     }
@@ -586,12 +587,13 @@ void POINT_EDITOR::finishItem()
 bool POINT_EDITOR::validatePolygon( SHAPE_POLY_SET& aModified, const SHAPE_POLY_SET* aOriginal ) const
 {
     if( !aModified.IsSelfIntersecting() )
+    {
+        m_statusPopup->Hide();
         return true;
+    }
 
     if( m_statusPopup )
     {
-        m_statusPopup->SetTextColor( wxColour( 255, 0, 0 ) );
-        m_statusPopup->SetText( _( "Self-intersecting polygons are not allowed" ) );
         wxPoint p = wxGetMousePosition() + wxPoint( 20, 20 );
         m_statusPopup->Move( p );
         m_statusPopup->Popup( getEditFrame<PCB_BASE_FRAME>() );

pcbnew/tools/zone_create_helper.cpp

@@ -65,7 +65,7 @@ std::unique_ptr<ZONE_CONTAINER> ZONE_CREATE_HELPER::createNewZone( bool aKeepout
     zoneInfo.m_NetcodeSelection = board.GetHighLightNetCode();
     zoneInfo.SetIsKeepout( m_params.m_keepout );
 
-    if ( m_params.m_mode != DRAWING_TOOL::ZONE_MODE::GRAPHIC_POLYGON  )
+    if( m_params.m_mode != DRAWING_TOOL::ZONE_MODE::GRAPHIC_POLYGON )
     {
         // Get the current default settings for zones
 
@@ -249,14 +249,15 @@ void ZONE_CREATE_HELPER::OnComplete( const POLYGON_GEOM_MANAGER& aMgr )
         // if m_params.m_mode == DRAWING_TOOL::ZONE_MODE::CUTOUT, m_zone
         // will be merged to the existing zone as a new hole.
         m_zone->Outline()->NewOutline();
+        auto* outline = m_zone->Outline();
 
         for( int i = 0; i < finalPoints.PointCount(); ++i )
         {
-            m_zone->Outline()->Append( finalPoints.CPoint( i ) );
+            outline->Append( finalPoints.CPoint( i ) );
         }
 
-        m_zone->Outline()->Outline( 0 ).SetClosed( true );
-        m_zone->Outline()->RemoveNullSegments();
+        outline->Outline( 0 ).SetClosed( true );
+        outline->RemoveNullSegments();
 
         // hand the zone over to the committer
         commitZone( std::move( m_zone ) );