Fix a bunch of issues with group selection and highlighting.

Fixes https://gitlab.com/kicad/code/kicad/issues/6686
This commit is contained in:
Jeff Young 2020-12-29 12:33:37 +00:00
parent dec68a782c
commit efd16dca66
12 changed files with 111 additions and 171 deletions

View File

@ -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; }

View File

@ -36,6 +36,7 @@
#include <track.h>
#include <zone.h>
#include <pcb_marker.h>
#include <pcb_group.h>
#include <pcb_target.h>
#include <core/arraydim.h>
#include <core/kicad_algo.h>
@ -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<const BOARD_ITEM*> 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<const PCB_GROUP*>( item );
std::unordered_set<const PCB_GROUP*> subgroupos;
std::queue<const PCB_GROUP*> 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<const PCB_GROUP*>( aChild );
subgroupos.insert( childGroup );
toCheck.push( childGroup );
}
}
}
for( EDA_ITEM* otherItem : selection )
{
if( otherItem != item
&& subgroupos.find( static_cast<PCB_GROUP*>( 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<BOARD_ITEM*>( 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;
}

View File

@ -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;
};

View File

@ -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<BOARD_ITEM*>( 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<BOARD_ITEM*>( 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();
}

View File

@ -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;

View File

@ -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();

View File

@ -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 );

View File

@ -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<const FOOTPRINT*>( 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<const PCB_GROUP*>( item );
group->RunOnChildren(
[this, aUpdateFlags]( BOARD_ITEM* aModItem )
{
VIEW::Update( aModItem, aUpdateFlags );
} );
}
VIEW::Update( item, aUpdateFlags );

View File

@ -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<FOOTPRINT*>( item );
footprint->RunOnChildren( [&]( BOARD_ITEM* bitem )
{
items.push_back( bitem );
} );
}
else if( item->Type() == PCB_GROUP_T )
{
PCB_GROUP* group = static_cast<PCB_GROUP*>( item );
group->RunOnChildren( [&]( BOARD_ITEM* bitem )
{
items.push_back( bitem );
} );
}
}
#endif
return items;
}

View File

@ -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<FOOTPRINT*>( 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<PCB_GROUP*>( 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<FOOTPRINT*>( 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<PCB_GROUP*>( 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

View File

@ -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

View File

@ -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 )
{
}