diff --git a/include/eda_item.h b/include/eda_item.h index f589b95de8..860709bb36 100644 --- a/include/eda_item.h +++ b/include/eda_item.h @@ -133,6 +133,7 @@ typedef const INSPECTOR_FUNC& INSPECTOR; #define UR_TRANSIENT (1 << 28) ///< indicates the item is owned by the undo/redo stack #define IS_DANGLING (1 << 29) ///< indicates a pin is dangling +#define ENTERED (1 << 30) // WARNING: if you add flags, you'll probably need to adjust the masks in GetEditFlags() and // ClearTempFlags(). @@ -168,6 +169,7 @@ public: inline bool IsDragging() const { return m_flags & IS_DRAGGED; } inline bool IsWireImage() const { return m_flags & IS_WIRE_IMAGE; } inline bool IsSelected() const { return m_flags & SELECTED; } + inline bool IsEntered() const { return m_flags & ENTERED; } inline bool IsResized() const { return m_flags & IS_RESIZED; } inline bool IsBrightened() const { return m_flags & BRIGHTENED; } diff --git a/pcbnew/board.cpp b/pcbnew/board.cpp index f695c800a9..0d7009f86b 100644 --- a/pcbnew/board.cpp +++ b/pcbnew/board.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -2074,93 +2075,24 @@ wxString BOARD::GroupsSanityCheckInternal( bool repair ) BOARD::GroupLegalOpsField BOARD::GroupLegalOps( const PCB_SELECTION& selection ) const { - GroupLegalOpsField legalOps = { false, false, false, false, false, false }; - - std::unordered_set allMembers; - - for( const PCB_GROUP* grp : m_groups ) - { - for( const BOARD_ITEM* member : grp->GetItems() ) - allMembers.insert( member ); - } - - bool hasGroup = ( SELECTION_CONDITIONS::HasType( PCB_GROUP_T ) )( selection ); - // All elements of selection are groups, and no element is a descendant group of any other. - bool onlyGroups = ( SELECTION_CONDITIONS::OnlyType( PCB_GROUP_T ) )( selection ); - // Any elements of the selections are already members of groups - bool anyGrouped = false; - // Any elements of the selections, except the first group, are already members of groups. - bool anyGroupedExceptFirst = false; - // All elements of the selections are already members of groups - bool allGrouped = true; - bool seenFirstGroup = false; - - if( onlyGroups ) - { - // Check that no groups are descendant subgroups of another group in the selection - for( EDA_ITEM* item : selection ) - { - const PCB_GROUP* group = static_cast( item ); - std::unordered_set subgroupos; - std::queue toCheck; - toCheck.push( group ); - - while( !toCheck.empty() ) - { - const PCB_GROUP* candidate = toCheck.front(); - toCheck.pop(); - - for( const BOARD_ITEM* aChild : candidate->GetItems() ) - { - if( aChild->Type() == PCB_GROUP_T ) - { - const PCB_GROUP* childGroup = static_cast( aChild ); - subgroupos.insert( childGroup ); - toCheck.push( childGroup ); - } - } - } - - for( EDA_ITEM* otherItem : selection ) - { - if( otherItem != item - && subgroupos.find( static_cast( otherItem ) ) != subgroupos.end() ) - { - // otherItem is a descendant subgroup of item - onlyGroups = false; - } - } - } - } + bool hasGroup = false; + bool hasMember = false; for( EDA_ITEM* item : selection ) { - BOARD_ITEM* board_item = static_cast( item ); - bool isFirstGroup = !seenFirstGroup && board_item->Type() == PCB_GROUP_T; + if( item->Type() == PCB_GROUP_T ) + hasGroup = true; - if( isFirstGroup ) - { - seenFirstGroup = true; - } - - if( allMembers.find( board_item ) == allMembers.end() ) - { - allGrouped = false; - } - else - { - anyGrouped = true; - - if( !isFirstGroup ) - anyGroupedExceptFirst = true; - } + if( item->GetParent() && item->GetParent()->Type() == PCB_GROUP_T ) + hasMember = true; } - legalOps.create = !anyGrouped; - legalOps.merge = hasGroup && !anyGroupedExceptFirst && ( selection.Size() > 1 ); - legalOps.ungroup = onlyGroups; - legalOps.removeItems = allGrouped; - legalOps.flatten = onlyGroups; - legalOps.enter = onlyGroups && selection.Size() == 1; + GroupLegalOpsField legalOps; + + legalOps.create = true; + legalOps.removeItems = hasMember; + legalOps.ungroup = hasGroup; + legalOps.enter = hasGroup && selection.Size() == 1; + return legalOps; } diff --git a/pcbnew/board.h b/pcbnew/board.h index 3497db6279..94ff3c15d1 100644 --- a/pcbnew/board.h +++ b/pcbnew/board.h @@ -1139,10 +1139,8 @@ public: struct GroupLegalOpsField { bool create : 1; - bool merge : 1; bool ungroup : 1; bool removeItems : 1; - bool flatten : 1; bool enter : 1; }; diff --git a/pcbnew/dialogs/dialog_group_properties.cpp b/pcbnew/dialogs/dialog_group_properties.cpp index 79d5a0f61b..5c9c693f19 100644 --- a/pcbnew/dialogs/dialog_group_properties.cpp +++ b/pcbnew/dialogs/dialog_group_properties.cpp @@ -54,6 +54,13 @@ DIALOG_GROUP_PROPERTIES::DIALOG_GROUP_PROPERTIES( PCB_BASE_EDIT_FRAME* aParent, } +DIALOG_GROUP_PROPERTIES::~DIALOG_GROUP_PROPERTIES() +{ + m_brdEditor->FocusOnItem( nullptr ); + m_brdEditor->GetCanvas()->Refresh(); +} + + bool DIALOG_GROUP_PROPERTIES::TransferDataToWindow() { // Don't do anything here; it gets called every time we re-show the dialog after @@ -67,8 +74,27 @@ bool DIALOG_GROUP_PROPERTIES::TransferDataFromWindow() BOARD_COMMIT commit( m_brdEditor ); commit.Modify( m_group ); - m_group->SetName( m_nameCtrl->GetValue() ); + 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 ) ); + PCB_GROUP* existingGroup = item->GetParentGroup(); + + if( existingGroup != m_group ) + { + commit.Modify( item ); + + if( existingGroup ) + commit.Modify( existingGroup ); + } + } + + m_group->SetName( m_nameCtrl->GetValue() ); m_toolMgr->RunAction( PCB_ACTIONS::selectionClear, true ); m_group->RemoveAll(); @@ -76,20 +102,12 @@ bool DIALOG_GROUP_PROPERTIES::TransferDataFromWindow() for( size_t ii = 0; ii < m_membersList->GetCount(); ++ii ) { BOARD_ITEM* item = static_cast( m_membersList->GetClientData( ii ) ); - PCB_GROUP* existingGroup = item->GetParentGroup(); - - if( existingGroup ) - { - commit.Modify( existingGroup ); - existingGroup->RemoveItem( item ); - } - m_group->AddItem( item ); } - commit.Push( _( "Modified group" ) ); m_toolMgr->RunAction( PCB_ACTIONS::selectItem, true, m_group ); + commit.Push( _( "Modified group" ) ); return true; } @@ -138,6 +156,9 @@ void DIALOG_GROUP_PROPERTIES::OnRemoveMember( wxCommandEvent& event ) if( selected >= 0 ) m_membersList->Delete( selected ); + + m_brdEditor->FocusOnItem( nullptr ); + m_brdEditor->GetCanvas()->Refresh(); } diff --git a/pcbnew/dialogs/dialog_group_properties.h b/pcbnew/dialogs/dialog_group_properties.h index 420824e9d9..a12e347463 100644 --- a/pcbnew/dialogs/dialog_group_properties.h +++ b/pcbnew/dialogs/dialog_group_properties.h @@ -40,7 +40,7 @@ private: public: DIALOG_GROUP_PROPERTIES( PCB_BASE_EDIT_FRAME* aParent, PCB_GROUP* aTarget ); - ~DIALOG_GROUP_PROPERTIES() { } + ~DIALOG_GROUP_PROPERTIES(); void OnMemberSelected( wxCommandEvent& event ) override; void OnAddMember( wxCommandEvent& event ) override; diff --git a/pcbnew/pcb_group.cpp b/pcbnew/pcb_group.cpp index 69566df88e..2b8ab7b760 100644 --- a/pcbnew/pcb_group.cpp +++ b/pcbnew/pcb_group.cpp @@ -85,6 +85,12 @@ PCB_GROUP* PCB_GROUP::TopLevelGroup( BOARD_ITEM* item, PCB_GROUP* scope ) bool PCB_GROUP::WithinScope( BOARD_ITEM* item, PCB_GROUP* scope ) { + for( PCB_GROUP* parent = item->GetParentGroup(); parent; parent = parent->GetParentGroup() ) + { + if( parent == scope ) + return true; + } + if( item->GetParent() && item->GetParent()->Type() == PCB_FOOTPRINT_T ) item = item->GetParent(); diff --git a/pcbnew/pcb_painter.cpp b/pcbnew/pcb_painter.cpp index e1b1e8d2b5..9108383475 100644 --- a/pcbnew/pcb_painter.cpp +++ b/pcbnew/pcb_painter.cpp @@ -1445,9 +1445,18 @@ void PCB_PAINTER::draw( const PCB_GROUP* aGroup, int aLayer ) { if( aLayer == LAYER_ANCHOR ) { - // Draw only when we're selected on our own - if( !aGroup->IsSelected() || ( aGroup->GetParent() && aGroup->GetParent()->IsSelected() ) ) + if( aGroup->IsSelected() && !( aGroup->GetParent() && aGroup->GetParent()->IsSelected() ) ) + { + // Selected on our own; draw enclosing box + } + else if( aGroup->IsEntered() ) + { + // Entered group; draw enclosing box + } + else + { return; + } const COLOR4D color = m_pcbSettings.GetColor( aGroup, LAYER_ANCHOR ); diff --git a/pcbnew/pcb_view.cpp b/pcbnew/pcb_view.cpp index 3dc269c67a..723a270750 100644 --- a/pcbnew/pcb_view.cpp +++ b/pcbnew/pcb_view.cpp @@ -98,11 +98,20 @@ void PCB_VIEW::Update( const KIGFX::VIEW_ITEM* aItem, int aUpdateFlags ) const if( item->Type() == PCB_FOOTPRINT_T ) { const FOOTPRINT* footprint = static_cast( item ); - - footprint->RunOnChildren( [this, aUpdateFlags]( BOARD_ITEM* aModItem ) - { - VIEW::Update( aModItem, aUpdateFlags ); - } ); + footprint->RunOnChildren( + [this, aUpdateFlags]( BOARD_ITEM* aModItem ) + { + VIEW::Update( aModItem, aUpdateFlags ); + } ); + } + else if( item->Type() == PCB_GROUP_T ) + { + const PCB_GROUP* group = static_cast( item ); + group->RunOnChildren( + [this, aUpdateFlags]( BOARD_ITEM* aModItem ) + { + VIEW::Update( aModItem, aUpdateFlags ); + } ); } VIEW::Update( item, aUpdateFlags ); diff --git a/pcbnew/tools/pcb_selection.cpp b/pcbnew/tools/pcb_selection.cpp index 7b448e7ef6..afb08e8cda 100644 --- a/pcbnew/tools/pcb_selection.cpp +++ b/pcbnew/tools/pcb_selection.cpp @@ -114,29 +114,6 @@ const KIGFX::VIEW_GROUP::ITEMS PCB_SELECTION::updateDrawList() const for( EDA_ITEM* item : m_items ) addItem( item ); -#if 0 - for( EDA_ITEM* item : m_items ) - { - items.push_back( item ); - - if( item->Type() == PCB_FOOTPRINT_T ) - { - FOOTPRINT* footprint = static_cast( item ); - footprint->RunOnChildren( [&]( BOARD_ITEM* bitem ) - { - items.push_back( bitem ); - } ); - } - else if( item->Type() == PCB_GROUP_T ) - { - PCB_GROUP* group = static_cast( item ); - group->RunOnChildren( [&]( BOARD_ITEM* bitem ) - { - items.push_back( bitem ); - } ); - } - } -#endif return items; } diff --git a/pcbnew/tools/pcb_selection_tool.cpp b/pcbnew/tools/pcb_selection_tool.cpp index 92a4ec2eb8..0acf352635 100644 --- a/pcbnew/tools/pcb_selection_tool.cpp +++ b/pcbnew/tools/pcb_selection_tool.cpp @@ -455,6 +455,7 @@ void PCB_SELECTION_TOOL::EnterGroup() ClearSelection(); m_enteredGroup = aGroup; + m_enteredGroup->SetFlags( ENTERED ); m_enteredGroup->RunOnChildren( [&]( BOARD_ITEM* titem ) { select( titem ); @@ -470,6 +471,7 @@ void PCB_SELECTION_TOOL::ExitGroup( bool aSelectGroup ) if( m_enteredGroup == nullptr ) return; + m_enteredGroup->ClearFlags( ENTERED ); ClearSelection(); if( aSelectGroup ) @@ -1666,16 +1668,22 @@ void PCB_SELECTION_TOOL::RebuildSelection() { EDA_ITEM* parent = item->GetParent(); - // Flags on footprint children might be set only because the parent is - // selected. - if( parent && parent->Type() == PCB_FOOTPRINT_T && parent->IsSelected() ) + // Let selected parents handle their children. + if( parent && parent->IsSelected() ) return SEARCH_RESULT::CONTINUE; highlight( (BOARD_ITEM*) item, SELECTED, &m_selection ); } if( item == m_enteredGroup ) + { + item->SetFlags( ENTERED ); enteredGroupFound = true; + } + else + { + item->ClearFlags( ENTERED ); + } return SEARCH_RESULT::CONTINUE; }; @@ -2071,9 +2079,7 @@ bool PCB_SELECTION_TOOL::Selectable( const BOARD_ITEM* aItem, bool checkVisibili void PCB_SELECTION_TOOL::select( BOARD_ITEM* aItem ) { if( aItem->IsSelected() ) - { return; - } if( aItem->Type() == PCB_PAD_T ) { @@ -2095,8 +2101,10 @@ void PCB_SELECTION_TOOL::unselect( BOARD_ITEM* aItem ) void PCB_SELECTION_TOOL::highlight( BOARD_ITEM* aItem, int aMode, PCB_SELECTION* aGroup ) { - highlightInternal( aItem, aMode, aGroup, false ); + if( aGroup ) + aGroup->Add( aItem ); + highlightInternal( aItem, aMode, aGroup != nullptr ); view()->Update( aItem, KIGFX::REPAINT ); // Many selections are very temporal and updating the display each time just @@ -2106,31 +2114,22 @@ void PCB_SELECTION_TOOL::highlight( BOARD_ITEM* aItem, int aMode, PCB_SELECTION* } -void PCB_SELECTION_TOOL::highlightInternal( BOARD_ITEM* aItem, int aMode, - PCB_SELECTION* aSelectionViewGroup, bool isChild ) +void PCB_SELECTION_TOOL::highlightInternal( BOARD_ITEM* aItem, int aMode, bool aUsingOverlay ) { if( aMode == SELECTED ) aItem->SetSelected(); else if( aMode == BRIGHTENED ) aItem->SetBrightened(); - if( aSelectionViewGroup ) - { - // Hide the original item, so it is shown only on overlay - view()->Hide( aItem, true ); + if( aUsingOverlay ) + view()->Hide( aItem, true ); // Hide the original item, so it is shown only on overlay - if( !isChild || aMode == BRIGHTENED ) - aSelectionViewGroup->Add( aItem ); - } - - // footprints are treated in a special way - when they are highlighted, we have to highlight - // all the parts that make the footprint, not the footprint itself if( aItem->Type() == PCB_FOOTPRINT_T ) { static_cast( aItem )->RunOnChildren( [&]( BOARD_ITEM* aChild ) { - highlightInternal( aChild, aMode, aSelectionViewGroup, true ); + highlightInternal( aChild, aMode, aUsingOverlay ); } ); } else if( aItem->Type() == PCB_GROUP_T ) @@ -2138,7 +2137,7 @@ void PCB_SELECTION_TOOL::highlightInternal( BOARD_ITEM* aItem, int aMode, static_cast( aItem )->RunOnChildren( [&]( BOARD_ITEM* aChild ) { - highlightInternal( aChild, aMode, aSelectionViewGroup, true ); + highlightInternal( aChild, aMode, aUsingOverlay ); } ); } } @@ -2146,8 +2145,10 @@ void PCB_SELECTION_TOOL::highlightInternal( BOARD_ITEM* aItem, int aMode, void PCB_SELECTION_TOOL::unhighlight( BOARD_ITEM* aItem, int aMode, PCB_SELECTION* aGroup ) { - unhighlightInternal( aItem, aMode, aGroup, false ); + if( aGroup ) + aGroup->Remove( aItem ); + unhighlightInternal( aItem, aMode, aGroup != nullptr ); view()->Update( aItem, KIGFX::REPAINT ); // Many selections are very temporal and updating the display each time just @@ -2157,35 +2158,22 @@ void PCB_SELECTION_TOOL::unhighlight( BOARD_ITEM* aItem, int aMode, PCB_SELECTIO } -void PCB_SELECTION_TOOL::unhighlightInternal( BOARD_ITEM* aItem, int aMode, - PCB_SELECTION* aSelectionViewGroup, bool isChild ) +void PCB_SELECTION_TOOL::unhighlightInternal( BOARD_ITEM* aItem, int aMode, bool aUsingOverlay ) { if( aMode == SELECTED ) aItem->ClearSelected(); else if( aMode == BRIGHTENED ) aItem->ClearBrightened(); - if( aSelectionViewGroup ) - { - aSelectionViewGroup->Remove( aItem ); + if( aUsingOverlay ) + view()->Hide( aItem, false ); // // Restore original item visibility - // Restore original item visibility - view()->Hide( aItem, false ); - - // N.B. if we clear the selection flag for sub-elements, we need to also - // remove the element from the selection group (if it exists) - if( isChild ) - view()->Update( aItem, KIGFX::REPAINT ); - } - - // footprints are treated in a special way - when they are highlighted, we have to - // highlight all the parts that make the footprint, not the footprint itself if( aItem->Type() == PCB_FOOTPRINT_T ) { static_cast( aItem )->RunOnChildren( [&]( BOARD_ITEM* aChild ) { - unhighlightInternal( aChild, aMode, aSelectionViewGroup, true ); + unhighlightInternal( aChild, aMode, aUsingOverlay ); } ); } else if( aItem->Type() == PCB_GROUP_T ) @@ -2193,7 +2181,7 @@ void PCB_SELECTION_TOOL::unhighlightInternal( BOARD_ITEM* aItem, int aMode, static_cast( aItem )->RunOnChildren( [&]( BOARD_ITEM* aChild ) { - unhighlightInternal( aChild, aMode, aSelectionViewGroup, true ); + unhighlightInternal( aChild, aMode, aUsingOverlay ); } ); } } @@ -2205,7 +2193,7 @@ bool PCB_SELECTION_TOOL::selectionContains( const VECTOR2I& aPoint ) const VECTOR2I margin = getView()->ToWorld( VECTOR2I( GRIP_MARGIN, GRIP_MARGIN ), false ); // Check if the point is located within any of the currently selected items bounding boxes - for( auto item : m_selection ) + for( EDA_ITEM* item : m_selection ) { BOX2I itemBox = item->ViewBBox(); itemBox.Inflate( margin.x, margin.y ); // Give some margin for gripping an item diff --git a/pcbnew/tools/pcb_selection_tool.h b/pcbnew/tools/pcb_selection_tool.h index 5f71b63d92..73b05e443d 100644 --- a/pcbnew/tools/pcb_selection_tool.h +++ b/pcbnew/tools/pcb_selection_tool.h @@ -366,11 +366,9 @@ private: const GENERAL_COLLECTORS_GUIDE getCollectorsGuide() const; private: - void highlightInternal( BOARD_ITEM* aItem, int aHighlightMode, - PCB_SELECTION* aSelectionViewGroup, bool isChild); + void highlightInternal( BOARD_ITEM* aItem, int aHighlightMode, bool aUsingOverlay ); - void unhighlightInternal( BOARD_ITEM* aItem, int aHighlightMode, - PCB_SELECTION* aSelectionViewGroup, bool isChild); + void unhighlightInternal( BOARD_ITEM* aItem, int aHighlightMode, bool aUsingOverlay ); PCB_BASE_FRAME* m_frame; // Pointer to the parent frame PCB_SELECTION m_selection; // Current state of selection diff --git a/qa/qa_utils/mocks.cpp b/qa/qa_utils/mocks.cpp index 93f04b86cc..9b92ccb76b 100644 --- a/qa/qa_utils/mocks.cpp +++ b/qa/qa_utils/mocks.cpp @@ -791,7 +791,7 @@ void PCB_SELECTION_TOOL::highlight( BOARD_ITEM* aItem, int aMode, PCB_SELECTION* } -void PCB_SELECTION_TOOL::highlightInternal( BOARD_ITEM* aItem, int aMode, PCB_SELECTION* aGroup, bool isChild ) +void PCB_SELECTION_TOOL::highlightInternal( BOARD_ITEM* aItem, int aMode, bool aUsingOverlay ) { } @@ -803,7 +803,7 @@ void PCB_SELECTION_TOOL::unhighlight( BOARD_ITEM* aItem, int aMode, PCB_SELECTIO } -void PCB_SELECTION_TOOL::unhighlightInternal( BOARD_ITEM* aItem, int aMode, PCB_SELECTION* aGroup, bool isChild ) +void PCB_SELECTION_TOOL::unhighlightInternal( BOARD_ITEM* aItem, int aMode, bool aUsingOverlay ) { }