From 673e23c2e6f50f71b1e261ff40469595cd8c28eb Mon Sep 17 00:00:00 2001 From: Roberto Fernandez Bautista Date: Sun, 7 Jan 2024 10:47:47 +0100 Subject: [PATCH] Fix and simplify SHAPE_LINE_CHAIN::Append( SHAPE_ARC ) + QA tests --- libs/kimath/src/geometry/shape_arc.cpp | 6 +- libs/kimath/src/geometry/shape_line_chain.cpp | 17 +--- .../kimath/geometry/test_shape_line_chain.cpp | 89 +++++++++++++++++++ 3 files changed, 98 insertions(+), 14 deletions(-) diff --git a/libs/kimath/src/geometry/shape_arc.cpp b/libs/kimath/src/geometry/shape_arc.cpp index 1593caa4b3..2257620961 100644 --- a/libs/kimath/src/geometry/shape_arc.cpp +++ b/libs/kimath/src/geometry/shape_arc.cpp @@ -477,6 +477,9 @@ const SHAPE_LINE_CHAIN SHAPE_ARC::ConvertToPolyline( double aAccuracy, VECTOR2I c = GetCenter(); EDA_ANGLE ca = GetCentralAngle(); + SEG startToEnd( GetP0(), GetP1() ); + double halfAccuracy = std::max( 1.0, aAccuracy / 2 ); + int n; // To calculate the arc to segment count, use the external radius instead of the radius. @@ -484,7 +487,8 @@ const SHAPE_LINE_CHAIN SHAPE_ARC::ConvertToPolyline( double aAccuracy, double external_radius = r+(m_width/2); double effectiveAccuracy; - if( external_radius < aAccuracy/2 ) // Should be a very rare case + if( external_radius < halfAccuracy + || startToEnd.Distance( GetArcMid() ) < halfAccuracy ) // Should be a very rare case { // In this case, the arc is approximated by one segment, with a effective error // between -aAccuracy/2 and +aAccuracy/2, as expected. diff --git a/libs/kimath/src/geometry/shape_line_chain.cpp b/libs/kimath/src/geometry/shape_line_chain.cpp index 797ab03df2..dc9c6f0660 100644 --- a/libs/kimath/src/geometry/shape_line_chain.cpp +++ b/libs/kimath/src/geometry/shape_line_chain.cpp @@ -1302,28 +1302,19 @@ void SHAPE_LINE_CHAIN::Append( const SHAPE_ARC& aArc ) void SHAPE_LINE_CHAIN::Append( const SHAPE_ARC& aArc, double aAccuracy ) { - SEG startToEnd( aArc.GetP0(), aArc.GetP1() ); + SHAPE_LINE_CHAIN chain = aArc.ConvertToPolyline( aAccuracy ); - if( startToEnd.Distance( aArc.GetArcMid() ) < 1 ) + if( chain.PointCount() > 2 ) { - // Not really a valid arc. Add as a straight line segment instead - Append( aArc.GetP0() ); - Append( aArc.GetP1() ); - } - else - { - SHAPE_LINE_CHAIN chain = aArc.ConvertToPolyline( aAccuracy ); - - // @todo should the below 4 LOC be moved to SHAPE_ARC::ConvertToPolyline ? chain.m_arcs.push_back( aArc ); chain.m_arcs.back().SetWidth( 0 ); for( auto& sh : chain.m_shapes ) sh.first = 0; - - Append( chain ); } + Append( chain ); + assert( m_shapes.size() == m_points.size() ); } 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 5666a01aeb..dd1794f0de 100644 --- a/qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp +++ b/qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp @@ -346,6 +346,95 @@ BOOST_AUTO_TEST_CASE( NextShape ) } + +BOOST_AUTO_TEST_CASE( AppendArc ) +{ + BOOST_TEST_CONTEXT( "Case 1: Arc mid point nearly collinear" ) + { + SHAPE_ARC arc( VECTOR2I( 100000, 0 ), VECTOR2I( 0, 2499 ), VECTOR2I( -100000, 0 ), 0 ); + SHAPE_LINE_CHAIN chain; + chain.Append( arc, 5000 ); + BOOST_CHECK( GEOM_TEST::IsOutlineValid( chain ) ); + BOOST_CHECK_EQUAL( chain.ArcCount(), 0 ); + BOOST_CHECK_EQUAL( chain.PointCount(), 2 ); + BOOST_CHECK_EQUAL( chain.GetPoint( 0 ), VECTOR2I( 100000, 0 ) ); //arc start + BOOST_CHECK_EQUAL( chain.GetPoint( 1 ), VECTOR2I( -100000, 0 ) ); //arc end + BOOST_CHECK_EQUAL( chain.GetPoint( -1 ), VECTOR2I( -100000, 0 ) ); //arc end + } + + BOOST_TEST_CONTEXT( "Case 2: Arc = Large Circle" ) + { + SHAPE_ARC arc( VECTOR2I( 100000, 0 ), VECTOR2I( 0, 0 ), VECTOR2I( 100000, 0 ), 0 ); + SHAPE_LINE_CHAIN chain; + chain.Append( arc, 5000 ); + BOOST_CHECK( GEOM_TEST::IsOutlineValid( chain ) ); + BOOST_CHECK_EQUAL( chain.ArcCount(), 1 ); + BOOST_CHECK_EQUAL( chain.PointCount(), 10 ); + BOOST_CHECK_EQUAL( chain.GetPoint( 0 ), VECTOR2I( 100000, 0 ) ); //arc start + BOOST_CHECK_EQUAL( chain.GetPoint( 9 ), VECTOR2I( 100000, 0 ) ); //arc end + BOOST_CHECK_EQUAL( chain.GetPoint( -1 ), VECTOR2I( 100000, 0 ) ); //arc end + } + + BOOST_TEST_CONTEXT( "Case 3: Arc = Small Circle (approximate to point)" ) + { + SHAPE_ARC arc( VECTOR2I( 2499, 0 ), VECTOR2I( 0, 0 ), VECTOR2I( 2499, 0 ), 0 ); + SHAPE_LINE_CHAIN chain; + chain.Append( arc, 5000 ); + BOOST_CHECK( GEOM_TEST::IsOutlineValid( chain ) ); + BOOST_CHECK_EQUAL( chain.ArcCount(), 0 ); + BOOST_CHECK_EQUAL( chain.PointCount(), 1 ); + BOOST_CHECK_EQUAL( chain.GetPoint( 0 ), VECTOR2I( 2499, 0 ) ); //arc start + } + + BOOST_TEST_CONTEXT( "Case 3: Small Arc (approximate to segment)" ) + { + SHAPE_ARC arc( VECTOR2I( 1767, 0 ), VECTOR2I( 2499, 2499 ), VECTOR2I( 0, 1767 ), 0 ); + SHAPE_LINE_CHAIN chain; + chain.Append( arc, 5000 ); + BOOST_CHECK( GEOM_TEST::IsOutlineValid( chain ) ); + BOOST_CHECK_EQUAL( chain.ArcCount(), 0 ); + BOOST_CHECK_EQUAL( chain.PointCount(), 2 ); + BOOST_CHECK_EQUAL( chain.GetPoint( 0 ), VECTOR2I( 1767, 0 ) ); //arc start + BOOST_CHECK_EQUAL( chain.GetPoint( 1 ), VECTOR2I( 0, 1767 ) ); //arc end + } + + BOOST_TEST_CONTEXT( "Case 4: Arc = null arc (all points coincident)" ) + { + SHAPE_ARC arc( VECTOR2I( 2499, 0 ), VECTOR2I( 2499, 0 ), VECTOR2I( 2499, 0 ), 0 ); + SHAPE_LINE_CHAIN chain; + chain.Append( arc, 5000 ); + BOOST_CHECK( GEOM_TEST::IsOutlineValid( chain ) ); + BOOST_CHECK_EQUAL( chain.ArcCount(), 0 ); + BOOST_CHECK_EQUAL( chain.PointCount(), 1 ); + BOOST_CHECK_EQUAL( chain.GetPoint( 0 ), VECTOR2I( 2499, 0 ) ); //arc start + } + + BOOST_TEST_CONTEXT( "Case 5: Arc = infinite radius (all points very close)" ) + { + SHAPE_ARC arc( VECTOR2I( 2499, 0 ), VECTOR2I( 2500, 0 ), VECTOR2I( 2501, 0 ), 0 ); + SHAPE_LINE_CHAIN chain; + chain.Append( arc, 5000 ); + BOOST_CHECK( GEOM_TEST::IsOutlineValid( chain ) ); + BOOST_CHECK_EQUAL( chain.ArcCount(), 0 ); + BOOST_CHECK_EQUAL( chain.PointCount(), 2 ); + BOOST_CHECK_EQUAL( chain.GetPoint( 0 ), VECTOR2I( 2499, 0 ) ); //arc start + BOOST_CHECK_EQUAL( chain.GetPoint( 1 ), VECTOR2I( 2501, 0 ) ); //arc end + } + + BOOST_TEST_CONTEXT( "Case 6: Arc = large radius (all points very close)" ) + { + SHAPE_ARC arc( VECTOR2I( -100000, 0 ), VECTOR2I( 0, 1 ), VECTOR2I( 100000, 0 ), 0 ); + SHAPE_LINE_CHAIN chain; + chain.Append( arc, 5000 ); + BOOST_CHECK( GEOM_TEST::IsOutlineValid( chain ) ); + BOOST_CHECK_EQUAL( chain.ArcCount(), 0 ); + BOOST_CHECK_EQUAL( chain.PointCount(), 2 ); + BOOST_CHECK_EQUAL( chain.GetPoint( 0 ), VECTOR2I( -100000, 0 ) ); //arc start + BOOST_CHECK_EQUAL( chain.GetPoint( 1 ), VECTOR2I( 100000, 0 ) ); //arc end + } +} + + // Test special case where the last arc in the chain has a shared point with the first arc BOOST_AUTO_TEST_CASE( ArcWrappingToStartSharedPoints ) {