From 75938632454bb310588ca9155f7910d7ac54ee9a Mon Sep 17 00:00:00 2001
From: Roberto Fernandez Bautista <roberto.fer.bau@gmail.com>
Date: Sun, 7 Jan 2024 10:40:11 +0100
Subject: [PATCH] SHAPE_LINE_CHAIN: Fix and simplify ShapeCount()

Fixes https://gitlab.com/kicad/code/kicad/-/issues/15754
---
 libs/kimath/src/geometry/shape_line_chain.cpp | 46 +++----------------
 .../kimath/geometry/test_shape_line_chain.cpp | 14 ++++++
 2 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/libs/kimath/src/geometry/shape_line_chain.cpp b/libs/kimath/src/geometry/shape_line_chain.cpp
index 6190f078ed..b08cb28800 100644
--- a/libs/kimath/src/geometry/shape_line_chain.cpp
+++ b/libs/kimath/src/geometry/shape_line_chain.cpp
@@ -980,49 +980,15 @@ int SHAPE_LINE_CHAIN::FindSegment( const VECTOR2I& aP, int aThreshold ) 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 < static_cast<int>( m_points.size() ) - 1; i++ )
-    {
-        if( m_shapes[i] == SHAPES_ARE_PT )
-        {
-            numShapes++;
-        }
-        else
-        {
-            // Expect that the second index only gets populated when the point is shared between
-            // two shapes. Otherwise, the shape index should always go on the first element of
-            // the pair.
-            assert( m_shapes[i].first != SHAPE_IS_PT );
+    wxCHECK2_MSG( m_points.size() == m_shapes.size(), return 0, "Invalid chain!" );
 
-            // Start assuming the point is shared with the previous arc
-            // If so, the new/next arc index should be located at the second
-            // element in the pair
-            arcIdx = m_shapes[i].second;
-
-            if( arcIdx == SHAPE_IS_PT )
-                arcIdx = m_shapes[i].first; // Not a shared point
-
-            numShapes++;
-
-            // Now skip the rest of the arc
-            while( i < numPoints && m_shapes[i].first == arcIdx )
-                i++;
+    if( m_points.size() < 2 )
+        return 0;
 
-            // Add the "hidden" segment at the end of the arc, if it exists
-            if( i < numPoints && m_points[i] != m_points[i - 1] )
-            {
-                numShapes++;
-            }
+    int numShapes = 1;
 
-            i--;
-        }
-    }
+    for( int i = NextShape( 0 ); i != -1; i = NextShape( i ) )
+        numShapes++;
 
     return numShapes;
 }
diff --git a/qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp b/qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp
index 51d8924ab8..a5f506ed4d 100644
--- a/qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp
+++ b/qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp
@@ -229,6 +229,20 @@ BOOST_AUTO_TEST_CASE( SimplifyDuplicatePoint )
 
 
 
+BOOST_AUTO_TEST_CASE( ShapeCount )
+{
+    BOOST_CHECK_EQUAL( Circle1Arc.ShapeCount(), 1 );
+    BOOST_CHECK_EQUAL( Circle2Arcs.ShapeCount(), 2 );
+    BOOST_CHECK_EQUAL( ArcsCoincident.ShapeCount(), 2 );
+    BOOST_CHECK_EQUAL( ArcsCoincidentClosed.ShapeCount(), 3 );
+    BOOST_CHECK_EQUAL( DuplicateArcs.ShapeCount(), 4 );
+    BOOST_CHECK_EQUAL( ArcAndPoint.ShapeCount(), 2 );
+    BOOST_CHECK_EQUAL( ArcsAndSegMixed.ShapeCount(), 4 );
+    BOOST_CHECK_EQUAL( EmptyChain.ShapeCount(), 0 );
+    BOOST_CHECK_EQUAL( OnePoint.ShapeCount(), 0 );
+}
+
+
 BOOST_AUTO_TEST_CASE( NextShape )
 {
     BOOST_CHECK_EQUAL( Circle1Arc.NextShape( 0 ), -1 ); //only one arc