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

Fix a corner case of wire merging. 

We want to look for a junction before merging two touching colinear
segments, but the old code would also check for a junction when
merging two overlapping segments, which is not what you want.

Fixes https://gitlab.com/kicad/code/kicad/issues/5960
This commit is contained in:
Jeff Young 2020-10-21 18:41:44 +01:00
parent 339fa5e0e3
commit 176f461b2c
4 changed files with 43 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
eeschema/bus-wire-junction.cpp
View File

@ -243,7 +243,7 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( SCH_SCREEN* aScreen )
continue; continue;
if( !secondLine->IsParallel( firstLine ) if( !secondLine->IsParallel( firstLine )
|| secondLine->IsStrokeEquivalent( firstLine ) || !secondLine->IsStrokeEquivalent( firstLine )
|| secondLine->GetLayer() != firstLine->GetLayer() ) || secondLine->GetLayer() != firstLine->GetLayer() )
{ {
continue; continue;
@ -257,15 +257,11 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( SCH_SCREEN* aScreen )
continue; continue;
} }
// If the end points overlap, check if we still need the junction // See if we can merge an overlap (or two colinear touching segments with
if( secondLine->IsEndPoint( firstLine->GetStartPoint() ) ) // no junction where they meet).
needed = aScreen->IsJunctionNeeded( firstLine->GetStartPoint() );
else if( secondLine->IsEndPoint( firstLine->GetEndPoint() ) )
needed = aScreen->IsJunctionNeeded( firstLine->GetEndPoint() );
SCH_LINE* mergedLine = nullptr; SCH_LINE* mergedLine = nullptr;
if( !needed && ( mergedLine = secondLine->MergeOverlap( firstLine ) ) ) if( mergedLine = secondLine->MergeOverlap( aScreen, firstLine, true ) )
{ {
remove_item( firstLine ); remove_item( firstLine );
remove_item( secondLine ); remove_item( secondLine );
@ -426,7 +422,7 @@ void SCH_EDIT_FRAME::DeleteJunction( SCH_ITEM* aJunction, bool aAppend )
} }
// Try to merge the remaining lines // 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( firstLine );
remove_item( secondLine ); remove_item( secondLine );

53
eeschema/sch_line.cpp
View File

@ -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 auto less = []( const wxPoint& lhs, const wxPoint& rhs ) -> bool
{ {
if( lhs.x == rhs.x ) if( lhs.x == rhs.x )
return lhs.y < rhs.y; return lhs.y < rhs.y;
return lhs.x < rhs.x; 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 ) ) if( rightmost_start != std::min( { rightmost_start, rightmost_end }, less ) )
std::swap( rightmost_start, rightmost_end ); std::swap( rightmost_start, rightmost_end );
// -leftmost is the line that starts farthest to the left // - leftmost is the line that starts farthest to the left
// -other is the line that is _not_ leftmost // - other is the line that is _not_ leftmost
// -rightmost is the line that ends farthest to the right. This may or // - rightmost is the line that ends farthest to the right. This may or may not be 'other'
// may not be 'other' as the second line may be completely covered by // as the second line may be completely covered by the first.
// the first.
if( less( rightmost_start, leftmost_start ) ) if( less( rightmost_start, leftmost_start ) )
{ {
std::swap( leftmost_start, rightmost_start ); std::swap( leftmost_start, rightmost_start );
std::swap( leftmost_end, rightmost_end ); std::swap( leftmost_end, rightmost_end );
} }
auto other_start = rightmost_start; wxPoint other_start = rightmost_start;
auto other_end = rightmost_end; wxPoint other_end = rightmost_end;
if( less( rightmost_end, leftmost_end ) ) if( less( rightmost_end, leftmost_end ) )
{ {
@ -482,10 +482,9 @@ SCH_LINE* SCH_LINE::MergeOverlap( SCH_LINE* aLine )
else else
{ {
// We use long long here to avoid overflow -- it enforces promotion // 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
// The slope of the left-most line is dy/dx. Then we check that the slope // left most start to the right most start is the same as well as the slope from the
// from the left most start to the right most start is the same as well as // left most start to right most end.
// the slope from the left most start to right most end.
long long dx = leftmost_end.x - leftmost_start.x; long long dx = leftmost_end.x - leftmost_start.x;
long long dy = leftmost_end.y - leftmost_start.y; long long dy = leftmost_end.y - leftmost_start.y;
colinear = ( ( ( other_start.y - leftmost_start.y ) * dx == 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 ) ); ( 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 // 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 ); auto ret = new SCH_LINE( *aLine );
ret->SetStartPoint( leftmost_start ); ret->SetStartPoint( leftmost_start );
ret->SetEndPoint( leftmost_end ); ret->SetEndPoint( leftmost_end );
if( IsSelected() || aLine->IsSelected() ) if( IsSelected() || aLine->IsSelected() )
ret->SetSelected(); ret->SetSelected();
return ret; return ret;
}
return nullptr;
} }

5
eeschema/sch_line.h
View File

@ -185,10 +185,13 @@ public:
* two lines overlap. This method is used to merge multiple line segments into a single * two lines overlap. This method is used to merge multiple line segments into a single
* line. * line.
* *
* @param aScreen - the current screen
* @param aLine - Line to compare. * @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. * @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 * Check if two lines are in the same quadrant as each other, using a reference point as

13
eeschema/tools/sch_line_wire_bus_tool.cpp
View File

@ -815,15 +815,12 @@ void SCH_LINE_WIRE_BUS_TOOL::simplifyWireList()
SCH_LINE* next_line = *next_it; 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;
delete line; it = m_wires.erase( it );
delete next_line; *it = merged;
it = m_wires.erase( it );
*it = merged;
}
} }
++it; ++it;