From cc721c490767b480dbaab9f4482a448dbc220b75 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Thu, 9 Nov 2023 13:55:00 +0000 Subject: [PATCH] Improve encapsulation of group internals. (It's still leaking into BOARD_COMMIT and some other places, but at least it no longer leaks into all the edit tools.) Also fixes some bugs when moving/copying/pasting multiple selections containing length-tuning patterns. --- common/widgets/footprint_diff_widget.cpp | 20 ++-- include/board_item.h | 9 +- include/pcb_group.h | 25 +---- pcbnew/array_creator.cpp | 22 +--- pcbnew/board_commit.cpp | 11 +- pcbnew/dialogs/dialog_group_properties.cpp | 6 -- pcbnew/footprint.cpp | 69 +++++++++---- pcbnew/footprint.h | 11 +- pcbnew/footprint_libraries_utils.cpp | 5 +- pcbnew/generators/pcb_tuning_pattern.cpp | 8 ++ pcbnew/kicad_clipboard.cpp | 20 ++-- pcbnew/load_select_footprint.cpp | 2 +- pcbnew/pcb_base_frame.cpp | 4 +- pcbnew/pcb_generator.cpp | 14 --- pcbnew/pcb_group.cpp | 3 +- pcbnew/tools/board_inspection_tool.cpp | 11 +- pcbnew/tools/drawing_stackup_table_tool.cpp | 4 - pcbnew/tools/edit_tool.cpp | 106 ++------------------ pcbnew/tools/edit_tool_move_fct.cpp | 38 ++----- pcbnew/tools/pcb_control.cpp | 2 +- pcbnew/tools/pcb_grid_helper.cpp | 2 +- pcbnew/tools/pcb_selection_tool.cpp | 48 ++++----- pcbnew/tools/position_relative_tool.cpp | 11 -- 23 files changed, 153 insertions(+), 298 deletions(-) diff --git a/common/widgets/footprint_diff_widget.cpp b/common/widgets/footprint_diff_widget.cpp index 0dfe469a8b..6335273ccb 100644 --- a/common/widgets/footprint_diff_widget.cpp +++ b/common/widgets/footprint_diff_widget.cpp @@ -64,11 +64,11 @@ void FOOTPRINT_DIFF_WIDGET::DisplayDiff( FOOTPRINT* aBoardFootprint, m_boardItemCopy->ClearSelected(); m_boardItemCopy->ClearBrightened(); - m_boardItemCopy->RunOnChildren( - [&]( BOARD_ITEM* child ) + m_boardItemCopy->RunOnDescendants( + [&]( BOARD_ITEM* item ) { - child->ClearSelected(); - child->ClearBrightened(); + item->ClearSelected(); + item->ClearBrightened(); } ); m_boardItemCopy->Move( -m_boardItemCopy->GetPosition() ); @@ -103,10 +103,10 @@ void FOOTPRINT_DIFF_WIDGET::onSlider( wxScrollEvent& aEvent ) m_boardItemCopy->SetForcedTransparency( val ); - m_boardItemCopy->RunOnChildren( - [&]( BOARD_ITEM* child ) + m_boardItemCopy->RunOnDescendants( + [&]( BOARD_ITEM* item ) { - child->SetForcedTransparency( val ); + item->SetForcedTransparency( val ); } ); } @@ -121,10 +121,10 @@ void FOOTPRINT_DIFF_WIDGET::onSlider( wxScrollEvent& aEvent ) m_libraryItem->SetForcedTransparency( val ); - m_libraryItem->RunOnChildren( - [&]( BOARD_ITEM* child ) + m_libraryItem->RunOnDescendants( + [&]( BOARD_ITEM* item ) { - child->SetForcedTransparency( val ); + item->SetForcedTransparency( val ); } ); } diff --git a/include/board_item.h b/include/board_item.h index e18bcbe3d3..fa3aa99279 100644 --- a/include/board_item.h +++ b/include/board_item.h @@ -190,13 +190,16 @@ public: /** * Invoke a function on all children. - * * @note This function should not add or remove items to the parent. - * - * @param aFunction is the function to be invoked. */ virtual void RunOnChildren( const std::function& aFunction ) const { } + /** + * Invoke a function on all descendants. + * @note This function should not add or remove items. + */ + virtual void RunOnDescendants( const std::function& aFunction ) const { } + BOARD_ITEM_CONTAINER* GetParent() const { return (BOARD_ITEM_CONTAINER*) m_parent; } FOOTPRINT* GetParentFootprint() const; diff --git a/include/pcb_group.h b/include/pcb_group.h index d277edbfdc..a8a1ec0e3c 100644 --- a/include/pcb_group.h +++ b/include/pcb_group.h @@ -196,35 +196,14 @@ public: /// @copydoc EDA_ITEM::GetMenuImage BITMAPS GetMenuImage() const override; - - /** - * Add all the immediate children of this group to the board commit. This function does not - * enter any subgroups of this group, or add the group itself. - * - * @param aCommit is the commit to add the children to. - */ - void AddChildrenToCommit( BOARD_COMMIT& aCommit ) - { - RunOnChildren( [&]( BOARD_ITEM* bItem ) - { - aCommit.Add( bItem ); - } ); - } - - /// @copydoc EDA_ITEM::GetMsgPanelInfo void GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector& aList ) override; ///< @copydoc BOARD_ITEM::RunOnChildren void RunOnChildren( const std::function& aFunction ) const override; - /** - * Invoke a function on all descendants of the group. - * - * @note This function should not add or remove items to the group or descendant groups. - * @param aFunction is the function to be invoked. - */ - void RunOnDescendants( const std::function& aFunction ) const; + ///< @copydoc BOARD_ITEM::RunOnDescendants + void RunOnDescendants( const std::function& aFunction ) const override; /** * Check if the proposed type can be added to a group diff --git a/pcbnew/array_creator.cpp b/pcbnew/array_creator.cpp index d76774601e..50895da123 100644 --- a/pcbnew/array_creator.cpp +++ b/pcbnew/array_creator.cpp @@ -157,23 +157,11 @@ void ARRAY_CREATOR::Invoke() // it this state, reset the selected stated of aItem: this_item->ClearSelected(); - if( this_item->Type() == PCB_GROUP_T ) - { - static_cast( this_item )->RunOnDescendants( - [&]( BOARD_ITEM* aItem ) - { - aItem->ClearSelected(); - commit.Add( aItem ); - }); - } - else if( this_item->Type() == PCB_FOOTPRINT_T ) - { - static_cast( this_item )->RunOnChildren( - [&]( BOARD_ITEM* aItem ) - { - aItem->ClearSelected(); - }); - } + this_item->RunOnDescendants( + [&]( BOARD_ITEM* aItem ) + { + aItem->ClearSelected(); + }); TransformItem( *array_opts, ptN, *this_item ); commit.Add( this_item ); diff --git a/pcbnew/board_commit.cpp b/pcbnew/board_commit.cpp index 0d4b750d09..c1b59df18d 100644 --- a/pcbnew/board_commit.cpp +++ b/pcbnew/board_commit.cpp @@ -76,11 +76,11 @@ COMMIT& BOARD_COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType, BASE_SCRE { wxCHECK( aItem, *this ); - if( aChangeType == CHT_MODIFY && aItem->Type() == PCB_GROUP_T ) + // Many operations (move, rotate, etc.) are applied directly to a group's children, so they + // must be staged as well. + if( PCB_GROUP* group = dynamic_cast( aItem ) ) { - // Many operations on group (move, rotate, etc.) are applied directly to their - // children, so it's the children that must be staged. - static_cast( aItem )->RunOnChildren( + group->RunOnChildren( [&]( BOARD_ITEM* child ) { COMMIT::Stage( child, aChangeType ); @@ -305,6 +305,9 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) switch( boardItem->Type() ) { case PCB_FIELD_T: + static_cast( boardItem )->SetVisible( false ); + break; + case PCB_TEXT_T: case PCB_PAD_T: case PCB_SHAPE_T: // a shape (normally not on copper layers) diff --git a/pcbnew/dialogs/dialog_group_properties.cpp b/pcbnew/dialogs/dialog_group_properties.cpp index d0a7aa3d4d..f1ca1a8f17 100644 --- a/pcbnew/dialogs/dialog_group_properties.cpp +++ b/pcbnew/dialogs/dialog_group_properties.cpp @@ -83,12 +83,6 @@ bool DIALOG_GROUP_PROPERTIES::TransferDataFromWindow() BOARD_COMMIT commit( m_brdEditor ); commit.Modify( m_group ); - m_group->RunOnDescendants( - [&]( BOARD_ITEM* descendant ) - { - commit.Modify( descendant ); - } ); - for( size_t ii = 0; ii < m_membersList->GetCount(); ++ii ) { BOARD_ITEM* item = static_cast( m_membersList->GetClientData( ii ) ); diff --git a/pcbnew/footprint.cpp b/pcbnew/footprint.cpp index ee30735778..d7268cc96d 100644 --- a/pcbnew/footprint.cpp +++ b/pcbnew/footprint.cpp @@ -1695,24 +1695,24 @@ EDA_ITEM* FOOTPRINT::Clone() const } -void FOOTPRINT::RunOnChildren( const std::function& aFunction ) const +void FOOTPRINT::RunOnChildren( const std::function& aFunction ) const { try { for( PCB_FIELD* field : m_fields ) - aFunction( static_cast( field ) ); + aFunction( field ); for( PAD* pad : m_pads ) - aFunction( static_cast( pad ) ); + aFunction( pad ); for( ZONE* zone : m_zones ) - aFunction( static_cast( zone ) ); + aFunction( zone ); for( PCB_GROUP* group : m_groups ) - aFunction( static_cast( group ) ); + aFunction( group ); for( BOARD_ITEM* drawing : m_drawings ) - aFunction( static_cast( drawing ) ); + aFunction( drawing ); } catch( std::bad_function_call& ) { @@ -1721,6 +1721,38 @@ void FOOTPRINT::RunOnChildren( const std::function& aFuncti } +void FOOTPRINT::RunOnDescendants( const std::function& aFunction ) const +{ + try + { + for( PCB_FIELD* field : m_fields ) + aFunction( field ); + + for( PAD* pad : m_pads ) + aFunction( pad ); + + for( ZONE* zone : m_zones ) + aFunction( zone ); + + for( PCB_GROUP* group : m_groups ) + { + aFunction( group ); + group->RunOnDescendants( aFunction ); + } + + for( BOARD_ITEM* drawing : m_drawings ) + { + aFunction( drawing ); + drawing->RunOnDescendants( aFunction ); + } + } + catch( std::bad_function_call& ) + { + wxFAIL_MSG( wxT( "Error running FOOTPRINT::RunOnDescendants" ) ); + } +} + + void FOOTPRINT::ViewGetLayers( int aLayers[], int& aCount ) const { aCount = 2; @@ -2063,10 +2095,10 @@ BOARD_ITEM* FOOTPRINT::Duplicate() const { FOOTPRINT* dupe = static_cast( BOARD_ITEM::Duplicate() ); - dupe->RunOnChildren( [&]( BOARD_ITEM* child ) - { - const_cast( child->m_Uuid ) = KIID(); - }); + dupe->RunOnDescendants( [&]( BOARD_ITEM* child ) + { + const_cast( child->m_Uuid ) = KIID(); + }); return dupe; } @@ -2755,17 +2787,14 @@ void FOOTPRINT::CheckNetTies( const std::functionIsOnCopperLayer() ) - { copperItems.push_back( item ); - } - else if( PCB_GROUP* group = dynamic_cast( item ) ) - { - group->RunOnDescendants( [&]( BOARD_ITEM* descendent ) - { - if( descendent->IsOnCopperLayer() ) - copperItems.push_back( descendent ); - } ); - } + + item->RunOnDescendants( + [&]( BOARD_ITEM* descendent ) + { + if( descendent->IsOnCopperLayer() ) + copperItems.push_back( descendent ); + } ); } for( ZONE* zone : m_zones ) diff --git a/pcbnew/footprint.h b/pcbnew/footprint.h index d33bd44c5e..b5372fd2a1 100644 --- a/pcbnew/footprint.h +++ b/pcbnew/footprint.h @@ -827,15 +827,12 @@ public: EDA_ITEM* Clone() const override; - /** - * Invoke a function on all BOARD_ITEMs that belong to the footprint (pads, drawings, texts). - * - * @note This function should not add or remove items to the footprint. - * - * @param aFunction is the function to be invoked. - */ + ///< @copydoc BOARD_ITEM::RunOnChildren void RunOnChildren( const std::function& aFunction ) const override; + ///< @copydoc BOARD_ITEM::RunOnDescendants + void RunOnDescendants( const std::function& aFunction ) const override; + virtual void ViewGetLayers( int aLayers[], int& aCount ) const override; double ViewGetLOD( int aLayer, KIGFX::VIEW* aView ) const override; diff --git a/pcbnew/footprint_libraries_utils.cpp b/pcbnew/footprint_libraries_utils.cpp index 410ec0fb1d..faba38c7da 100644 --- a/pcbnew/footprint_libraries_utils.cpp +++ b/pcbnew/footprint_libraries_utils.cpp @@ -894,7 +894,8 @@ bool FOOTPRINT_EDIT_FRAME::SaveFootprintToBoard( bool aAddNew ) }; fixUuid( const_cast( newFootprint->m_Uuid ) ); - newFootprint->RunOnChildren( + + newFootprint->RunOnDescendants( [&]( BOARD_ITEM* aChild ) { fixUuid( const_cast( aChild->m_Uuid ) ); @@ -1311,7 +1312,7 @@ FOOTPRINT* PCB_BASE_FRAME::CreateNewFootprint( const wxString& aFootprintName, if( footprint->GetValue().IsEmpty() ) footprint->SetValue( footprintName ); - footprint->RunOnChildren( + footprint->RunOnDescendants( [&]( BOARD_ITEM* aChild ) { if( aChild->Type() == PCB_FIELD_T || aChild->Type() == PCB_TEXT_T ) diff --git a/pcbnew/generators/pcb_tuning_pattern.cpp b/pcbnew/generators/pcb_tuning_pattern.cpp index ae6c8423b5..64729b7f8d 100644 --- a/pcbnew/generators/pcb_tuning_pattern.cpp +++ b/pcbnew/generators/pcb_tuning_pattern.cpp @@ -127,6 +127,14 @@ public: { m_origin += aMoveVector; m_end += aMoveVector; + + PCB_GROUP::Move( aMoveVector ); + + if( m_baseLine ) + m_baseLine->Move( aMoveVector ); + + if( m_baseLineCoupled ) + m_baseLineCoupled->Move( aMoveVector ); } void Rotate( const VECTOR2I& aRotCentre, const EDA_ANGLE& aAngle ) override diff --git a/pcbnew/kicad_clipboard.cpp b/pcbnew/kicad_clipboard.cpp index abf7387fd5..849f61e687 100644 --- a/pcbnew/kicad_clipboard.cpp +++ b/pcbnew/kicad_clipboard.cpp @@ -144,7 +144,7 @@ void CLIPBOARD_IO::SaveSelection( const PCB_SELECTION& aSelected, bool isFootpri if( group ) { - static_cast( clone )->RunOnDescendants( + clone->RunOnDescendants( [&]( BOARD_ITEM* descendant ) { // One cannot add an additional mandatory field to a given footprint: @@ -239,14 +239,9 @@ void CLIPBOARD_IO::SaveSelection( const PCB_SELECTION& aSelected, bool isFootpri copy = static_cast( boardItem->Clone() ); } - auto prepItem = [&]( BOARD_ITEM* aItem ) - { - aItem->SetLocked( false ); - }; - if( copy ) { - prepItem( copy ); + copy->SetLocked( false ); // locate the reference point at (0, 0) in the copied items copy->Move( -refPoint ); @@ -255,11 +250,12 @@ void CLIPBOARD_IO::SaveSelection( const PCB_SELECTION& aSelected, bool isFootpri if( copy->Type() == PCB_GROUP_T ) { - static_cast( copy )->RunOnDescendants( prepItem ); - static_cast( copy )->RunOnDescendants( [&]( BOARD_ITEM* titem ) - { - Format( titem, 1 ); - } ); + copy->RunOnDescendants( + [&]( BOARD_ITEM* titem ) + { + titem->SetLocked( false ); + Format( titem, 1 ); + } ); } copy->SetParentGroup( nullptr ); diff --git a/pcbnew/load_select_footprint.cpp b/pcbnew/load_select_footprint.cpp index e14e3dfe7a..7ce9a4b4ac 100644 --- a/pcbnew/load_select_footprint.cpp +++ b/pcbnew/load_select_footprint.cpp @@ -121,7 +121,7 @@ bool FOOTPRINT_EDIT_FRAME::LoadFootprintFromBoard( FOOTPRINT* aFootprint ) newFootprint->ClearFlags(); recordAndUpdateUuid( newFootprint ); - newFootprint->RunOnChildren( + newFootprint->RunOnDescendants( [&]( BOARD_ITEM* aItem ) { if( aItem->Type() == PCB_PAD_T ) diff --git a/pcbnew/pcb_base_frame.cpp b/pcbnew/pcb_base_frame.cpp index 59ac800b72..de47c2e72b 100644 --- a/pcbnew/pcb_base_frame.cpp +++ b/pcbnew/pcb_base_frame.cpp @@ -290,7 +290,7 @@ void PCB_BASE_FRAME::FocusOnItems( std::vector aItems, PCB_LAYER_ID { lastItem->ClearBrightened(); - lastItem->RunOnChildren( + lastItem->RunOnDescendants( [&]( BOARD_ITEM* child ) { child->ClearBrightened(); @@ -337,7 +337,7 @@ void PCB_BASE_FRAME::FocusOnItems( std::vector aItems, PCB_LAYER_ID { item->SetBrightened(); - item->RunOnChildren( + item->RunOnDescendants( [&]( BOARD_ITEM* child ) { child->SetBrightened(); diff --git a/pcbnew/pcb_generator.cpp b/pcbnew/pcb_generator.cpp index 91cc28fece..0186c78e67 100644 --- a/pcbnew/pcb_generator.cpp +++ b/pcbnew/pcb_generator.cpp @@ -40,12 +40,6 @@ void PCB_GENERATOR::EditStart( GENERATOR_TOOL* aTool, BOARD* aBoard, PCB_BASE_ED BOARD_COMMIT* aCommit ) { aCommit->Modify( this ); - - RunOnDescendants( - [&]( BOARD_ITEM* aItem ) - { - aCommit->Modify( aItem ); - } ); } @@ -66,14 +60,6 @@ void PCB_GENERATOR::EditRevert( GENERATOR_TOOL* aTool, BOARD* aBoard, PCB_BASE_E void PCB_GENERATOR::Remove( GENERATOR_TOOL* aTool, BOARD* aBoard, PCB_BASE_EDIT_FRAME* aFrame, BOARD_COMMIT* aCommit ) { - RunOnDescendants( - [&]( BOARD_ITEM* aItem ) - { - aCommit->Stage( aItem, CHT_MODIFY ); - } ); - - RemoveAll(); - aCommit->Remove( this ); } diff --git a/pcbnew/pcb_group.cpp b/pcbnew/pcb_group.cpp index a260cab672..9abbedded7 100644 --- a/pcbnew/pcb_group.cpp +++ b/pcbnew/pcb_group.cpp @@ -426,7 +426,7 @@ void PCB_GROUP::RunOnDescendants( const std::function& aFun aFunction( item ); if( item->Type() == PCB_GROUP_T ) - static_cast( item )->RunOnDescendants( aFunction ); + item->RunOnDescendants( aFunction ); } } catch( std::bad_function_call& ) @@ -435,6 +435,7 @@ void PCB_GROUP::RunOnDescendants( const std::function& aFun } } + bool PCB_GROUP::operator==( const BOARD_ITEM& aOther ) const { if( aOther.Type() != Type() ) diff --git a/pcbnew/tools/board_inspection_tool.cpp b/pcbnew/tools/board_inspection_tool.cpp index d55e444ab2..f57d89a5d8 100644 --- a/pcbnew/tools/board_inspection_tool.cpp +++ b/pcbnew/tools/board_inspection_tool.cpp @@ -1942,13 +1942,12 @@ void BOARD_INSPECTION_TOOL::calculateSelectionRatsnest( const VECTOR2I& aDelta ) items.push_back( pad ); } } - else if( item->Type() == PCB_GROUP_T ) + else if( item->Type() == PCB_GROUP_T || item->Type() == PCB_GENERATOR_T ) { - PCB_GROUP *group = static_cast( item ); - group->RunOnDescendants( [ &queued_items ]( BOARD_ITEM *aItem ) - { - queued_items.push_back( aItem ); - } ); + item->RunOnDescendants( [ &queued_items ]( BOARD_ITEM *aItem ) + { + queued_items.push_back( aItem ); + } ); } else if( BOARD_CONNECTED_ITEM* boardItem = dyn_cast( item ) ) { diff --git a/pcbnew/tools/drawing_stackup_table_tool.cpp b/pcbnew/tools/drawing_stackup_table_tool.cpp index 816c76cbb2..0776193a49 100644 --- a/pcbnew/tools/drawing_stackup_table_tool.cpp +++ b/pcbnew/tools/drawing_stackup_table_tool.cpp @@ -648,10 +648,6 @@ int DRAWING_TOOL::InteractivePlaceWithPreview( const TOOL_EVENT& aEvent, for( BOARD_ITEM* item : aItems ) { item->Move( cursorPosition ); - - if( item->Type() == PCB_GROUP_T ) - static_cast( item )->AddChildrenToCommit( commit ); - commit.Add( item ); } diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp index 037d92f902..ee1bd2f2aa 100644 --- a/pcbnew/tools/edit_tool.cpp +++ b/pcbnew/tools/edit_tool.cpp @@ -1760,20 +1760,8 @@ int EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent ) for( EDA_ITEM* item : selection ) { if( !item->IsNew() && !item->IsMoving() ) - { commit->Modify( item ); - // If rotating a group, record position of all the descendants for undo - if( item->Type() == PCB_GROUP_T || item->Type() == PCB_GENERATOR_T ) - { - static_cast( item )->RunOnDescendants( - [&]( BOARD_ITEM* bItem ) - { - commit->Modify( bItem ); - }); - } - } - if( BOARD_ITEM* board_item = dynamic_cast( item ) ) { board_item->Rotate( refPt, rotateAngle ); @@ -2046,19 +2034,8 @@ int EDIT_TOOL::Flip( const TOOL_EVENT& aEvent ) if( BOARD_ITEM* boardItem = dynamic_cast( item ) ) { if( !boardItem->IsNew() && !boardItem->IsMoving() ) - { commit->Modify( boardItem ); - if( boardItem->Type() == PCB_GROUP_T || boardItem->Type() == PCB_GENERATOR_T ) - { - static_cast( boardItem )->RunOnDescendants( - [&]( BOARD_ITEM* descendant ) - { - commit->Modify( descendant ); - }); - } - } - boardItem->Flip( refPt, leftRight ); boardItem->Normalize(); } @@ -2125,6 +2102,7 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) case PCB_DIM_CENTER_T: case PCB_DIM_RADIAL_T: case PCB_DIM_ORTHOGONAL_T: + case PCB_GROUP_T: if( parentFP ) { commit.Modify( parentFP ); @@ -2193,61 +2171,20 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) break; - case PCB_GROUP_T: - { - PCB_GROUP* group = static_cast( board_item ); - - auto removeItem = - [&]( BOARD_ITEM* bItem ) - { - if( bItem->GetParent() && bItem->GetParent()->Type() == PCB_FOOTPRINT_T ) - { - // Just make fields invisible if they happen to be in group. - if( bItem->Type() == PCB_FIELD_T ) - { - commit.Modify( bItem->GetParent() ); - static_cast( board_item )->SetVisible( false ); - getView()->Update( board_item ); - - return; - } - else if( bItem->Type() == PCB_PAD_T ) - { - if( !IsFootprintEditor() - && !frame()->GetPcbNewSettings()->m_AllowFreePads ) - { - return; - } - } - - commit.Modify( bItem->GetParent() ); - getView()->Remove( bItem ); - bItem->GetParent()->Remove( bItem ); - } - else - { - commit.Remove( bItem ); - } - }; - - removeItem( group ); - - group->RunOnDescendants( [&]( BOARD_ITEM* aDescendant ) - { - removeItem( aDescendant ); - }); - break; - } - case PCB_GENERATOR_T: - { - PCB_GENERATOR* generator = static_cast( board_item ); + if( aItems.Size() == 1 ) + { + PCB_GENERATOR* generator = static_cast( board_item ); - m_toolMgr->RunSynchronousAction( PCB_ACTIONS::genRemove, &commit, - generator ); + m_toolMgr->RunSynchronousAction( PCB_ACTIONS::genRemove, &commit, + generator ); + } + else + { + commit.Remove( board_item ); + } break; - } default: wxASSERT_MSG( parentFP == nullptr, wxT( "Try to delete an item living in a footrprint" ) ); @@ -2392,20 +2329,8 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent ) wxCHECK2( boardItem, continue ); if( !boardItem->IsNew() ) - { commit.Modify( boardItem ); - if( boardItem->Type() == PCB_GROUP_T || boardItem->Type() == PCB_GENERATOR_T ) - { - PCB_GROUP* group = static_cast( boardItem ); - - group->RunOnDescendants( [&]( BOARD_ITEM* descendant ) - { - commit.Modify( descendant ); - }); - } - } - if( !boardItem->GetParent() || !boardItem->GetParent()->IsSelected() ) boardItem->Move( translation ); @@ -2554,15 +2479,6 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent ) if( dupe_item ) { - if( dupe_item->Type() == PCB_GROUP_T ) - { - static_cast( dupe_item )->RunOnDescendants( - [&]( BOARD_ITEM* bItem ) - { - commit.Add( bItem ); - }); - } - // Clear the selection flag here, otherwise the PCB_SELECTION_TOOL // will not properly select it later on dupe_item->ClearSelected(); diff --git a/pcbnew/tools/edit_tool_move_fct.cpp b/pcbnew/tools/edit_tool_move_fct.cpp index cbf738db32..c6cd7b8f1f 100644 --- a/pcbnew/tools/edit_tool_move_fct.cpp +++ b/pcbnew/tools/edit_tool_move_fct.cpp @@ -91,19 +91,7 @@ int EDIT_TOOL::Swap( const TOOL_EVENT& aEvent ) for( EDA_ITEM* item : selection ) { if( !item->IsNew() && !item->IsMoving() ) - { commit->Modify( item ); - - // If swapping a group, record position of all the descendants for undo - if( item->Type() == PCB_GROUP_T || item->Type() == PCB_GENERATOR_T ) - { - PCB_GROUP* group = static_cast( item ); - group->RunOnDescendants( [&]( BOARD_ITEM* descendant ) - { - commit->Modify( descendant ); - }); - } - } } for( size_t i = 0; i < sorted.size() - 1; i++ ) @@ -559,15 +547,14 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit if( !item->GetParent() || !item->GetParent()->IsSelected() ) static_cast( item )->Move( movement ); - if( item->Type() == PCB_GENERATOR_T ) + if( item->Type() == PCB_GENERATOR_T && sel_items.size() == 1 ) { m_toolMgr->RunSynchronousAction( PCB_ACTIONS::genUpdateEdit, aCommit, static_cast( item ) ); } - else if( item->Type() == PCB_FOOTPRINT_T ) - { + + if( item->Type() == PCB_FOOTPRINT_T ) redraw3D = true; - } } if( redraw3D && allowRedraw3D ) @@ -595,7 +582,7 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit if( !item->IsNew() && !item->IsMoving() ) { - if( item->Type() == PCB_GENERATOR_T ) + if( item->Type() == PCB_GENERATOR_T && sel_items.size() == 1 ) { m_toolMgr->RunSynchronousAction( PCB_ACTIONS::genStartEdit, aCommit, static_cast( item ) ); @@ -603,19 +590,14 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit else { aCommit->Modify( item ); + item->SetFlags( IS_MOVING ); - // If moving a group, record position of all the descendants for undo - if( item->Type() == PCB_GROUP_T ) - { - PCB_GROUP* group = static_cast( item ); - group->RunOnDescendants( - [&]( BOARD_ITEM* bItem ) - { - aCommit->Modify( bItem ); - item->SetFlags( IS_MOVING ); - } ); - } + static_cast( item )->RunOnDescendants( + [&]( BOARD_ITEM* bItem ) + { + item->SetFlags( IS_MOVING ); + } ); } } } diff --git a/pcbnew/tools/pcb_control.cpp b/pcbnew/tools/pcb_control.cpp index e3b4293000..c0dfbd7f25 100644 --- a/pcbnew/tools/pcb_control.cpp +++ b/pcbnew/tools/pcb_control.cpp @@ -1203,7 +1203,7 @@ bool PCB_CONTROL::placeBoardItems( BOARD_COMMIT* aCommit, std::vectorGetTool()->ReannotateDuplicatesInSelection(); - for( BOARD_ITEM* item : aItems ) + for( BOARD_ITEM* item : itemsToSel ) { if( aIsNew ) aCommit->Add( item ); diff --git a/pcbnew/tools/pcb_grid_helper.cpp b/pcbnew/tools/pcb_grid_helper.cpp index 9d4dfcbcba..2584fffea6 100644 --- a/pcbnew/tools/pcb_grid_helper.cpp +++ b/pcbnew/tools/pcb_grid_helper.cpp @@ -526,7 +526,7 @@ std::set PCB_GRID_HELPER::queryVisible( const BOX2I& aArea, { items.erase( aItem ); - aItem->RunOnChildren( + aItem->RunOnDescendants( [&]( BOARD_ITEM* aChild ) { skipItem( aChild ); diff --git a/pcbnew/tools/pcb_selection_tool.cpp b/pcbnew/tools/pcb_selection_tool.cpp index f034700020..2956d1aed8 100644 --- a/pcbnew/tools/pcb_selection_tool.cpp +++ b/pcbnew/tools/pcb_selection_tool.cpp @@ -641,26 +641,17 @@ PCB_SELECTION& PCB_SELECTION_TOOL::RequestSelection( CLIENT_SELECTION_FILTER aCl for( EDA_ITEM* item : m_selection ) { BOARD_ITEM* boardItem = static_cast( item ); + bool lockedDescendant = false; - if( boardItem->Type() == PCB_GROUP_T ) - { - PCB_GROUP* group = static_cast( boardItem ); - bool lockedDescendant = false; - - group->RunOnDescendants( - [&lockedDescendant]( BOARD_ITEM* child ) - { - if( child->IsLocked() ) - lockedDescendant = true; - } ); + boardItem->RunOnDescendants( + [&]( BOARD_ITEM* item ) + { + if( item->IsLocked() ) + lockedDescendant = true; + } ); - if( lockedDescendant ) - lockedItems.push_back( group ); - } - else if( boardItem->IsLocked() ) - { + if( boardItem->IsLocked() || lockedDescendant ) lockedItems.push_back( boardItem ); - } } if( !lockedItems.empty() ) @@ -2882,8 +2873,8 @@ void PCB_SELECTION_TOOL::highlightInternal( EDA_ITEM* aItem, int aMode, bool aUs if( BOARD_ITEM* boardItem = dynamic_cast( aItem ) ) { - boardItem->RunOnChildren( std::bind( &PCB_SELECTION_TOOL::highlightInternal, this, _1, - aMode, aUsingOverlay ) ); + boardItem->RunOnDescendants( std::bind( &PCB_SELECTION_TOOL::highlightInternal, this, _1, + aMode, aUsingOverlay ) ); } } @@ -2914,8 +2905,8 @@ void PCB_SELECTION_TOOL::unhighlightInternal( EDA_ITEM* aItem, int aMode, bool a if( BOARD_ITEM* boardItem = dynamic_cast( aItem ) ) { - boardItem->RunOnChildren( std::bind( &PCB_SELECTION_TOOL::unhighlightInternal, this, _1, - aMode, aUsingOverlay ) ); + boardItem->RunOnDescendants( std::bind( &PCB_SELECTION_TOOL::unhighlightInternal, this, _1, + aMode, aUsingOverlay ) ); } } @@ -2938,15 +2929,12 @@ bool PCB_SELECTION_TOOL::selectionContains( const VECTOR2I& aPoint ) const bool found = false; - if( PCB_GROUP* group = dynamic_cast( item ) ) - { - group->RunOnDescendants( - [&]( BOARD_ITEM* aItem ) - { - if( aItem->HitTest( aPoint, margin ) ) - found = true; - } ); - } + static_cast( item )->RunOnDescendants( + [&]( BOARD_ITEM* aItem ) + { + if( aItem->HitTest( aPoint, margin ) ) + found = true; + } ); if( found ) return true; diff --git a/pcbnew/tools/position_relative_tool.cpp b/pcbnew/tools/position_relative_tool.cpp index b50ad5c7fe..0b5f9910b5 100644 --- a/pcbnew/tools/position_relative_tool.cpp +++ b/pcbnew/tools/position_relative_tool.cpp @@ -139,17 +139,6 @@ int POSITION_RELATIVE_TOOL::RelativeItemSelectionMove( const VECTOR2I& aPosAncho } m_commit->Modify( boardItem ); - - // If moving a group, record position of all the descendants for undo - if( boardItem->Type() == PCB_GROUP_T ) - { - PCB_GROUP* group = static_cast( boardItem ); - group->RunOnDescendants( [&]( BOARD_ITEM* descendant ) - { - m_commit->Modify( descendant ); - }); - } - boardItem->Move( aggregateTranslation ); } }