kicad-developers team mailing list archive

[Simon Richter <Simon.Richter@xxxxxxxxxx>]

Hi Wayne,

On 27.06.2016 20:21, Wayne Stambaugh wrote:

> This patch and 0014-Reserve-appropriate-space-in-local-vector.patch have
> been committed to the product branch r6950.

I have an even better implementation by now, using std::min and
std::max, that I posted a few days later. That patch, rebased onto the
current state, is attached.

Sorry for the extra work. :/

    Simon

commit e07a906e90e461c77fbef21c2100d297fe73b85f
Author: Simon Richter <Simon.Richter@xxxxxxxxxx>
Date:   Mon Jun 20 03:07:52 2016 +0200

    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.

diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index c70ff12..e044927 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,18 @@ bool SCH_LINE::MergeOverlap( SCH_LINE* aLine )
     // for horizontal segments the uppermost and the lowest point
     if( colinear )
     {
-        std::vector <wxPoint*> candidates;
-        candidates.reserve( 4 );
-        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.x;
+        };
+
+        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 }, less );
+
+        m_start = top_left;
+        m_end = bottom_right;
         return true;
     }
     return false;

Attachment: signature.asc
Description: OpenPGP digital signature