kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #25129
[PATCH] Rewrite line merging without static vector and sort
The static vector is a massive code smell, and as we're only interested in
the most extreme elements, we can simply use std::min and std::max with
custom comparison functions instead.
---
eeschema/sch_line.cpp | 40 +++++++++++++++++++---------------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index 72efb5c..f6a493a 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -270,17 +270,6 @@ void SCH_LINE::Rotate( wxPoint aPosition )
/*
- * helper sort function, used by MergeOverlap
- * sorts ref and test by x values, or (for same x values) by y values
- */
-bool sort_by_ends_position(const wxPoint * ref, const wxPoint * tst )
-{
- if( ref->x == tst->x )
- return ref->y < tst->y;
- return ref->x < tst->x;
-}
-
-/*
* MergeOverlap try to merge 2 lines that are colinear.
* this function expects these 2 lines have at least a common end
*/
@@ -346,16 +335,25 @@ bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
// for horizontal segments the uppermost and the lowest point
if( colinear )
{
- static std::vector <wxPoint*> candidates;
- candidates.clear();
- candidates.push_back( &m_start );
- candidates.push_back( &m_end );
- candidates.push_back( &aLine->m_start );
- candidates.push_back( &aLine->m_end );
- sort( candidates.begin(), candidates.end(), sort_by_ends_position );
- wxPoint tmp = *candidates[3];
- m_start = *candidates[0];
- m_end = tmp;
+ auto less = []( const wxPoint& lhs, const wxPoint& rhs ) -> bool
+ {
+ if( lhs.x == rhs.x )
+ return lhs.y < rhs.y;
+ return lhs.x < rhs.y;
+ };
+
+ auto greater = []( const wxPoint& lhs, const wxPoint& rhs ) -> bool
+ {
+ if( lhs.x == rhs.x )
+ return lhs.y > rhs.y;
+ return lhs.x > rhs.y;
+ };
+
+ wxPoint top_left = std::min( { m_start, m_end, aLine->m_start, aLine->m_end }, less );
+ wxPoint bottom_right = std::max( { m_start, m_end, aLine->m_start, aLine->m_end }, greater );
+
+ m_start = top_left;
+ m_end = bottom_right;
return true;
}
return false;
Follow ups