From d6bc33bd4203e6bd2f09270baa29d3cab8341ddb Mon Sep 17 00:00:00 2001 From: Maciej Suminski Date: Fri, 24 Jul 2015 09:42:46 +0200 Subject: [PATCH] Removed a few more memory leaks and fixed crash on exit. --- common/tool/context_menu.cpp | 7 +++++-- common/tool/tool_manager.cpp | 6 +++++- pcbnew/router/router_tool.cpp | 8 +++++--- pcbnew/tools/conditional_menu.cpp | 26 +++++++------------------- pcbnew/tools/conditional_menu.h | 13 ++++--------- pcbnew/tools/pcb_editor_control.cpp | 6 ++++-- pcbnew/tools/pcb_editor_control.h | 4 ++++ pcbnew/tools/placement_tool.cpp | 21 +++++++++++---------- pcbnew/tools/placement_tool.h | 2 ++ pcbnew/tools/selection_tool.cpp | 20 +++++++++++++++----- pcbnew/tools/selection_tool.h | 6 ++++++ 11 files changed, 68 insertions(+), 51 deletions(-) diff --git a/common/tool/context_menu.cpp b/common/tool/context_menu.cpp index ed51b44d41..4d87160b67 100644 --- a/common/tool/context_menu.cpp +++ b/common/tool/context_menu.cpp @@ -131,6 +131,9 @@ wxMenuItem* CONTEXT_MENU::Add( const TOOL_ACTION& aAction ) std::list CONTEXT_MENU::Add( CONTEXT_MENU* aMenu, const wxString& aLabel, bool aExpand ) { std::list items; + CONTEXT_MENU* menuCopy = new CONTEXT_MENU( *aMenu ); + m_submenus.push_back( menuCopy ); + menuCopy->m_parent = this; if( aExpand ) { @@ -146,12 +149,12 @@ std::list CONTEXT_MENU::Add( CONTEXT_MENU* aMenu, const wxString& a { wxMenuItem* newItem = new wxMenuItem( this, -1, aLabel, wxEmptyString, wxITEM_NORMAL ); newItem->SetBitmap( KiBitmap( aMenu->m_icon ) ); - newItem->SetSubMenu( aMenu ); + newItem->SetSubMenu( menuCopy ); items.push_back( Append( newItem ) ); } else { - items.push_back( AppendSubMenu( aMenu, aLabel ) ); + items.push_back( AppendSubMenu( menuCopy, aLabel ) ); } } diff --git a/common/tool/tool_manager.cpp b/common/tool/tool_manager.cpp index 3da57c9961..c69bb4ff62 100644 --- a/common/tool/tool_manager.cpp +++ b/common/tool/tool_manager.cpp @@ -592,6 +592,8 @@ void TOOL_MANAGER::dispatchContextMenu( const TOOL_EVENT& aEvent ) st->pendingWait = true; st->waitEvents = TOOL_EVENT( TC_ANY, TA_ANY ); + CONTEXT_MENU* m = st->contextMenu; + if( st->contextMenuTrigger == CMENU_NOW ) st->contextMenuTrigger = CMENU_OFF; @@ -604,17 +606,19 @@ void TOOL_MANAGER::dispatchContextMenu( const TOOL_EVENT& aEvent ) // Run update handlers st->contextMenu->UpdateAll(); - boost::scoped_ptr menu( new CONTEXT_MENU( *st->contextMenu ) ); + boost::scoped_ptr menu( new CONTEXT_MENU( *m ) ); GetEditFrame()->PopupMenu( menu.get() ); // If nothing was chosen from the context menu, we must notify the tool as well if( menu->GetSelected() < 0 ) { TOOL_EVENT evt( TC_COMMAND, TA_CONTEXT_MENU_CHOICE, -1 ); + evt.SetParameter( m ); dispatchInternal( evt ); } TOOL_EVENT evt( TC_COMMAND, TA_CONTEXT_MENU_CLOSED ); + evt.SetParameter( m ); dispatchInternal( evt ); m_viewControls->ForceCursorPosition( forcedCursor, cursorPos ); diff --git a/pcbnew/router/router_tool.cpp b/pcbnew/router/router_tool.cpp index 533bed91a6..d721de8d6e 100644 --- a/pcbnew/router/router_tool.cpp +++ b/pcbnew/router/router_tool.cpp @@ -211,9 +211,8 @@ public: AppendSeparator(); - CONTEXT_TRACK_WIDTH_MENU* trackMenu = new CONTEXT_TRACK_WIDTH_MENU; - trackMenu->SetBoard( aBoard ); - Add( trackMenu, _( "Select Track/Via Width" ) ); + m_widthMenu.SetBoard( aBoard ); + Add( &m_widthMenu, _( "Select Track/Via Width" ) ); Add( ACT_CustomTrackWidth ); @@ -223,6 +222,9 @@ public: AppendSeparator(); Add( PNS_TOOL_BASE::ACT_RouterOptions ); } + +private: + CONTEXT_TRACK_WIDTH_MENU m_widthMenu; }; diff --git a/pcbnew/tools/conditional_menu.cpp b/pcbnew/tools/conditional_menu.cpp index 95954b2169..db00c05479 100644 --- a/pcbnew/tools/conditional_menu.cpp +++ b/pcbnew/tools/conditional_menu.cpp @@ -23,16 +23,7 @@ */ #include "conditional_menu.h" - -CONDITIONAL_MENU::~CONDITIONAL_MENU() -{ - for( std::list::iterator it = m_entries.begin(); it != m_entries.end(); ++it ) - { - if( it->Type() == ENTRY::MENU ) - delete it->Menu(); - } -} - +#include void CONDITIONAL_MENU::AddItem( const TOOL_ACTION& aAction, const SELECTION_CONDITION& aCondition, int aOrder ) @@ -55,12 +46,9 @@ void CONDITIONAL_MENU::AddSeparator( const SELECTION_CONDITION& aCondition, int } -CONTEXT_MENU& CONDITIONAL_MENU::Generate( SELECTION& aSelection ) +CONTEXT_MENU* CONDITIONAL_MENU::Generate( SELECTION& aSelection ) { - wxMenuItemList items = m_menu.GetMenuItems(); - - for( wxMenuItemList::iterator it = items.begin(); it != items.end(); ++it ) - m_menu.Remove( (*it) ); + CONTEXT_MENU* m_menu = new CONTEXT_MENU; for( std::list::iterator it = m_entries.begin(); it != m_entries.end(); ++it ) { @@ -72,19 +60,19 @@ CONTEXT_MENU& CONDITIONAL_MENU::Generate( SELECTION& aSelection ) switch( it->Type() ) { case ENTRY::ACTION: - m_menu.Add( *it->Action() ); + m_menu->Add( *it->Action() ); break; case ENTRY::MENU: - m_menu.Add( it->Menu(), it->Label(), it->Expand() ); + m_menu->Add( it->Menu(), it->Label(), it->Expand() ); break; case ENTRY::WXITEM: - m_menu.Append( it->wxItem() ); + m_menu->Append( it->wxItem() ); break; case ENTRY::SEPARATOR: - m_menu.AppendSeparator(); + m_menu->AppendSeparator(); break; default: diff --git a/pcbnew/tools/conditional_menu.h b/pcbnew/tools/conditional_menu.h index 68a705c8b1..fe49a28bc7 100644 --- a/pcbnew/tools/conditional_menu.h +++ b/pcbnew/tools/conditional_menu.h @@ -25,19 +25,17 @@ #ifndef CONDITIONAL_MENU_H #define CONDITIONAL_MENU_H -#include #include "selection_conditions.h" - #include +#include class SELECTION_TOOL; +class TOOL_ACTION; +class CONTEXT_MENU; class CONDITIONAL_MENU { public: - CONDITIONAL_MENU() {} - ~CONDITIONAL_MENU(); - ///> Constant to indicate that we do not care about an ENTRY location in the menu. static const int ANY_ORDER = -1; @@ -89,12 +87,9 @@ public: * @param aSelection is selection for which the conditions are checked against. * @return Menu filtered by the entry conditions. */ - CONTEXT_MENU& Generate( SELECTION& aSelection ); + CONTEXT_MENU* Generate( SELECTION& aSelection ); private: - ///> Returned menu instance, prepared by Generate() function. - CONTEXT_MENU m_menu; - ///> Helper class to organize menu entries. class ENTRY { diff --git a/pcbnew/tools/pcb_editor_control.cpp b/pcbnew/tools/pcb_editor_control.cpp index 79d2713579..9772a34dcd 100644 --- a/pcbnew/tools/pcb_editor_control.cpp +++ b/pcbnew/tools/pcb_editor_control.cpp @@ -82,7 +82,7 @@ private: PCB_EDITOR_CONTROL::PCB_EDITOR_CONTROL() : - TOOL_INTERACTIVE( "pcbnew.EditorControl" ), m_frame( NULL ) + TOOL_INTERACTIVE( "pcbnew.EditorControl" ), m_frame( NULL ), m_zoneMenu( NULL ) { m_placeOrigin = new KIGFX::ORIGIN_VIEWITEM( KIGFX::COLOR4D( 0.8, 0.0, 0.0, 1.0 ), KIGFX::ORIGIN_VIEWITEM::CROSS ); @@ -93,6 +93,7 @@ PCB_EDITOR_CONTROL::PCB_EDITOR_CONTROL() : PCB_EDITOR_CONTROL::~PCB_EDITOR_CONTROL() { delete m_placeOrigin; + delete m_zoneMenu; } @@ -115,7 +116,8 @@ bool PCB_EDITOR_CONTROL::Init() if( selTool ) { - selTool->GetMenu().AddMenu( new ZONE_CONTEXT_MENU, _( "Zones" ), false, + m_zoneMenu = new ZONE_CONTEXT_MENU; + selTool->GetMenu().AddMenu( m_zoneMenu, _( "Zones" ), false, SELECTION_CONDITIONS::OnlyType( PCB_ZONE_AREA_T ) ); } diff --git a/pcbnew/tools/pcb_editor_control.h b/pcbnew/tools/pcb_editor_control.h index f3536a588a..606ee43f4d 100644 --- a/pcbnew/tools/pcb_editor_control.h +++ b/pcbnew/tools/pcb_editor_control.h @@ -30,7 +30,9 @@ namespace KIGFX { class ORIGIN_VIEWITEM; } + class PCB_EDIT_FRAME; +class ZONE_CONTEXT_MENU; /** * Class PCB_EDITOR_CONTROL @@ -104,6 +106,8 @@ private: // How does line width change after one -/+ key press. static const int WIDTH_STEP; + + ZONE_CONTEXT_MENU* m_zoneMenu; }; #endif diff --git a/pcbnew/tools/placement_tool.cpp b/pcbnew/tools/placement_tool.cpp index fc9a71a8af..d962492e07 100644 --- a/pcbnew/tools/placement_tool.cpp +++ b/pcbnew/tools/placement_tool.cpp @@ -35,12 +35,13 @@ #include PLACEMENT_TOOL::PLACEMENT_TOOL() : - TOOL_INTERACTIVE( "pcbnew.Placement" ), m_selectionTool( NULL ) + TOOL_INTERACTIVE( "pcbnew.Placement" ), m_selectionTool( NULL ), m_placementMenu( NULL ) { } PLACEMENT_TOOL::~PLACEMENT_TOOL() { + delete m_placementMenu; } @@ -56,15 +57,15 @@ bool PLACEMENT_TOOL::Init() } // Create a context menu and make it available through selection tool - CONTEXT_MENU* menu = new CONTEXT_MENU; - menu->Add( COMMON_ACTIONS::alignTop ); - menu->Add( COMMON_ACTIONS::alignBottom ); - menu->Add( COMMON_ACTIONS::alignLeft ); - menu->Add( COMMON_ACTIONS::alignRight ); - menu->AppendSeparator(); - menu->Add( COMMON_ACTIONS::distributeHorizontally ); - menu->Add( COMMON_ACTIONS::distributeVertically ); - m_selectionTool->GetMenu().AddMenu( menu, _( "Align/distribute" ), false, + m_placementMenu = new CONTEXT_MENU; + m_placementMenu->Add( COMMON_ACTIONS::alignTop ); + m_placementMenu->Add( COMMON_ACTIONS::alignBottom ); + m_placementMenu->Add( COMMON_ACTIONS::alignLeft ); + m_placementMenu->Add( COMMON_ACTIONS::alignRight ); + m_placementMenu->AppendSeparator(); + m_placementMenu->Add( COMMON_ACTIONS::distributeHorizontally ); + m_placementMenu->Add( COMMON_ACTIONS::distributeVertically ); + m_selectionTool->GetMenu().AddMenu( m_placementMenu, _( "Align/distribute" ), false, SELECTION_CONDITIONS::MoreThan( 1 ) ); return true; diff --git a/pcbnew/tools/placement_tool.h b/pcbnew/tools/placement_tool.h index e7beb75de4..db8ec92965 100644 --- a/pcbnew/tools/placement_tool.h +++ b/pcbnew/tools/placement_tool.h @@ -82,6 +82,8 @@ public: private: SELECTION_TOOL* m_selectionTool; + + CONTEXT_MENU* m_placementMenu; }; #endif /* PLACEMENT_TOOL_H_ */ diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp index f4a8299d87..587818722f 100644 --- a/pcbnew/tools/selection_tool.cpp +++ b/pcbnew/tools/selection_tool.cpp @@ -71,7 +71,7 @@ public: SELECTION_TOOL::SELECTION_TOOL() : TOOL_INTERACTIVE( "pcbnew.InteractiveSelection" ), m_frame( NULL ), m_additive( false ), m_multiple( false ), - m_editModules( false ), m_locked( true ) + m_editModules( false ), m_locked( true ), m_contextMenu( NULL ), m_selectMenu( NULL ) { // Do not leave uninitialized members: m_preliminary = false; @@ -81,6 +81,8 @@ SELECTION_TOOL::SELECTION_TOOL() : SELECTION_TOOL::~SELECTION_TOOL() { delete m_selection.group; + delete m_contextMenu; + delete m_selectMenu; } @@ -88,7 +90,8 @@ bool SELECTION_TOOL::Init() { m_selection.group = new KIGFX::VIEW_GROUP; - m_menu.AddMenu( new SELECT_MENU, _( "Select..." ), false, + m_selectMenu = new SELECT_MENU; + m_menu.AddMenu( m_selectMenu, _( "Select..." ), false, ( SELECTION_CONDITIONS::OnlyType( PCB_VIA_T ) || SELECTION_CONDITIONS::OnlyType( PCB_TRACE_T ) ) && SELECTION_CONDITIONS::Count( 1 ) ); @@ -165,10 +168,11 @@ int SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) if( emptySelection ) selectPoint( evt->Position() ); - CONTEXT_MENU& contextMenu = m_menu.Generate( m_selection ); + delete m_contextMenu; + m_contextMenu = m_menu.Generate( m_selection ); - if( contextMenu.GetMenuItemCount() > 0 ) - SetContextMenu( &contextMenu, CMENU_NOW ); + if( m_contextMenu->GetMenuItemCount() > 0 ) + SetContextMenu( m_contextMenu, CMENU_NOW ); m_preliminary = emptySelection; } @@ -274,6 +278,12 @@ int SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) { if( m_preliminary ) clearSelection(); + + if( evt->Parameter() == m_contextMenu ) + { + delete m_contextMenu; + m_contextMenu = NULL; + } } } diff --git a/pcbnew/tools/selection_tool.h b/pcbnew/tools/selection_tool.h index f9ccee73ac..b8937129c6 100644 --- a/pcbnew/tools/selection_tool.h +++ b/pcbnew/tools/selection_tool.h @@ -38,6 +38,7 @@ class PCB_BASE_FRAME; class SELECTION_AREA; class BOARD_ITEM; class GENERAL_COLLECTOR; +class SELECT_MENU; namespace KIGFX { @@ -343,6 +344,11 @@ private: /// Menu displayed by the tool. CONDITIONAL_MENU m_menu; + + // TODO + CONTEXT_MENU* m_contextMenu; + + SELECT_MENU* m_selectMenu; }; #endif