diff --git a/libs/kimath/include/geometry/shape_line_chain.h b/libs/kimath/include/geometry/shape_line_chain.h index b15942262d..3535e73082 100644 --- a/libs/kimath/include/geometry/shape_line_chain.h +++ b/libs/kimath/include/geometry/shape_line_chain.h @@ -368,16 +368,10 @@ public: * 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 or -1 if * the end was reached. */ - int NextShape( int aPointIndex, bool aForwards = true ) const; - - int PrevShape( int aPointIndex ) const - { - return NextShape( aPointIndex, false ); - } + int NextShape( int aPointIndex ) const; /** * Move a point to a specific location. @@ -864,15 +858,14 @@ public: if( nextIdx > m_shapes.size() - 1 ) { - if( nextIdx == m_shapes.size() && m_closed ) + if( nextIdx == m_shapes.size() && m_closed && IsSharedPt( 0 ) ) nextIdx = 0; // segment between end point and first point else return false; } return ( IsPtOnArc( aSegment ) - && ( IsSharedPt( aSegment ) - || m_shapes[aSegment].first == m_shapes[nextIdx].first ) ); + && ( ArcIndex( aSegment ) == m_shapes[nextIdx].first ) ); } diff --git a/libs/kimath/src/geometry/shape_line_chain.cpp b/libs/kimath/src/geometry/shape_line_chain.cpp index dab84a6422..367d1e52f1 100644 --- a/libs/kimath/src/geometry/shape_line_chain.cpp +++ b/libs/kimath/src/geometry/shape_line_chain.cpp @@ -1028,60 +1028,49 @@ int SHAPE_LINE_CHAIN::ShapeCount() const } -int SHAPE_LINE_CHAIN::NextShape( int aPointIndex, bool aForwards ) const +int SHAPE_LINE_CHAIN::NextShape( int aPointIndex ) const { if( aPointIndex < 0 ) aPointIndex += PointCount(); + if( aPointIndex < 0 ) + return -1; + int lastIndex = PointCount() - 1; - // First or last point? - if( ( aForwards && aPointIndex == lastIndex ) || - ( !aForwards && aPointIndex == 0 ) ) - { - return -1; // we don't want to wrap around - } - - int delta = aForwards ? 1 : -1; + // Last point? + if( aPointIndex >= lastIndex ) + return -1; // we don't want to wrap around if( m_shapes[aPointIndex] == SHAPES_ARE_PT ) - return aPointIndex + delta; + return aPointIndex + 1; int arcStart = aPointIndex; // The second element should only get populated when the point is shared between two shapes. // If not a shared point, then the index should always go on the first element. - assert( m_shapes[aPointIndex].first != SHAPE_IS_PT ); + wxCHECK2_MSG( m_shapes[aPointIndex].first != SHAPE_IS_PT, return -1, "malformed chain!" ); - // Start with the assumption the point is shared - auto arcIndex = [&]( int aIndex ) -> ssize_t - { - if( aForwards ) - return ArcIndex( aIndex ); - else - return reversedArcIndex( aIndex ); - }; - - ssize_t currentArcIdx = arcIndex( aPointIndex ); + ssize_t currentArcIdx = ArcIndex( aPointIndex ); // Now skip the rest of the arc - while( aPointIndex < lastIndex && aPointIndex >= 0 && arcIndex( aPointIndex ) == currentArcIdx ) - aPointIndex += delta; - - if( aPointIndex == lastIndex ) - { - if( !m_closed && arcIndex( aPointIndex ) == currentArcIdx ) - return -1; - else - return lastIndex; // Segment between last point and the start - } + while( aPointIndex < lastIndex && ArcIndex( aPointIndex ) == currentArcIdx ) + aPointIndex += 1; bool indexStillOnArc = alg::pair_contains( m_shapes[aPointIndex], currentArcIdx ); // 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 && !indexStillOnArc ) - aPointIndex -= delta; + aPointIndex -= 1; + + if( aPointIndex == lastIndex ) + { + if( !m_closed || IsArcSegment( aPointIndex ) ) + return -1; //no shape + else + return lastIndex; // Segment between last point and the start of the chain + } return aPointIndex; } diff --git a/pcbnew/router/pns_line_placer.cpp b/pcbnew/router/pns_line_placer.cpp index d6e48f6492..60949f2e87 100644 --- a/pcbnew/router/pns_line_placer.cpp +++ b/pcbnew/router/pns_line_placer.cpp @@ -217,9 +217,7 @@ bool LINE_PLACER::handlePullback() if( pullback_1 || pullback_2 ) { - lastSegIdx = tail.PrevShape( -1 ); - - if( !tail.IsPtOnArc( lastSegIdx ) ) + if( !tail.IsArcSegment( lastSegIdx ) ) { const SEG& seg = tail.CSegment( lastSegIdx ); m_direction = DIRECTION_45( seg ); 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 37992e9eca..c360e0f972 100644 --- a/qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp +++ b/qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp @@ -35,7 +35,78 @@ * NOTE: Collision of SHAPE_LINE_CHAIN with arcs is tested in test_shape_arc.cpp */ -BOOST_AUTO_TEST_SUITE( ShapeLineChain ) +struct SLC_CASES +{ + SHAPE_LINE_CHAIN Circle1Arc; + SHAPE_LINE_CHAIN Circle2Arcs; + SHAPE_LINE_CHAIN ArcsCoincident; + SHAPE_LINE_CHAIN ArcsCoincidentClosed; + SHAPE_LINE_CHAIN ArcsIndependent; + SHAPE_LINE_CHAIN DuplicateArcs; + SHAPE_LINE_CHAIN ArcsAndSegMixed; + SHAPE_LINE_CHAIN ArcAndPoint; + SHAPE_LINE_CHAIN EmptyChain; + SHAPE_LINE_CHAIN OnePoint; + + SHAPE_ARC ArcCircle; ///< Full Circle arc + SHAPE_ARC Arc0a; ///< First half of a circle + SHAPE_ARC Arc0b; ///< Second half of a circle + SHAPE_ARC Arc1; ///< start coincident with Arc0a end + SHAPE_ARC Arc2; ///< Independent arc + + SLC_CASES() + { + ArcCircle = SHAPE_ARC( VECTOR2I( 183450000, 128360000 ), + VECTOR2I( 183850000, 128360000 ), + VECTOR2I( 183450000, 128360000 ), 0 ); + + Arc0a = SHAPE_ARC( VECTOR2I( 183450000, 128360000 ), + VECTOR2I( 183650000, 128560000 ), + VECTOR2I( 183850000, 128360000 ), 0 ); + + Arc0b = SHAPE_ARC( VECTOR2I( 183850000, 128360000 ), + VECTOR2I( 183650000, 128160000 ), + VECTOR2I( 183450000, 128360000 ), 0 ); + + Arc1 = SHAPE_ARC( VECTOR2I( 183850000, 128360000 ), + VECTOR2I( 183638550, 128640305 ), + VECTOR2I( 183500000, 129204974 ), 0 ); + + Arc2 = SHAPE_ARC( VECTOR2I( 283450000, 228360000 ), + VECTOR2I( 283650000, 228560000 ), + VECTOR2I( 283850000, 228360000 ), 0 ); + + Circle1Arc.Append( ArcCircle ); + Circle1Arc.SetClosed( true ); + + Circle2Arcs.Append( Arc0a ); + Circle2Arcs.Append( Arc0b ); + Circle2Arcs.SetClosed( true ); + + ArcsCoincident.Append( Arc0a ); + ArcsCoincident.Append( Arc1 ); + + ArcsCoincidentClosed=ArcsCoincident; + ArcsCoincidentClosed.SetClosed( true ); + + ArcsIndependent.Append( Arc0a ); + ArcsIndependent.Append( Arc2 ); + + DuplicateArcs=ArcsCoincident; + DuplicateArcs.Append( Arc1 ); //should add a segment between end of the chain and new copy of the arc + + ArcAndPoint.Append( Arc0a ); + ArcAndPoint.Append( VECTOR2I( 233450000, 228360000 ) ); + + ArcsAndSegMixed = ArcAndPoint; + ArcsAndSegMixed.Append( Arc2 ); + + OnePoint.Append( VECTOR2I( 233450000, 228360000 ) ); + } +}; + + +BOOST_FIXTURE_TEST_SUITE( TestShapeLineChain, SLC_CASES ) BOOST_AUTO_TEST_CASE( ArcToPolyline ) { @@ -156,6 +227,50 @@ BOOST_AUTO_TEST_CASE( SimplifyDuplicatePoint ) } + + +BOOST_AUTO_TEST_CASE( NextShape ) +{ + BOOST_CHECK_EQUAL( Circle1Arc.NextShape( 0 ), -1 ); //only one arc + + BOOST_CHECK_EQUAL( Circle2Arcs.NextShape( 0 ), 8 ); // next shape "Arc0b" + BOOST_CHECK_EQUAL( Circle2Arcs.NextShape( 8 ), -1 ); //no more shapes (last point joins with first, part of arc) + + BOOST_CHECK_EQUAL( ArcsCoincident.NextShape( 0 ), 8 ); // next shape "Arc1" + BOOST_CHECK_EQUAL( ArcsCoincident.NextShape( 8 ), -1 ); //no more shapes + + BOOST_CHECK_EQUAL( ArcsCoincidentClosed.NextShape( 0 ), 8 ); // next shape "Arc1" + BOOST_CHECK_EQUAL( ArcsCoincidentClosed.NextShape( 8 ), 13 ); //next shape is hidden segment joining last/first + BOOST_CHECK_EQUAL( ArcsCoincidentClosed.NextShape( 13 ), -1 ); //no more shapes + + BOOST_CHECK_EQUAL( DuplicateArcs.NextShape( 0 ), 8 ); // next shape "Arc1" + BOOST_CHECK_EQUAL( DuplicateArcs.NextShape( 8 ), 13 ); // next shape hidden segment joining the 2 duplicate arcs + BOOST_CHECK_EQUAL( DuplicateArcs.NextShape( 13 ), 14 ); // next shape "Arc1" (duplicate) + BOOST_CHECK_EQUAL( DuplicateArcs.NextShape( 14 ), -1 ); //no more shapes + + BOOST_CHECK_EQUAL( ArcAndPoint.NextShape( 0 ), 8 ); // next shape straight segment (end of arc->point) + BOOST_CHECK_EQUAL( ArcAndPoint.NextShape( 8 ), -1 ); //no more shapes + + BOOST_CHECK_EQUAL( ArcsAndSegMixed.NextShape( 0 ), 8 ); // next shape straight segment (end of arc->point) + BOOST_CHECK_EQUAL( ArcsAndSegMixed.NextShape( 8 ), 9 ); // next shape straight segment (point->begining of arc) + BOOST_CHECK_EQUAL( ArcsAndSegMixed.NextShape( 9 ), 10 ); //next shape second arc + BOOST_CHECK_EQUAL( ArcsAndSegMixed.NextShape( 10 ), -1 ); //no more shapes + BOOST_CHECK_EQUAL( ArcsAndSegMixed.NextShape( 20 ), -1 ); //invalid indices should still work + BOOST_CHECK_EQUAL( ArcsAndSegMixed.NextShape( -50 ), -1 ); //invalid indices should still work + + BOOST_CHECK_EQUAL( EmptyChain.NextShape( 0 ), -1 ); + BOOST_CHECK_EQUAL( EmptyChain.NextShape( 1 ), -1 ); //invalid indices should still work + BOOST_CHECK_EQUAL( EmptyChain.NextShape( 2 ), -1 ); //invalid indices should still work + BOOST_CHECK_EQUAL( EmptyChain.NextShape( -2 ), -1 ); //invalid indices should still work + + BOOST_CHECK_EQUAL( OnePoint.NextShape( 0 ), -1 ); + BOOST_CHECK_EQUAL( OnePoint.NextShape( -1 ), -1 ); + BOOST_CHECK_EQUAL( OnePoint.NextShape( 1 ), -1 ); //invalid indices should still work + BOOST_CHECK_EQUAL( OnePoint.NextShape( 2 ), -1 ); //invalid indices should still work + BOOST_CHECK_EQUAL( OnePoint.NextShape( -2 ), -1 ); //invalid indices should still work +} + + // Test special case where the last arc in the chain has a shared point with the first arc BOOST_AUTO_TEST_CASE( ArcWrappingToStartSharedPoints ) {