haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

SHAPE_LINE_CHAIN: Fix SetClosed() logic 

SetClosed should unwrap the chain when there are coincident points at the
start.
This commit is contained in:
Roberto Fernandez Bautista 2024-01-07 10:46:41 +01:00 committed by Alex Shvartzkop
parent 7593863245
commit 34942290a2
3 changed files with 97 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
libs/kimath/src/geometry/shape_line_chain.cpp
View File

@ -215,14 +215,15 @@ void SHAPE_LINE_CHAIN::fixIndicesRotation()
{
wxCHECK( m_shapes.size() == m_points.size(), /*void*/ );
if( m_shapes.size() <= 1 || m_arcs.size() <= 1 )
if( m_shapes.size() <= 1 )
return;
size_t rotations = 0;
size_t numPoints = m_points.size();
while( ArcIndex( 0 ) != SHAPE_IS_PT
&& ArcIndex( 0 ) == ArcIndex( numPoints - 1 ) )
&& ArcIndex( 0 ) == ArcIndex( numPoints - 1 )
&& !IsSharedPt( 0 ) )
{
// Rotate right
std::rotate( m_points.rbegin(), m_points.rbegin() + 1, m_points.rend() );
@ -253,6 +254,17 @@ void SHAPE_LINE_CHAIN::mergeFirstLastPointIfNeeded()
fixIndicesRotation();
}
}
else
{
if( m_points.size() > 1 && IsSharedPt( 0 ) )
{
// Create a duplicate point at the end
m_points.push_back( m_points.front() );
m_shapes.push_back( { m_shapes.front().first, SHAPE_IS_PT });
m_shapes.front().first = m_shapes.front().second;
m_shapes.front().second = SHAPE_IS_PT;
}
}
}

21
qa/tests/libs/kimath/geometry/geom_test_utils.h
View File

@ -232,6 +232,9 @@ inline bool IsOutlineValid( const SHAPE_LINE_CHAIN& aChain )
ssize_t prevArcIdx = -1;
std::set<size_t> testedArcs;
if( aChain.PointCount() > 0 && !aChain.IsClosed() && aChain.IsSharedPt( 0 ) )
return false; //can't have first point being shared on an open chain
for( int i = 0; i < aChain.PointCount(); i++ )
{
ssize_t arcIdx = aChain.ArcIndex( i );
@ -286,6 +289,24 @@ inline bool IsOutlineValid( const SHAPE_LINE_CHAIN& aChain )
prevArcIdx = arcIdx;
}
// Make sure last arc point matches the end of the arc
if( prevArcIdx >= 0 )
{
if( aChain.IsClosed() && aChain.IsSharedPt( 0 ) )
{
if( aChain.CShapes()[0].first != prevArcIdx )
return false;
if( aChain.Arc( prevArcIdx ).GetP1() != aChain.CPoint( 0 ) )
return false;
}
else
{
if( aChain.Arc( prevArcIdx ).GetP1() != aChain.CPoint( -1 ) )
return false;
}
}
return true;
}

63
qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp
View File

@ -183,7 +183,7 @@ BOOST_AUTO_TEST_CASE( ArcToPolylineLargeCoords )
BOOST_CHECK_EQUAL( base_chain.Area(), areaPriorToArcRemoval ); // Area should not have changed
}
// Test that duplicate point gets removed when line is set to be closed
// Test that duplicate point gets removed when line is set to be closed and added where required
BOOST_AUTO_TEST_CASE( SetClosedDuplicatePoint )
{
// Test from issue #9843
@ -205,8 +205,69 @@ BOOST_AUTO_TEST_CASE( SetClosedDuplicatePoint )
BOOST_CHECK_EQUAL( chain.CPoints().size(), chain.CShapes().size() );
BOOST_CHECK_EQUAL( chain.PointCount(), 30 ); // (-1) should have removed coincident points
BOOST_CHECK( GEOM_TEST::IsOutlineValid( chain ) );
// Special case: arc wrapping around to start (e.g. circle)
BOOST_CHECK( GEOM_TEST::IsOutlineValid( Circle2Arcs ) );
BOOST_CHECK_EQUAL( Circle2Arcs.IsClosed(), true );
BOOST_CHECK_EQUAL( Circle2Arcs.PointCount(), 16 );
BOOST_CHECK_EQUAL( Circle2Arcs.IsArcSegment( 15 ), true );
BOOST_CHECK_EQUAL( Circle2Arcs.ShapeCount(), 2 );
Circle2Arcs.SetClosed( false );
BOOST_CHECK( GEOM_TEST::IsOutlineValid( Circle2Arcs ) );
BOOST_CHECK_EQUAL( Circle2Arcs.IsClosed(), false );
BOOST_CHECK_EQUAL( Circle2Arcs.PointCount(), 17 );
BOOST_CHECK_EQUAL( Circle2Arcs.IsArcSegment( 15 ), true );
BOOST_CHECK_EQUAL( Circle2Arcs.IsArcSegment( 16 ), false ); // last point doesn't join up
}
struct CLOSE_TOGGLE_SHAPE_CASE
{
std::string m_ctx_name;
SHAPE_LINE_CHAIN m_chain;
bool m_closed;
int m_shape_count;
int m_point_count;
int m_expected_shape_count;
int m_expected_point_count;
};
static const std::vector<CLOSE_TOGGLE_SHAPE_CASE> close_toggle_shape_cases =
{
{ "Circle1Arc", SLC_CASES().Circle1Arc, true, 1, 15, 1, 16 },
{ "Circle2Arcs", SLC_CASES().Circle2Arcs, true, 2, 16, 2, 17 },
{ "ArcsCoincident", SLC_CASES().ArcsCoincident, false, 2, 14, 3, 14 },
{ "ArcsCoincidentClosed", SLC_CASES().ArcsCoincidentClosed, true, 3, 14, 2, 14 },
{ "DuplicateArcs", SLC_CASES().DuplicateArcs, false, 4, 20, 5, 20 },
{ "ArcAndPoint", SLC_CASES().ArcAndPoint, false, 2, 10, 3, 10 },
{ "ArcsAndSegMixed", SLC_CASES().ArcsAndSegMixed, false, 4, 19, 5, 19 }
};
BOOST_AUTO_TEST_CASE( ToggleClosed )
{
for( const CLOSE_TOGGLE_SHAPE_CASE& c : close_toggle_shape_cases )
{
BOOST_TEST_CONTEXT( c.m_ctx_name )
{
SHAPE_LINE_CHAIN slc_case = c.m_chain; // make a copy to edit
BOOST_CHECK( GEOM_TEST::IsOutlineValid( slc_case ) );
BOOST_CHECK_EQUAL( slc_case.IsClosed(), c.m_closed );
BOOST_CHECK_EQUAL( slc_case.ShapeCount(), c.m_shape_count );
BOOST_CHECK_EQUAL( slc_case.PointCount(), c.m_point_count );
slc_case.SetClosed( !c.m_closed );
BOOST_CHECK( GEOM_TEST::IsOutlineValid( slc_case ) );
BOOST_CHECK_EQUAL( slc_case.IsClosed(), !c.m_closed );
BOOST_CHECK_EQUAL( slc_case.ShapeCount(), c.m_expected_shape_count );
BOOST_CHECK_EQUAL( slc_case.PointCount(), c.m_expected_point_count );
slc_case.SetClosed( c.m_closed ); // toggle back to normal
BOOST_CHECK( GEOM_TEST::IsOutlineValid( slc_case ) );
BOOST_CHECK_EQUAL( slc_case.IsClosed(), c.m_closed );
BOOST_CHECK_EQUAL( slc_case.ShapeCount(), c.m_shape_count );
BOOST_CHECK_EQUAL( slc_case.PointCount(), c.m_point_count );
}
}
}
// Test that duplicate point gets removed when we call simplify
BOOST_AUTO_TEST_CASE( SimplifyDuplicatePoint )
{