Don't leave stale pointers in groups when exchanging modules.

Also simplifies groups so that other areas of code that have to know
about them at least don't have to know as much. One of the simplifications
is to not worry so much about empty groups until save time; others are in
the access logic to parent groups.

Also simplifies user model slightly by removing Merge and Flatten
(which are just ungroup/group and ungroup/ungroup/.../group).

Also allows multiple groups to have the same name.  This is useful when
using groups for a classification system.

Fixes https://gitlab.com/kicad/code/kicad/issues/5788
This commit is contained in:
Jeff Young 2020-09-25 18:37:03 +01:00
parent d1322e7d1d
commit 6fde9ea8a5
19 changed files with 157 additions and 613 deletions

View File

@ -90,25 +90,18 @@ class BOARD_ITEM : public EDA_ITEM
{
protected:
PCB_LAYER_ID m_Layer;
KIID m_groupUuid;
PCB_GROUP* m_group;
public:
BOARD_ITEM( BOARD_ITEM* aParent, KICAD_T idtype )
: EDA_ITEM( aParent, idtype ),
m_Layer( F_Cu ),
m_groupUuid( 0 )
BOARD_ITEM( BOARD_ITEM* aParent, KICAD_T idtype ) :
EDA_ITEM( aParent, idtype ),
m_Layer( F_Cu ),
m_group( nullptr )
{
}
void SetGroup( PCB_GROUP* aGroup );
PCB_GROUP* GetGroup() const;
/**
* Test if this item is inside a group.
*
* @return true if inside a group
*/
bool IsInGroup() const { return m_groupUuid != niluuid; }
void SetParentGroup( PCB_GROUP* aGroup ) { m_group = aGroup; }
PCB_GROUP* GetParentGroup() const { return m_group; }
// Do not create a copy constructor & operator=.
// The ones generated by the compiler are adequate.

View File

@ -45,8 +45,6 @@ namespace KIGFX
class VIEW;
}
typedef std::unordered_set<BOARD_ITEM*> BOARD_ITEM_SET;
/**
* PCB_GROUP is a set of BOARD_ITEMs (i.e., without duplicates)
*/
@ -68,7 +66,7 @@ public:
wxString GetName() const { return m_name; }
void SetName( wxString aName ) { m_name = aName; }
const BOARD_ITEM_SET& GetItems() const
const std::unordered_set<BOARD_ITEM*>& GetItems() const
{
return m_items;
}
@ -196,8 +194,8 @@ public:
void RunOnDescendants( const std::function<void( BOARD_ITEM* )>& aFunction );
private:
BOARD_ITEM_SET m_items; // Members of the group
wxString m_name; // Optional group name
std::unordered_set<BOARD_ITEM*> m_items; // Members of the group
wxString m_name; // Optional group name
};
#endif // CLASS_PCB_GROUP_H_

View File

@ -726,7 +726,7 @@ void BOARD::DeleteMARKERs( bool aWarningsAndErrors, bool aExclusions )
}
BOARD_ITEM* BOARD::GetItem( const KIID& aID )
BOARD_ITEM* BOARD::GetItem( const KIID& aID ) const
{
if( aID == niluuid )
return nullptr;
@ -786,7 +786,7 @@ BOARD_ITEM* BOARD::GetItem( const KIID& aID )
}
if( m_Uuid == aID )
return this;
return const_cast<BOARD*>( this );
// Not found; weak reference has been deleted.
return DELETED_BOARD_ITEM::GetInstance();
@ -1967,52 +1967,17 @@ void BOARD::HighLightON( bool aValue )
}
PCB_GROUP* BOARD::TopLevelGroup( BOARD_ITEM* item, PCB_GROUP* scope )
PCB_GROUP* BOARD::TopLevelGroup( BOARD_ITEM* item, PCB_GROUP* scope )
{
PCB_GROUP* candidate = NULL;
bool foundParent;
PCB_GROUP* candidate = item->GetParentGroup();
do
{
foundParent = false;
for( PCB_GROUP* group : m_groups )
{
BOARD_ITEM* toFind = ( candidate == NULL ) ? item : candidate;
if( group->GetItems().find( toFind ) != group->GetItems().end() )
{
if( scope == group && candidate != NULL )
{
wxCHECK( candidate->Type() == PCB_GROUP_T, NULL );
return candidate;
}
candidate = group;
foundParent = true;
}
}
} while( foundParent );
if( scope != NULL )
{
return NULL;
}
while( candidate && candidate->GetParentGroup() && candidate->GetParentGroup() != scope )
candidate = candidate->GetParentGroup();
return candidate;
}
PCB_GROUP* BOARD::ParentGroup( BOARD_ITEM* item )
{
for( PCB_GROUP* group : m_groups )
{
if( group->GetItems().find( item ) != group->GetItems().end() )
return group;
}
return NULL;
}
wxString BOARD::GroupsSanityCheck( bool repair )
{
if( repair )
@ -2026,116 +1991,6 @@ wxString BOARD::GroupsSanityCheck( bool repair )
wxString BOARD::GroupsSanityCheckInternal( bool repair )
{
BOARD& board = *this;
GROUPS& groups = board.Groups();
std::unordered_set<wxString> groupNames;
std::unordered_set<wxString> allMembers;
// To help with cycle detection, construct a mapping from
// each group to the at most single parent group it could belong to.
std::vector<int> parentGroupIdx( groups.size(), -1 );
for( size_t idx = 0; idx < groups.size(); idx++ )
{
PCB_GROUP& group = *( groups[idx] );
BOARD_ITEM* testItem = board.GetItem( group.m_Uuid );
if( testItem != groups[idx] )
{
if( repair )
board.Groups().erase( board.Groups().begin() + idx );
return wxString::Format( _( "Group Uuid %s maps to 2 different BOARD_ITEMS: %p and %p" ),
group.m_Uuid.AsString(),
testItem, groups[idx] );
}
// Non-blank group names must be unique
if( !group.GetName().empty() )
{
if( groupNames.find( group.GetName() ) != groupNames.end() )
{
if( repair )
group.SetName( group.GetName() + "-" + group.m_Uuid.AsString() );
return wxString::Format( _( "Two groups of identical name: %s" ), group.GetName() );
}
wxCHECK( groupNames.insert( group.GetName() ).second == true,
"Insert failed of new group" );
}
for( BOARD_ITEM* member : group.GetItems() )
{
BOARD_ITEM* item = board.GetItem( member->m_Uuid );
if( ( item == nullptr ) || ( item->Type() == NOT_USED ) )
{
if( repair )
group.RemoveItem( member );
return wxString::Format( _( "Group %s contains deleted item %s" ),
group.m_Uuid.AsString(),
member->m_Uuid.AsString() );
}
if( item != member )
{
if( repair )
group.RemoveItem( member );
return wxString::Format( _( "Uuid %s maps to 2 different BOARD_ITEMS: %s %p %s and %p %s" ),
member->m_Uuid.AsString(),
item->m_Uuid.AsString(),
item,
item->GetSelectMenuText( EDA_UNITS::MILLIMETRES ),
member,
member->GetSelectMenuText( EDA_UNITS::MILLIMETRES )
);
}
if( allMembers.find( member->m_Uuid.AsString() ) != allMembers.end() )
{
if( repair )
group.RemoveItem( member );
return wxString::Format(
_( "BOARD_ITEM %s appears multiple times in groups (either in the "
"same group or in multiple groups) " ),
item->m_Uuid.AsString() );
}
wxCHECK( allMembers.insert( member->m_Uuid.AsString() ).second == true,
"Insert failed of new member" );
if( item->Type() == PCB_GROUP_T )
{
// Could speed up with a map structure if needed
size_t childIdx = std::distance(
groups.begin(), std::find( groups.begin(), groups.end(), item ) );
// This check of childIdx should never fail, because if a group
// is not found in the groups list, then the board.GetItem()
// check above should have failed.
wxCHECK( childIdx < groups.size(),
wxString::Format( "Group %s not found in groups list",
item->m_Uuid.AsString() ) );
wxCHECK( parentGroupIdx[childIdx] == -1,
wxString::Format( "Duplicate group despite allMembers check previously: %s",
item->m_Uuid.AsString() ) );
parentGroupIdx[childIdx] = idx;
}
}
if( group.GetItems().size() == 0 )
{
if( repair )
board.Groups().erase( board.Groups().begin() + idx );
return wxString::Format( _( "Group must have at least one member: %s" ),
group.m_Uuid.AsString() );
}
}
// Cycle detection
//
// Each group has at most one parent group.
@ -2147,46 +2002,43 @@ wxString BOARD::GroupsSanityCheckInternal( bool repair )
// There may be extra time taken due to the container access calls and iterators.
//
// Groups we know are cycle free
std::unordered_set<int> knownCycleFreeGroups;
std::unordered_set<PCB_GROUP*> knownCycleFreeGroups;
// Groups in the current chain we're exploring.
std::unordered_set<int> currentChainGroups;
std::unordered_set<PCB_GROUP*> currentChainGroups;
// Groups we haven't checked yet.
std::unordered_set<int> toCheckGroups;
std::unordered_set<PCB_GROUP*> toCheckGroups;
// Initialize set of groups to check that could participate in a cycle.
for( size_t idx = 0; idx < groups.size(); idx++ )
{
wxCHECK( toCheckGroups.insert( idx ).second == true, "Insert of ints failed" );
}
for( PCB_GROUP* group : Groups() )
toCheckGroups.insert( group);
while( !toCheckGroups.empty() )
{
currentChainGroups.clear();
int currIdx = *toCheckGroups.begin();
PCB_GROUP* group = *toCheckGroups.begin();
while( true )
{
if( currentChainGroups.find( currIdx ) != currentChainGroups.end() )
if( currentChainGroups.find( group ) != currentChainGroups.end() )
{
if( repair )
board.Groups().erase( board.Groups().begin() + currIdx );
Remove( group );
return "Cycle detected in group membership";
}
else if( knownCycleFreeGroups.find( currIdx ) != knownCycleFreeGroups.end() )
else if( knownCycleFreeGroups.find( group ) != knownCycleFreeGroups.end() )
{
// Parent is a group we know does not lead to a cycle
break;
}
wxCHECK( currentChainGroups.insert( currIdx ).second == true,
"Insert of new group to check failed" );
currentChainGroups.insert( group );
// We haven't visited currIdx yet, so it must be in toCheckGroups
wxCHECK( toCheckGroups.erase( currIdx ) == 1,
"Erase of idx for group just checked failed" );
currIdx = parentGroupIdx[currIdx];
toCheckGroups.erase( group );
if( currIdx == -1 )
group = group->GetParentGroup();
if( !group )
{
// end of chain and no cycles found in this chain
break;
@ -2208,16 +2060,13 @@ BOARD::GroupLegalOpsField BOARD::GroupLegalOps( const PCBNEW_SELECTION& selectio
GroupLegalOpsField legalOps = { false, false, false, false, false, false };
std::unordered_set<const BOARD_ITEM*> allMembers;
for( const PCB_GROUP* grp : m_groups )
{
for( const BOARD_ITEM* member : grp->GetItems() )
{
// Item can be member of at most one group.
wxCHECK( allMembers.insert( member ).second == true, legalOps );
}
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 );
@ -2286,9 +2135,7 @@ BOARD::GroupLegalOpsField BOARD::GroupLegalOps( const PCBNEW_SELECTION& selectio
anyGrouped = true;
if( !isFirstGroup )
{
anyGroupedExceptFirst = true;
}
}
}
@ -2300,81 +2147,3 @@ BOARD::GroupLegalOpsField BOARD::GroupLegalOps( const PCBNEW_SELECTION& selectio
legalOps.enter = onlyGroups && selection.Size() == 1;
return legalOps;
}
void BOARD::GroupRemoveItems( const PCBNEW_SELECTION& selection, BOARD_COMMIT* commit )
{
std::unordered_set<BOARD_ITEM*> emptyGroups;
std::unordered_set<PCB_GROUP*> emptyGroupParents;
// groups who have had children removed, either items or empty groups.
std::unordered_set<PCB_GROUP*> itemParents;
std::unordered_set<BOARD_ITEM*> itemsToRemove;
for( EDA_ITEM* item : selection )
{
BOARD_ITEM* board_item = static_cast<BOARD_ITEM*>( item );
itemsToRemove.insert( board_item );
}
for( BOARD_ITEM* item : itemsToRemove )
{
PCB_GROUP* parentGroup = ParentGroup( item );
itemParents.insert( parentGroup );
while( parentGroup != nullptr )
{
// Test if removing this item would make parent empty
bool allRemoved = true;
for( BOARD_ITEM* grpItem : parentGroup->GetItems() )
{
if( ( itemsToRemove.find( grpItem ) == itemsToRemove.end() )
&& ( emptyGroups.find( grpItem ) == emptyGroups.end() ) )
{
allRemoved = false;
}
}
if( allRemoved )
{
emptyGroups.insert( parentGroup );
parentGroup = ParentGroup( parentGroup );
if( parentGroup != nullptr )
itemParents.insert( parentGroup );
}
else
{
break;
}
}
}
// Items themselves are removed outside the context of this function
// First let's check the parents of items that are no empty
for( PCB_GROUP* grp : itemParents )
{
if( emptyGroups.find( grp ) == emptyGroups.end() )
{
commit->Modify( grp );
BOARD_ITEM_SET members = grp->GetItems();
bool removedSomething = false;
for( BOARD_ITEM* member : members )
{
if( ( itemsToRemove.find( member ) != itemsToRemove.end() )
|| ( emptyGroups.find( member ) != emptyGroups.end() ) )
{
grp->RemoveItem( member );
removedSomething = true;
}
}
wxCHECK_RET( removedSomething, "Item to be removed not found in it's parent group" );
}
}
for( BOARD_ITEM* grp : emptyGroups )
commit->Remove( grp );
}

View File

@ -248,6 +248,7 @@ public:
const MODULES& Modules() const { return m_modules; }
DRAWINGS& Drawings() { return m_drawings; }
const DRAWINGS& Drawings() const { return m_drawings; }
ZONE_CONTAINERS& Zones() { return m_zones; }
const ZONE_CONTAINERS& Zones() const { return m_zones; }
@ -325,7 +326,7 @@ public:
* @return null if aID is null. Returns an object of Type() == NOT_USED if
* the aID is not found.
*/
BOARD_ITEM* GetItem( const KIID& aID );
BOARD_ITEM* GetItem( const KIID& aID ) const;
void FillItemMap( std::map<KIID, EDA_ITEM*>& aMap );
@ -1105,20 +1106,6 @@ public:
*/
PCB_GROUP* TopLevelGroup( BOARD_ITEM* item, PCB_GROUP* scope );
/*
* @return The group containing item as a child, or NULL if there is no
* such group.
*/
PCB_GROUP* ParentGroup( BOARD_ITEM* item );
/*
* Given a selection of items, remove them from their groups and also
* recursively remove empty groups that result.
*/
void GroupRemoveItems( const PCBNEW_SELECTION& selection, BOARD_COMMIT* commit );
struct GroupLegalOpsField
{
bool create : 1;

View File

@ -59,21 +59,6 @@ BOARD* BOARD_ITEM::GetBoard() const
}
PCB_GROUP* BOARD_ITEM::GetGroup() const
{
if( IsInGroup() && GetBoard() )
return dynamic_cast<PCB_GROUP*>( GetBoard()->GetItem( m_groupUuid ) );
return nullptr;
}
void BOARD_ITEM::SetGroup( PCB_GROUP* aGroup )
{
m_groupUuid = aGroup ? aGroup->m_Uuid : niluuid;
}
wxString BOARD_ITEM::GetLayerName() const
{
BOARD* board = GetBoard();

View File

@ -37,11 +37,11 @@ PCB_GROUP::PCB_GROUP( BOARD*aParent ) : BOARD_ITEM( aParent, PCB_GROUP_T )
bool PCB_GROUP::AddItem( BOARD_ITEM* aItem )
{
// Items can only be in one group at a time
if( aItem->IsInGroup() )
return false;
if( aItem->GetParentGroup() )
aItem->GetParentGroup()->RemoveItem( aItem );
m_items.insert( aItem );
aItem->SetGroup( this );
aItem->SetParentGroup( this );
return true;
}
@ -51,7 +51,7 @@ bool PCB_GROUP::RemoveItem( BOARD_ITEM* aItem )
// Only clear the item's group field if it was inside this group
if( m_items.erase( aItem ) == 1 )
{
aItem->SetGroup( nullptr );
aItem->SetParentGroup( nullptr );
return true;
}
@ -62,7 +62,7 @@ bool PCB_GROUP::RemoveItem( BOARD_ITEM* aItem )
void PCB_GROUP::RemoveAll()
{
for( BOARD_ITEM* item : m_items )
item->SetGroup( nullptr );
item->SetParentGroup( nullptr );
m_items.clear();
}

View File

@ -473,30 +473,38 @@ TEXTE_MODULE* getMatchingTextItem( TEXTE_MODULE* aRefItem, MODULE* aModule )
}
void PCB_EDIT_FRAME::Exchange_Module( MODULE* aSrc, MODULE* aDest, BOARD_COMMIT& aCommit,
void PCB_EDIT_FRAME::Exchange_Module( MODULE* aExisting, MODULE* aNew, BOARD_COMMIT& aCommit,
bool deleteExtraTexts, bool resetTextLayers,
bool resetTextEffects, bool resetFabricationAttrs,
bool reset3DModels )
{
aDest->SetParent( GetBoard() );
PCB_GROUP* parentGroup = aExisting->GetParentGroup();
PlaceModule( aDest, false );
if( parentGroup )
{
parentGroup->RemoveItem( aExisting );
parentGroup->AddItem( aNew );
}
aNew->SetParent( GetBoard() );
PlaceModule( aNew, false );
// PlaceModule will move the module to the cursor position, which we don't want. Copy
// the original position across.
aDest->SetPosition( aSrc->GetPosition() );
aNew->SetPosition( aExisting->GetPosition() );
if( aDest->GetLayer() != aSrc->GetLayer() )
aDest->Flip( aDest->GetPosition(), m_Settings->m_FlipLeftRight );
if( aNew->GetLayer() != aExisting->GetLayer() )
aNew->Flip( aNew->GetPosition(), m_Settings->m_FlipLeftRight );
if( aDest->GetOrientation() != aSrc->GetOrientation() )
aDest->SetOrientation( aSrc->GetOrientation() );
if( aNew->GetOrientation() != aExisting->GetOrientation() )
aNew->SetOrientation( aExisting->GetOrientation() );
aDest->SetLocked( aSrc->IsLocked() );
aNew->SetLocked( aExisting->IsLocked() );
for( D_PAD* pad : aDest->Pads() )
for( D_PAD* pad : aNew->Pads() )
{
D_PAD* oldPad = aSrc->FindPadByName( pad->GetName() );
D_PAD* oldPad = aExisting->FindPadByName( pad->GetName() );
if( oldPad )
{
@ -507,51 +515,51 @@ void PCB_EDIT_FRAME::Exchange_Module( MODULE* aSrc, MODULE* aDest, BOARD_COMMIT&
}
// Copy reference
processTextItem( aSrc->Reference(), aDest->Reference(),
processTextItem( aExisting->Reference(), aNew->Reference(),
// never reset reference text
false,
resetTextLayers, resetTextEffects );
// Copy value
processTextItem( aSrc->Value(), aDest->Value(),
processTextItem( aExisting->Value(), aNew->Value(),
// reset value text only when it is a proxy for the footprint ID
// (cf replacing value "MountingHole-2.5mm" with "MountingHole-4.0mm")
aSrc->GetValue() == aSrc->GetFPID().GetLibItemName(),
aExisting->GetValue() == aExisting->GetFPID().GetLibItemName(),
resetTextLayers, resetTextEffects );
// Copy fields in accordance with the reset* flags
for( BOARD_ITEM* item : aSrc->GraphicalItems() )
for( BOARD_ITEM* item : aExisting->GraphicalItems() )
{
TEXTE_MODULE* srcItem = dyn_cast<TEXTE_MODULE*>( item );
if( srcItem )
{
TEXTE_MODULE* destItem = getMatchingTextItem( srcItem, aDest );
TEXTE_MODULE* destItem = getMatchingTextItem( srcItem, aNew );
if( destItem )
processTextItem( *srcItem, *destItem, false, resetTextLayers, resetTextEffects );
else if( !deleteExtraTexts )
aDest->Add( new TEXTE_MODULE( *srcItem ) );
aNew->Add( new TEXTE_MODULE( *srcItem ) );
}
}
if( !resetFabricationAttrs )
aDest->SetAttributes( aSrc->GetAttributes() );
aNew->SetAttributes( aExisting->GetAttributes() );
// Copy 3D model settings in accordance with the reset* flag
if( !reset3DModels )
aDest->Models() = aSrc->Models(); // Linked list of 3D models.
aNew->Models() = aExisting->Models(); // Linked list of 3D models.
// Updating other parameters
const_cast<KIID&>( aDest->m_Uuid ) = aSrc->m_Uuid;
aDest->SetProperties( aSrc->GetProperties() );
aDest->SetPath( aSrc->GetPath() );
aDest->CalculateBoundingBox();
const_cast<KIID&>( aNew->m_Uuid ) = aExisting->m_Uuid;
aNew->SetProperties( aExisting->GetProperties() );
aNew->SetPath( aExisting->GetPath() );
aNew->CalculateBoundingBox();
aCommit.Remove( aSrc );
aCommit.Add( aDest );
aCommit.Remove( aExisting );
aCommit.Add( aNew );
aDest->ClearFlags();
aNew->ClearFlags();
}

View File

@ -1552,16 +1552,25 @@ void PCB_IO::format( TEXTE_PCB* aText, int aNestLevel ) const
void PCB_IO::format( PCB_GROUP* aGroup, int aNestLevel ) const
{
m_out->Print( aNestLevel, "(group %s (id %s)\n", m_out->Quotew( aGroup->GetName() ).c_str(),
TO_UTF8( aGroup->m_Uuid.AsString() ) );
m_out->Print( aNestLevel + 2, "(members\n" );
std::set<BOARD_ITEM*, BOARD_ITEM::ptr_cmp> sorted_items( aGroup->GetItems().begin(),
aGroup->GetItems().end() );
// Don't write empty groups
if( aGroup->GetItems().empty() )
return;
for( const auto& item : sorted_items )
{
m_out->Print( aNestLevel + 4, "%s\n", TO_UTF8( item->m_Uuid.AsString() ) );
}
m_out->Print( aNestLevel, "(group %s (id %s)\n",
m_out->Quotew( aGroup->GetName() ).c_str(),
TO_UTF8( aGroup->m_Uuid.AsString() ) );
m_out->Print( aNestLevel + 1, "(members\n" );
wxArrayString memberIds;
for( BOARD_ITEM* member : aGroup->GetItems() )
memberIds.Add( member->m_Uuid.AsString() );
memberIds.Sort();
for( const wxString& memberId : memberIds )
m_out->Print( aNestLevel + 2, "%s\n", TO_UTF8( memberId ) );
m_out->Print( 0, " )\n" );
m_out->Print( aNestLevel, ")\n" );

View File

@ -770,11 +770,11 @@ public:
* Replaces OldModule by NewModule, using OldModule settings:
* position, orientation, pad netnames ...)
* OldModule is deleted or put in undo list.
* @param aSrc = footprint to replace
* @param aDest = footprint to put
* @param aExisting = footprint to replace
* @param aNew = footprint to put
* @param aCommit = commit that should store the changes
*/
void Exchange_Module( MODULE* aSrc, MODULE* aDest, BOARD_COMMIT& aCommit,
void Exchange_Module( MODULE* aExisting, MODULE* aNew, BOARD_COMMIT& aCommit,
bool deleteExtraTexts = true, bool resetTextLayers = true,
bool resetTextEffects = true, bool resetFabricationAttrs = true,
bool reset3DModels = true );

View File

@ -232,11 +232,10 @@ static void memberOf( LIBEVAL::CONTEXT* aCtx, void* self )
if( !item )
return;
BOARD* board = item->GetBoard();
PCB_GROUP* group = board->ParentGroup( item );
PCB_GROUP* group = item->GetParentGroup();
if( !group && item->GetParent() && item->GetParent()->Type() == PCB_MODULE_T )
group = board->ParentGroup( item->GetParent() );
group = item->GetParent()->GetParentGroup();
while( group )
{
@ -246,7 +245,7 @@ static void memberOf( LIBEVAL::CONTEXT* aCtx, void* self )
return;
}
group = board->ParentGroup( group );
group = group->GetParentGroup();
}
}

View File

@ -1089,6 +1089,11 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
for( EDA_ITEM* item : selectionCopy )
{
PCB_GROUP* group = static_cast<BOARD_ITEM*>( item )->GetParentGroup();
if( group )
group->RemoveItem( static_cast<BOARD_ITEM*>( item ) );
if( m_editModules )
{
m_commit->Remove( item );
@ -1201,33 +1206,17 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent )
}
}
// Figure out status of a group containing items to be removed. if entered
// group is not set in the selection tool, then any groups to be removed are
// removed in their entirety and so no empty group could remain. If entered
// group is set, then we could be removing all items of the entered group,
// in which case we need to remove the group itself.
// If the entered group has been emptied then leave it.
PCB_GROUP* enteredGroup = m_selectionTool->GetEnteredGroup();
if( enteredGroup != nullptr )
{
board()->GroupRemoveItems( removed, m_commit.get() );
if( m_commit->HasRemoveEntry( enteredGroup ) )
m_selectionTool->ExitGroup();
}
if( enteredGroup && enteredGroup->GetItems().empty() )
m_selectionTool->ExitGroup();
if( isCut )
m_commit->Push( _( "Cut" ) );
else
m_commit->Push( _( "Delete" ) );
if( enteredGroup != nullptr )
{
wxString check = board()->GroupsSanityCheck();
wxCHECK_MSG( check == wxEmptyString, 0,
_( "Remove of items in entered group resulted in inconsistent state: " )+ check );
}
if( !m_lockedSelected && !lockedItems.empty() )
{
///> Popup nag for deleting locked items

View File

@ -100,7 +100,7 @@ bool DIALOG_GROUP_PROPERTIES::TransferDataFromWindow()
for( size_t ii = 0; ii < m_membersList->GetCount(); ++ii )
{
BOARD_ITEM* item = static_cast<BOARD_ITEM*>( m_membersList->GetClientData( ii ) );
PCB_GROUP* existingGroup = item->GetGroup();
PCB_GROUP* existingGroup = item->GetParentGroup();
if( existingGroup )
{

View File

@ -647,11 +647,6 @@ TOOL_ACTION PCB_ACTIONS::groupCreate( "pcbnew.EditorControl.groupCreate",
_( "Group" ), _( "Add the selected items to a new group" ),
locked_xpm );
TOOL_ACTION PCB_ACTIONS::groupMerge( "pcbnew.EditorControl.groupMerge",
AS_GLOBAL, 0, "",
_( "Merge" ), "",
unlocked_xpm );
TOOL_ACTION PCB_ACTIONS::groupUngroup( "pcbnew.EditorControl.groupUngroup",
AS_GLOBAL, 0, "",
_( "Ungroup" ), "",
@ -662,11 +657,6 @@ TOOL_ACTION PCB_ACTIONS::groupRemoveItems( "pcbnew.EditorControl.groupRemoveItem
_( "Remove Items" ), _( "Remove items from group" ),
unlocked_xpm );
TOOL_ACTION PCB_ACTIONS::groupFlatten( "pcbnew.EditorControl.groupFlatten",
AS_GLOBAL, 0, "",
_( "Flatten Group" ), "",
unlocked_xpm );
TOOL_ACTION PCB_ACTIONS::groupEnter( "pcbnew.EditorControl.groupEnter",
AS_GLOBAL, 0, "",
_( "Enter Group" ), _( "Enter the group to edit items" ),

View File

@ -410,10 +410,8 @@ public:
// Grouping
static TOOL_ACTION groupCreate;
static TOOL_ACTION groupMerge;
static TOOL_ACTION groupUngroup;
static TOOL_ACTION groupRemoveItems;
static TOOL_ACTION groupFlatten;
static TOOL_ACTION groupEnter;
static TOOL_ACTION groupLeave;

View File

@ -129,9 +129,7 @@ public:
Add( PCB_ACTIONS::groupCreate );
Add( PCB_ACTIONS::groupUngroup );
Add( PCB_ACTIONS::groupMerge );
Add( PCB_ACTIONS::groupRemoveItems );
Add( PCB_ACTIONS::groupFlatten );
Add( PCB_ACTIONS::groupEnter );
}
@ -154,10 +152,8 @@ private:
BOARD::GroupLegalOpsField legalOps = board->GroupLegalOps( selection );
Enable( PCB_ACTIONS::groupCreate.GetUIId(), legalOps.create );
Enable( PCB_ACTIONS::groupMerge.GetUIId(), legalOps.merge );
Enable( PCB_ACTIONS::groupUngroup.GetUIId(), legalOps.ungroup );
Enable( PCB_ACTIONS::groupRemoveItems.GetUIId(), legalOps.removeItems );
Enable( PCB_ACTIONS::groupFlatten.GetUIId(), legalOps.flatten );
Enable( PCB_ACTIONS::groupEnter.GetUIId(), legalOps.enter );
}
};
@ -1006,7 +1002,7 @@ int PCB_EDITOR_CONTROL::modifyLockSelected( MODIFY_MODE aMode )
}
int PCB_EDITOR_CONTROL::GroupSelected( const TOOL_EVENT& aEvent )
int PCB_EDITOR_CONTROL::Group( const TOOL_EVENT& aEvent )
{
SELECTION_TOOL* selTool = m_toolMgr->GetTool<SELECTION_TOOL>();
const PCBNEW_SELECTION& selection = selTool->GetSelection();
@ -1015,23 +1011,18 @@ int PCB_EDITOR_CONTROL::GroupSelected( const TOOL_EVENT& aEvent )
if( selection.Empty() )
m_toolMgr->RunAction( PCB_ACTIONS::selectionCursor, true );
// why don't we have to update the selection after selectionCursor action?
PCB_GROUP* group = new PCB_GROUP( board );
for( EDA_ITEM* item : selection )
group->AddItem( static_cast<BOARD_ITEM*>( item ) );
commit.Add( group );
commit.Push( _( "GroupCreate" ) );
wxString check = board->GroupsSanityCheck();
wxCHECK_MSG( check == wxEmptyString, 0,wxT( "Group create resulted in inconsistent state: " ) + check );
commit.Push( _( "Group Items" ) );
selTool->ClearSelection();
selTool->select( group );
// Should I call PostEvent and onModify() ?
m_toolMgr->PostEvent( EVENTS::SelectedItemsModified );
m_frame->OnModify();
@ -1039,51 +1030,35 @@ int PCB_EDITOR_CONTROL::GroupSelected( const TOOL_EVENT& aEvent )
}
int PCB_EDITOR_CONTROL::GroupMergeSelected( const TOOL_EVENT& aEvent )
int PCB_EDITOR_CONTROL::Ungroup( const TOOL_EVENT& aEvent )
{
SELECTION_TOOL* selTool = m_toolMgr->GetTool<SELECTION_TOOL>();
const PCBNEW_SELECTION& selection = selTool->GetSelection();
BOARD* board = getModel<BOARD>();
BOARD_COMMIT commit( m_frame );
if( selection.Empty() )
m_toolMgr->RunAction( PCB_ACTIONS::selectionCursor, true );
// why don't we have to update the selection after selectionCursor action?
PCB_GROUP* firstGroup = NULL;
for( EDA_ITEM* item : selection )
{
BOARD_ITEM* board_item = static_cast<BOARD_ITEM*>( item );
if( firstGroup == NULL && board_item->Type() == PCB_GROUP_T )
{
firstGroup = static_cast<PCB_GROUP*>( board_item );
break;
}
}
// The group submenu update() call only enabled merge if there was a group
// in the selection.
wxCHECK_MSG( firstGroup != NULL, 0, "Group not found in selection though selection was checked" );
commit.Modify( firstGroup );
for( EDA_ITEM* item : selection )
{
BOARD_ITEM* board_item = static_cast<BOARD_ITEM*>( item );
if( board_item != firstGroup )
firstGroup->AddItem( board_item );
}
commit.Push( "GroupMerge" );
wxString check = board->GroupsSanityCheck();
wxCHECK_MSG( check == wxEmptyString, 0, wxT( "Group merge resulted in inconsistent state: " ) + check );
PCBNEW_SELECTION selCopy = selection;
selTool->ClearSelection();
selTool->select( firstGroup );
// Should I call PostEvent and onModify() ?
for( EDA_ITEM* item : selCopy )
{
PCB_GROUP* group = dynamic_cast<PCB_GROUP*>( item );
if( group )
{
for( BOARD_ITEM* member : group->GetItems() )
selTool->select( member );
group->RemoveAll();
commit.Remove( group );
}
}
commit.Push( "Ungroup Items" );
m_toolMgr->PostEvent( EVENTS::SelectedItemsModified );
m_frame->OnModify();
@ -1091,49 +1066,36 @@ int PCB_EDITOR_CONTROL::GroupMergeSelected( const TOOL_EVENT& aEvent )
}
int PCB_EDITOR_CONTROL::UngroupSelected( const TOOL_EVENT& aEvent )
int PCB_EDITOR_CONTROL::RemoveFromGroup( const TOOL_EVENT& aEvent )
{
SELECTION_TOOL* selTool = m_toolMgr->GetTool<SELECTION_TOOL>();
const PCBNEW_SELECTION& selection = selTool->GetSelection();
BOARD* board = getModel<BOARD>();
BOARD_COMMIT commit( m_frame );
std::unordered_set<BOARD_ITEM*> ungroupedItems;
if( selection.Empty() )
m_toolMgr->RunAction( PCB_ACTIONS::selectionCursor, true );
// why don't we have to update the selection after selectionCursor action?
std::map<PCB_GROUP*, std::vector<BOARD_ITEM*>> groupMap;
for( EDA_ITEM* item : selection )
{
BOARD_ITEM* board_item = static_cast<BOARD_ITEM*>( item );
BOARD_ITEM* boardItem = static_cast<BOARD_ITEM*>( item );
PCB_GROUP* group = boardItem->GetParentGroup();
wxCHECK_MSG( board_item->Type() == PCB_GROUP_T, 0,
"Selection for ungroup should only have groups in it - was checked." );
commit.Remove( board_item );
for( BOARD_ITEM* bItem : static_cast<PCB_GROUP*>( board_item )->GetItems() )
{
ungroupedItems.insert( bItem );
}
if( group )
groupMap[ group ].push_back( boardItem );
}
commit.Push( "GroupUngroup" );
wxString check = board->GroupsSanityCheck();
wxCHECK_MSG( check == wxEmptyString, 0, wxT( "Group merge resulted in inconsistent state: " ) + check );
selTool->ClearSelection();
for( BOARD_ITEM* item : ungroupedItems )
for( std::pair<PCB_GROUP*, std::vector<BOARD_ITEM*>> pair : groupMap )
{
// commit.Remove() on the group recursively removed children from the view.
// Add them back to the view
//getView()->Add( item );
commit.Modify( pair.first );
selTool->select( item );
for( BOARD_ITEM* item : pair.second )
pair.first->RemoveItem( item );
}
// Should I call PostEvent and onModify() ?
commit.Push( "Remove Group Items" );
m_toolMgr->PostEvent( EVENTS::SelectedItemsModified );
m_frame->OnModify();
@ -1141,107 +1103,7 @@ int PCB_EDITOR_CONTROL::UngroupSelected( const TOOL_EVENT& aEvent )
}
int PCB_EDITOR_CONTROL::GroupRemoveItemsSelected( const TOOL_EVENT& aEvent )
{
SELECTION_TOOL* selTool = m_toolMgr->GetTool<SELECTION_TOOL>();
const PCBNEW_SELECTION& selection = selTool->GetSelection();
BOARD* board = getModel<BOARD>();
BOARD_COMMIT commit( m_frame );
if( selection.Empty() )
m_toolMgr->RunAction( PCB_ACTIONS::selectionCursor, true );
// why don't we have to update the selection after selectionCursor action?
board->GroupRemoveItems( selection, &commit );
commit.Push( "GroupRemoveItems" );
wxString check = board->GroupsSanityCheck();
wxCHECK_MSG( check == wxEmptyString, 0, wxT( "Group removeItems resulted in inconsistent state: " ) + check );
// Should I call PostEvent and onModify() ?
m_toolMgr->PostEvent( EVENTS::SelectedItemsModified );
m_frame->OnModify();
return 0;
}
int PCB_EDITOR_CONTROL::GroupFlattenSelected( const TOOL_EVENT& aEvent )
{
SELECTION_TOOL* selTool = m_toolMgr->GetTool<SELECTION_TOOL>();
const PCBNEW_SELECTION& selection = selTool->GetSelection();
BOARD* board = getModel<BOARD>();
BOARD_COMMIT commit( m_frame );
const PCBNEW_SELECTION origGroups = selTool->GetSelection();
// These items were moved up to the top-level group that need to be readded to
// the view. That's becuase commit.Remove(group) recursively removed them from
// the view.
//std::unordered_set<BOARD_ITEM*> movedItems;
if( selection.Empty() )
m_toolMgr->RunAction( PCB_ACTIONS::selectionCursor, true );
// why don't we have to update the selection after selectionCursor action?
for( EDA_ITEM* item : selection )
{
BOARD_ITEM* board_item = static_cast<BOARD_ITEM*>( item );
wxCHECK_MSG( board_item->Type() == PCB_GROUP_T, 0,
"Selection for ungroup should only have groups in it - was checked." );
std::queue<PCB_GROUP*> groupsToFlatten;
groupsToFlatten.push( static_cast<PCB_GROUP*>( board_item ) );
PCB_GROUP* topGroup = groupsToFlatten.front();
commit.Modify( topGroup );
std::unordered_set<BOARD_ITEM*> topSubgroupsToRemove;
while( !groupsToFlatten.empty() )
{
PCB_GROUP* grp = groupsToFlatten.front();
groupsToFlatten.pop();
for( BOARD_ITEM* grpItem : grp->GetItems() )
{
if( grpItem->Type() == PCB_GROUP_T )
{
groupsToFlatten.push( static_cast<PCB_GROUP*>( grpItem ) );
commit.Remove( grpItem );
if( grp == topGroup )
topSubgroupsToRemove.insert( grpItem );
}
else
{
if( grp != topGroup )
{
wxCHECK( topGroup->AddItem( grpItem ), 0 );
//movedItems.insert( grpItem );
}
}
}
}
for( BOARD_ITEM* group : topSubgroupsToRemove )
{
topGroup->RemoveItem( group );
}
}
commit.Push( "GroupFlatten" );
wxString check = board->GroupsSanityCheck();
wxCHECK_MSG( check == wxEmptyString, 0, wxT( "Group flatten resulted in inconsistent state: " ) + check );
// Removing subgroups deselects the items in them. So reselect everything no that it's flattened.
selTool->ClearSelection();
for( EDA_ITEM* item : origGroups )
selTool->select( static_cast<BOARD_ITEM*>( item ) );
// Should I call PostEvent and onModify() ?
m_toolMgr->PostEvent( EVENTS::SelectedItemsModified );
m_frame->OnModify();
return 0;
}
int PCB_EDITOR_CONTROL::GroupEnterSelected( const TOOL_EVENT& aEvent )
int PCB_EDITOR_CONTROL::EnterGroup( const TOOL_EVENT& aEvent )
{
SELECTION_TOOL* selTool = m_toolMgr->GetTool<SELECTION_TOOL>();
const PCBNEW_SELECTION& selection = selTool->GetSelection();
@ -1253,7 +1115,7 @@ int PCB_EDITOR_CONTROL::GroupEnterSelected( const TOOL_EVENT& aEvent )
}
int PCB_EDITOR_CONTROL::GroupLeave( const TOOL_EVENT& aEvent )
int PCB_EDITOR_CONTROL::LeaveGroup( const TOOL_EVENT& aEvent )
{
m_toolMgr->GetTool<SELECTION_TOOL>()->ExitGroup( true /* Select the group */ );
return 0;
@ -1625,13 +1487,11 @@ void PCB_EDITOR_CONTROL::setTransitions()
Go( &PCB_EDITOR_CONTROL::ToggleLockSelected, PCB_ACTIONS::toggleLock.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::LockSelected, PCB_ACTIONS::lock.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::UnlockSelected, PCB_ACTIONS::unlock.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::GroupSelected, PCB_ACTIONS::groupCreate.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::GroupMergeSelected, PCB_ACTIONS::groupMerge.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::UngroupSelected, PCB_ACTIONS::groupUngroup.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::GroupRemoveItemsSelected, PCB_ACTIONS::groupRemoveItems.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::GroupFlattenSelected, PCB_ACTIONS::groupFlatten.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::GroupEnterSelected, PCB_ACTIONS::groupEnter.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::GroupLeave, PCB_ACTIONS::groupLeave.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::Group, PCB_ACTIONS::groupCreate.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::Ungroup, PCB_ACTIONS::groupUngroup.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::RemoveFromGroup, PCB_ACTIONS::groupRemoveItems.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::EnterGroup, PCB_ACTIONS::groupEnter.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::LeaveGroup, PCB_ACTIONS::groupLeave.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::UpdatePCBFromSchematic, ACTIONS::updatePcbFromSchematic.MakeEvent() );
Go( &PCB_EDITOR_CONTROL::UpdateSchematicFromPCB, ACTIONS::updateSchematicFromPcb.MakeEvent() );

View File

@ -111,25 +111,19 @@ public:
int UnlockSelected( const TOOL_EVENT& aEvent );
///> Groups selected items.
int GroupSelected( const TOOL_EVENT& aEvent );
///> Merges selected items.
int GroupMergeSelected( const TOOL_EVENT& aEvent );
int Group( const TOOL_EVENT& aEvent );
///> Ungroups selected items.
int UngroupSelected( const TOOL_EVENT& aEvent );
int Ungroup( const TOOL_EVENT& aEvent );
///> Remove selection from group.
int GroupRemoveItemsSelected( const TOOL_EVENT& aEvent );
///> Collaps subgroups to single group.
int GroupFlattenSelected( const TOOL_EVENT& aEvent );
int RemoveFromGroup( const TOOL_EVENT& aEvent );
///> Restrict seletion to only member of the group.
int GroupEnterSelected( const TOOL_EVENT& aEvent );
int EnterGroup( const TOOL_EVENT& aEvent );
///> Leave the current group (deselect its members and select the group as a whole)
int GroupLeave( const TOOL_EVENT& aEvent );
int LeaveGroup( const TOOL_EVENT& aEvent );
///> Runs the drill origin tool for setting the origin for drill and pick-and-place files.
int DrillOrigin( const TOOL_EVENT& aEvent );

View File

@ -849,15 +849,6 @@ int PCBNEW_CONTROL::placeBoardItems( std::vector<BOARD_ITEM*>& aItems, bool aIsN
{
static_cast<MODULE*>( item )->SetPath( KIID_PATH() );
}
else if( item->Type() == PCB_GROUP_T )
{
PCB_GROUP* group = static_cast<PCB_GROUP*>( item );
// If pasting a group, its immediate children must be updated to have its new KIID
group->RunOnChildren( [group]( BOARD_ITEM* aBrdItem )
{
aBrdItem->SetGroup( group );
} );
}
}
// Add or just select items for the move/place command
@ -875,34 +866,11 @@ int PCBNEW_CONTROL::placeBoardItems( std::vector<BOARD_ITEM*>& aItems, bool aIsN
// selection, so descendents of groups should not be in the selection
// object.
item->SetSelected();
}
// Filter out from selection any items that are in groups that are also in the selection
// For PCB_GROUP_T, a selection including the group should not include its descendants.
std::unordered_set<PCB_GROUP*> groups;
for( BOARD_ITEM* item : aItems )
{
if( item->Type() == PCB_GROUP_T )
groups.insert( static_cast<PCB_GROUP*>( item ) );
}
for( BOARD_ITEM* item : aItems )
{
bool inGroup = false;
for( PCB_GROUP* grp : groups )
{
if( grp->GetItems().find( item ) != grp->GetItems().end() )
{
inGroup = true;
break;
}
}
if( !inGroup )
{
if( !item->GetParentGroup() || !item->GetParentGroup()->IsSelected() )
selection.Add( item );
}
}
if( selection.Size() > 0 )
{
if( aAnchorAtOrigin )

View File

@ -1545,10 +1545,8 @@ bool SELECTION_TOOL::itemPassesFilter( BOARD_ITEM* aItem )
return false;
}
if( m_enteredGroup != NULL )
{
return m_enteredGroup->GetItems().find( aItem ) != m_enteredGroup->GetItems().end();
}
if( m_enteredGroup && aItem->GetParentGroup() != m_enteredGroup )
return false;
return true;
}
@ -1954,7 +1952,7 @@ bool SELECTION_TOOL::Selectable( const BOARD_ITEM* aItem, bool checkVisibilityOn
PCB_GROUP* group = const_cast<PCB_GROUP*>( static_cast<const PCB_GROUP*>( aItem ) );
// Similar to logic for module, a group is selectable if any of its
// members are. (This recurses)
// members are. (This recurses.)
for( BOARD_ITEM* item : group->GetItems() )
{
if( Selectable( item, true ) )
@ -2579,8 +2577,7 @@ void SELECTION_TOOL::FilterCollectorForGroups( GENERAL_COLLECTOR& aCollector ) c
aCollector.Remove( aCollector[j] );
}
}
else if( m_enteredGroup != NULL &&
m_enteredGroup->GetItems().find( aCollector[j] ) == m_enteredGroup->GetItems().end() )
else if( m_enteredGroup && aCollector[j]->GetParentGroup() != m_enteredGroup )
{
// If a group is entered, no selections of objects not in the group.
aCollector.Remove( aCollector[j] );

View File

@ -163,8 +163,8 @@ void testGroupEqual( const PCB_GROUP& group1, const PCB_GROUP& group2 )
BOOST_CHECK_EQUAL( group1.m_Uuid.AsString(), group2.m_Uuid.AsString() );
BOOST_CHECK_EQUAL( group1.GetName(), group2.GetName() );
const BOARD_ITEM_SET& items1 = group1.GetItems();
const BOARD_ITEM_SET& items2 = group2.GetItems();
const std::unordered_set<BOARD_ITEM*>& items1 = group1.GetItems();
const std::unordered_set<BOARD_ITEM*>& items2 = group2.GetItems();
BOOST_CHECK_EQUAL( items1.size(), items2.size() );