diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp index f672f50636..0da5598fd9 100644 --- a/eeschema/bus-wire-junction.cpp +++ b/eeschema/bus-wire-junction.cpp @@ -243,7 +243,7 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( SCH_SCREEN* aScreen ) continue; if( !secondLine->IsParallel( firstLine ) - || secondLine->IsStrokeEquivalent( firstLine ) + || !secondLine->IsStrokeEquivalent( firstLine ) || secondLine->GetLayer() != firstLine->GetLayer() ) { continue; @@ -257,15 +257,11 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( SCH_SCREEN* aScreen ) continue; } - // If the end points overlap, check if we still need the junction - if( secondLine->IsEndPoint( firstLine->GetStartPoint() ) ) - needed = aScreen->IsJunctionNeeded( firstLine->GetStartPoint() ); - else if( secondLine->IsEndPoint( firstLine->GetEndPoint() ) ) - needed = aScreen->IsJunctionNeeded( firstLine->GetEndPoint() ); - + // See if we can merge an overlap (or two colinear touching segments with + // no junction where they meet). SCH_LINE* mergedLine = nullptr; - if( !needed && ( mergedLine = secondLine->MergeOverlap( firstLine ) ) ) + if( mergedLine = secondLine->MergeOverlap( aScreen, firstLine, true ) ) { remove_item( firstLine ); remove_item( secondLine ); @@ -426,7 +422,7 @@ void SCH_EDIT_FRAME::DeleteJunction( SCH_ITEM* aJunction, bool aAppend ) } // Try to merge the remaining lines - if( SCH_LINE* line = secondLine->MergeOverlap( firstLine ) ) + if( SCH_LINE* line = secondLine->MergeOverlap( screen, firstLine, false ) ) { remove_item( firstLine ); remove_item( secondLine ); diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp index 69bb94307a..66e6098045 100644 --- a/eeschema/sch_line.cpp +++ b/eeschema/sch_line.cpp @@ -400,12 +400,13 @@ bool SCH_LINE::IsParallel( SCH_LINE* aLine ) } -SCH_LINE* SCH_LINE::MergeOverlap( SCH_LINE* aLine ) +SCH_LINE* SCH_LINE::MergeOverlap( SCH_SCREEN* aScreen, SCH_LINE* aLine, bool aCheckJunctions ) { auto less = []( const wxPoint& lhs, const wxPoint& rhs ) -> bool { if( lhs.x == rhs.x ) return lhs.y < rhs.y; + return lhs.x < rhs.x; }; @@ -427,19 +428,18 @@ SCH_LINE* SCH_LINE::MergeOverlap( SCH_LINE* aLine ) if( rightmost_start != std::min( { rightmost_start, rightmost_end }, less ) ) std::swap( rightmost_start, rightmost_end ); - // -leftmost is the line that starts farthest to the left - // -other is the line that is _not_ leftmost - // -rightmost is the line that ends farthest to the right. This may or - // may not be 'other' as the second line may be completely covered by - // the first. + // - leftmost is the line that starts farthest to the left + // - other is the line that is _not_ leftmost + // - rightmost is the line that ends farthest to the right. This may or may not be 'other' + // as the second line may be completely covered by the first. if( less( rightmost_start, leftmost_start ) ) { std::swap( leftmost_start, rightmost_start ); std::swap( leftmost_end, rightmost_end ); } - auto other_start = rightmost_start; - auto other_end = rightmost_end; + wxPoint other_start = rightmost_start; + wxPoint other_end = rightmost_end; if( less( rightmost_end, leftmost_end ) ) { @@ -482,10 +482,9 @@ SCH_LINE* SCH_LINE::MergeOverlap( SCH_LINE* aLine ) else { // We use long long here to avoid overflow -- it enforces promotion - // Don't use double as we need to make a direct comparison - // The slope of the left-most line is dy/dx. Then we check that the slope - // from the left most start to the right most start is the same as well as - // the slope from the left most start to right most end. + // The slope of the left-most line is dy/dx. Then we check that the slope from the + // left most start to the right most start is the same as well as the slope from the + // left most start to right most end. long long dx = leftmost_end.x - leftmost_start.x; long long dy = leftmost_end.y - leftmost_start.y; colinear = ( ( ( other_start.y - leftmost_start.y ) * dx == @@ -494,22 +493,28 @@ SCH_LINE* SCH_LINE::MergeOverlap( SCH_LINE* aLine ) ( other_end.x - leftmost_start.x ) * dy ) ); } + if( !colinear ) + return nullptr; + + // We either have a true overlap or colinear touching segments. We always want to merge + // the former, but the later only get merged if there no junction at the touch point. + + bool touching = leftmost_end == rightmost_start; + + if( touching && aCheckJunctions && aScreen->IsJunctionNeeded( leftmost_end ) ) + return nullptr; + // Make a new segment that merges the 2 segments - if( colinear ) - { - leftmost_end = rightmost_end; + leftmost_end = rightmost_end; - auto ret = new SCH_LINE( *aLine ); - ret->SetStartPoint( leftmost_start ); - ret->SetEndPoint( leftmost_end ); + auto ret = new SCH_LINE( *aLine ); + ret->SetStartPoint( leftmost_start ); + ret->SetEndPoint( leftmost_end ); - if( IsSelected() || aLine->IsSelected() ) - ret->SetSelected(); + if( IsSelected() || aLine->IsSelected() ) + ret->SetSelected(); - return ret; - } - - return nullptr; + return ret; } diff --git a/eeschema/sch_line.h b/eeschema/sch_line.h index f54f114c3b..acc85334ae 100644 --- a/eeschema/sch_line.h +++ b/eeschema/sch_line.h @@ -185,10 +185,13 @@ public: * two lines overlap. This method is used to merge multiple line segments into a single * line. * + * @param aScreen - the current screen * @param aLine - Line to compare. + * @param aCheckJunctions - indicates we need to check for a junction if the two segments + * are colinear and touch * @return New line that combines the two or NULL on non-overlapping segments. */ - SCH_LINE* MergeOverlap( SCH_LINE* aLine ); + SCH_LINE* MergeOverlap( SCH_SCREEN* aScreen, SCH_LINE* aLine, bool aCheckJunctions ); /** * Check if two lines are in the same quadrant as each other, using a reference point as diff --git a/eeschema/tools/sch_line_wire_bus_tool.cpp b/eeschema/tools/sch_line_wire_bus_tool.cpp index 6cb214640e..2967f9297e 100644 --- a/eeschema/tools/sch_line_wire_bus_tool.cpp +++ b/eeschema/tools/sch_line_wire_bus_tool.cpp @@ -815,15 +815,12 @@ void SCH_LINE_WIRE_BUS_TOOL::simplifyWireList() SCH_LINE* next_line = *next_it; - if( line->IsParallel( next_line ) ) + if( SCH_LINE* merged = line->MergeOverlap( m_frame->GetScreen(), next_line, false ) ) { - if( SCH_LINE* merged = line->MergeOverlap( next_line ) ) - { - delete line; - delete next_line; - it = m_wires.erase( it ); - *it = merged; - } + delete line; + delete next_line; + it = m_wires.erase( it ); + *it = merged; } ++it;