From 3e190cee4b491e19634cd99e378aaec39942ee30 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sun, 28 Jan 2018 09:35:33 +0000 Subject: [PATCH] Implement selection brightening for DRC. The old item pointers (which aren't safe to keep around) were removed in favour of opaque references (void*) which are then compared against existing items when needed. Also improves brightening by brightening the whole footprint (ie: its pads, drawings, reference and value) rather than just its target cross. (cherry picked from commit 30e90b0) --- common/marker_base.cpp | 28 ++-- .../netlist_exporter_generic.h | 4 +- include/drc_item.h | 114 ++++++++------ include/marker_base.h | 40 +++-- pcbnew/class_board.cpp | 68 +++++++- pcbnew/class_board.h | 2 + pcbnew/class_marker_pcb.cpp | 12 +- pcbnew/class_marker_pcb.h | 35 ++--- pcbnew/dialogs/dialog_drc.cpp | 145 ++++++------------ pcbnew/dialogs/dialog_drc.h | 4 +- pcbnew/drc.cpp | 67 +++----- pcbnew/drc.h | 60 ++++---- pcbnew/drc_clearance_test_functions.cpp | 2 +- pcbnew/drc_item.cpp | 12 ++ pcbnew/drc_marker_functions.cpp | 131 ++++------------ pcbnew/tools/pcb_actions.h | 3 + pcbnew/tools/selection_tool.cpp | 107 ++++++++----- pcbnew/tools/selection_tool.h | 40 +++-- 18 files changed, 447 insertions(+), 427 deletions(-) diff --git a/common/marker_base.cpp b/common/marker_base.cpp index 8e4f44951c..65332bf17b 100644 --- a/common/marker_base.cpp +++ b/common/marker_base.cpp @@ -103,6 +103,17 @@ MARKER_BASE::MARKER_BASE() } +MARKER_BASE::MARKER_BASE( int aErrorCode, const wxPoint& aMarkerPos, + EDA_ITEM* aItem, const wxPoint& aPos, + EDA_ITEM* bItem, const wxPoint& bPos ) +{ + m_ScalingFactor = M_SHAPE_SCALE; + init(); + + SetData( aErrorCode, aMarkerPos, aItem, aPos, bItem, bPos ); +} + + MARKER_BASE::MARKER_BASE( int aErrorCode, const wxPoint& aMarkerPos, const wxString& aText, const wxPoint& aPos, const wxString& bText, const wxPoint& bPos ) @@ -110,9 +121,7 @@ MARKER_BASE::MARKER_BASE( int aErrorCode, const wxPoint& aMarkerPos, m_ScalingFactor = M_SHAPE_SCALE; init(); - SetData( aErrorCode, aMarkerPos, - aText, aPos, - bText, bPos ); + SetData( aErrorCode, aMarkerPos, aText, aPos, bText, bPos ); } @@ -131,22 +140,21 @@ MARKER_BASE::~MARKER_BASE() void MARKER_BASE::SetData( int aErrorCode, const wxPoint& aMarkerPos, - const wxString& aText, const wxPoint& aPos, - const wxString& bText, const wxPoint& bPos ) + EDA_ITEM* aItem, const wxPoint& aPos, + EDA_ITEM* bItem, const wxPoint& bPos ) { m_Pos = aMarkerPos; - m_drc.SetData( aErrorCode, - aText, bText, aPos, bPos ); + m_drc.SetData( aErrorCode, aItem, aPos, bItem, bPos ); m_drc.SetParent( this ); } void MARKER_BASE::SetData( int aErrorCode, const wxPoint& aMarkerPos, - const wxString& aText, const wxPoint& aPos ) + const wxString& aText, const wxPoint& aPos, + const wxString& bText, const wxPoint& bPos ) { m_Pos = aMarkerPos; - m_drc.SetData( aErrorCode, - aText, aPos ); + m_drc.SetData( aErrorCode, aText, aPos, bText, bPos ); m_drc.SetParent( this ); } diff --git a/eeschema/netlist_exporters/netlist_exporter_generic.h b/eeschema/netlist_exporters/netlist_exporter_generic.h index eb694b985b..16b2aae680 100644 --- a/eeschema/netlist_exporters/netlist_exporter_generic.h +++ b/eeschema/netlist_exporters/netlist_exporter_generic.h @@ -3,7 +3,7 @@ * * Copyright (C) 1992-2013 jp.charras at wanadoo.fr * Copyright (C) 2013 SoftPLC Corporation, Dick Hollenbeck - * Copyright (C) 1992-2017 KiCad Developers + * Copyright (C) 1992-2018 KiCad Developers * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -57,7 +57,6 @@ enum GNL_T class NETLIST_EXPORTER_GENERIC : public NETLIST_EXPORTER { private: - SCH_EDIT_FRAME* m_frame; std::set< wxString > m_libraries; ///< Set of library nicknames. SYMBOL_LIB_TABLE* m_libTable; @@ -65,7 +64,6 @@ private: public: NETLIST_EXPORTER_GENERIC( SCH_EDIT_FRAME* aFrame, NETLIST_OBJECT_LIST* aMasterList ) : NETLIST_EXPORTER( aMasterList ), - m_frame( aFrame ), m_libTable( aFrame->Prj().SchSymbolLibTable() ) {} diff --git a/include/drc_item.h b/include/drc_item.h index 5f2441aad2..5889757e9e 100644 --- a/include/drc_item.h +++ b/include/drc_item.h @@ -28,6 +28,8 @@ #include class MARKER_BASE; +class BOARD; +class BOARD_ITEM; /** @@ -45,40 +47,34 @@ class MARKER_BASE; class DRC_ITEM { protected: - int m_ErrorCode; ///< the error code's numeric value - wxString m_MainText; ///< text for the first BOARD_ITEM or SCH_ITEM - wxString m_AuxiliaryText; ///< text for the second BOARD_ITEM or SCH_ITEM - wxPoint m_MainPosition; ///< the location of the first (or main ) BOARD_ITEM or SCH_ITEM. This is also the position of the marker - wxPoint m_AuxiliaryPosition; ///< the location of the second BOARD_ITEM or SCH_ITEM - bool m_hasSecondItem; ///< true when 2 items create a DRC/ERC error, false if only one item - bool m_noCoordinate; + int m_ErrorCode; // the error code's numeric value + wxString m_MainText; // text for the first BOARD_ITEM or SCH_ITEM + wxString m_AuxiliaryText; // text for the second BOARD_ITEM or SCH_ITEM + wxPoint m_MainPosition; // the location of the first (or main ) BOARD_ITEM or SCH_ITEM. + wxPoint m_AuxiliaryPosition; // the location of the second BOARD_ITEM or SCH_ITEM + bool m_hasSecondItem; // true when 2 items create a DRC/ERC error, false if only one item + bool m_noCoordinate; - /// The marker this item belongs to, if any - MARKER_BASE* m_parent; + MARKER_BASE* m_parent; // The marker this item belongs to, if any + void* m_mainItemWeakRef; // search the current BOARD_ITEMs or SCH_ITEMs for a match + void* m_auxItemWeakRef; // search the current BOARD_ITEMs or SCH_ITEMs for a match public: DRC_ITEM() { - m_ErrorCode = 0; - m_hasSecondItem = false; - m_noCoordinate = false; - m_parent = nullptr; + m_ErrorCode = 0; + m_hasSecondItem = false; + m_noCoordinate = false; + m_parent = nullptr; + m_mainItemWeakRef = nullptr; + m_auxItemWeakRef = nullptr; } - DRC_ITEM( int aErrorCode, - const wxString& aMainText, const wxString& bAuxiliaryText, - const wxPoint& aMainPos, const wxPoint& bAuxiliaryPos ) + DRC_ITEM( int aErrorCode, EDA_ITEM* aMainItem, const wxPoint& aMainPos, + EDA_ITEM* bAuxiliaryItem, const wxPoint& bAuxiliaryPos ) { - SetData( aErrorCode, - aMainText, bAuxiliaryText, - aMainPos, bAuxiliaryPos ); - } - - DRC_ITEM( int aErrorCode, - const wxString& aText, const wxPoint& aPos ) - { - SetData( aErrorCode, aText, aPos ); + SetData( aErrorCode, aMainItem, aMainPos, bAuxiliaryItem, bAuxiliaryPos ); } @@ -86,39 +82,51 @@ public: * Function SetData * initialize all data in item * @param aErrorCode = error code - * @param aMainText = the text concerning the schematic or board item - * @param aMainPos = position the item and therefore of this issue - */ - void SetData( int aErrorCode, - const wxString& aMainText, const wxPoint& aMainPos ) - { - SetData( aErrorCode, - aMainText, aMainText, - aMainPos, aMainPos ); - m_hasSecondItem = false; - m_parent = nullptr; - } - - /** - * Function SetData - * initialize all data in item - * @param aErrorCode = error code - * @param aMainText = the first text (main text) concerning the main schematic or board item - * @param bAuxiliaryText = the second text (main text) concerning the second schematic or board item + * @param aMainItem = the first (main) schematic or board item + * @param bAuxiliaryItem = the second schematic or board item * @param aMainPos = position the first item and therefore of this issue * @param bAuxiliaryPos = position the second item */ - void SetData( int aErrorCode, - const wxString& aMainText, const wxString& bAuxiliaryText, - const wxPoint& aMainPos, const wxPoint& bAuxiliaryPos ) + void SetData( int aErrorCode, EDA_ITEM* aMainItem, const wxPoint& aMainPos, + EDA_ITEM* bAuxiliaryItem = nullptr, const wxPoint& bAuxiliaryPos = wxPoint() ) + { + m_ErrorCode = aErrorCode; + m_MainText = aMainItem->GetSelectMenuText(); + m_AuxiliaryText = bAuxiliaryItem ? bAuxiliaryItem->GetSelectMenuText() : wxString( wxEmptyString ); + m_MainPosition = aMainPos; + m_AuxiliaryPosition = bAuxiliaryPos; + m_hasSecondItem = bAuxiliaryItem != nullptr; + m_noCoordinate = false; + m_parent = nullptr; + + // Weak references (void*). One must search the BOARD_ITEMS or SCH_ITEMS for a match. + m_mainItemWeakRef = aMainItem; + m_auxItemWeakRef = bAuxiliaryItem; + } + + /** + * Function SetData + * initialize all data in item + * @param aErrorCode = error code + * @param aMainItem = the first (main) schematic or board item + * @param bAuxiliaryItem = the second schematic or board item + * @param aMainPos = position the first item and therefore of this issue + * @param bAuxiliaryPos = position the second item + */ + void SetData( int aErrorCode, const wxString& aMainText, const wxPoint& aMainPos, + const wxString& bAuxiliaryText = wxEmptyString, const wxPoint& bAuxiliaryPos = wxPoint() ) { m_ErrorCode = aErrorCode; m_MainText = aMainText; m_AuxiliaryText = bAuxiliaryText; m_MainPosition = aMainPos; m_AuxiliaryPosition = bAuxiliaryPos; - m_hasSecondItem = true; + m_hasSecondItem = bAuxiliaryText.Length(); m_noCoordinate = false; + m_parent = nullptr; + + m_mainItemWeakRef = nullptr; + m_auxItemWeakRef = nullptr; } /** @@ -132,6 +140,8 @@ public: m_AuxiliaryText = aAuxiliaryText; m_AuxiliaryPosition = aAuxiliaryPos; m_hasSecondItem = true; + + m_auxItemWeakRef = nullptr; } void SetParent( MARKER_BASE* aMarker ) { m_parent = aMarker; } @@ -142,11 +152,17 @@ public: void SetShowNoCoordinate() { m_noCoordinate = true; } - /** acces to A and B texts + /** + * Access to A and B texts */ wxString GetMainText() const { return m_MainText; } wxString GetAuxiliaryText() const { return m_AuxiliaryText; } + /** + * Access to A and B items for BOARDs + */ + BOARD_ITEM* GetMainItem( BOARD* aBoard ) const; + BOARD_ITEM* GetAuxiliaryItem( BOARD* aBoard ) const; /** * Function ShowHtml diff --git a/include/marker_base.h b/include/marker_base.h index fcbba97e0a..7f1bcc0d20 100644 --- a/include/marker_base.h +++ b/include/marker_base.h @@ -69,6 +69,19 @@ public: MARKER_BASE(); + /** + * Constructor + * @param aErrorCode The categorizing identifier for an error + * @param aMarkerPos The position of the MARKER on the BOARD + * @param aItem The first of two objects + * @param aPos The position of the first of two objects + * @param bItem The second of the two conflicting objects + * @param bPos The position of the second of two objects + */ + MARKER_BASE( int aErrorCode, const wxPoint& aMarkerPos, + EDA_ITEM* aItem, const wxPoint& aPos, + EDA_ITEM* bItem, const wxPoint& bPos ); + /** * Constructor * @param aErrorCode The categorizing identifier for an error @@ -152,6 +165,20 @@ public: return m_MarkerType; } + /** + * Function SetData + * fills in all the reportable data associated with a MARKER. + * @param aErrorCode The categorizing identifier for an error + * @param aMarkerPos The position of the MARKER on the BOARD + * @param aItem The first of two objects + * @param aPos The position of the first of two objects + * @param bItem The second of the two conflicting objects + * @param bPos The position of the second of two objects + */ + void SetData( int aErrorCode, const wxPoint& aMarkerPos, + EDA_ITEM* aItem, const wxPoint& aPos, + EDA_ITEM* bItem = nullptr, const wxPoint& bPos = wxPoint() ); + /** * Function SetData * fills in all the reportable data associated with a MARKER. @@ -164,18 +191,7 @@ public: */ void SetData( int aErrorCode, const wxPoint& aMarkerPos, const wxString& aText, const wxPoint& aPos, - const wxString& bText, const wxPoint& bPos ); - - /** - * Function SetData - * fills in all the reportable data associated with a MARKER. - * @param aErrorCode The categorizing identifier for an error - * @param aMarkerPos The position of the MARKER on the BOARD - * @param aText Text describing the object - * @param aPos The position of the object - */ - void SetData( int aErrorCode, const wxPoint& aMarkerPos, - const wxString& aText, const wxPoint& aPos ); + const wxString& bText = wxEmptyString, const wxPoint& bPos = wxPoint() ); /** * Function SetAuxiliaryData diff --git a/pcbnew/class_board.cpp b/pcbnew/class_board.cpp index f2dc2dc4fc..2beb6dc298 100644 --- a/pcbnew/class_board.cpp +++ b/pcbnew/class_board.cpp @@ -61,6 +61,33 @@ #include +/** + * A singleton item of this class is returned for a weak reference that no longer exists. + * Its sole purpose is to flag the item as having been deleted. + */ +class DELETED_BOARD_ITEM : public BOARD_ITEM +{ +public: + DELETED_BOARD_ITEM() : + BOARD_ITEM( nullptr, NOT_USED ) + {} + + wxString GetSelectMenuText() const override { return _( "(Deleted Item)" ); } + wxString GetClass() const override { return wxT( "DELETED_BOARD_ITEM" ); } + + // pure virtuals: + const wxPoint GetPosition() const override { return wxPoint(); } + void SetPosition( const wxPoint& ) override {} + void Draw( EDA_DRAW_PANEL* , wxDC* , GR_DRAWMODE , const wxPoint& ) override {} + +#if defined(DEBUG) + void Show( int , std::ostream& ) const override {} +#endif +}; + +DELETED_BOARD_ITEM g_DeletedItem; + + /* This is an odd place for this, but CvPcb won't link if it is * in class_board_item.cpp like I first tried it. */ @@ -1008,6 +1035,45 @@ void BOARD::DeleteZONEOutlines() } +BOARD_ITEM* BOARD::GetItem( void* aWeakReference, bool includeDrawings ) +{ + for( TRACK* track : Tracks() ) + if( track == aWeakReference ) + return track; + + for( MODULE* module : Modules() ) + { + if( module == aWeakReference ) + return module; + + for( D_PAD* pad : module->Pads() ) + if( pad == aWeakReference ) + return pad; + + if( includeDrawings ) + { + for( BOARD_ITEM* drawing : module->GraphicalItems() ) + if( drawing == aWeakReference ) + return drawing; + } + } + + for( ZONE_CONTAINER* zone : Zones() ) + if( zone == aWeakReference ) + return zone; + + if( includeDrawings ) + { + for( BOARD_ITEM* drawing : Drawings() ) + if( drawing == aWeakReference ) + return drawing; + } + + // Not found; weak reference has been deleted. + return &g_DeletedItem; +} + + int BOARD::GetNumSegmTrack() const { return m_Track.GetCount(); @@ -1642,8 +1708,8 @@ D_PAD* BOARD::GetPadFast( const wxPoint& aPosition, LSET aLayerSet ) // Pad found, it must be on the correct layer if( ( pad->GetLayerSet() & aLayerSet ).any() ) return pad; + } } -} return nullptr; } diff --git a/pcbnew/class_board.h b/pcbnew/class_board.h index d03d7defdf..d78ce55077 100644 --- a/pcbnew/class_board.h +++ b/pcbnew/class_board.h @@ -283,6 +283,8 @@ public: void Remove( BOARD_ITEM* aBoardItem ) override; + BOARD_ITEM* GetItem( void* aWeakReference, bool includeDrawings = true ); + BOARD_ITEM* Duplicate( const BOARD_ITEM* aItem, bool aAddToBoard = false ); /** diff --git a/pcbnew/class_marker_pcb.cpp b/pcbnew/class_marker_pcb.cpp index a44594400f..40e61cf593 100644 --- a/pcbnew/class_marker_pcb.cpp +++ b/pcbnew/class_marker_pcb.cpp @@ -54,19 +54,21 @@ MARKER_PCB::MARKER_PCB( BOARD_ITEM* aParent ) : MARKER_PCB::MARKER_PCB( int aErrorCode, const wxPoint& aMarkerPos, - const wxString& aText, const wxPoint& aPos, - const wxString& bText, const wxPoint& bPos ) : + BOARD_ITEM* aItem, const wxPoint& aPos, + BOARD_ITEM* bItem, const wxPoint& bPos ) : BOARD_ITEM( NULL, PCB_MARKER_T ), // parent set during BOARD::Add() - MARKER_BASE( aErrorCode, aMarkerPos, aText, aPos, bText, bPos ), m_item( NULL ) + MARKER_BASE( aErrorCode, aMarkerPos, aItem, aPos, bItem, bPos ), m_item( NULL ) { m_Color = WHITE; m_ScalingFactor = SCALING_FACTOR; } + MARKER_PCB::MARKER_PCB( int aErrorCode, const wxPoint& aMarkerPos, - const wxString& aText, const wxPoint& aPos ) : + const wxString& aText, const wxPoint& aPos, + const wxString& bText, const wxPoint& bPos ) : BOARD_ITEM( NULL, PCB_MARKER_T ), // parent set during BOARD::Add() - MARKER_BASE( aErrorCode, aMarkerPos, aText, aPos ), m_item( NULL ) + MARKER_BASE( aErrorCode, aMarkerPos, aText, aPos, bText, bPos ), m_item( NULL ) { m_Color = WHITE; m_ScalingFactor = SCALING_FACTOR; diff --git a/pcbnew/class_marker_pcb.h b/pcbnew/class_marker_pcb.h index 9a95e5ac2d..712852b8d8 100644 --- a/pcbnew/class_marker_pcb.h +++ b/pcbnew/class_marker_pcb.h @@ -45,6 +45,19 @@ public: MARKER_PCB( BOARD_ITEM* aParent ); + /** + * Constructor + * @param aErrorCode The categorizing identifier for an error + * @param aMarkerPos The position of the MARKER_PCB on the BOARD + * @param aItem The first of two objects + * @param aPos The position of the first of two objects + * @param bItem The second of the two conflicting objects + * @param bPos The position of the second of two objects + */ + MARKER_PCB( int aErrorCode, const wxPoint& aMarkerPos, + BOARD_ITEM* aItem, const wxPoint& aPos, + BOARD_ITEM* bItem = nullptr, const wxPoint& bPos = wxPoint() ); + /** * Constructor * @param aErrorCode The categorizing identifier for an error @@ -56,17 +69,7 @@ public: */ MARKER_PCB( int aErrorCode, const wxPoint& aMarkerPos, const wxString& aText, const wxPoint& aPos, - const wxString& bText, const wxPoint& bPos ); - - /** - * Constructor - * @param aErrorCode The categorizing identifier for an error - * @param aMarkerPos The position of the MARKER_PCB on the BOARD - * @param aText Text describing the object - * @param aPos The position of the object - */ - MARKER_PCB( int aErrorCode, const wxPoint& aMarkerPos, - const wxString& aText, const wxPoint& aPos ); + const wxString& bText = wxEmptyString, const wxPoint& bPos = wxPoint() ); ~MARKER_PCB(); @@ -88,16 +91,6 @@ public: const wxPoint GetPosition() const override { return m_Pos; } void SetPosition( const wxPoint& aPos ) override { m_Pos = aPos; } - void SetItem( const BOARD_ITEM* aItem ) - { - m_item = aItem; - } - - const BOARD_ITEM* GetItem() const - { - return m_item; - } - bool HitTest( const wxPoint& aPosition ) const override { return HitTestMarker( aPosition ); diff --git a/pcbnew/dialogs/dialog_drc.cpp b/pcbnew/dialogs/dialog_drc.cpp index 6ec28c08e9..8db2e02196 100644 --- a/pcbnew/dialogs/dialog_drc.cpp +++ b/pcbnew/dialogs/dialog_drc.cpp @@ -39,6 +39,9 @@ #include #include #include +#include +#include +#include #include #include @@ -53,6 +56,35 @@ #define RefillZonesBeforeDrc wxT( "RefillZonesBeforeDrc" ) +struct BOARD_THAWER +{ + BOARD_THAWER( PCB_EDIT_FRAME* aBoardEditor ) + { + m_boardEditor = aBoardEditor; + m_freezeCount = 0; + + while( m_boardEditor->IsFrozen() ) + { + m_boardEditor->Thaw(); + m_freezeCount++; + } + } + + ~BOARD_THAWER() + { + while( m_freezeCount > 0 ) + { + m_boardEditor->Freeze(); + m_freezeCount--; + } + } + +protected: + PCB_EDIT_FRAME* m_boardEditor; + int m_freezeCount; +}; + + DIALOG_DRC_CONTROL::DIALOG_DRC_CONTROL( DRC* aTester, PCB_EDIT_FRAME* aEditorFrame, wxWindow* aParent ) : DIALOG_DRC_CONTROL_BASE( aParent ) @@ -88,8 +120,6 @@ DIALOG_DRC_CONTROL::~DIALOG_DRC_CONTROL() m_UnconnectedListBox->Disconnect( ID_UNCONNECTED_LIST, wxEVT_RIGHT_UP, wxMouseEventHandler( DIALOG_DRC_CONTROL::OnRightUpUnconnected ), NULL, this ); - - this->Disconnect( wxEVT_MENU, wxCommandEventHandler( DIALOG_DRC_CONTROL::OnPopupMenu ), NULL, this ); } void DIALOG_DRC_CONTROL::OnActivateDlg( wxActivateEvent& event ) @@ -141,10 +171,6 @@ void DIALOG_DRC_CONTROL::InitValues() wxMouseEventHandler( DIALOG_DRC_CONTROL::OnRightUpUnconnected ), NULL, this ); - this->Connect( wxEVT_MENU, wxCommandEventHandler( DIALOG_DRC_CONTROL::OnPopupMenu ), NULL, - this ); - - m_DeleteCurrentMarkerButton->Enable( false ); DisplayDRCValues(); @@ -421,55 +447,6 @@ void DIALOG_DRC_CONTROL::OnLeftDClickClearance( wxMouseEvent& event ) } -void DIALOG_DRC_CONTROL::OnPopupMenu( wxCommandEvent& event ) -{ - int source = event.GetId(); - - const DRC_ITEM* item = nullptr; - wxPoint pos; - - int selection; - - switch( source ) - { - case ID_POPUP_UNCONNECTED_A: - selection = m_UnconnectedListBox->GetSelection(); - item = m_UnconnectedListBox->GetItem( selection ); - pos = item->GetPointA(); - break; - - case ID_POPUP_UNCONNECTED_B: - selection = m_UnconnectedListBox->GetSelection(); - item = m_UnconnectedListBox->GetItem( selection ); - pos = item->GetPointB(); - break; - - case ID_POPUP_MARKERS_A: - selection = m_ClearanceListBox->GetSelection(); - item = m_ClearanceListBox->GetItem( selection ); - pos = item->GetPointA(); - break; - - case ID_POPUP_MARKERS_B: - selection = m_ClearanceListBox->GetSelection(); - item = m_ClearanceListBox->GetItem( selection ); - pos = item->GetPointB(); - break; - } - - if( item ) - { - // When selecting a item, center it on GAL and just move the graphic - // cursor in legacy mode gives the best result - bool center = m_brdEditor->IsGalCanvasActive() ? true : false; - m_brdEditor->FocusOnLocation( pos, true, center ); - - if( !IsModal() ) - Show( false ); - } -} - - void DIALOG_DRC_CONTROL::OnRightUpUnconnected( wxMouseEvent& event ) { // popup menu to go to either of the items listed in the DRC_ITEM. @@ -477,22 +454,7 @@ void DIALOG_DRC_CONTROL::OnRightUpUnconnected( wxMouseEvent& event ) int selection = m_UnconnectedListBox->GetSelection(); if( selection != wxNOT_FOUND ) - { - wxMenu menu; - wxMenuItem* mItem; - const DRC_ITEM* dItem = m_UnconnectedListBox->GetItem( selection ); - - mItem = new wxMenuItem( &menu, ID_POPUP_UNCONNECTED_A, dItem->GetTextA() ); - menu.Append( mItem ); - - if( dItem->HasSecondItem() ) - { - mItem = new wxMenuItem( &menu, ID_POPUP_UNCONNECTED_B, dItem->GetTextB() ); - menu.Append( mItem ); - } - - PopupMenu( &menu ); - } + doSelectionMenu( m_UnconnectedListBox->GetItem( selection ) ); } @@ -503,22 +465,23 @@ void DIALOG_DRC_CONTROL::OnRightUpClearance( wxMouseEvent& event ) int selection = m_ClearanceListBox->GetSelection(); if( selection != wxNOT_FOUND ) - { - wxMenu menu; - wxMenuItem* mItem; - const DRC_ITEM* dItem = m_ClearanceListBox->GetItem( selection ); + doSelectionMenu( m_ClearanceListBox->GetItem( selection ) ); +} - mItem = new wxMenuItem( &menu, ID_POPUP_MARKERS_A, dItem->GetTextA() ); - menu.Append( mItem ); - if( dItem->HasSecondItem() ) - { - mItem = new wxMenuItem( &menu, ID_POPUP_MARKERS_B, dItem->GetTextB() ); - menu.Append( mItem ); - } +void DIALOG_DRC_CONTROL::doSelectionMenu( const DRC_ITEM* aItem ) +{ + // popup menu to go to either of the items listed in the DRC_ITEM. + GENERAL_COLLECTOR items; - PopupMenu( &menu ); - } + items.Append( aItem->GetMainItem( m_brdEditor->GetBoard() ) ); + + if( aItem->HasSecondItem() ) + items.Append( aItem->GetAuxiliaryItem( m_brdEditor->GetBoard() ) ); + + BOARD_THAWER thawer( m_brdEditor ); + m_brdEditor->GetToolManager()->RunAction( PCB_ACTIONS::selectionMenu, true, &items ); + m_brdEditor->GetCanvas()->Refresh(); } @@ -630,21 +593,9 @@ void DIALOG_DRC_CONTROL::OnUnconnectedSelectionEvent( wxCommandEvent& event ) void DIALOG_DRC_CONTROL::RedrawDrawPanel() { - int freezeCount = 0; - - while( m_brdEditor->IsFrozen() ) - { - m_brdEditor->Thaw(); - freezeCount++; - } + BOARD_THAWER thawer( m_brdEditor ); m_brdEditor->GetCanvas()->Refresh(); - - while( freezeCount > 0 ) - { - m_brdEditor->Freeze(); - freezeCount--; - } } diff --git a/pcbnew/dialogs/dialog_drc.h b/pcbnew/dialogs/dialog_drc.h index 00b0dab0fb..00f9f64a94 100644 --- a/pcbnew/dialogs/dialog_drc.h +++ b/pcbnew/dialogs/dialog_drc.h @@ -140,7 +140,9 @@ private: void DelDRCMarkers(); void RedrawDrawPanel(); - void OnPopupMenu( wxCommandEvent& event ); + /// Run the SELECTION_TOOL's disambiguation menu to highlight the two BOARD_ITEMs + /// in the DRC_ITEM. + void doSelectionMenu( const DRC_ITEM* aItem ); BOARD* m_currentBoard; // the board currently on test DRC* m_tester; diff --git a/pcbnew/drc.cpp b/pcbnew/drc.cpp index b7aaad04ab..853ce36d55 100644 --- a/pcbnew/drc.cpp +++ b/pcbnew/drc.cpp @@ -280,10 +280,8 @@ int DRC::TestZoneToZoneOutline( ZONE_CONTAINER* aZone, bool aCreateMarkers ) if( aCreateMarkers ) { wxPoint pt( currentVertex.x, currentVertex.y ); - wxString msg1 = zoneRef->GetSelectMenuText(); - wxString msg2 = zoneToTest->GetSelectMenuText(); MARKER_PCB* marker = new MARKER_PCB( COPPERAREA_INSIDE_COPPERAREA, - pt, msg1, pt, msg2, pt ); + pt, zoneRef, pt, zoneToTest, pt ); commit.Add( marker ); } @@ -302,10 +300,8 @@ int DRC::TestZoneToZoneOutline( ZONE_CONTAINER* aZone, bool aCreateMarkers ) if( aCreateMarkers ) { wxPoint pt( currentVertex.x, currentVertex.y ); - wxString msg1 = zoneToTest->GetSelectMenuText(); - wxString msg2 = zoneRef->GetSelectMenuText(); MARKER_PCB* marker = new MARKER_PCB( COPPERAREA_INSIDE_COPPERAREA, - pt, msg1, pt, msg2, pt ); + pt, zoneToTest, pt, zoneRef, pt ); commit.Add( marker ); } @@ -350,10 +346,8 @@ int DRC::TestZoneToZoneOutline( ZONE_CONTAINER* aZone, bool aCreateMarkers ) // COPPERAREA_COPPERAREA error : intersect or too close if( aCreateMarkers ) { - wxString msg1 = zoneRef->GetSelectMenuText(); - wxString msg2 = zoneToTest->GetSelectMenuText(); MARKER_PCB* marker = new MARKER_PCB( COPPERAREA_CLOSE_TO_COPPERAREA, - pt, msg1, pt, msg2, pt ); + pt, zoneRef, pt, zoneToTest, pt ); commit.Add( marker ); } @@ -791,16 +785,14 @@ void DRC::testUnconnected() for( const auto& edge : edges ) { - wxString t_src = edge.GetSourceNode()->Parent()->GetSelectMenuText(); - wxString t_dst = edge.GetTargetNode()->Parent()->GetSelectMenuText(); auto src = edge.GetSourcePos(); auto dst = edge.GetTargetPos(); - DRC_ITEM* uncItem = new DRC_ITEM( DRCE_UNCONNECTED_ITEMS, - t_src, - t_dst, - wxPoint( src.x, src.y ), wxPoint( dst.x, dst.y ) ); + edge.GetSourceNode()->Parent(), + wxPoint( src.x, src.y ), + edge.GetTargetNode()->Parent(), + wxPoint( dst.x, dst.y ) ); m_unconnected.push_back( uncItem ); } @@ -834,7 +826,7 @@ void DRC::testZones() if( ( netcode < 0 ) || pads_in_net == 0 ) { - addMarkerToPcb( fillMarker( test_area, + addMarkerToPcb( fillMarker( test_area, test_area->GetPosition(), DRCE_SUSPICIOUS_NET_FOR_ZONE_OUTLINE, m_currentMarker ) ); m_currentMarker = nullptr; } @@ -1031,10 +1023,8 @@ void DRC::testDisabledLayers() auto createMarker = [&]( BOARD_ITEM* aItem ) { - wxString msg; - msg.Printf( _( "\"%s\" is on a disabled layer" ), aItem->GetSelectMenuText() ); - m_currentMarker = fillMarker( aItem->GetPosition(), DRCE_DISABLED_LAYER_ITEM, - msg, m_currentMarker ); + m_currentMarker = fillMarker( aItem, aItem->GetPosition(), DRCE_DISABLED_LAYER_ITEM, + m_currentMarker ); addMarkerToPcb( m_currentMarker ); m_currentMarker = nullptr; }; @@ -1047,10 +1037,11 @@ void DRC::testDisabledLayers() for( auto module : board->Modules() ) { - module->RunOnChildren( [&]( BOARD_ITEM* aItem ) { - if( disabledLayers.test( aItem->GetLayer() ) ) - createMarker( aItem ); - } ); + module->RunOnChildren( [&]( BOARD_ITEM* aItem ) + { + if( disabledLayers.test( aItem->GetLayer() ) ) + createMarker( aItem ); + } ); } for( auto zone : board->Zones() ) @@ -1258,11 +1249,9 @@ bool DRC::doFootprintOverlappingDrc() if( !is_ok && m_doFootprintOverlapping ) { - msg.Printf( _( "footprint \"%s\" has malformed courtyard" ), - footprint->GetReference().GetData() ); - m_currentMarker = fillMarker( footprint->GetPosition(), + m_currentMarker = fillMarker( footprint, footprint->GetPosition(), DRCE_MALFORMED_COURTYARD_IN_FOOTPRINT, - msg, m_currentMarker ); + m_currentMarker ); addMarkerToPcb( m_currentMarker ); m_currentMarker = nullptr; success = false; @@ -1275,11 +1264,9 @@ bool DRC::doFootprintOverlappingDrc() footprint->GetPolyCourtyardBack().OutlineCount() == 0 && is_ok ) { - msg.Printf( _( "footprint \"%s\" has no courtyard defined" ), - footprint->GetReference().GetData() ); - m_currentMarker = fillMarker( footprint->GetPosition(), + m_currentMarker = fillMarker( footprint, footprint->GetPosition(), DRCE_MISSING_COURTYARD_IN_FOOTPRINT, - msg, m_currentMarker ); + m_currentMarker ); addMarkerToPcb( m_currentMarker ); m_currentMarker = nullptr; success = false; @@ -1314,13 +1301,9 @@ bool DRC::doFootprintOverlappingDrc() if( courtyard.OutlineCount() ) { //Overlap between footprint and candidate - msg.Printf( _( "footprints \"%s\" and \"%s\" overlap on front (top) layer" ), - footprint->GetReference().GetData(), - candidate->GetReference().GetData() ); VECTOR2I& pos = courtyard.Vertex( 0, 0, -1 ); - wxPoint loc( pos.x, pos.y ); - m_currentMarker = fillMarker( loc, DRCE_OVERLAPPING_FOOTPRINTS, msg, - m_currentMarker ); + m_currentMarker = fillMarker( wxPoint( pos.x, pos.y ), footprint, candidate, + DRCE_OVERLAPPING_FOOTPRINTS, m_currentMarker ); addMarkerToPcb( m_currentMarker ); m_currentMarker = nullptr; success = false; @@ -1351,13 +1334,9 @@ bool DRC::doFootprintOverlappingDrc() if( courtyard.OutlineCount() ) { //Overlap between footprint and candidate - msg.Printf( _( "footprints \"%s\" and \"%s\" overlap on back (bottom) layer" ), - footprint->GetReference().GetData(), - candidate->GetReference().GetData() ); VECTOR2I& pos = courtyard.Vertex( 0, 0, -1 ); - wxPoint loc( pos.x, pos.y ); - m_currentMarker = fillMarker( loc, DRCE_OVERLAPPING_FOOTPRINTS, msg, - m_currentMarker ); + m_currentMarker = fillMarker( wxPoint( pos.x, pos.y ), footprint, candidate, + DRCE_OVERLAPPING_FOOTPRINTS, m_currentMarker ); addMarkerToPcb( m_currentMarker ); m_currentMarker = nullptr; success = false; diff --git a/pcbnew/drc.h b/pcbnew/drc.h index 9d9501c1d9..dfcc61074d 100644 --- a/pcbnew/drc.h +++ b/pcbnew/drc.h @@ -229,43 +229,47 @@ private: /** - * Creates a marker and fills it in with information but does not add it to the BOARD. + * Function fillMarker + * Optionally creates a marker and fills it in with information, but does not add it to + * the BOARD. Use this to report any kind of DRC problem, or unconnected pad problem. * - * Use this to report any kind of DRC problem or unconnected pad problem. - * - * @param aTrack The reference track. - * @param aItem Another item on the BOARD, such as a VIA, SEGZONE, - * or TRACK. - * @param aErrorCode A categorizing identifier for the particular type - * of error that is being reported. - * @param fillMe A MARKER_PCB* which is to be filled in, or NULL if one is to - * first be allocated, then filled. + * @param aTrack/aPad The reference item. + * @param aItem Another item on the BOARD, such as a VIA, SEGZONE, or TRACK, which is + * in conflict with the reference item. + * @param aErrorCode An ID for the particular type of error that is being reported. + * @param fillMe A MARKER_PCB* which is to be filled in, or NULL if one is to be created. */ - MARKER_PCB* fillMarker( const TRACK* aTrack, BOARD_ITEM* aItem, int aErrorCode, - MARKER_PCB* fillMe ); + MARKER_PCB* fillMarker( TRACK* aTrack, BOARD_ITEM* aItem, int aErrorCode, MARKER_PCB* fillMe ); MARKER_PCB* fillMarker( D_PAD* aPad, BOARD_ITEM* aItem, int aErrorCode, MARKER_PCB* fillMe ); - MARKER_PCB* fillMarker( ZONE_CONTAINER* aArea, int aErrorCode, MARKER_PCB* fillMe ); - - MARKER_PCB* fillMarker( const wxPoint& aPos, int aErrorCode, - const wxString& aMessage, MARKER_PCB* fillMe ); + /** + * Function fillMarker + * Optionally creates a marker and fills it in with information, but does not add it to + * the BOARD. Use this to report any kind of DRC problem, or unconnected pad problem. + * + * @param aItem The reference item. + * @param aPos Usually the position of the item, but could be more specific for a zone. + * @param aErrorCode An ID for the particular type of error that is being reported. + * @param fillMe A MARKER_PCB* which is to be filled in, or NULL if one is to be created. + */ + MARKER_PCB* fillMarker( BOARD_ITEM* aItem, const wxPoint& aPos, int aErrorCode, + MARKER_PCB* fillMe ); /** - * Create a marker and fills it in with information but do not add it to the BOARD. + * Function fillMarker + * Optionally creates a marker and fills it in with information, but does not add it to + * the BOARD. Use this to report any kind of DRC problem, or unconnected pad problem. * - * Use this to report any kind of DRC problem, or unconnected pad problem. - * - * @param aArea The zone to test - * @param aPos position of error - * @param aErrorCode Type of error - * @param fillMe A MARKER_PCB* which is to be filled in, or NULL if one is to - * first be allocated, then filled. + * @param aPos The reference location. + * @param aErrorCode An ID for the particular type of error that is being reported. + * @param aItem The first item in conflict. + * @param bItem (Optional) The second item in conflict or NULL. + * @param aErrorCode An ID for the particular type of error that is being reported. + * @param fillMe A MARKER_PCB* which is to be filled in, or NULL if one is to be created. */ - MARKER_PCB* fillMarker( const ZONE_CONTAINER* aArea, - const wxPoint& aPos, - int aErrorCode, - MARKER_PCB* fillMe ); + MARKER_PCB* fillMarker( const wxPoint& aPos, BOARD_ITEM* aItem, BOARD_ITEM* bItem, + int aErrorCode, MARKER_PCB* fillMe ); /** * Fill a MARKER which will report on a generic problem with the board which is diff --git a/pcbnew/drc_clearance_test_functions.cpp b/pcbnew/drc_clearance_test_functions.cpp index 7099310d4a..2d839780aa 100644 --- a/pcbnew/drc_clearance_test_functions.cpp +++ b/pcbnew/drc_clearance_test_functions.cpp @@ -200,7 +200,7 @@ bool DRC::doTrackDrc( TRACK* aRefSeg, TRACK* aStart, bool testPads ) // Phase 0 : Test vias if( aRefSeg->Type() == PCB_VIA_T ) { - const VIA *refvia = static_cast( aRefSeg ); + VIA *refvia = static_cast( aRefSeg ); // test if the via size is smaller than minimum if( refvia->GetViaType() == VIA_MICROVIA ) { diff --git a/pcbnew/drc_item.cpp b/pcbnew/drc_item.cpp index 4c3918e820..d0e25a433e 100644 --- a/pcbnew/drc_item.cpp +++ b/pcbnew/drc_item.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include @@ -154,3 +155,14 @@ wxString DRC_ITEM::ShowCoord( const wxPoint& aPos ) ret << aPos; return ret; } + +BOARD_ITEM* DRC_ITEM::GetMainItem( BOARD* aBoard ) const +{ + return aBoard->GetItem( m_mainItemWeakRef, false /* copper only */ ); +} + +BOARD_ITEM* DRC_ITEM::GetAuxiliaryItem( BOARD* aBoard ) const +{ + return aBoard->GetItem( m_auxItemWeakRef, false /* copper only */ ); +} + diff --git a/pcbnew/drc_marker_functions.cpp b/pcbnew/drc_marker_functions.cpp index 671cee40f2..59d728787f 100644 --- a/pcbnew/drc_marker_functions.cpp +++ b/pcbnew/drc_marker_functions.cpp @@ -41,35 +41,29 @@ #include #include #include -#include #include #include +#include -MARKER_PCB* DRC::fillMarker( const TRACK* aTrack, BOARD_ITEM* aItem, int aErrorCode, - MARKER_PCB* fillMe ) +MARKER_PCB* DRC::fillMarker( TRACK* aTrack, BOARD_ITEM* bItem, int aErrorCode, MARKER_PCB* fillMe ) { - wxString textA = aTrack->GetSelectMenuText(); - wxString textB; + wxPoint posA; + wxPoint posB = wxPoint(); - wxPoint position; - wxPoint posB; - - if( aItem ) // aItem might be NULL + if( bItem ) // aItem might be NULL { - textB = aItem->GetSelectMenuText(); - - if( aItem->Type() == PCB_PAD_T ) + if( bItem->Type() == PCB_PAD_T ) { - posB = position = ((D_PAD*)aItem)->GetPosition(); + posB = posA = ((D_PAD*)bItem)->GetPosition(); } - else if( aItem->Type() == PCB_VIA_T ) + else if( bItem->Type() == PCB_VIA_T ) { - posB = position = ((VIA*)aItem)->GetPosition(); + posB = posA = ((VIA*)bItem)->GetPosition(); } - else if( aItem->Type() == PCB_TRACE_T ) + else if( bItem->Type() == PCB_TRACE_T ) { - TRACK* track = (TRACK*) aItem; + TRACK* track = (TRACK*) bItem; posB = track->GetPosition(); @@ -78,48 +72,27 @@ MARKER_PCB* DRC::fillMarker( const TRACK* aTrack, BOARD_ITEM* aItem, int aErrorC // either of aItem's start or end will be used for the marker position // first assume start, then switch at end if needed. decision made on // distance from end of aTrack. - position = track->GetStart(); + posA = track->GetStart(); double dToEnd = GetLineLength( endPos, aTrack->GetEnd() ); - double dToStart = GetLineLength( position, aTrack->GetEnd() ); + double dToStart = GetLineLength( posA, aTrack->GetEnd() ); if( dToEnd < dToStart ) - position = endPos; + posA = endPos; } - else if( aItem->Type() == PCB_TEXT_T ) + else if( bItem->Type() == PCB_TEXT_T ) { - position = aTrack->GetPosition(); - posB = ((TEXTE_PCB*) aItem)->GetPosition(); + posA = aTrack->GetPosition(); + posB = ((TEXTE_PCB*) bItem)->GetPosition(); } } else - position = aTrack->GetPosition(); + posA = aTrack->GetPosition(); if( fillMe ) - { - if( aItem ) - fillMe->SetData( aErrorCode, position, - textA, aTrack->GetPosition(), - textB, posB ); - else - fillMe->SetData( aErrorCode, position, - textA, aTrack->GetPosition() ); - } + fillMe->SetData( aErrorCode, posA, aTrack, aTrack->GetPosition(), bItem, posB ); else - { - if( aItem ) - { - fillMe = new MARKER_PCB( aErrorCode, position, - textA, aTrack->GetPosition(), - textB, posB ); - fillMe->SetItem( aItem ); - } - else - { - fillMe = new MARKER_PCB( aErrorCode, position, - textA, aTrack->GetPosition() ); - } - } + fillMe = new MARKER_PCB( aErrorCode, posA, aTrack, aTrack->GetPosition(), bItem, posB ); return fillMe; } @@ -127,16 +100,11 @@ MARKER_PCB* DRC::fillMarker( const TRACK* aTrack, BOARD_ITEM* aItem, int aErrorC MARKER_PCB* DRC::fillMarker( D_PAD* aPad, BOARD_ITEM* aItem, int aErrorCode, MARKER_PCB* fillMe ) { - wxString textA = aPad->GetSelectMenuText(); - wxString textB; - wxPoint posA = aPad->GetPosition(); wxPoint posB; if( aItem ) { - textB = aItem->GetSelectMenuText(); - switch( aItem->Type() ) { case PCB_PAD_T: @@ -154,57 +122,28 @@ MARKER_PCB* DRC::fillMarker( D_PAD* aPad, BOARD_ITEM* aItem, int aErrorCode, MAR } if( fillMe ) - { - fillMe->SetData( aErrorCode, posA, textA, posA, textB, posB ); - } + fillMe->SetData( aErrorCode, posA, aPad, posA, aItem, posB ); else - { - fillMe = new MARKER_PCB( aErrorCode, posA, textA, posA, textB, posB ); - fillMe->SetItem( aPad ); // TODO it has to be checked - } + fillMe = new MARKER_PCB( aErrorCode, posA, aPad, posA, aItem, posB ); return fillMe; } -MARKER_PCB* DRC::fillMarker( ZONE_CONTAINER* aArea, int aErrorCode, MARKER_PCB* fillMe ) +MARKER_PCB* DRC::fillMarker(BOARD_ITEM *aItem, const wxPoint &aPos, int aErrorCode, + MARKER_PCB *fillMe) { - wxString textA = aArea->GetSelectMenuText(); - - wxPoint posA = aArea->GetPosition(); - - if( fillMe ) - { - fillMe->SetData( aErrorCode, posA, textA, posA ); - } - else - { - fillMe = new MARKER_PCB( aErrorCode, posA, textA, posA ); - fillMe->SetItem( aArea ); - } - - return fillMe; + return fillMarker(aPos, aItem, nullptr, aErrorCode, fillMe ); } -MARKER_PCB* DRC::fillMarker( const ZONE_CONTAINER* aArea, - const wxPoint& aPos, - int aErrorCode, - MARKER_PCB* fillMe ) +MARKER_PCB* DRC::fillMarker( const wxPoint& aPos, BOARD_ITEM* aItem, BOARD_ITEM* bItem, + int aErrorCode, MARKER_PCB* fillMe ) { - wxString textA = aArea->GetSelectMenuText(); - - wxPoint posA = aPos; - if( fillMe ) - { - fillMe->SetData( aErrorCode, posA, textA, posA ); - } + fillMe->SetData( aErrorCode, aPos, aItem, aPos, bItem, aPos ); else - { - fillMe = new MARKER_PCB( aErrorCode, posA, textA, posA ); - fillMe->SetItem( aArea ); - } + fillMe = new MARKER_PCB( aErrorCode, aPos, aItem, aPos, bItem, aPos ); return fillMe; } @@ -225,15 +164,3 @@ MARKER_PCB* DRC::fillMarker( int aErrorCode, const wxString& aMessage, MARKER_PC } -MARKER_PCB* DRC::fillMarker( const wxPoint& aPos, int aErrorCode, const wxString& aMessage, MARKER_PCB* fillMe ) -{ - wxPoint posA = aPos; - - if( fillMe ) - fillMe->SetData( aErrorCode, posA, aMessage, posA ); - else - fillMe = new MARKER_PCB( aErrorCode, posA, aMessage, posA ); - - return fillMe; -} - diff --git a/pcbnew/tools/pcb_actions.h b/pcbnew/tools/pcb_actions.h index cdd8f15698..b7ebf859dc 100644 --- a/pcbnew/tools/pcb_actions.h +++ b/pcbnew/tools/pcb_actions.h @@ -64,6 +64,9 @@ public: /// Unselects a list of items (specified as the event parameter) static TOOL_ACTION unselectItems; + /// Runs a selection menu to select from a list of items + static TOOL_ACTION selectionMenu; + /// Selects a connection between junctions. static TOOL_ACTION selectConnection; diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp index 348d2b83c8..b86fa984cf 100644 --- a/pcbnew/tools/selection_tool.cpp +++ b/pcbnew/tools/selection_tool.cpp @@ -92,6 +92,10 @@ TOOL_ACTION PCB_ACTIONS::selectionClear( "pcbnew.InteractiveSelection.Clear", AS_GLOBAL, 0, "", "" ); // No description, it is not supposed to be shown anywhere +TOOL_ACTION PCB_ACTIONS::selectionMenu( "pcbnew.InteractiveSelection.SelectionMenu", + AS_GLOBAL, 0, + "", "" ); // No description, it is not supposed to be shown anywhere + TOOL_ACTION PCB_ACTIONS::selectConnection( "pcbnew.InteractiveSelection.SelectConnection", AS_GLOBAL, TOOL_ACTION::LegacyHotKey( HK_SEL_TRIVIAL_CONNECTION ), _( "Trivial Connection" ), _( "Selects a connection between two junctions." ), add_tracks_xpm ); @@ -520,7 +524,7 @@ bool SELECTION_TOOL::selectPoint( const VECTOR2I& aWhere, bool aOnDrag, Wait( TOOL_EVENT( TC_ANY, TA_MOUSE_UP, BUT_LEFT ) ); } - BOARD_ITEM* item = disambiguationMenu( &collector ); + BOARD_ITEM* item = doSelectionMenu( &collector, _( "Clarify Selection" ) ); if( item ) { @@ -682,6 +686,7 @@ void SELECTION_TOOL::setTransitions() Go( &SELECTION_TOOL::SelectItems, PCB_ACTIONS::selectItems.MakeEvent() ); Go( &SELECTION_TOOL::UnselectItem, PCB_ACTIONS::unselectItem.MakeEvent() ); Go( &SELECTION_TOOL::UnselectItems, PCB_ACTIONS::unselectItems.MakeEvent() ); + Go( &SELECTION_TOOL::SelectionMenu, PCB_ACTIONS::selectionMenu.MakeEvent() ); Go( &SELECTION_TOOL::find, PCB_ACTIONS::find.MakeEvent() ); Go( &SELECTION_TOOL::findMove, PCB_ACTIONS::findMove.MakeEvent() ); Go( &SELECTION_TOOL::filterSelection, PCB_ACTIONS::filterSelection.MakeEvent() ); @@ -1368,10 +1373,11 @@ void SELECTION_TOOL::clearSelection() if( m_selection.Empty() ) return; - for( auto item : m_selection ) - unselectVisually( static_cast( item ) ); + while( m_selection.GetSize() ) + unhighlight( static_cast( m_selection.Front() ), SELECTED, m_selection ); + + view()->Update( &m_selection ); - m_selection.Clear(); m_selection.SetIsHover( false ); m_selection.ClearReferencePoint(); @@ -1387,10 +1393,20 @@ void SELECTION_TOOL::clearSelection() } -BOARD_ITEM* SELECTION_TOOL::disambiguationMenu( GENERAL_COLLECTOR* aCollector ) +int SELECTION_TOOL::SelectionMenu( const TOOL_EVENT& aEvent ) +{ + GENERAL_COLLECTOR* collector = aEvent.Parameter(); + doSelectionMenu( collector, wxEmptyString ); + + return 0; +} + + +BOARD_ITEM* SELECTION_TOOL::doSelectionMenu( GENERAL_COLLECTOR* aCollector, + const wxString& aTitle ) { BOARD_ITEM* current = NULL; - KIGFX::VIEW_GROUP highlightGroup; + SELECTION highlightGroup; CONTEXT_MENU menu; highlightGroup.SetLayer( LAYER_GP_OVERLAY ); @@ -1408,7 +1424,8 @@ BOARD_ITEM* SELECTION_TOOL::disambiguationMenu( GENERAL_COLLECTOR* aCollector ) menu.Add( menuText, i + 1, item->GetMenuImage() ); } - menu.SetTitle( _( "Clarify Selection" ) ); + if( aTitle.Length() ) + menu.SetTitle( aTitle ); menu.SetIcon( info_xpm ); menu.DisplayTitle( true ); SetContextMenu( &menu, CMENU_NOW ); @@ -1418,12 +1435,7 @@ BOARD_ITEM* SELECTION_TOOL::disambiguationMenu( GENERAL_COLLECTOR* aCollector ) if( evt->Action() == TA_CONTEXT_MENU_UPDATE ) { if( current ) - { - current->ClearBrightened(); - getView()->Hide( current, false ); - highlightGroup.Remove( current ); - getView()->MarkTargetDirty( KIGFX::TARGET_OVERLAY ); - } + unhighlight( current, BRIGHTENED, highlightGroup ); int id = *evt->GetCommandId(); @@ -1431,10 +1443,7 @@ BOARD_ITEM* SELECTION_TOOL::disambiguationMenu( GENERAL_COLLECTOR* aCollector ) if( id > 0 && id <= limit ) { current = ( *aCollector )[id - 1]; - current->SetBrightened(); - getView()->Hide( current, true ); - highlightGroup.Add( current ); - getView()->MarkTargetDirty( KIGFX::TARGET_OVERLAY ); + highlight( current, BRIGHTENED, highlightGroup ); } else { @@ -1444,12 +1453,7 @@ BOARD_ITEM* SELECTION_TOOL::disambiguationMenu( GENERAL_COLLECTOR* aCollector ) else if( evt->Action() == TA_CONTEXT_MENU_CHOICE ) { if( current ) - { - current->ClearBrightened(); - getView()->Hide( current, false ); - highlightGroup.Remove( current ); - getView()->MarkTargetDirty( KIGFX::TARGET_OVERLAY ); - } + unhighlight( current, BRIGHTENED, highlightGroup ); OPT id = evt->GetCommandId(); @@ -1464,7 +1468,6 @@ BOARD_ITEM* SELECTION_TOOL::disambiguationMenu( GENERAL_COLLECTOR* aCollector ) } getView()->Remove( &highlightGroup ); - return current; } @@ -1743,8 +1746,8 @@ void SELECTION_TOOL::select( BOARD_ITEM* aItem ) return; } - m_selection.Add( aItem ); - selectVisually( aItem ); + highlight( aItem, SELECTED, m_selection ); + view()->Update( &m_selection ); if( m_frame ) { @@ -1767,8 +1770,8 @@ void SELECTION_TOOL::unselect( BOARD_ITEM* aItem ) if( !aItem->IsSelected() ) return; - m_selection.Remove( aItem ); - unselectVisually( aItem ); + unhighlight( aItem, SELECTED, m_selection ); + view()->Update( &m_selection ); if( m_selection.Empty() ) { @@ -1781,49 +1784,77 @@ void SELECTION_TOOL::unselect( BOARD_ITEM* aItem ) } -void SELECTION_TOOL::selectVisually( BOARD_ITEM* aItem ) +void SELECTION_TOOL::highlight( BOARD_ITEM* aItem, int aMode, SELECTION& aGroup ) { + if( aMode == SELECTED ) + aItem->SetSelected(); + else if( aMode == BRIGHTENED ) + aItem->SetBrightened(); + // Hide the original item, so it is shown only on overlay - aItem->SetSelected(); view()->Hide( aItem, true ); + aGroup.Add( aItem ); + // Modules are treated in a special way - when they are selected, we have to // unselect all the parts that make the module, not the module itself - if( aItem->Type() == PCB_MODULE_T ) { static_cast( aItem )->RunOnChildren( [&] ( BOARD_ITEM* item ) { - item->SetSelected(); + if( aMode == SELECTED ) + item->SetSelected(); + else if( aMode == BRIGHTENED ) + { + item->SetBrightened(); + aGroup.Add( item ); + } view()->Hide( item, true ); }); } - view()->Update( &m_selection ); + // Many selections are very temporal and updating the display each time just + // creates noise. + if( aMode == BRIGHTENED ) + getView()->MarkTargetDirty( KIGFX::TARGET_OVERLAY ); } -void SELECTION_TOOL::unselectVisually( BOARD_ITEM* aItem ) +void SELECTION_TOOL::unhighlight( BOARD_ITEM* aItem, int aMode, SELECTION& aGroup ) { + if( aMode == SELECTED ) + aItem->ClearSelected(); + else if( aMode == BRIGHTENED ) + aItem->ClearBrightened(); + + aGroup.Remove( aItem ); + // Restore original item visibility - aItem->ClearSelected(); view()->Hide( aItem, false ); view()->Update( aItem ); // Modules are treated in a special way - when they are selected, we have to // unselect all the parts that make the module, not the module itself - if( aItem->Type() == PCB_MODULE_T ) { static_cast( aItem )->RunOnChildren( [&] ( BOARD_ITEM* item ) { - item->ClearSelected(); + if( aMode == SELECTED ) + item->ClearSelected(); + else if( aMode == BRIGHTENED ) + { + item->ClearBrightened(); + aGroup.Remove( item ); + } view()->Hide( item, false ); view()->Update( item ); }); } - view()->Update( &m_selection ); + // Many selections are very temporal and updating the display each time just + // creates noise. + if( aMode == BRIGHTENED ) + getView()->MarkTargetDirty( KIGFX::TARGET_OVERLAY ); } diff --git a/pcbnew/tools/selection_tool.h b/pcbnew/tools/selection_tool.h index 54eab30049..35e097415a 100644 --- a/pcbnew/tools/selection_tool.h +++ b/pcbnew/tools/selection_tool.h @@ -49,6 +49,7 @@ namespace KIGFX typedef void (*CLIENT_SELECTION_FILTER)( const VECTOR2I&, GENERAL_COLLECTOR& ); + /** * Class SELECTION_TOOL * @@ -127,6 +128,13 @@ public: ///> Multiple item unselection event handler int UnselectItems( const TOOL_EVENT& aEvent ); + /** + * Function SelectionMenu() + * Allows the selection of a single item from a list of items via a popup menu. The + * list is passed as aEvent's parameter. + */ + int SelectionMenu( const TOOL_EVENT& aEvent ); + ///> Event sent after an item is selected. static const TOOL_EVENT SelectedEvent; @@ -179,6 +187,13 @@ private: */ bool selectMultiple(); + /** + * Allows the selection of a single item from a list via pop-up menu. The items are + * highlighted on the canvas when hovered in the menu. + * @param aTitle (optional) Allows the menu to be titled (ie: "Clarify Selection"). + */ + BOARD_ITEM* doSelectionMenu( GENERAL_COLLECTOR* aItems, const wxString& aTitle ); + ///> Selects a trivial connection (between two junctions) of items in selection int selectConnection( const TOOL_EVENT& aEvent ); @@ -241,15 +256,6 @@ private: */ void clearSelection(); - /** - * Function disambiguationMenu() - * Handles the menu that allows one to select one of many items in case - * there is more than one item at the selected point (@see selectCursor()). - * - * @param aItems contains list of items that are displayed to the user. - */ - BOARD_ITEM* disambiguationMenu( GENERAL_COLLECTOR* aItems ); - /** * Function pickSmallestComponent() * Allows one to find the smallest (in terms of bounding box area) item from the list. @@ -301,17 +307,21 @@ private: /** * Function selectVisually() - * Marks item as selected, but does not add it to the ITEMS_PICKED_LIST. - * @param aItem is an item to be be marked. + * Highlights the item visually. + * @param aItem is an item to be be highlighted. + * @param aHighlightMode should be either SELECTED or BRIGHTENED + * @param aGroup is the group to add the item to in the BRIGHTENED mode. */ - void selectVisually( BOARD_ITEM* aItem ); + void highlight( BOARD_ITEM* aItem, int aHighlightMode, SELECTION& aGroup ); /** * Function unselectVisually() - * Marks item as selected, but does not add it to the ITEMS_PICKED_LIST. - * @param aItem is an item to be be marked. + * Unhighlights the item visually. + * @param aItem is an item to be be highlighted. + * @param aHighlightMode should be either SELECTED or BRIGHTENED + * @param aGroup is the group to remove the item from. */ - void unselectVisually( BOARD_ITEM* aItem ); + void unhighlight( BOARD_ITEM* aItem, int aHighlightMode, SELECTION& aGroup ); /** * Function selectionContains()