SHAPE_LINE_CHAIN fix rotation of indices after going through Clipper

Clipper might mess up the rotation of the indices such that an arc can be split between
the end point and wrap around to the start point. Detect if this happened and fix it as
required.

Also, handle arcs at the last segment of the chain correctly, meaning we can have arcs
towards the end of the chain that finish at the starting point of the chain.

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

libs/kimath/include/geometry/shape_line_chain.h

@@ -256,6 +256,7 @@ public:
     void SetClosed( bool aClosed )
     {
         m_closed = aClosed;
+        mergeFirstLastPointIfNeeded();
     }
 
     /**
@@ -806,7 +807,7 @@ public:
     */
     bool IsSharedPt( size_t aIndex ) const
     {
-        return aIndex < m_shapes.size() - 1
+        return aIndex < m_shapes.size()
                && m_shapes[aIndex].first != SHAPE_IS_PT
                && m_shapes[aIndex].second != SHAPE_IS_PT;
     }
@@ -828,7 +829,12 @@ public:
         size_t nextIdx = aSegment + 1;
 
         if( nextIdx > m_shapes.size() - 1 )
-            return false; // Always false, even if the shape is closed
+        {
+            if( nextIdx == m_shapes.size() && m_closed )
+                nextIdx = 0; // segment between end point and first point
+            else
+                return false;
+        }
 
         return ( IsPtOnArc( aSegment )
                  && ( IsSharedPt( aSegment )
@@ -847,9 +853,6 @@ public:
 
     bool IsArcEnd( size_t aIndex ) const
     {
-        if( aIndex == static_cast<size_t>( PointCount() ) - 1 )
-            return IsPtOnArc( aIndex );
-
         return ( IsSharedPt( aIndex ) || ( IsPtOnArc( aIndex ) && !IsArcSegment( aIndex ) ) );
     }
 
@@ -912,6 +915,16 @@ protected:
                                        std::vector<CLIPPER_Z_VALUE>& aZValueBuffer,
                                        std::vector<SHAPE_ARC>&       aArcBuffer ) const;
 
+    /**
+     * Fix indices of this chain to ensure arcs are not split between the end and start indices
+     */
+    void fixIndicesRotation();
+
+    /**
+     * Merge the first and last point if they are the same and this chain is closed.
+     */
+    void mergeFirstLastPointIfNeeded();
+
 private:
 
     static const ssize_t SHAPE_IS_PT;

libs/kimath/src/geometry/shape_line_chain.cpp

@@ -77,7 +77,7 @@ SHAPE_LINE_CHAIN::SHAPE_LINE_CHAIN( const ClipperLib::Path&             aPath,
                 m_arcs.push_back( aArcBuffer.at( aArcIndex ) );
             }
 
-            return loadedArcs.at(aArcIndex);
+            return loadedArcs.at( aArcIndex );
         };
 
     for( size_t ii = 0; ii < aPath.size(); ++ii )
@@ -87,6 +87,14 @@ SHAPE_LINE_CHAIN::SHAPE_LINE_CHAIN( const ClipperLib::Path&             aPath,
         m_shapes[ii].first = loadArc( aZValueBuffer[aPath[ii].Z].m_FirstArcIdx );
         m_shapes[ii].second = loadArc( aZValueBuffer[aPath[ii].Z].m_SecondArcIdx );
     }
+
+    // Clipper shouldn't return duplicate contiguous points. if it did, these would be
+    // removed during Append() and we would have different number of shapes to points
+    wxASSERT( m_shapes.size() == m_points.size() );
+
+    // Clipper might mess up the rotation of the indices such that an arc can be split between
+    // the end point and wrap around to the start point. Lets fix the indices up now
+    fixIndicesRotation();
 }
 
 ClipperLib::Path SHAPE_LINE_CHAIN::convertToClipper( bool aRequiredOrientation,
@@ -120,6 +128,45 @@ ClipperLib::Path SHAPE_LINE_CHAIN::convertToClipper( bool aRequiredOrientation,
 }
 
 
+void SHAPE_LINE_CHAIN::fixIndicesRotation()
+{
+    wxCHECK( m_shapes.size() == m_points.size(), /*void*/ );
+
+    if( m_shapes.size() <= 1 || m_arcs.size() <= 1 )
+        return;
+
+    size_t rotations = 0;
+    size_t numPoints = m_points.size();
+
+    while( ArcIndex( 0 ) != SHAPE_IS_PT
+        && ArcIndex( 0 ) == ArcIndex( numPoints - 1 ) )
+    {
+        // Rotate right
+        std::rotate( m_points.rbegin(), m_points.rbegin() + 1, m_points.rend() );
+        std::rotate( m_shapes.rbegin(), m_shapes.rbegin() + 1, m_shapes.rend() );
+
+        // Sanity check - avoid infinite loops
+        wxCHECK( rotations++ <= m_shapes.size(), /* void */ );
+    }
+}
+
+
+void SHAPE_LINE_CHAIN::mergeFirstLastPointIfNeeded()
+{
+    if( m_closed )
+    {
+        if( m_points.size() > 1 && m_points.front() == m_points.back() )
+        {
+            m_shapes.front().second = m_shapes.front().first;
+            m_shapes.front().first = m_shapes.back().first;
+
+            m_points.pop_back();
+            m_shapes.pop_back();
+        }
+    }
+}
+
+
 void SHAPE_LINE_CHAIN::convertArc( ssize_t aArcIndex )
 {
     if( aArcIndex < 0 )
@@ -1185,6 +1232,8 @@ void SHAPE_LINE_CHAIN::Append( const SHAPE_LINE_CHAIN& aOtherLine )
         m_bbox.Merge( p );
     }
 
+    mergeFirstLastPointIfNeeded();
+
     assert( m_shapes.size() == m_points.size() );
 }