Fixed VIEW_ITEM memory leaks

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 <tomasz.wlostowski@cern.ch>
  * @author Maciej Suminski <maciej.suminski@cern.ch>
  *
@@ -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();
 

include/pcb_draw_panel_gal.h

@@ -107,10 +107,10 @@ protected:
     void setDefaultLayerDeps();
 
     ///> Currently used worksheet
-    KIGFX::WORKSHEET_VIEWITEM* m_worksheet;
+    std::unique_ptr<KIGFX::WORKSHEET_VIEWITEM> m_worksheet;
 
     ///> Ratsnest view item
-    KIGFX::RATSNEST_VIEWITEM* m_ratsnest;
+    std::unique_ptr<KIGFX::RATSNEST_VIEWITEM> m_ratsnest;
 };
 
 #endif /* PCB_DRAW_PANEL_GAL_H_ */

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 <maciej.suminski@cern.ch>
  *
  * 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_ratsnest.reset( new KIGFX::RATSNEST_VIEWITEM( aBoard->GetRatsnest() ) );
-    {
+    m_view->Add( m_ratsnest.get() );
-        m_view->Remove( m_ratsnest );
-        delete m_ratsnest;
-    }
-
-    m_ratsnest = new KIGFX::RATSNEST_VIEWITEM( aBoard->GetRatsnest() );
-    m_view->Add( m_ratsnest );
 
     // 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_worksheet.reset( aWorksheet );
-    {
+    m_view->Add( m_worksheet.get() );
-        m_view->Remove( m_worksheet );
-        delete m_worksheet;
-    }
-
-    m_worksheet = aWorksheet;
-    m_view->Add( m_worksheet );
 }
 
 

pcbnew/router/pns_kicad_iface.cpp

@@ -338,7 +338,6 @@ public:
     ~PNS_PCBNEW_DEBUG_DECORATOR()
     {
         Clear();
-        m_view->Remove( m_items );
         delete m_items;
     }
 

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 ),
+    m_placeOrigin.reset( new KIGFX::ORIGIN_VIEWITEM( KIGFX::COLOR4D( 0.8, 0.0, 0.0, 1.0 ),
-                                                KIGFX::ORIGIN_VIEWITEM::CIRCLE_CROSS );
+                                                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<BOARD>()->GetAuxOrigin() );
-        getView()->Remove( m_placeOrigin );
+        getView()->Remove( m_placeOrigin.get() );
-        getView()->Add( m_placeOrigin );
+        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();
 

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<KIGFX::ORIGIN_VIEWITEM> m_placeOrigin;
 
     ///> Flag to ignore a single crossprobe message from eeschema.
     bool m_probingSchToPcb;

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<BOARD>()->GetGridOrigin() );
-        getView()->Remove( m_gridOrigin );
+        getView()->Remove( m_gridOrigin.get() );
-        getView()->Add( m_gridOrigin );
+        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();
     }

pcbnew/tools/pcbnew_control.h

@@ -26,6 +26,7 @@
 #define PCBNEW_CONTROL_H
 
 #include <tool/tool_interactive.h>
+#include <memory>
 
 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<KIGFX::ORIGIN_VIEWITEM> m_gridOrigin;
 
     ///> Applies the legacy canvas grid settings for GAL.
     void updateGrid();