From e51ab862255ad0dcdd84f19d1c0dd08dd90f86e5 Mon Sep 17 00:00:00 2001 From: Mike Williams Date: Thu, 3 Mar 2022 09:35:57 -0500 Subject: [PATCH] Schematic Drag: fixes and improvements Fixes: * Various special cases around junctions on pins and dragging. * Some rotations of endpoints resulting in a 45 degree rotate. * Some cases where it was possible to get a line with neither endpoint selected, and also substractive unselecting only one of two selected endpoints unselecting both. * Use line midpoint for sorting. Start and endpoints aren't consistent in the order they appear on the X or Y axis. So, we need to use the midpoint for our position for consistent sorting when dragging groups of parallel lines where some have the start and end reversed. Other: * Rename TEMP_SELECTED TO SELECTED_BY_DRAG. This is the actual meaning of the flag, and should reduce confusion as to when it should be used. * Move usage of TEMP_SELECTED as an algorithmic mark to CANDIDATE instead. * Fix mistaken clearing of START_POINT and ENDPOINT. * Move endpoint setting and clearing out of narrowSelection, and into selectPoint and selectMultiple. * Don't show dangling end warnings on new lines --- eeschema/sch_line.h | 1 + eeschema/sch_painter.cpp | 5 + eeschema/tools/ee_point_editor.cpp | 4 +- eeschema/tools/ee_selection_tool.cpp | 143 +++++++++++++++------------ eeschema/tools/ee_selection_tool.h | 7 +- eeschema/tools/ee_tool_base.h | 4 +- eeschema/tools/sch_edit_tool.cpp | 4 +- eeschema/tools/sch_move_tool.cpp | 88 ++++++++++++----- include/eda_item.h | 11 ++- include/eda_item_flags.h | 6 +- include/tool/selection.h | 13 ++- pcbnew/tools/pcb_selection_tool.cpp | 10 +- 12 files changed, 180 insertions(+), 116 deletions(-) diff --git a/eeschema/sch_line.h b/eeschema/sch_line.h index bd1380cacb..05659c2400 100644 --- a/eeschema/sch_line.h +++ b/eeschema/sch_line.h @@ -256,6 +256,7 @@ public: VECTOR2I GetPosition() const override { return m_start; } void SetPosition( const VECTOR2I& aPosition ) override; + VECTOR2I GetSortPosition() const override { return GetMidPoint(); } bool IsPointClickableAnchor( const VECTOR2I& aPos ) const override { diff --git a/eeschema/sch_painter.cpp b/eeschema/sch_painter.cpp index 0b238f8546..2e7696b390 100644 --- a/eeschema/sch_painter.cpp +++ b/eeschema/sch_painter.cpp @@ -1602,6 +1602,11 @@ void SCH_PAINTER::draw( const SCH_LINE *aLine, int aLayer ) if( drawingShadows && !( aLine->IsBrightened() || aLine->IsSelected() ) ) return; + // Line end dangling status isn't updated until the line is finished drawing, + // so don't warn them about ends that are probably connected + if( aLine->IsNew() && drawingDangling ) + return; + COLOR4D color = getRenderColor( aLine, aLine->GetLayer(), drawingShadows ); float width = getLineWidth( aLine, drawingShadows ); PLOT_DASH_TYPE lineStyle = aLine->GetEffectiveLineStyle(); diff --git a/eeschema/tools/ee_point_editor.cpp b/eeschema/tools/ee_point_editor.cpp index 4129142660..53ac2409e9 100644 --- a/eeschema/tools/ee_point_editor.cpp +++ b/eeschema/tools/ee_point_editor.cpp @@ -1010,9 +1010,9 @@ void EE_POINT_EDITOR::updateParentItem() const if( connected.first ) { if( connected.second == STARTPOINT ) - static_cast( connected.first )->SetStartPoint( line->GetPosition() ); + static_cast( connected.first )->SetStartPoint( line->GetStartPoint() ); else if( connected.second == ENDPOINT ) - static_cast( connected.first )->SetEndPoint( line->GetPosition() ); + static_cast( connected.first )->SetEndPoint( line->GetStartPoint() ); updateItem( connected.first, true ); } diff --git a/eeschema/tools/ee_selection_tool.cpp b/eeschema/tools/ee_selection_tool.cpp index cd1fc02b6e..6a789a53cf 100644 --- a/eeschema/tools/ee_selection_tool.cpp +++ b/eeschema/tools/ee_selection_tool.cpp @@ -377,7 +377,7 @@ int EE_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) // Collect items at the clicked location (doesn't select them yet) EE_COLLECTOR collector; CollectHits( collector, evt->Position() ); - narrowSelection( collector, evt->Position(), false, false ); + narrowSelection( collector, evt->Position(), false ); if( collector.GetCount() == 1 && !m_isSymbolEditor && !modifier_enabled ) { @@ -405,7 +405,8 @@ int EE_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) if( !selCancelled ) { - selectPoint( collector, nullptr, nullptr, m_additive, m_subtractive, m_exclusive_or ); + selectPoint( collector, evt->Position(), nullptr, nullptr, m_additive, + m_subtractive, m_exclusive_or ); m_selection.SetIsHover( false ); } } @@ -580,7 +581,7 @@ int EE_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) if( CollectHits( collector, evt->Position() ) ) { - narrowSelection( collector, evt->Position(), false, false ); + narrowSelection( collector, evt->Position(), false ); if( collector.GetCount() == 1 && !modifier_enabled ) { @@ -797,7 +798,7 @@ bool EE_SELECTION_TOOL::CollectHits( EE_COLLECTOR& aCollector, const VECTOR2I& a void EE_SELECTION_TOOL::narrowSelection( EE_COLLECTOR& collector, const VECTOR2I& aWhere, - bool aCheckLocked, bool aSelectPoints ) + bool aCheckLocked ) { for( int i = collector.GetCount() - 1; i >= 0; --i ) { @@ -812,20 +813,6 @@ void EE_SELECTION_TOOL::narrowSelection( EE_COLLECTOR& collector, const VECTOR2I collector.Remove( i ); continue; } - - // SelectPoint, unlike other selection routines, can select line ends - if( aSelectPoints && collector[i]->Type() == SCH_LINE_T ) - { - SCH_LINE* line = (SCH_LINE*) collector[i]; - line->ClearFlags( STARTPOINT | ENDPOINT ); - - if( HitTestPoints( line->GetStartPoint(), aWhere, collector.m_Threshold ) ) - line->SetFlags( STARTPOINT ); - else if( HitTestPoints( line->GetEndPoint(), aWhere, collector.m_Threshold ) ) - line->SetFlags( ENDPOINT ); - else - line->SetFlags( STARTPOINT | ENDPOINT ); - } } // Apply some ugly heuristics to avoid disambiguation menus whenever possible @@ -836,9 +823,9 @@ void EE_SELECTION_TOOL::narrowSelection( EE_COLLECTOR& collector, const VECTOR2I } -bool EE_SELECTION_TOOL::selectPoint( EE_COLLECTOR& aCollector, EDA_ITEM** aItem, - bool* aSelectionCancelledFlag, bool aAdd, bool aSubtract, - bool aExclusiveOr ) +bool EE_SELECTION_TOOL::selectPoint( EE_COLLECTOR& aCollector, const VECTOR2I& aWhere, + EDA_ITEM** aItem, bool* aSelectionCancelledFlag, bool aAdd, + bool aSubtract, bool aExclusiveOr ) { m_selection.ClearReferencePoint(); @@ -873,13 +860,33 @@ bool EE_SELECTION_TOOL::selectPoint( EE_COLLECTOR& aCollector, EDA_ITEM** aItem, { for( int i = 0; i < aCollector.GetCount(); ++i ) { + EDA_ITEM_FLAGS flags = 0; + + // Handle line ends specially + if( aCollector[i]->Type() == SCH_LINE_T ) + { + SCH_LINE* line = (SCH_LINE*) aCollector[i]; + + if( HitTestPoints( line->GetStartPoint(), aWhere, aCollector.m_Threshold ) ) + flags = STARTPOINT; + else if( HitTestPoints( line->GetEndPoint(), aWhere, aCollector.m_Threshold ) ) + flags = ENDPOINT; + else + flags = STARTPOINT | ENDPOINT; + } + if( aSubtract || ( aExclusiveOr && aCollector[i]->IsSelected() ) ) { - unselect( aCollector[i] ); - anySubtracted = true; + aCollector[i]->ClearFlags( flags ); + if( !aCollector[i]->HasFlag( STARTPOINT ) && !aCollector[i]->HasFlag( ENDPOINT ) ) + { + unselect( aCollector[i] ); + anySubtracted = true; + } } else { + aCollector[i]->SetFlags( flags ); select( aCollector[i] ); anyAdded = true; } @@ -915,9 +922,10 @@ bool EE_SELECTION_TOOL::SelectPoint( const VECTOR2I& aWhere, const KICAD_T* aFil if( !CollectHits( collector, aWhere, aFilterList ) ) return false; - narrowSelection( collector, aWhere, aCheckLocked, true ); + narrowSelection( collector, aWhere, aCheckLocked ); - return selectPoint( collector, aItem, aSelectionCancelledFlag, aAdd, aSubtract, aExclusiveOr ); + return selectPoint( collector, aWhere, aItem, aSelectionCancelledFlag, aAdd, aSubtract, + aExclusiveOr ); } @@ -1234,7 +1242,7 @@ bool EE_SELECTION_TOOL::selectMultiple() if( item ) { - item->ClearFlags( TEMP_SELECTED | STARTPOINT | ENDPOINT ); + item->ClearFlags( CANDIDATE ); nearbyItems.push_back( item ); } @@ -1253,49 +1261,53 @@ bool EE_SELECTION_TOOL::selectMultiple() bool anyAdded = false; bool anySubtracted = false; - auto selectItem = - [&]( EDA_ITEM* aItem ) + auto selectItem = [&]( EDA_ITEM* aItem ) + { + EDA_ITEM_FLAGS flags = 0; + + // Handle line ends specially + if( aItem->Type() == SCH_LINE_T ) + { + SCH_LINE* line = (SCH_LINE*) aItem; + + if( selectionRect.Contains( line->GetStartPoint() ) ) + flags |= STARTPOINT; + + if( selectionRect.Contains( line->GetEndPoint() ) ) + flags |= ENDPOINT; + + + // If no ends were selected, select whole line (both ends) + // Also select both ends if the selection overlaps the midpoint + if( ( !( flags & STARTPOINT ) && !( flags & ENDPOINT ) ) + || selectionRect.Contains( line->GetMidPoint() ) ) { - if( m_subtractive || ( m_exclusive_or && aItem->IsSelected() ) ) - { - unselect( aItem ); - anySubtracted = true; - } - else - { - select( aItem ); + flags = STARTPOINT | ENDPOINT; + } + } - // Lines can have just one end selected - if( aItem->Type() == SCH_LINE_T ) - { - SCH_LINE* line = static_cast( aItem ); - - line->ClearFlags( STARTPOINT | ENDPOINT ); - - if( selectionRect.Contains( line->GetStartPoint() ) ) - line->SetFlags( STARTPOINT ); - - if( selectionRect.Contains( line->GetEndPoint() ) ) - line->SetFlags( ENDPOINT ); - - // If no ends were selected, select whole line (both ends) - // Also select both ends if the selection overlaps the midpoint - if( ( !line->HasFlag( STARTPOINT ) && !line->HasFlag( ENDPOINT ) ) - || selectionRect.Contains( line->GetMidPoint() ) ) - { - line->SetFlags( STARTPOINT | ENDPOINT ); - } - } - - anyAdded = true; - } - }; + if( m_subtractive || ( m_exclusive_or && aItem->IsSelected() ) ) + { + aItem->ClearFlags( flags ); + if( !aItem->HasFlag( STARTPOINT ) && !aItem->HasFlag( ENDPOINT ) ) + { + unselect( aItem ); + anySubtracted = true; + } + } + else + { + aItem->SetFlags( flags ); + select( aItem ); + anyAdded = true; + } + }; for( EDA_ITEM* item : nearbyItems ) { if( Selectable( item ) && item->HitTest( selectionRect, isWindowSelection ) ) { - item->SetFlags( TEMP_SELECTED ); + item->SetFlags( CANDIDATE ); selectItem( item ); } } @@ -1303,7 +1315,7 @@ bool EE_SELECTION_TOOL::selectMultiple() for( EDA_ITEM* item : nearbyChildren ) { if( Selectable( item ) - && !item->GetParent()->HasFlag( TEMP_SELECTED ) + && !item->GetParent()->HasFlag( CANDIDATE ) && item->HitTest( selectionRect, isWindowSelection ) ) { selectItem( item ); @@ -1886,7 +1898,12 @@ void EE_SELECTION_TOOL::unhighlight( EDA_ITEM* aItem, int aMode, EE_SELECTION* a KICAD_T itemType = aItem->Type(); if( aMode == SELECTED ) + { aItem->ClearSelected(); + // Lines need endpoints cleared here + if( aItem->Type() == SCH_LINE_T ) + aItem->ClearFlags( STARTPOINT | ENDPOINT ); + } else if( aMode == BRIGHTENED ) aItem->ClearBrightened(); diff --git a/eeschema/tools/ee_selection_tool.h b/eeschema/tools/ee_selection_tool.h index 807397a67e..9029bc3b62 100644 --- a/eeschema/tools/ee_selection_tool.h +++ b/eeschema/tools/ee_selection_tool.h @@ -197,10 +197,8 @@ private: * @param collector EE_COLLECTOR with elements to filter * @param aWhere point where we should narrow (if relevant) * @param aCheckLocked If false, remove locked elements from #collector - * @param aSelectPoints If true, set STARTPOINT/ENDPOINT flags on individual wires */ - void narrowSelection( EE_COLLECTOR& collector, const VECTOR2I& aWhere, bool aCheckLocked, - bool aSelectPoints ); + void narrowSelection( EE_COLLECTOR& collector, const VECTOR2I& aWhere, bool aCheckLocked ); /** * This is the primary SelectPoint method that will prompt the user with a menu to disambiguate @@ -208,6 +206,7 @@ private: * actual selection group. * * @param aCollector is an EE_COLLECTOR that already has collected items + * @param aWhere position of the selected point * @param aItem is set to the newly selected item if only one was selected, otherwise is * unchanged. * @param aSelectionCancelledFlag allows the function to inform its caller that a selection @@ -217,7 +216,7 @@ private: * @param aSubtract indicates if found item(s) should be subtracted from the selection * @param aExclusiveOr indicates if found item(s) should be toggle in the selection */ - bool selectPoint( EE_COLLECTOR& aCollector, EDA_ITEM** aItem = nullptr, + bool selectPoint( EE_COLLECTOR& aCollector, const VECTOR2I& aWhere, EDA_ITEM** aItem = nullptr, bool* aSelectionCancelledFlag = nullptr, bool aAdd = false, bool aSubtract = false, bool aExclusiveOr = false ); diff --git a/eeschema/tools/ee_tool_base.h b/eeschema/tools/ee_tool_base.h index 781f6923ca..204b62e57a 100644 --- a/eeschema/tools/ee_tool_base.h +++ b/eeschema/tools/ee_tool_base.h @@ -139,7 +139,7 @@ protected: // IS_SELECTED flag should not be set on undo items which were added for // a drag operation. - if( selected && aItem->HasFlag( TEMP_SELECTED ) ) + if( selected && aItem->HasFlag( SELECTED_BY_DRAG ) ) aItem->ClearSelected(); if( m_isSymbolEditor ) @@ -168,7 +168,7 @@ protected: } } - if( selected && aItem->HasFlag( TEMP_SELECTED ) ) + if( selected && aItem->HasFlag( SELECTED_BY_DRAG ) ) aItem->SetSelected(); } diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index 947602cc37..aa3d2a046b 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -498,7 +498,7 @@ int SCH_EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent ) { SCH_ITEM* item = static_cast( selection.GetItem( ii ) ); - if( item->HasFlag( TEMP_SELECTED ) ) + if( item->HasFlag( SELECTED_BY_DRAG ) ) continue; principalItemCount++; @@ -652,7 +652,7 @@ int SCH_EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent ) // We've already rotated the user selected item if there was only one. We're just // here to rotate the ends of wires that were attached to it. - if( principalItemCount == 1 && !item->HasFlag( TEMP_SELECTED ) ) + if( principalItemCount == 1 && !item->HasFlag( SELECTED_BY_DRAG ) ) continue; if( !moving ) diff --git a/eeschema/tools/sch_move_tool.cpp b/eeschema/tools/sch_move_tool.cpp index ddead26039..c1c22728c4 100644 --- a/eeschema/tools/sch_move_tool.cpp +++ b/eeschema/tools/sch_move_tool.cpp @@ -189,7 +189,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) for( SCH_ITEM* it : m_frame->GetScreen()->Items() ) { - it->ClearFlags( TEMP_SELECTED ); + it->ClearFlags( SELECTED_BY_DRAG ); if( !it->IsSelected() ) it->ClearFlags( STARTPOINT | ENDPOINT ); @@ -268,7 +268,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) if( item->IsNew() ) { - if( item->HasFlag( TEMP_SELECTED ) && m_isDrag ) + if( item->HasFlag( SELECTED_BY_DRAG ) && m_isDrag ) { // Item was added in getConnectedDragItems saveCopyInUndoList( (SCH_ITEM*) item, UNDO_REDO::NEWITEM, appendUndo ); @@ -447,15 +447,18 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) // Look for pre-existing lines we can drag with us instead of creating new ones bool foundAttachment = false; - bool foundJunction = false; - SCH_LINE* foundLine = nullptr; + bool foundJunction = false; + bool foundPin = false; + SCH_LINE* foundLine = nullptr; for( EDA_ITEM* cItem : m_lineConnectionCache[line] ) { foundAttachment = true; // If the move is the same angle as a connected line, // we can shrink/extend that line endpoint - if( cItem->Type() == SCH_LINE_T ) + switch( cItem->Type() ) + { + case SCH_LINE_T: { SCH_LINE* cLine = static_cast( cItem ); @@ -469,9 +472,19 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) // when we can (otherwise the zero length line will draw overlapping segments on them) if( foundLine == nullptr && cLine->GetLength() == 0 ) foundLine = cLine; + + break; } - else if( cItem->Type() == SCH_JUNCTION_T ) + case SCH_JUNCTION_T: foundJunction = true; + break; + + case SCH_PIN_T: + foundPin = true; + break; + + default: break; + } } // Ok... what if our original line is length zero from moving in its direction, @@ -491,8 +504,9 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) } // If we have found an attachment, but not a line, we want to check if it's // a junction. These are special-cased and get a single line added instead of a - // 90-degree bend. - else if( !foundLine && foundJunction ) + // 90-degree bend. Except when we're on a pin, because pins always need bends, + // and junctions are just added to pins for visual clarity. + else if( !foundLine && foundJunction && !foundPin ) { // Create a new wire ending at the unselected end foundLine = new SCH_LINE( unselectedEnd, line->GetLayer() ); @@ -884,18 +898,38 @@ void SCH_MOVE_TOOL::getConnectedItems( SCH_ITEM* aOriginalItem, const VECTOR2I& { EE_RTREE& items = m_frame->GetScreen()->Items(); EE_RTREE::EE_TYPE itemsOverlapping = items.Overlapping( aOriginalItem->GetBoundingBox() ); + SCH_ITEM* foundJunction = nullptr; + SCH_ITEM* foundSymbol = nullptr; - // If you're connected to a junction, you're only connected to the junction - // (unless you are the junction) + // If you're connected to a junction, you're only connected to the junction. + // + // But, if you're connected to a junction on a pin, you're only connected to the pin. This + // is because junctions and pins have different logic for how bend lines are generated + // and we need to prioritize the pin version in some cases. for( SCH_ITEM* item : itemsOverlapping ) { - if( item != aOriginalItem && item->Type() == SCH_JUNCTION_T && item->IsConnected( aPoint ) ) + if( item != aOriginalItem && item->IsConnected( aPoint ) ) { - aList.push_back( item ); - return; + if( item->Type() == SCH_JUNCTION_T ) + foundJunction = item; + else if( item->Type() == SCH_SYMBOL_T ) + foundSymbol = item; } } + if( foundSymbol && foundJunction ) + { + aList.push_back( foundSymbol ); + return; + } + + if( foundJunction ) + { + aList.push_back( foundJunction ); + return; + } + + for( SCH_ITEM* test : itemsOverlapping ) { if( test == aOriginalItem || !test->CanConnect( aOriginalItem ) ) @@ -1028,11 +1062,11 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, const VECTOR // already been grabbed during the partial selection process. line->SetFlags( STARTPOINT ); - if( line->HasFlag( SELECTED ) || line->HasFlag( TEMP_SELECTED ) ) + if( line->HasFlag( SELECTED ) || line->HasFlag( SELECTED_BY_DRAG ) ) continue; else { - line->SetFlags( TEMP_SELECTED ); + line->SetFlags( SELECTED_BY_DRAG ); aList.push_back( line ); } } @@ -1040,11 +1074,11 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, const VECTOR { line->SetFlags( ENDPOINT ); - if( line->HasFlag( SELECTED ) || line->HasFlag( TEMP_SELECTED ) ) + if( line->HasFlag( SELECTED ) || line->HasFlag( SELECTED_BY_DRAG ) ) continue; else { - line->SetFlags( TEMP_SELECTED ); + line->SetFlags( SELECTED_BY_DRAG ); aList.push_back( line ); } } @@ -1068,7 +1102,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, const VECTOR newWire->SetFlags( IS_NEW ); m_frame->AddToScreen( newWire, m_frame->GetScreen() ); - newWire->SetFlags( TEMP_SELECTED | STARTPOINT ); + newWire->SetFlags( SELECTED_BY_DRAG | STARTPOINT ); aList.push_back( newWire ); //We need to add a connection reference here because the normal @@ -1100,12 +1134,12 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, const VECTOR if( label->IsSelected() ) continue; // These will be moved on their own because they're selected - if( label->HasFlag( TEMP_SELECTED ) ) + if( label->HasFlag( SELECTED_BY_DRAG ) ) continue; if( label->CanConnect( line ) && line->HitTest( label->GetPosition(), 1 ) ) { - label->SetFlags( TEMP_SELECTED ); + label->SetFlags( SELECTED_BY_DRAG ); aList.push_back( label ); SPECIAL_CASE_LABEL_INFO info; @@ -1141,17 +1175,17 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, const VECTOR newWire->SetFlags( IS_NEW ); m_frame->AddToScreen( newWire, m_frame->GetScreen() ); - newWire->SetFlags( TEMP_SELECTED | STARTPOINT ); + newWire->SetFlags( SELECTED_BY_DRAG | STARTPOINT ); aList.push_back( newWire ); } break; case SCH_NO_CONNECT_T: // Select no-connects that are connected to items being moved. - if( !test->HasFlag( TEMP_SELECTED ) && test->IsConnected( aPoint ) ) + if( !test->HasFlag( SELECTED_BY_DRAG ) && test->IsConnected( aPoint ) ) { aList.push_back( test ); - test->SetFlags( TEMP_SELECTED ); + test->SetFlags( SELECTED_BY_DRAG ); } break; @@ -1161,7 +1195,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, const VECTOR case SCH_HIER_LABEL_T: case SCH_DIRECTIVE_LABEL_T: // Performance optimization: - if( test->HasFlag( TEMP_SELECTED ) ) + if( test->HasFlag( SELECTED_BY_DRAG ) ) break; // Select labels that are connected to a wire (or bus) being moved. @@ -1185,7 +1219,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, const VECTOR } else { - label->SetFlags( TEMP_SELECTED ); + label->SetFlags( SELECTED_BY_DRAG ); aList.push_back( label ); if( oneEndFixed ) @@ -1204,7 +1238,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, const VECTOR case SCH_BUS_WIRE_ENTRY_T: case SCH_BUS_BUS_ENTRY_T: // Performance optimization: - if( test->HasFlag( TEMP_SELECTED ) ) + if( test->HasFlag( SELECTED_BY_DRAG ) ) break; // Select bus entries that are connected to a bus being moved. @@ -1224,7 +1258,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, const VECTOR { if( line->HitTest( point, 1 ) ) { - test->SetFlags( TEMP_SELECTED ); + test->SetFlags( SELECTED_BY_DRAG ); aList.push_back( test ); // A bus entry needs its wire & label as well diff --git a/include/eda_item.h b/include/eda_item.h index 07d1d9e708..c324244807 100644 --- a/include/eda_item.h +++ b/include/eda_item.h @@ -164,7 +164,7 @@ public: void ClearTempFlags() { - ClearFlags( CANDIDATE | TEMP_SELECTED | IS_LINKED | SKIP_STRUCT | DO_NOT_DRAW ); + ClearFlags( CANDIDATE | SELECTED_BY_DRAG | IS_LINKED | SKIP_STRUCT | DO_NOT_DRAW ); } void ClearEditFlags() @@ -256,6 +256,15 @@ public: */ virtual const VECTOR2I GetFocusPosition() const { return GetPosition(); } + /** + * Return the coordinates that should be used for sorting this element + * visually compared to other elements. For instance, for lines the midpoint + * might be a better sorting point than either end. + * + * @return X,Y coordinate of the sort point + */ + virtual VECTOR2I GetSortPosition() const { return GetPosition(); } + /** * Create a duplicate of this item with linked list members set to NULL. * diff --git a/include/eda_item_flags.h b/include/eda_item_flags.h index b65440c7dd..2928f6d08a 100644 --- a/include/eda_item_flags.h +++ b/include/eda_item_flags.h @@ -45,8 +45,8 @@ #define IS_WIRE_IMAGE (1 << 8) ///< Item to be drawn as wireframe while editing #define STARTPOINT (1 << 9) ///< When a line is selected, these flags indicate which #define ENDPOINT (1 << 10) ///< ends. (Used to support dragging.) -#define SELECTED (1 << 11) -#define TEMP_SELECTED (1 << 12) ///< flag indicating that the structure has already selected +#define SELECTED (1 << 11) ///< Item was manually selected by the user +#define SELECTED_BY_DRAG (1 << 12) ///< Item was algorithmically selected as a dragged item #define STRUCT_DELETED (1 << 13) ///< flag indication structures to be erased #define CANDIDATE (1 << 14) ///< flag indicating that the structure is connected #define SKIP_STRUCT (1 << 15) ///< flag indicating that the structure should be ignored @@ -77,4 +77,4 @@ typedef std::uint32_t EDA_ITEM_FLAGS; -#endif \ No newline at end of file +#endif diff --git a/include/tool/selection.h b/include/tool/selection.h index f040bfb200..0ef833b8cc 100644 --- a/include/tool/selection.h +++ b/include/tool/selection.h @@ -34,7 +34,6 @@ #include #include -class EDA_ITEM; class SELECTION : public KIGFX::VIEW_GROUP { @@ -126,21 +125,21 @@ public: std::sort( sorted_items.begin(), sorted_items.end(), [&]( EDA_ITEM* a, EDA_ITEM* b ) { if( a->Type() == b->Type() ) { - if( a->GetPosition().x == b->GetPosition().x ) + if( a->GetSortPosition().x == b->GetSortPosition().x ) { // Ensure deterministic sort - if( a->GetPosition().y == b->GetPosition().y ) + if( a->GetSortPosition().y == b->GetSortPosition().y ) return a->m_Uuid < b->m_Uuid; if( topBeforeBottom ) - return a->GetPosition().y < b->GetPosition().y; + return a->GetSortPosition().y < b->GetSortPosition().y; else - return a->GetPosition().y > b->GetPosition().y; + return a->GetSortPosition().y > b->GetSortPosition().y; } else if( leftBeforeRight ) - return a->GetPosition().x < b->GetPosition().x; + return a->GetSortPosition().x < b->GetSortPosition().x; else - return a->GetPosition().x > b->GetPosition().x; + return a->GetSortPosition().x > b->GetSortPosition().x; } else return a->Type() < b->Type(); diff --git a/pcbnew/tools/pcb_selection_tool.cpp b/pcbnew/tools/pcb_selection_tool.cpp index 03c80bd80e..6f83c4022e 100644 --- a/pcbnew/tools/pcb_selection_tool.cpp +++ b/pcbnew/tools/pcb_selection_tool.cpp @@ -3016,18 +3016,18 @@ void PCB_SELECTION_TOOL::FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollec { std::unordered_set toAdd; - // Set TEMP_SELECTED on all parents which are included in the GENERAL_COLLECTOR. This + // Set CANDIDATE on all parents which are included in the GENERAL_COLLECTOR. This // algorithm is O3n, whereas checking for the parent inclusion could potentially be On^2. for( int j = 0; j < aCollector.GetCount(); j++ ) { if( aCollector[j]->GetParent() ) - aCollector[j]->GetParent()->ClearFlags( TEMP_SELECTED ); + aCollector[j]->GetParent()->ClearFlags( CANDIDATE ); } if( aMultiselect ) { for( int j = 0; j < aCollector.GetCount(); j++ ) - aCollector[j]->SetFlags( TEMP_SELECTED ); + aCollector[j]->SetFlags( CANDIDATE ); } for( int j = 0; j < aCollector.GetCount(); ) @@ -3048,7 +3048,7 @@ void PCB_SELECTION_TOOL::FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollec if( aTop != item ) { toAdd.insert( aTop ); - aTop->SetFlags( TEMP_SELECTED ); + aTop->SetFlags( CANDIDATE ); aCollector.Remove( item ); continue; @@ -3063,7 +3063,7 @@ void PCB_SELECTION_TOOL::FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollec } // Footprints are a bit easier as they can't be nested. - if( parent && ( parent->GetFlags() & TEMP_SELECTED ) ) + if( parent && ( parent->GetFlags() & CANDIDATE ) ) { // Remove children of selected items aCollector.Remove( item );