Simplify the "pick up attached labels" logic.

This should vastly reduce the number of side-effects we were seeing.

Fixes https://gitlab.com/kicad/code/kicad/issues/4311

Fixes https://gitlab.com/kicad/code/kicad/issues/git

Fixes https://gitlab.com/kicad/code/kicad/issues/4320
This commit is contained in:
Jeff Young 2020-05-06 11:53:56 +01:00
parent 7d7e0143e4
commit 0f34fc2e5f
2 changed files with 43 additions and 45 deletions

View File

@ -188,6 +188,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
// Setup a drag or a move // Setup a drag or a move
// //
m_dragAdditions.clear(); m_dragAdditions.clear();
m_specialCaseLabels.clear();
internalPoints.clear(); internalPoints.clear();
@ -509,6 +510,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, wxPoint aPoi
{ {
// Select the connected end of wires/bus connections. // Select the connected end of wires/bus connections.
SCH_LINE* testLine = static_cast<SCH_LINE*>( test ); SCH_LINE* testLine = static_cast<SCH_LINE*>( test );
wxPoint otherEnd;
if( testLine->GetStartPoint() == aPoint ) if( testLine->GetStartPoint() == aPoint )
{ {
@ -516,6 +518,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, wxPoint aPoi
aList.push_back( testLine ); aList.push_back( testLine );
testLine->SetFlags( STARTPOINT | TEMP_SELECTED ); testLine->SetFlags( STARTPOINT | TEMP_SELECTED );
otherEnd = testLine->GetEndPoint();
} }
else if( testLine->GetEndPoint() == aPoint ) else if( testLine->GetEndPoint() == aPoint )
{ {
@ -523,7 +526,31 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, wxPoint aPoi
aList.push_back( testLine ); aList.push_back( testLine );
testLine->SetFlags( ENDPOINT | TEMP_SELECTED ); testLine->SetFlags( ENDPOINT | TEMP_SELECTED );
otherEnd = testLine->GetStartPoint();
} }
// Since only one end is going to move, the movement vector of any labels attached
// to it is scaled by the proportion of the line length the label is from the moving
// end.
for( SCH_ITEM* item : m_frame->GetScreen()->Items().OfType( SCH_LABEL_T ) )
{
if( item->IsSelected() )
continue; // These will be moved on their own because they're selected
if( item->CanConnect( testLine ) && testLine->HitTest( item->GetPosition(), 1 ) )
{
SCH_TEXT* label = static_cast<SCH_TEXT*>( item );
if( !label->HasFlag( TEMP_SELECTED ) )
aList.push_back( label );
double scale = GetLineLength( label->GetPosition(), aPoint ) /
GetLineLength( otherEnd, aPoint );
m_specialCaseLabels[label] = scale;
}
}
break; break;
} }
@ -610,63 +637,18 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, wxPoint aPoi
void SCH_MOVE_TOOL::moveItem( EDA_ITEM* aItem, const VECTOR2I& aDelta ) void SCH_MOVE_TOOL::moveItem( EDA_ITEM* aItem, const VECTOR2I& aDelta )
{ {
std::vector< std::pair<double, SCH_TEXT*> > labels;
auto collectLabels =
[&] ( SCH_LINE* aLine )
{
for( SCH_ITEM* test : m_frame->GetScreen()->Items().OfType( SCH_LABEL_T ) )
{
if( test->IsSelected() )
continue; // These will be moved on their own because they're selected
if( !test->CanConnect( aLine ) || !aLine->HitTest( test->GetPosition(), 1 ) )
continue;
SCH_TEXT* label = static_cast<SCH_TEXT*>( test );
double pct = GetLineLength( label->GetPosition(), aLine->GetStartPoint() ) /
GetLineLength( aLine->GetEndPoint(), aLine->GetStartPoint() );
labels.emplace_back( pct, label );
}
};
auto adjustLabels =
[&] ( SCH_LINE* aLine )
{
for( std::pair<double, SCH_TEXT*> pair : labels )
{
double pct = pair.first;
SCH_TEXT* label = pair.second;
wxPoint lineVector( aLine->GetEndPoint() - aLine->GetStartPoint() );
label->SetPosition( aLine->GetStartPoint() + ( lineVector * pct ) );
m_frame->RefreshItem( label );
}
};
switch( aItem->Type() ) switch( aItem->Type() )
{ {
case SCH_LINE_T: case SCH_LINE_T:
{ {
SCH_LINE* line = static_cast<SCH_LINE*>( aItem ); SCH_LINE* line = static_cast<SCH_LINE*>( aItem );
if( !aItem->IsNew() )
{
if( aItem->HasFlag( STARTPOINT ) || aItem->HasFlag( ENDPOINT ) )
collectLabels( line );
}
if( aItem->HasFlag( STARTPOINT ) ) if( aItem->HasFlag( STARTPOINT ) )
line->MoveStart( (wxPoint) aDelta ); line->MoveStart( (wxPoint) aDelta );
if( aItem->HasFlag( ENDPOINT ) ) if( aItem->HasFlag( ENDPOINT ) )
line->MoveEnd( (wxPoint) aDelta ); line->MoveEnd( (wxPoint) aDelta );
if( !aItem->IsNew() )
{
adjustLabels( line );
}
} }
break; break;
@ -699,6 +681,17 @@ void SCH_MOVE_TOOL::moveItem( EDA_ITEM* aItem, const VECTOR2I& aDelta )
pin->ConstrainOnEdge( pin->GetStoredPos() ); pin->ConstrainOnEdge( pin->GetStoredPos() );
break; break;
} }
case SCH_LABEL_T:
{
SCH_TEXT* label = static_cast<SCH_TEXT*>( aItem );
if( m_specialCaseLabels.count( label ) )
label->Move( (wxPoint) aDelta * m_specialCaseLabels[ label ] );
else
label->Move( (wxPoint) aDelta );
break;
}
default: default:
static_cast<SCH_ITEM*>( aItem )->Move( (wxPoint) aDelta ); static_cast<SCH_ITEM*>( aItem )->Move( (wxPoint) aDelta );
break; break;

View File

@ -76,6 +76,11 @@ private:
VECTOR2I m_cursor; VECTOR2I m_cursor;
boost::optional<VECTOR2I> m_anchorPos; boost::optional<VECTOR2I> m_anchorPos;
// A map of labels to scaling factors. Used to scale the movement vector for labels that
// are attached to wires which have only one end moving.
std::map<SCH_TEXT*, double> m_specialCaseLabels;
}; };
#endif //KICAD_SCH_MOVE_TOOL_H #endif //KICAD_SCH_MOVE_TOOL_H