From 7e0ba42f0eb719881e1c12aae547de1c32fb9adf Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Fri, 25 Nov 2022 15:04:36 +0000 Subject: [PATCH] Bug fixes for dragging labels attached to wires in orthogonal mode. Fixes https://gitlab.com/kicad/code/kicad/issues/12677 --- eeschema/bus-wire-junction.cpp | 10 ++- eeschema/sch_line.h | 8 ++- eeschema/tools/sch_move_tool.cpp | 110 +++++++++++++++++++++---------- eeschema/tools/sch_move_tool.h | 3 +- 4 files changed, 92 insertions(+), 39 deletions(-) diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp index 0f474cd0d3..eb76b29d36 100644 --- a/eeschema/bus-wire-junction.cpp +++ b/eeschema/bus-wire-junction.cpp @@ -159,9 +159,13 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( SCH_SCREEN* aScreen ) auto remove_item = [&]( SCH_ITEM* aItem ) -> void { changed = true; - aItem->SetFlags( STRUCT_DELETED ); - itemList.PushItem( ITEM_PICKER( aScreen, aItem, UNDO_REDO::DELETED ) ); - deletedItems.push_back( aItem ); + + if( !( aItem->GetFlags() & STRUCT_DELETED ) ) + { + aItem->SetFlags( STRUCT_DELETED ); + itemList.PushItem( ITEM_PICKER( aScreen, aItem, UNDO_REDO::DELETED ) ); + deletedItems.push_back( aItem ); + } }; BreakSegmentsOnJunctions( aScreen ); diff --git a/eeschema/sch_line.h b/eeschema/sch_line.h index ed599c423d..4f78821444 100644 --- a/eeschema/sch_line.h +++ b/eeschema/sch_line.h @@ -111,7 +111,13 @@ public: * Saves the current line angle. Useful when dragging a line and its important to * be able to restart the line from length 0 in the correct direction. */ - inline void StoreAngle() { m_storedAngle = Angle(); } + inline void StoreAngle() + { + if( !IsNull() ) + m_storedAngle = Angle(); + } + + inline void StoreAngle( const EDA_ANGLE& aAngle ) { m_storedAngle = aAngle; } /** * Returns the angle stored by StoreAngle() diff --git a/eeschema/tools/sch_move_tool.cpp b/eeschema/tools/sch_move_tool.cpp index d452c4a5fb..8c39a02a0a 100644 --- a/eeschema/tools/sch_move_tool.cpp +++ b/eeschema/tools/sch_move_tool.cpp @@ -29,13 +29,13 @@ #include #include #include -#include #include #include #include #include #include #include +#include #include #include #include @@ -492,7 +492,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) connections = item->GetConnectionPoints(); for( VECTOR2I point : connections ) - getConnectedDragItems( item, point, connectedDragItems ); + getConnectedDragItems( item, point, connectedDragItems, appendUndo ); } for( EDA_ITEM* item : connectedDragItems ) @@ -1053,7 +1053,7 @@ void SCH_MOVE_TOOL::getConnectedItems( SCH_ITEM* aOriginalItem, const VECTOR2I& void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aSelectedItem, const VECTOR2I& aPoint, - EDA_ITEMS& aList ) + EDA_ITEMS& aList, bool& aAppendUndo ) { EE_RTREE& items = m_frame->GetScreen()->Items(); EE_RTREE::EE_TYPE itemsOverlappingRTree = items.Overlapping( aSelectedItem->GetBoundingBox() ); @@ -1061,28 +1061,40 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aSelectedItem, const VECTOR bool ptHasUnselectedJunction = false; auto makeNewWire = - [&]( SCH_ITEM* fixed, SCH_ITEM* selected ) + [&]( SCH_ITEM* fixed, SCH_ITEM* selected, const VECTOR2I& start, const VECTOR2I& end ) { SCH_LINE* newWire; // Add a new newWire between the fixed item and the selected item so the selected // item can be dragged. if( fixed->GetLayer() == LAYER_BUS_JUNCTION || selected->GetLayer() == LAYER_BUS ) - newWire = new SCH_LINE( aPoint, LAYER_BUS ); + newWire = new SCH_LINE( start, LAYER_BUS ); else - newWire = new SCH_LINE( aPoint, LAYER_WIRE ); + newWire = new SCH_LINE( start, LAYER_WIRE ); - newWire->SetFlags(IS_NEW ); + newWire->SetFlags( IS_NEW ); newWire->SetConnectivityDirty( true ); newWire->SetLastResolvedState( selected ); + + newWire->SetEndPoint( end ); m_frame->AddToScreen( newWire, m_frame->GetScreen() ); - newWire->SetFlags(SELECTED_BY_DRAG | STARTPOINT ); - aList.push_back( newWire ); - return newWire; }; + auto makeNewJunction = + [&]( SCH_LINE* line, const VECTOR2I pt ) + { + SCH_JUNCTION* junction = new SCH_JUNCTION( pt ); + junction->SetFlags( IS_NEW ); + junction->SetConnectivityDirty( true ); + junction->SetLastResolvedState( line ); + + m_frame->AddToScreen( junction, m_frame->GetScreen() ); + + return junction; + }; + for( SCH_ITEM* item : itemsOverlappingRTree ) { // Skip ourselves, skip already selected items (but not lines, they need both ends tested) @@ -1165,17 +1177,40 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aSelectedItem, const VECTOR // selected they'll move together if( line->HitTest( aPoint, 1 ) && !line->HasFlag( SELECTED ) ) { - newWire = makeNewWire( line, aSelectedItem ); + newWire = makeNewWire( line, aSelectedItem, aPoint, aPoint ); + newWire->SetFlags( SELECTED_BY_DRAG | STARTPOINT ); + newWire->StoreAngle( ( line->Angle() + ANGLE_90 ).Normalize() ); + aList.push_back( newWire ); if( aPoint != line->GetStartPoint() && aPoint != line->GetEndPoint() ) { - newWire->SetEndPoint( line->GetEndPoint() ); - line->SetEndPoint( aPoint ); - } + // Split line in half + if( !line->IsNew() ) + { + saveCopyInUndoList( line, UNDO_REDO::CHANGED, aAppendUndo ); + aAppendUndo = true; + } - // We need to add a connection reference here because the normal algorithm - // won't find a new line with a point in the middle of an existing line - m_lineConnectionCache[newWire] = { line }; + VECTOR2I oldEnd = line->GetEndPoint(); + line->SetEndPoint( aPoint ); + + SCH_LINE* secondHalf = makeNewWire( line, line, aPoint, oldEnd ); + SCH_JUNCTION* junction = makeNewJunction( line, aPoint ); + + saveCopyInUndoList( secondHalf, UNDO_REDO::NEWITEM, aAppendUndo ); + aAppendUndo = true; + + saveCopyInUndoList( junction, UNDO_REDO::NEWITEM, aAppendUndo ); + aAppendUndo = true; + + m_frame->AddToScreen( secondHalf, m_frame->GetScreen() ); + m_frame->AddToScreen( junction, m_frame->GetScreen() ); + } + else + { + m_lineConnectionCache[ newWire ] = { line }; + m_lineConnectionCache[ line ] = { newWire }; + } } break; @@ -1227,7 +1262,9 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aSelectedItem, const VECTOR { // Add a new wire between the sheetpin and the selected item so the // selected item can be dragged. - newWire = makeNewWire( pin, aSelectedItem ); + newWire = makeNewWire( pin, aSelectedItem, aPoint, aPoint ); + newWire->SetFlags( SELECTED_BY_DRAG | STARTPOINT ); + aList.push_back( newWire ); } } } @@ -1240,7 +1277,9 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aSelectedItem, const VECTOR { // Add a new wire between the symbol or junction and the selected item so // the selected item can be dragged. - newWire = makeNewWire( test, aSelectedItem ); + newWire = makeNewWire( test, aSelectedItem, aPoint, aPoint ); + newWire->SetFlags( SELECTED_BY_DRAG | STARTPOINT ); + aList.push_back( newWire ); } break; @@ -1301,7 +1340,9 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aSelectedItem, const VECTOR { // Add a new wire between the label and the selected item so the selected item // can be dragged. - newWire = makeNewWire( test, aSelectedItem ); + newWire = makeNewWire( test, aSelectedItem, aPoint, aPoint ); + newWire->SetFlags( SELECTED_BY_DRAG | STARTPOINT ); + aList.push_back( newWire ); } break; @@ -1341,7 +1382,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aSelectedItem, const VECTOR else otherEnd = ends[0]; - getConnectedDragItems( test, otherEnd, aList ); + getConnectedDragItems( test, otherEnd, aList, aAppendUndo ); // No need to test the other end of the bus entry break; @@ -1442,13 +1483,13 @@ int SCH_MOVE_TOOL::AlignElements( const TOOL_EVENT& aEvent ) { EE_GRID_HELPER grid( m_toolMgr); EE_SELECTION& selection = m_selectionTool->RequestSelection( EE_COLLECTOR::MovableItems ); - bool append_undo = false; + bool appendUndo = false; auto doMoveItem = [&]( EDA_ITEM* item, const VECTOR2I& delta ) { - saveCopyInUndoList( item, UNDO_REDO::CHANGED, append_undo ); - append_undo = true; + saveCopyInUndoList( item, UNDO_REDO::CHANGED, appendUndo ); + appendUndo = true; moveItem( item, delta ); item->ClearFlags( IS_MOVING ); @@ -1476,8 +1517,8 @@ int SCH_MOVE_TOOL::AlignElements( const TOOL_EVENT& aEvent ) { if( item->Type() == SCH_LINE_T ) { - SCH_LINE* line = static_cast( item ); - std::vector flags{ STARTPOINT, ENDPOINT }; + SCH_LINE* line = static_cast( item ); + std::vector flags{ STARTPOINT, ENDPOINT }; std::vector pts{ line->GetStartPoint(), line->GetEndPoint() }; for( int ii = 0; ii < 2; ++ii ) @@ -1486,7 +1527,7 @@ int SCH_MOVE_TOOL::AlignElements( const TOOL_EVENT& aEvent ) line->ClearFlags(); line->SetFlags( SELECTED ); line->SetFlags( flags[ii] ); - getConnectedDragItems( line, pts[ii], drag_items ); + getConnectedDragItems( line, pts[ii], drag_items, appendUndo ); std::set unique_items( drag_items.begin(), drag_items.end() ); VECTOR2I gridpt = grid.AlignGrid( pts[ii] ) - pts[ii]; @@ -1538,7 +1579,8 @@ int SCH_MOVE_TOOL::AlignElements( const TOOL_EVENT& aEvent ) if( gridpt != VECTOR2I( 0, 0 ) ) { EDA_ITEMS drag_items; - getConnectedDragItems( pin, pin->GetConnectionPoints()[0], drag_items ); + getConnectedDragItems( pin, pin->GetConnectionPoints()[0], drag_items, + appendUndo ); doMoveItem( pin, gridpt ); @@ -1555,16 +1597,16 @@ int SCH_MOVE_TOOL::AlignElements( const TOOL_EVENT& aEvent ) } else { - std::vector connections; - EDA_ITEMS drag_items{ item }; - connections = static_cast( item )->GetConnectionPoints(); + SCH_ITEM* schItem = static_cast( item ); + std::vector connections = schItem->GetConnectionPoints(); + EDA_ITEMS drag_items{ item }; for( const VECTOR2I& point : connections ) - getConnectedDragItems( static_cast( item ), point, drag_items ); + getConnectedDragItems( schItem, point, drag_items, appendUndo ); std::map shifts; - VECTOR2I most_common( 0, 0 ); - int max_count = 0; + VECTOR2I most_common( 0, 0 ); + int max_count = 0; for( const VECTOR2I& conn : connections ) { diff --git a/eeschema/tools/sch_move_tool.h b/eeschema/tools/sch_move_tool.h index e8a4e55903..5f58e0ec4b 100644 --- a/eeschema/tools/sch_move_tool.h +++ b/eeschema/tools/sch_move_tool.h @@ -69,7 +69,8 @@ private: ///< Connected items with no wire are included (as there is no wire to adjust for the drag). ///< Connected wires are included with any un-connected ends flagged (STARTPOINT or ENDPOINT). void getConnectedItems( SCH_ITEM* aOriginalItem, const VECTOR2I& aPoint, EDA_ITEMS& aList ); - void getConnectedDragItems( SCH_ITEM* fixed, const VECTOR2I& selected, EDA_ITEMS& aList ); + void getConnectedDragItems( SCH_ITEM* fixed, const VECTOR2I& selected, EDA_ITEMS& aList, + bool& aAppendUndo ); void orthoLineDrag( SCH_LINE* line, const VECTOR2I& splitDelta, int& xBendCount, int& yBendCount, const EE_GRID_HELPER& grid );