From 906ee77dbf87a80c43672eddaa188ff73ac2b4bc Mon Sep 17 00:00:00 2001 From: Maciej Suminski Date: Thu, 2 Mar 2017 23:57:13 +0100 Subject: [PATCH] Fixed VIEW_ITEM memory leaks --- common/view/view.cpp | 16 +++++++++++----- include/pcb_draw_panel_gal.h | 4 ++-- pcbnew/pcb_draw_panel_gal.cpp | 27 +++++---------------------- pcbnew/router/pns_kicad_iface.cpp | 1 - pcbnew/tools/pcb_editor_control.cpp | 13 +++++-------- pcbnew/tools/pcb_editor_control.h | 2 +- pcbnew/tools/pcbnew_control.cpp | 12 +++++------- pcbnew/tools/pcbnew_control.h | 3 ++- 8 files changed, 31 insertions(+), 47 deletions(-) diff --git a/common/view/view.cpp b/common/view/view.cpp index 6018e7b743..726e0a734b 100644 --- a/common/view/view.cpp +++ b/common/view/view.cpp @@ -1,7 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * - * Copyright (C) 2013-2016 CERN + * Copyright (C) 2013-2017 CERN * @author Tomasz Wlostowski * @author Maciej Suminski * @@ -250,7 +250,10 @@ void VIEW::OnDestroy( VIEW_ITEM* aItem ) if( !data ) return; - data->m_view->Remove( aItem ); + if( data->m_view ) + data->m_view->Remove( aItem ); + + delete data; } @@ -311,7 +314,9 @@ void VIEW::Add( VIEW_ITEM* aItem, int aDrawPriority ) if( aDrawPriority < 0 ) aDrawPriority = m_nextDrawPriority++; - aItem->m_viewPrivData = new VIEW_ITEM_DATA; + if( !aItem->m_viewPrivData ) + aItem->m_viewPrivData = new VIEW_ITEM_DATA; + aItem->m_viewPrivData->m_view = this; aItem->m_viewPrivData->m_drawPriority = aDrawPriority; @@ -342,6 +347,7 @@ void VIEW::Remove( VIEW_ITEM* aItem ) if( !viewData ) return; + wxASSERT( viewData->m_view == this ); auto item = std::find( m_allItems.begin(), m_allItems.end(), aItem ); if( item != m_allItems.end() ) @@ -367,6 +373,7 @@ void VIEW::Remove( VIEW_ITEM* aItem ) } viewData->deleteGroups(); + viewData->m_view = nullptr; } @@ -826,8 +833,8 @@ struct VIEW::drawItem bool operator()( VIEW_ITEM* aItem ) { + wxASSERT( aItem->viewPrivData() ); - assert( aItem->viewPrivData() ); // Conditions that have te be fulfilled for an item to be drawn bool drawCondition = aItem->viewPrivData()->isRenderable() && aItem->ViewGetLOD( layer, view ) < view->m_scale; @@ -974,7 +981,6 @@ struct VIEW::recacheItem void VIEW::Clear() { BOX2I r; - r.SetMaximum(); m_allItems.clear(); diff --git a/include/pcb_draw_panel_gal.h b/include/pcb_draw_panel_gal.h index db84a479a5..dfd3a9808f 100644 --- a/include/pcb_draw_panel_gal.h +++ b/include/pcb_draw_panel_gal.h @@ -107,10 +107,10 @@ protected: void setDefaultLayerDeps(); ///> Currently used worksheet - KIGFX::WORKSHEET_VIEWITEM* m_worksheet; + std::unique_ptr m_worksheet; ///> Ratsnest view item - KIGFX::RATSNEST_VIEWITEM* m_ratsnest; + std::unique_ptr m_ratsnest; }; #endif /* PCB_DRAW_PANEL_GAL_H_ */ diff --git a/pcbnew/pcb_draw_panel_gal.cpp b/pcbnew/pcb_draw_panel_gal.cpp index 3742f137ca..6c80c2ceb4 100644 --- a/pcbnew/pcb_draw_panel_gal.cpp +++ b/pcbnew/pcb_draw_panel_gal.cpp @@ -1,7 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * - * Copyright (C) 2014 CERN + * Copyright (C) 2014-2017 CERN * @author Maciej Suminski * * This program is free software; you can redistribute it and/or @@ -101,9 +101,6 @@ PCB_DRAW_PANEL_GAL::PCB_DRAW_PANEL_GAL( wxWindow* aParentWindow, wxWindowID aWin KIGFX::GAL_DISPLAY_OPTIONS& aOptions, GAL_TYPE aGalType ) : EDA_DRAW_PANEL_GAL( aParentWindow, aWindowId, aPosition, aSize, aOptions, aGalType ) { - m_worksheet = NULL; - m_ratsnest = NULL; - setDefaultLayerOrder(); setDefaultLayerDeps(); @@ -126,8 +123,6 @@ EDA_DRAW_PANEL_GAL( aParentWindow, aWindowId, aPosition, aSize, aOptions, aGalTy PCB_DRAW_PANEL_GAL::~PCB_DRAW_PANEL_GAL() { delete m_painter; - delete m_worksheet; - delete m_ratsnest; } @@ -159,14 +154,8 @@ void PCB_DRAW_PANEL_GAL::DisplayBoard( const BOARD* aBoard ) m_view->Add( zone ); // Ratsnest - if( m_ratsnest ) - { - m_view->Remove( m_ratsnest ); - delete m_ratsnest; - } - - m_ratsnest = new KIGFX::RATSNEST_VIEWITEM( aBoard->GetRatsnest() ); - m_view->Add( m_ratsnest ); + m_ratsnest.reset( new KIGFX::RATSNEST_VIEWITEM( aBoard->GetRatsnest() ) ); + m_view->Add( m_ratsnest.get() ); // Display settings UseColorScheme( aBoard->GetColorsSettings() ); @@ -175,14 +164,8 @@ void PCB_DRAW_PANEL_GAL::DisplayBoard( const BOARD* aBoard ) void PCB_DRAW_PANEL_GAL::SetWorksheet( KIGFX::WORKSHEET_VIEWITEM* aWorksheet ) { - if( m_worksheet ) - { - m_view->Remove( m_worksheet ); - delete m_worksheet; - } - - m_worksheet = aWorksheet; - m_view->Add( m_worksheet ); + m_worksheet.reset( aWorksheet ); + m_view->Add( m_worksheet.get() ); } diff --git a/pcbnew/router/pns_kicad_iface.cpp b/pcbnew/router/pns_kicad_iface.cpp index 5816ce9588..a8b291c363 100644 --- a/pcbnew/router/pns_kicad_iface.cpp +++ b/pcbnew/router/pns_kicad_iface.cpp @@ -338,7 +338,6 @@ public: ~PNS_PCBNEW_DEBUG_DECORATOR() { Clear(); - m_view->Remove( m_items ); delete m_items; } diff --git a/pcbnew/tools/pcb_editor_control.cpp b/pcbnew/tools/pcb_editor_control.cpp index 9888978346..ef6cd88900 100644 --- a/pcbnew/tools/pcb_editor_control.cpp +++ b/pcbnew/tools/pcb_editor_control.cpp @@ -232,17 +232,14 @@ PCB_EDITOR_CONTROL::PCB_EDITOR_CONTROL() : PCB_TOOL( "pcbnew.EditorControl" ), m_frame( nullptr ) { - m_placeOrigin = new KIGFX::ORIGIN_VIEWITEM( KIGFX::COLOR4D( 0.8, 0.0, 0.0, 1.0 ), - KIGFX::ORIGIN_VIEWITEM::CIRCLE_CROSS ); + m_placeOrigin.reset( new KIGFX::ORIGIN_VIEWITEM( KIGFX::COLOR4D( 0.8, 0.0, 0.0, 1.0 ), + KIGFX::ORIGIN_VIEWITEM::CIRCLE_CROSS ) ); m_probingSchToPcb = false; } PCB_EDITOR_CONTROL::~PCB_EDITOR_CONTROL() { - getView()->Remove( m_placeOrigin ); - - delete m_placeOrigin; } @@ -253,8 +250,8 @@ void PCB_EDITOR_CONTROL::Reset( RESET_REASON aReason ) if( aReason == MODEL_RELOAD || aReason == GAL_SWITCH ) { m_placeOrigin->SetPosition( getModel()->GetAuxOrigin() ); - getView()->Remove( m_placeOrigin ); - getView()->Add( m_placeOrigin ); + getView()->Remove( m_placeOrigin.get() ); + getView()->Add( m_placeOrigin.get() ); } } @@ -977,7 +974,7 @@ int PCB_EDITOR_CONTROL::DrillOrigin( const TOOL_EVENT& aEvent ) assert( picker ); m_frame->SetToolID( ID_PCB_PLACE_OFFSET_COORD_BUTT, wxCURSOR_PENCIL, _( "Adjust zero" ) ); - picker->SetClickHandler( std::bind( setDrillOrigin, getView(), m_frame, m_placeOrigin, _1 ) ); + picker->SetClickHandler( std::bind( setDrillOrigin, getView(), m_frame, m_placeOrigin.get(), _1 ) ); picker->Activate(); Wait(); diff --git a/pcbnew/tools/pcb_editor_control.h b/pcbnew/tools/pcb_editor_control.h index 5f8ba280b7..24bcd1f7b5 100644 --- a/pcbnew/tools/pcb_editor_control.h +++ b/pcbnew/tools/pcb_editor_control.h @@ -110,7 +110,7 @@ private: PCB_EDIT_FRAME* m_frame; ///> Place & drill origin marker. - KIGFX::ORIGIN_VIEWITEM* m_placeOrigin; + std::unique_ptr m_placeOrigin; ///> Flag to ignore a single crossprobe message from eeschema. bool m_probingSchToPcb; diff --git a/pcbnew/tools/pcbnew_control.cpp b/pcbnew/tools/pcbnew_control.cpp index a96a77b31e..3ff1cc3dbb 100644 --- a/pcbnew/tools/pcbnew_control.cpp +++ b/pcbnew/tools/pcbnew_control.cpp @@ -229,14 +229,12 @@ TOOL_ACTION PCB_ACTIONS::toBeDone( "pcbnew.Control.toBeDone", PCBNEW_CONTROL::PCBNEW_CONTROL() : TOOL_INTERACTIVE( "pcbnew.Control" ), m_frame( NULL ) { - m_gridOrigin = new KIGFX::ORIGIN_VIEWITEM(); + m_gridOrigin.reset( new KIGFX::ORIGIN_VIEWITEM() ); } PCBNEW_CONTROL::~PCBNEW_CONTROL() { - getView()->Remove( m_gridOrigin ); - delete m_gridOrigin; } @@ -247,8 +245,8 @@ void PCBNEW_CONTROL::Reset( RESET_REASON aReason ) if( aReason == MODEL_RELOAD || aReason == GAL_SWITCH ) { m_gridOrigin->SetPosition( getModel()->GetGridOrigin() ); - getView()->Remove( m_gridOrigin ); - getView()->Add( m_gridOrigin ); + getView()->Remove( m_gridOrigin.get() ); + getView()->Add( m_gridOrigin.get() ); } } @@ -731,7 +729,7 @@ int PCBNEW_CONTROL::GridSetOrigin( const TOOL_EVENT& aEvent ) if( origin ) { - setOrigin( getView(), m_frame, m_gridOrigin, *origin ); + setOrigin( getView(), m_frame, m_gridOrigin.get(), *origin ); delete origin; } else @@ -743,7 +741,7 @@ int PCBNEW_CONTROL::GridSetOrigin( const TOOL_EVENT& aEvent ) // TODO it will not check the toolbar button in module editor, as it uses a different ID.. m_frame->SetToolID( ID_PCB_PLACE_GRID_COORD_BUTT, wxCURSOR_PENCIL, _( "Adjust grid origin" ) ); - picker->SetClickHandler( std::bind( setOrigin, getView(), m_frame, m_gridOrigin, _1 ) ); + picker->SetClickHandler( std::bind( setOrigin, getView(), m_frame, m_gridOrigin.get(), _1 ) ); picker->Activate(); Wait(); } diff --git a/pcbnew/tools/pcbnew_control.h b/pcbnew/tools/pcbnew_control.h index 6ca4ee1088..2e017cb05a 100644 --- a/pcbnew/tools/pcbnew_control.h +++ b/pcbnew/tools/pcbnew_control.h @@ -26,6 +26,7 @@ #define PCBNEW_CONTROL_H #include +#include namespace KIGFX { class ORIGIN_VIEWITEM; @@ -94,7 +95,7 @@ private: PCB_BASE_FRAME* m_frame; ///> Grid origin marker. - KIGFX::ORIGIN_VIEWITEM* m_gridOrigin; + std::unique_ptr m_gridOrigin; ///> Applies the legacy canvas grid settings for GAL. void updateGrid();