From 0f34fc2e5f3acac160cb7204a63a87a35985fda0 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Wed, 6 May 2020 11:53:56 +0100
Subject: [PATCH] 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
---
 eeschema/tools/sch_move_tool.cpp | 83 +++++++++++++++-----------------
 eeschema/tools/sch_move_tool.h   |  5 ++
 2 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/eeschema/tools/sch_move_tool.cpp b/eeschema/tools/sch_move_tool.cpp
index 054fdec67c..f129a2e959 100644
--- a/eeschema/tools/sch_move_tool.cpp
+++ b/eeschema/tools/sch_move_tool.cpp
@@ -188,6 +188,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
                 // Setup a drag or a move
                 //
                 m_dragAdditions.clear();
+                m_specialCaseLabels.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.
             SCH_LINE* testLine = static_cast<SCH_LINE*>( test );
+            wxPoint   otherEnd;
 
             if( testLine->GetStartPoint() == aPoint )
             {
@@ -516,6 +518,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, wxPoint aPoi
                     aList.push_back( testLine );
 
                 testLine->SetFlags( STARTPOINT | TEMP_SELECTED );
+                otherEnd = testLine->GetEndPoint();
             }
             else if( testLine->GetEndPoint() == aPoint )
             {
@@ -523,7 +526,31 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, wxPoint aPoi
                     aList.push_back( testLine );
 
                 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;
         }
 
@@ -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 )
 {
-    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() )
     {
     case SCH_LINE_T:
     {
         SCH_LINE* line = static_cast<SCH_LINE*>( aItem );
 
-        if( !aItem->IsNew() )
-        {
-            if( aItem->HasFlag( STARTPOINT ) || aItem->HasFlag( ENDPOINT ) )
-                collectLabels( line );
-        }
-
         if( aItem->HasFlag( STARTPOINT ) )
             line->MoveStart( (wxPoint) aDelta );
 
         if( aItem->HasFlag( ENDPOINT ) )
             line->MoveEnd( (wxPoint) aDelta );
 
-        if( !aItem->IsNew() )
-        {
-            adjustLabels( line );
-        }
     }
         break;
 
@@ -699,6 +681,17 @@ void SCH_MOVE_TOOL::moveItem( EDA_ITEM* aItem, const VECTOR2I& aDelta )
         pin->ConstrainOnEdge( pin->GetStoredPos() );
         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:
         static_cast<SCH_ITEM*>( aItem )->Move( (wxPoint) aDelta );
         break;
diff --git a/eeschema/tools/sch_move_tool.h b/eeschema/tools/sch_move_tool.h
index ce32df7f14..ad250457bb 100644
--- a/eeschema/tools/sch_move_tool.h
+++ b/eeschema/tools/sch_move_tool.h
@@ -76,6 +76,11 @@ private:
     VECTOR2I              m_cursor;
 
     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