PNS: Fix loop removal, make walkaround less broken for arcs

libs/kimath/include/geometry/direction45.h

@@ -90,9 +90,20 @@ public:
     DIRECTION_45( const SEG& aSeg, bool a90 = false ) :
             m_90deg( a90 )
     {
+
         construct_( aSeg.B - aSeg.A );
     }
 
+    /**
+     * Creates a DIRECTION_45 from the endpoints of a given arc
+     * @param aArc will be translated into the closest DIRECTION_45
+     */
+    DIRECTION_45( const SHAPE_ARC& aArc, bool a90 = false ) :
+            m_90deg( a90 )
+    {
+        construct_( aArc.GetP1() - aArc.GetP0() );
+    }
+
     /**
      * Function Format()
      * Formats the direction in a human readable word.

libs/kimath/include/geometry/shape_line_chain.h

@@ -232,6 +232,13 @@ public:
         return std::max( 0, c );
     }
 
+    /**
+     * Returns the number of shapes (line segments or arcs) in this line chain.
+     * This is kind of like SegmentCount() but will only count arcs as 1 segment
+     * @return ArcCount() + the number of non-arc segments
+     */
+    int ShapeCount() const;
+
     /**
      * Function PointCount()
      *
@@ -283,6 +290,22 @@ public:
                         const_cast<VECTOR2I&>( m_points[aIndex + 1] ), aIndex );
     }
 
+    /**
+     * Returns the vertex index of the next shape in the chain, or -1 if aPoint is in the last shape
+     * If aPoint is the start of a segment, this will be ( aPoint + 1 ).
+     * If aPoint is part of an arc, this will be the index of the start of the next shape after the
+     * arc, in other words, the last point of the arc.
+     * @param aPointIndex is a vertex in the chain
+     * @param aForwards is true if the next shape is desired, false for previous shape
+     * @return the vertex index of the start of the next shape after aPoint's shape
+     */
+    int NextShape( int aPointIndex, bool aForwards = true ) const;
+
+    int PrevShape( int aPointIndex ) const
+    {
+        return NextShape( aPointIndex, false );
+    }
+
     /**
      * Accessor Function to move a point to a specific location
      * @param aIndex Index (wrapping) of the point to move
@@ -489,6 +512,14 @@ public:
         Remove( aIndex, aIndex );
     }
 
+    /**
+     * Removes the shape at the given index from the line chain.
+     * If the given index is inside an arc, the entire arc will be removed.
+     * Otherwise this is equivalent to Remove( aPointIndex ).
+     * @param aPointIndex is the index of the point to remove
+     */
+    void RemoveShape( int aPointIndex );
+
     /**
      * Function Split()
      *

libs/kimath/src/geometry/direction_45.cpp

@@ -19,6 +19,7 @@
 
 #include <geometry/direction45.h>
 
+
 const SHAPE_LINE_CHAIN DIRECTION_45::BuildInitialTrace( const VECTOR2I& aP0, const VECTOR2I& aP1,
                                                         bool aStartDiagonal, bool aFillet ) const
 {

libs/kimath/src/geometry/shape_line_chain.cpp

@@ -420,6 +420,104 @@ int SHAPE_LINE_CHAIN::FindSegment( const VECTOR2I& aP ) const
 }
 
 
+int SHAPE_LINE_CHAIN::ShapeCount() const
+{
+    if( m_points.empty() )
+        return 0;
+
+    int numPoints = static_cast<int>( m_shapes.size() );
+    int numShapes = 0;
+    int arcIdx    = -1;
+
+    for( int i = 0; i < m_points.size() - 1; i++ )
+    {
+        if( m_shapes[i] == SHAPE_IS_PT )
+        {
+            numShapes++;
+        }
+        else
+        {
+            arcIdx = m_shapes[i];
+            numShapes++;
+
+            // Now skip the rest of the arc
+            while( i < numPoints && m_shapes[i] == arcIdx )
+                i++;
+
+            // Is there another arc right after?  Add the "hidden" segment
+            if( i < numPoints &&
+                m_shapes[i] != SHAPE_IS_PT &&
+                m_shapes[i] != arcIdx &&
+                m_points[i] != m_points[i - 1] )
+            {
+                numShapes++;
+            }
+
+            i--;
+        }
+    }
+
+    return numShapes;
+}
+
+
+int SHAPE_LINE_CHAIN::NextShape( int aPointIndex, bool aForwards ) const
+{
+    if( aPointIndex < 0 )
+        aPointIndex += PointCount();
+
+    // First or last point?
+    if( ( aForwards && aPointIndex == PointCount() - 1 ) ||
+        ( !aForwards && aPointIndex == 0 ) )
+    {
+        return -1;
+    }
+
+    int delta = aForwards ? 1 : -1;
+
+    if( m_shapes[aPointIndex] == SHAPE_IS_PT )
+        return aPointIndex + delta;
+
+    int arcIndex = m_shapes[aPointIndex];
+    int arcStart = aPointIndex;
+
+    while( aPointIndex < static_cast<int>( m_shapes.size() ) && m_shapes[aPointIndex] == arcIndex )
+        aPointIndex += delta;
+
+    // We want the last vertex of the arc if the initial point was the start of one
+    // Well-formed arcs should generate more than one point to travel above
+    if( aPointIndex - arcStart > 1 )
+        aPointIndex -= delta;
+
+    return aPointIndex;
+}
+
+
+void SHAPE_LINE_CHAIN::RemoveShape( int aPointIndex )
+{
+    if( aPointIndex < 0 )
+        aPointIndex += PointCount();
+
+    if( m_shapes[aPointIndex] == SHAPE_IS_PT )
+    {
+        Remove( aPointIndex );
+        return;
+    }
+
+    int start  = aPointIndex;
+    int end    = aPointIndex;
+    int arcIdx = m_shapes[aPointIndex];
+
+    while( start >= 0 && m_shapes[start] == arcIdx )
+        start--;
+
+    while( end < static_cast<int>( m_shapes.size() ) - 1 && m_shapes[end] == arcIdx )
+        end++;
+
+    Remove( start, end );
+}
+
+
 const SHAPE_LINE_CHAIN SHAPE_LINE_CHAIN::Slice( int aStartIndex, int aEndIndex ) const
 {
     SHAPE_LINE_CHAIN rv;
@@ -431,7 +529,33 @@ const SHAPE_LINE_CHAIN SHAPE_LINE_CHAIN::Slice( int aStartIndex, int aEndIndex )
         aStartIndex += PointCount();
 
     for( int i = aStartIndex; i <= aEndIndex && static_cast<size_t>( i ) < m_points.size(); i++ )
+#if 0
+    {
+        if( m_shapes[i] != SHAPE_IS_PT )
+        {
+            int arcIdx = m_shapes[i];
+
+            // wxASSERT_MSG( i == 0 || ( m_shapes[i - 1] != arcIdx ),
+            //               "SHAPE_LINE_CHAIN::Slice in the middle of an arc!" );
+
+            rv.Append( m_arcs[arcIdx] );
+
+            while( m_shapes[i] == arcIdx && static_cast<size_t>( i ) < m_shapes.size() )
+                i++;
+
+            i--;
+
+            // FIXME: PNS currently slices in the middle of arcs all the time (LINE::Walkaround)
+            // wxASSERT_MSG( i <= aEndIndex, "SHAPE_LINE_CHAIN::Slice in the middle of an arc!" );
+        }
+        else
+        {
             rv.Append( m_points[i] );
+        }
+    }
+#else
+            rv.Append( m_points[i] );
+#endif
 
     return rv;
 }

pcbnew/router/pns_kicad_iface.cpp

@@ -1331,8 +1331,6 @@ void PNS_KICAD_IFACE_BASE::SetDebugDecorator( PNS::DEBUG_DECORATOR *aDec )
 
 void PNS_KICAD_IFACE::DisplayItem( const PNS::ITEM* aItem, int aClearance, bool aEdit )
 {
-    wxLogTrace( "PNS", "DisplayItem %p", aItem );
-
     ROUTER_PREVIEW_ITEM* pitem = new ROUTER_PREVIEW_ITEM( aItem, m_view );
 
     if( aClearance >= 0 )

pcbnew/router/pns_line.cpp

@@ -868,20 +868,67 @@ int LINE::Rank() const
 
 void LINE::ClipVertexRange( int aStart, int aEnd )
 {
+    /**
+     * We need to figure out which joints to keep after the clip operation, because arcs will have
+     * multiple vertices.  It is assumed that anything calling this method will have determined the
+     * vertex range to clip based on joints, meaning we will never clip in the middle of an arc.
+     * Clipping in the middle of an arc would break this and various other things...
+     */
+    int firstLink = 0;
+    int lastLink  = std::max( 0, static_cast<int>( m_links.size() ) - 1 );
+    int arcIdx    = -1;
+    int linkIdx   = 0;
+
+    const std::vector<long>& shapes = m_line.CShapes();
+    int numPoints = static_cast<int>( shapes.size() );
+
+    for( int i = 0; i < m_line.PointCount(); i++ )
+    {
+        if( i <= aStart )
+            firstLink = linkIdx;
+
+        if( shapes[i] >= 0 )
+        {
+            // Account for "hidden segments" between two arcs
+            if( i > aStart && ( shapes[i - 1] >= 0 ) && ( shapes[i - 1] != shapes[i] ) )
+                linkIdx++;
+
+            arcIdx = shapes[i];
+
+            // Skip over the rest of the arc vertices
+            while( i < numPoints && shapes[i] == arcIdx )
+                i++;
+
+            // Back up two vertices to restart at the segment coincident with the end of the arc
+            i -= 2;
+        }
+
+        if( i >= aEnd - 1 || linkIdx >= lastLink )
+        {
+            lastLink = linkIdx;
+            break;
+        }
+
+        linkIdx++;
+    }
+
+    wxASSERT( lastLink >= firstLink );
+
     m_line = m_line.Slice( aStart, aEnd );
 
-    if( IsLinked() ) {
+    if( IsLinked() )
-        assert( m_links.size() < INT_MAX );
+    {
-        assert( (int) m_links.size() >= (aEnd - aStart) );
+        wxASSERT( m_links.size() < INT_MAX );
+        wxASSERT( static_cast<int>( m_links.size() ) >= ( lastLink - firstLink ) );
 
         // Note: The range includes aEnd, but we have n-1 segments.
         std::rotate(
             m_links.begin(),
-            m_links.begin() + aStart,
+            m_links.begin() + firstLink,
-            m_links.begin() + aEnd
+            m_links.begin() + lastLink
         );
 
-        m_links.resize( aEnd - aStart );
+        m_links.resize( lastLink - firstLink + 1 );
     }
 }
 

pcbnew/router/pns_line.h

@@ -147,6 +147,7 @@ public:
     int SegmentCount() const { return m_line.SegmentCount(); }
     int PointCount() const { return m_line.PointCount(); }
     int ArcCount() const { return m_line.ArcCount(); }
+    int ShapeCount() const { return m_line.ShapeCount(); }
 
     ///> Returns the aIdx-th point of the line
     const VECTOR2I& CPoint( int aIdx ) const { return m_line.CPoint( aIdx ); }

pcbnew/router/pns_line_placer.cpp

@@ -189,8 +189,24 @@ bool LINE_PLACER::handlePullback()
         return true;
     }
 
-    DIRECTION_45 first_head( head.CSegment( 0 ) );
+    DIRECTION_45 first_head, last_tail;
-    DIRECTION_45 last_tail( tail.CSegment( -1 ) );
+
+    const std::vector<ssize_t>& headShapes = head.CShapes();
+    const std::vector<ssize_t>& tailShapes = tail.CShapes();
+
+    wxASSERT( tail.PointCount() >= 2 );
+    if( headShapes[0] == -1 )
+        first_head = DIRECTION_45( head.CSegment( 0 ) );
+    else
+        first_head = DIRECTION_45( head.CArcs()[ headShapes[0] ] );
+
+    int lastSegIdx = tail.PointCount() - 2;
+
+    if( tailShapes[lastSegIdx] == -1 )
+        last_tail = DIRECTION_45( tail.CSegment( lastSegIdx ) );
+    else
+        last_tail = DIRECTION_45( tail.CArcs()[tailShapes[lastSegIdx]] );
+
     DIRECTION_45::AngleType angle = first_head.Angle( last_tail );
 
     // case 1: we have a defined routing direction, and the currently computed
@@ -204,9 +220,20 @@ bool LINE_PLACER::handlePullback()
 
     if( pullback_1 || pullback_2 )
     {
-        const SEG last = tail.CSegment( -1 );
+        lastSegIdx = tail.PrevShape( -1 );
-        m_direction = DIRECTION_45( last );
+
-        m_p_start = last.A;
+        if( tailShapes[lastSegIdx] == -1 )
+        {
+            const SEG& seg = tail.CSegment( lastSegIdx );
+            m_direction    = DIRECTION_45( seg );
+            m_p_start      = seg.A;
+        }
+        else
+        {
+            const SHAPE_ARC& arc = tail.CArcs()[tailShapes[lastSegIdx]];
+            m_direction          = DIRECTION_45( arc );
+            m_p_start            = arc.GetP0();
+        }
 
         wxLogTrace( "PNS", "Placer: pullback triggered [%d] [%s %s]",
                 n, last_tail.Format().c_str(), first_head.Format().c_str() );
@@ -217,7 +244,7 @@ bool LINE_PLACER::handlePullback()
         if( n < 2 )
             tail.Clear(); // don't leave a single-point tail
         else
-            tail.Remove( -1, -1 );
+            tail.RemoveShape( -1 );
 
         if( !tail.SegmentCount() )
             m_direction = m_initial_direction;
@@ -294,15 +321,6 @@ bool LINE_PLACER::reduceTail( const VECTOR2I& aEnd )
 }
 
 
-bool LINE_PLACER::checkObtusity( const SEG& aA, const SEG& aB ) const
-{
-    const DIRECTION_45 dir_a( aA );
-    const DIRECTION_45 dir_b( aB );
-
-    return dir_a.IsObtuse( dir_b ) || dir_a == dir_b;
-}
-
-
 bool LINE_PLACER::mergeHead()
 {
     SHAPE_LINE_CHAIN& head = m_head.Line();
@@ -315,8 +333,8 @@ bool LINE_PLACER::mergeHead()
     head.Simplify();
     tail.Simplify();
 
-    int n_head  = head.SegmentCount();
+    int n_head  = head.ShapeCount();
-    int n_tail  = tail.SegmentCount();
+    int n_tail  = tail.ShapeCount();
 
     if( n_head < 3 )
     {
@@ -335,25 +353,41 @@ bool LINE_PLACER::mergeHead()
 
     DIRECTION_45 dir_tail, dir_head;
 
+    const std::vector<ssize_t>& headShapes = head.CShapes();
+    const std::vector<ssize_t>& tailShapes = tail.CShapes();
+
+    if( headShapes[0] == -1 )
         dir_head = DIRECTION_45( head.CSegment( 0 ) );
+    else
+        dir_head = DIRECTION_45( head.CArcs()[ headShapes[0] ] );
 
     if( n_tail )
     {
+        wxASSERT( tail.PointCount() >= 2 );
+        int lastSegIdx = tail.PointCount() - 2;
+
+        if( tailShapes[lastSegIdx] == -1 )
             dir_tail = DIRECTION_45( tail.CSegment( -1 ) );
+        else
+            dir_tail = DIRECTION_45( tail.CArcs()[ tailShapes[lastSegIdx] ] );
 
         if( dir_head.Angle( dir_tail ) & ForbiddenAngles )
             return false;
     }
 
     tail.Append( head );
-    tail.Remove( -1 );
 
     tail.Simplify();
 
     SEG last  = tail.CSegment( -1 );
-
     m_p_start = last.B;
-    m_direction = DIRECTION_45( last ).Right();
+
+    int lastSegIdx = tail.PointCount() - 2;
+
+    if( tailShapes[lastSegIdx] == -1 )
+        m_direction = DIRECTION_45( tail.CSegment( -1 ) );
+    else
+        m_direction = DIRECTION_45( tail.CArcs()[ tailShapes[lastSegIdx] ] );
 
     head.Remove( 0, -1 );
 
@@ -646,7 +680,7 @@ bool LINE_PLACER::optimizeTailHeadTransition()
 
     int threshold = std::min( tail.PointCount(), tailLookbackSegments + 1 );
 
-    if( tail.SegmentCount() < 3 )
+    if( tail.ShapeCount() < 3 )
         return false;
 
     // assemble TailLookbackSegments tail segments with the current head
@@ -691,10 +725,10 @@ void LINE_PLACER::routeStep( const VECTOR2I& aP )
 
     LINE new_head;
 
-    wxLogTrace( "PNS", "INIT-DIR: %s head: %d, tail: %d segs",
+    wxLogTrace( "PNS", "routeStep: direction: %s head: %d, tail: %d shapes",
-                m_initial_direction.Format().c_str(),
+                m_direction.Format().c_str(),
-                m_head.SegmentCount(),
+                m_head.ShapeCount(),
-                m_tail.SegmentCount() );
+                m_tail.ShapeCount() );
 
     for( i = 0; i < n_iter; i++ )
     {
@@ -1064,6 +1098,7 @@ bool LINE_PLACER::FixRoute( const VECTOR2I& aP, ITEM* aEndItem, bool aForceFinis
     if( !fixAll && l.ArcCount() )
         fixAll = true;
 
+    // TODO: lastDirSeg will be calculated incorrectly if we end on an arc
     SEG lastDirSeg = ( !fixAll && l.SegmentCount() > 1 ) ? l.CSegment( -2 ) : l.CSegment( -1 );
     lastDirSeg.A.y = -lastDirSeg.A.y;
     lastDirSeg.B.y = -lastDirSeg.B.y;
@@ -1118,9 +1153,7 @@ bool LINE_PLACER::FixRoute( const VECTOR2I& aP, ITEM* aEndItem, bool aForceFinis
         setInitialDirection( d_last );
         m_currentStart = ( m_placingVia || fixAll ) ? p_last : p_pre_last;
 
-        NODE* commit = fixAll ? m_lastNode : m_currentNode;
+        m_fixedTail.AddStage( m_p_start, m_currentLayer, m_placingVia, m_direction, m_currentNode );
-
-        m_fixedTail.AddStage( m_p_start, m_currentLayer, m_placingVia, m_direction, commit );
 
         m_startItem = NULL;
         m_placingVia = false;
@@ -1323,6 +1356,9 @@ bool LINE_PLACER::buildInitialLine( const VECTOR2I& aP, LINE& aHead )
     SHAPE_LINE_CHAIN l;
     DIRECTION_45 guessedDir = m_postureSolver.GetPosture( aP );
 
+    wxLogTrace( "PNS", "buildInitialLine: m_direction %s, guessedDir %s, tail points %d",
+                m_direction.Format(), guessedDir.Format(), m_tail.PointCount() );
+
     // Rounded corners don't make sense when routing orthogonally (single track at a time)
     bool fillet = !m_orthoMode && Settings().GetCornerMode() == CORNER_MODE::ROUNDED_45;
 

pcbnew/router/pns_line_placer.h

@@ -328,15 +328,6 @@ private:
      */
     void simplifyNewLine( NODE* aNode, SEGMENT* aLatest );
 
-    /**
-     * Function checkObtusity()
-     *
-     * Helper function, checking if segments a and b form an obtuse angle
-     * (in 45-degree regime).
-     * @return true, if angle (aA, aB) is obtuse
-     */
-    bool checkObtusity( const SEG& aA, const SEG& aB ) const;
-
     /**
      * Function handleSelfIntersections()
      *

pcbnew/router/pns_node.cpp

@@ -48,7 +48,6 @@ static std::unordered_set<NODE*> allocNodes;
 
 NODE::NODE()
 {
-    wxLogTrace( "PNS", "NODE::create %p", this );
     m_depth = 0;
     m_root = this;
     m_parent = NULL;
@@ -64,8 +63,6 @@ NODE::NODE()
 
 NODE::~NODE()
 {
-    wxLogTrace( "PNS", "NODE::delete %p", this );
-
     if( !m_children.empty() )
     {
         wxLogTrace( "PNS", "attempting to free a node that has kids." );
@@ -127,8 +124,6 @@ NODE* NODE::Branch()
 {
     NODE* child = new NODE;
 
-    wxLogTrace( "PNS", "NODE::branch %p (parent %p)", child, this );
-
     m_children.insert( child );
 
     child->m_depth = m_depth + 1;
@@ -150,10 +145,12 @@ NODE* NODE::Branch()
         child->m_override = m_override;
     }
 
+#if 0
     wxLogTrace( "PNS", "%d items, %d joints, %d overrides",
                 child->m_index->Size(),
                 (int) child->m_joints.size(),
                 (int) child->m_override.size() );
+#endif
 
     return child;
 }
@@ -932,25 +929,37 @@ const LINE NODE::AssembleLine( LINKED_ITEM* aSeg, int* aOriginSegmentIndex,
     for( int i = i_start + 1; i < i_end; i++ )
     {
         const VECTOR2I& p  = corners[i];
+        LINKED_ITEM*    li = segs[i];
 
+        if( !li || li->Kind() != ITEM::ARC_T )
             pl.Line().Append( p );
 
-        if( segs[i] && prev_seg != segs[i] )
+        if( li && prev_seg != li )
         {
-            pl.Link( segs[i] );
+            if( li->Kind() == ITEM::ARC_T )
+            {
+                const ARC*       arc = static_cast<const ARC*>( li );
+                const SHAPE_ARC* sa  = static_cast<const SHAPE_ARC*>( arc->Shape() );
+                pl.Line().Append( *sa );
+            }
+
+            pl.Link( li );
 
             // latter condition to avoid loops
-            if( segs[i] == aSeg && aOriginSegmentIndex && !originSet )
+            if( li == aSeg && aOriginSegmentIndex && !originSet )
             {
                 *aOriginSegmentIndex = n;
                 originSet = true;
             }
+
             n++;
         }
 
-        prev_seg = segs[i];
+        prev_seg = li;
     }
 
+    pl.Line().Simplify();
+
     assert( pl.SegmentCount() != 0 );
 
     return pl;

pcbnew/router/pns_optimizer.cpp

@@ -577,6 +577,7 @@ bool OPTIMIZER::Optimize( LINE* aLine, LINE* aResult )
         aResult->ClearLinks();
     }
 
+    bool hasArcs = aLine->ArcCount();
     bool rv = false;
 
     if( m_effortLevel & PRESERVE_VERTEX )
@@ -598,19 +599,23 @@ bool OPTIMIZER::Optimize( LINE* aLine, LINE* aResult )
         AddConstraint( c );
     }
 
-    if( m_effortLevel & MERGE_SEGMENTS )
+    // TODO: Fix for arcs
+    if( !hasArcs && m_effortLevel & MERGE_SEGMENTS )
         rv |= mergeFull( aResult );
 
-    if( m_effortLevel & MERGE_OBTUSE )
+    // TODO: Fix for arcs
+    if( !hasArcs && m_effortLevel & MERGE_OBTUSE )
         rv |= mergeObtuse( aResult );
 
     if( m_effortLevel & MERGE_COLINEAR )
         rv |= mergeColinear( aResult );
 
-    if( m_effortLevel & SMART_PADS )
+    // TODO: Fix for arcs
+    if( !hasArcs && m_effortLevel & SMART_PADS )
         rv |= runSmartPads( aResult );
 
-    if( m_effortLevel & FANOUT_CLEANUP )
+    // TODO: Fix for arcs
+    if( !hasArcs && m_effortLevel & FANOUT_CLEANUP )
         rv |= fanoutCleanup( aResult );
 
     return rv;
@@ -1023,10 +1028,6 @@ bool OPTIMIZER::Optimize( LINE* aLine, int aEffortLevel, NODE* aWorld, const VEC
     if( aEffortLevel & OPTIMIZER::PRESERVE_VERTEX )
         opt.SetPreserveVertex( aV );
 
-    // TODO: fix optimizer for arcs
-    if( aLine->ArcCount() )
-        return -1;
-
     return opt.Optimize( aLine );
 }