Mirrors
/
kicad
mirror of https://github.com/KiCad/kicad-source-mirror
3
0
Fork 0
Code Releases Activity
Browse Source

Fix SHAPE_LINE_CHAIN::Remove and SHAPE_LINE_CHAIN::RemoveShape

newinvert
Roberto Fernandez Bautista 2 years ago
committed by Alex Shvartzkop
parent
673e23c2e6
commit
effed5dfdf
2 changed files with 161 additions and 24 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 65
      libs/kimath/src/geometry/shape_line_chain.cpp
  2. 120
      qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp

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

@ -821,7 +821,12 @@ void SHAPE_LINE_CHAIN::Replace( int aStartIndex, int aEndIndex, const SHAPE_LINE
void SHAPE_LINE_CHAIN::Remove( int aStartIndex, int aEndIndex )
{
assert( m_shapes.size() == m_points.size() );
wxCHECK( m_shapes.size() == m_points.size(), /*void*/ );
// Unwrap the chain first (correctly handling removing arc at
// end of chain coincident with start)
bool closedState = IsClosed();
SetClosed( false );
if( aEndIndex < 0 )
aEndIndex += PointCount();
@ -829,19 +834,31 @@ void SHAPE_LINE_CHAIN::Remove( int aStartIndex, int aEndIndex )
if( aStartIndex < 0 )
aStartIndex += PointCount();
if( aStartIndex >= PointCount() )
if( aStartIndex >= PointCount() || aEndIndex >= PointCount() || aStartIndex > aEndIndex)
{
SetClosed( closedState );
return;
}
aEndIndex = std::min( aEndIndex, PointCount() - 1 );
// Split arcs at start index and end just after the end index
if( IsPtOnArc( aStartIndex ) )
splitArc( aStartIndex );
// Split arcs, making arcs coincident
if( !IsArcStart( aStartIndex ) && IsPtOnArc( aStartIndex ) )
splitArc( aStartIndex, false );
size_t nextIndex = static_cast<size_t>( aEndIndex ) + 1;
if( IsSharedPt( aStartIndex ) ) // Don't delete the shared point
aStartIndex += 1;
if( IsPtOnArc( nextIndex ) )
splitArc( nextIndex );
if( !IsArcEnd( aEndIndex ) && IsPtOnArc( aEndIndex ) && aEndIndex < PointCount() - 1 )
splitArc( aEndIndex + 1, true );
if( IsSharedPt( aEndIndex ) ) // Don't delete the shared point
aEndIndex -= 1;
if( aStartIndex > aEndIndex )
{
SetClosed( closedState );
return;
}
std::set<size_t> extra_arcs;
auto logArcIdxRemoval = [&]( ssize_t& aShapeIndex )
@ -869,14 +886,16 @@ void SHAPE_LINE_CHAIN::Remove( int aStartIndex, int aEndIndex )
logArcIdxRemoval( m_shapes[i].first ); // Only remove the arc on the first index
// Ensure that m_shapes has been built correctly.
assert( i > aStartIndex || IsSharedPt( i - 1 )
assert( i > aStartIndex || ( IsSharedPt( i - 1 )
? m_shapes[i - 1].second == m_shapes[i].first
: m_shapes[i - 1].first == m_shapes[i].first );
: m_shapes[i - 1].first == m_shapes[i].first ) );
continue;
}
}
alg::run_on_pair( m_shapes[i], logArcIdxRemoval );
else
{
alg::run_on_pair( m_shapes[i], logArcIdxRemoval );
}
}
for( auto arc : extra_arcs )
@ -885,6 +904,8 @@ void SHAPE_LINE_CHAIN::Remove( int aStartIndex, int aEndIndex )
m_shapes.erase( m_shapes.begin() + aStartIndex, m_shapes.begin() + aEndIndex + 1 );
m_points.erase( m_points.begin() + aStartIndex, m_points.begin() + aEndIndex + 1 );
assert( m_shapes.size() == m_points.size() );
SetClosed( closedState );
}
@ -1077,32 +1098,28 @@ void SHAPE_LINE_CHAIN::RemoveShape( int aPointIndex )
if( aPointIndex < 0 )
aPointIndex += PointCount();
if( aPointIndex >= PointCount() || aPointIndex < 0 )
return; // Invalid index, fail gracefully
if( m_shapes[aPointIndex] == SHAPES_ARE_PT )
{
Remove( aPointIndex );
return;
}
//@todo should this be replaced to use NextShape() / PrevShape()?
int start = aPointIndex;
int end = aPointIndex;
int arcIdx = ArcIndex( aPointIndex );
if( !IsSharedPt( aPointIndex ) )
if( !IsArcStart( start ) )
{
// aPointIndex is not a shared point, so iterate backwards to find the start of the arc
while( start >= 0 && m_shapes[start].first == arcIdx )
start--;
// Check if the previous point might be a shared point and decrement 'start' if so
if( start >= 1 && m_shapes[static_cast<ssize_t>( start ) - 1].second == arcIdx )
while( start > 0 && ArcIndex( static_cast<ssize_t>( start ) - 1 ) == arcIdx )
start--;
}
// For the end point we only need to check the first element in m_shapes (the second one is only
// populated if there is an arc after the current one sharing the same point).
while( end < static_cast<int>( m_shapes.size() ) - 1 && m_shapes[end].first == arcIdx )
end++;
if( !IsArcEnd( end ) || start == end )
end = NextShape( end ); // can be -1 to indicate end of chain
Remove( start, end );
}

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

@ -288,6 +288,126 @@ BOOST_AUTO_TEST_CASE( SimplifyDuplicatePoint )
}
struct REMOVE_SHAPE_CASE
{
std::string m_ctx_name;
SHAPE_LINE_CHAIN m_chain;
int m_shape_count;
int m_arc_count;
int m_remove_index;
int m_expected_shape_count;
int m_expected_arc_count;
};
static const std::vector<REMOVE_SHAPE_CASE> remove_shape_cases =
{
{ "Circle1Arc - 1st arc - index on start", SLC_CASES().Circle1Arc, 1, 1, 0, 0, 0 },
{ "Circle1Arc - 1st arc - index on mid", SLC_CASES().Circle1Arc, 1, 1, 8, 0, 0 },
{ "Circle1Arc - 1st arc - index on end", SLC_CASES().Circle1Arc, 1, 1, 14, 0, 0 },
{ "Circle1Arc - 1st arc - index on -1", SLC_CASES().Circle1Arc, 1, 1, -1, 0, 0 },
{ "Circle1Arc - invalid index", SLC_CASES().Circle1Arc, 1, 1, 15, 1, 1 },
{ "Circle2Arcs - 1st arc - index on start", SLC_CASES().Circle2Arcs, 2, 2, 0, 2, 1 },
{ "Circle2Arcs - 1st arc - index on mid", SLC_CASES().Circle2Arcs, 2, 2, 3, 2, 1 },
{ "Circle2Arcs - 1st arc - index on end", SLC_CASES().Circle2Arcs, 2, 2, 7, 2, 1 },
{ "Circle2Arcs - 2nd arc - index on start", SLC_CASES().Circle2Arcs, 2, 2, 8, 2, 1 },
{ "Circle2Arcs - 2nd arc - index on mid", SLC_CASES().Circle2Arcs, 2, 2, 11, 2, 1 },
{ "Circle2Arcs - 2nd arc - index on end", SLC_CASES().Circle2Arcs, 2, 2, 15, 2, 1 },
{ "Circle2Arcs - 2nd arc - index on -1", SLC_CASES().Circle2Arcs, 2, 2, -1, 2, 1 },
{ "Circle2Arcs - invalid index", SLC_CASES().Circle2Arcs, 2, 2, 16, 2, 2 },
{ "ArcsCoinc. - 1st arc - idx on start", SLC_CASES().ArcsCoincident, 2, 2, 0, 1, 1 },
{ "ArcsCoinc. - 1st arc - idx on mid", SLC_CASES().ArcsCoincident, 2, 2, 3, 1, 1 },
{ "ArcsCoinc. - 1st arc - idx on end", SLC_CASES().ArcsCoincident, 2, 2, 7, 1, 1 },
{ "ArcsCoinc. - 2nd arc - idx on start", SLC_CASES().ArcsCoincident, 2, 2, 8, 1, 1 },
{ "ArcsCoinc. - 2nd arc - idx on mid", SLC_CASES().ArcsCoincident, 2, 2, 10, 1, 1 },
{ "ArcsCoinc. - 2nd arc - idx on end", SLC_CASES().ArcsCoincident, 2, 2, 13, 1, 1 },
{ "ArcsCoinc. - 2nd arc - idx on -1", SLC_CASES().ArcsCoincident, 2, 2, -1, 1, 1 },
{ "ArcsCoinc. - invalid idx", SLC_CASES().ArcsCoincident, 2, 2, 14, 2, 2 },
{ "ArcsCoinc. - 1st arc - idx on start", SLC_CASES().ArcsCoincident, 2, 2, 0, 1, 1 },
{ "A.Co.Closed - 1st arc - idx on start", SLC_CASES().ArcsCoincidentClosed, 3, 2, 1, 2, 1 },
{ "A.Co.Closed - 1st arc - idx on mid", SLC_CASES().ArcsCoincidentClosed, 3, 2, 3, 2, 1 },
{ "A.Co.Closed - 1st arc - idx on end", SLC_CASES().ArcsCoincidentClosed, 3, 2, 7, 2, 1 },
{ "A.Co.Closed - 2nd arc - idx on start", SLC_CASES().ArcsCoincidentClosed, 3, 2, 8, 2, 1 },
{ "A.Co.Closed - 2nd arc - idx on mid", SLC_CASES().ArcsCoincidentClosed, 3, 2, 10, 2, 1 },
{ "A.Co.Closed - 2nd arc - idx on end", SLC_CASES().ArcsCoincidentClosed, 3, 2, 13, 2, 1 },
{ "A.Co.Closed - 2nd arc - idx on -1", SLC_CASES().ArcsCoincidentClosed, 3, 2, -1, 2, 1 },
{ "A.Co.Closed - invalid idx", SLC_CASES().ArcsCoincidentClosed, 3, 2, 14, 3, 2 },
{ "ArcsIndep. - 1st arc - idx on start", SLC_CASES().ArcsIndependent, 3, 2, 0, 1, 1 },
{ "ArcsIndep. - 1st arc - idx on mid", SLC_CASES().ArcsIndependent, 3, 2, 3, 1, 1 },
{ "ArcsIndep. - 1st arc - idx on end", SLC_CASES().ArcsIndependent, 3, 2, 8, 1, 1 },
{ "ArcsIndep. - 2nd arc - idx on start", SLC_CASES().ArcsIndependent, 3, 2, 9, 1, 1 },
{ "ArcsIndep. - 2nd arc - idx on mid", SLC_CASES().ArcsIndependent, 3, 2, 12, 1, 1 },
{ "ArcsIndep. - 2nd arc - idx on end", SLC_CASES().ArcsIndependent, 3, 2, 17, 1, 1 },
{ "ArcsIndep. - 2nd arc - idx on -1", SLC_CASES().ArcsIndependent, 3, 2, -1, 1, 1 },
{ "ArcsIndep. - invalid idx", SLC_CASES().ArcsIndependent, 3, 2, 18, 3, 2 },
{ "Dup.Arcs - 1st arc - idx on start", SLC_CASES().DuplicateArcs, 4, 3, 0, 3, 2 },
{ "Dup.Arcs - 1st arc - idx on mid", SLC_CASES().DuplicateArcs, 4, 3, 3, 3, 2 },
{ "Dup.Arcs - 1st arc - idx on end", SLC_CASES().DuplicateArcs, 4, 3, 7, 3, 2 },
{ "Dup.Arcs - 2nd arc - idx on start", SLC_CASES().DuplicateArcs, 4, 3, 8, 3, 2 },
{ "Dup.Arcs - 2nd arc - idx on mid", SLC_CASES().DuplicateArcs, 4, 3, 10, 3, 2 },
{ "Dup.Arcs - 2nd arc - idx on end", SLC_CASES().DuplicateArcs, 4, 3, 13, 3, 2 },
{ "Dup.Arcs - 3rd arc - idx on start", SLC_CASES().DuplicateArcs, 4, 3, 14, 2, 2 },
{ "Dup.Arcs - 3rd arc - idx on mid", SLC_CASES().DuplicateArcs, 4, 3, 17, 2, 2 },
{ "Dup.Arcs - 3rd arc - idx on end", SLC_CASES().DuplicateArcs, 4, 3, 19, 2, 2 },
{ "Dup.Arcs - 3rd arc - idx on -1", SLC_CASES().DuplicateArcs, 4, 3, -1, 2, 2 },
{ "Dup.Arcs - invalid idx", SLC_CASES().DuplicateArcs, 4, 3, 20, 4, 3 },
{ "Arcs Mixed - 1st arc - idx on start", SLC_CASES().ArcsAndSegMixed, 4, 2, 0, 2, 1 },
{ "Arcs Mixed - 1st arc - idx on mid", SLC_CASES().ArcsAndSegMixed, 4, 2, 3, 2, 1 },
{ "Arcs Mixed - 1st arc - idx on end", SLC_CASES().ArcsAndSegMixed, 4, 2, 8, 2, 1 },
{ "Arcs Mixed - Straight segment", SLC_CASES().ArcsAndSegMixed, 4, 2, 9, 3, 2 },
{ "Arcs Mixed - 2nd arc - idx on start", SLC_CASES().ArcsAndSegMixed, 4, 2, 10, 2, 1 },
{ "Arcs Mixed - 2nd arc - idx on mid", SLC_CASES().ArcsAndSegMixed, 4, 2, 14, 2, 1 },
{ "Arcs Mixed - 2nd arc - idx on end", SLC_CASES().ArcsAndSegMixed, 4, 2, 18, 2, 1 },
{ "Arcs Mixed - 2nd arc - idx on -1", SLC_CASES().ArcsAndSegMixed, 4, 2, -1, 2, 1 },
{ "Arcs Mixed - invalid idx", SLC_CASES().ArcsAndSegMixed, 4, 2, 19, 4, 2 }
};
BOOST_AUTO_TEST_CASE( RemoveShape )
{
for( const REMOVE_SHAPE_CASE& c : remove_shape_cases )
{
BOOST_TEST_CONTEXT( c.m_ctx_name )
{
SHAPE_LINE_CHAIN slc_case = c.m_chain; // make a copy to edit
BOOST_CHECK_EQUAL( slc_case.ShapeCount(), c.m_shape_count );
BOOST_CHECK_EQUAL( slc_case.ArcCount(), c.m_arc_count );
BOOST_CHECK_EQUAL( GEOM_TEST::IsOutlineValid( slc_case ), true );
slc_case.RemoveShape( c.m_remove_index );
BOOST_CHECK_EQUAL( slc_case.ShapeCount(), c.m_expected_shape_count );
BOOST_CHECK_EQUAL( slc_case.ArcCount(), c.m_expected_arc_count );
BOOST_CHECK_EQUAL( GEOM_TEST::IsOutlineValid( slc_case ), true );
}
}
}
BOOST_AUTO_TEST_CASE( RemoveShapeAfterSimplify )
{
for( const REMOVE_SHAPE_CASE& c : remove_shape_cases )
{
BOOST_TEST_CONTEXT( c.m_ctx_name )
{
SHAPE_LINE_CHAIN slc_case = c.m_chain; // make a copy to edit
BOOST_CHECK_EQUAL( GEOM_TEST::IsOutlineValid( slc_case ), true );
BOOST_CHECK_EQUAL( slc_case.ShapeCount(), c.m_shape_count );
BOOST_CHECK_EQUAL( slc_case.ArcCount(), c.m_arc_count );
slc_case.Simplify();
BOOST_CHECK_EQUAL( GEOM_TEST::IsOutlineValid( slc_case ), true );
BOOST_CHECK_EQUAL( slc_case.ShapeCount(), c.m_shape_count );
BOOST_CHECK_EQUAL( slc_case.ArcCount(), c.m_arc_count );
slc_case.RemoveShape( c.m_remove_index );
BOOST_CHECK_EQUAL( GEOM_TEST::IsOutlineValid( slc_case ), true );
BOOST_CHECK_EQUAL( slc_case.ShapeCount(), c.m_expected_shape_count );
BOOST_CHECK_EQUAL( slc_case.ArcCount(), c.m_expected_arc_count );
}
}
}
BOOST_AUTO_TEST_CASE( ShapeCount )

Write Preview
Loading…
Cancel
Save