From d8fd132100fec9b8705b222eb91883edb1a0ec6e Mon Sep 17 00:00:00 2001 From: Ian McInerney Date: Thu, 10 Oct 2019 01:11:14 +0200 Subject: [PATCH] Ensure that all tools are cancelled before destroying frames Fixes: lp:1847532 * https://bugs.launchpad.net/kicad/+bug/1847532 --- common/eda_base_frame.cpp | 10 ++++++++++ cvpcb/cvpcb_mainframe.cpp | 5 ++--- cvpcb/display_footprints_frame.cpp | 7 +++---- eeschema/libedit/lib_edit_frame.cpp | 3 +++ eeschema/sch_edit_frame.cpp | 3 +++ eeschema/viewlib_frame.cpp | 3 +++ gerbview/gerbview_frame.cpp | 6 +++--- include/eda_base_frame.h | 15 ++++++++++----- kicad/kicad_manager_frame.cpp | 5 ++--- pagelayout_editor/pl_editor_frame.cpp | 7 +++++++ pagelayout_editor/pl_editor_frame.h | 2 +- pcbnew/footprint_edit_frame.cpp | 3 +++ pcbnew/footprint_viewer_frame.cpp | 3 +++ pcbnew/footprint_wizard_frame.cpp | 7 +++---- pcbnew/pcb_edit_frame.cpp | 2 ++ 15 files changed, 58 insertions(+), 23 deletions(-) diff --git a/common/eda_base_frame.cpp b/common/eda_base_frame.cpp index 05863e1ce..bb4cca502 100644 --- a/common/eda_base_frame.cpp +++ b/common/eda_base_frame.cpp @@ -161,6 +161,16 @@ EDA_BASE_FRAME::~EDA_BASE_FRAME() } +void EDA_BASE_FRAME::EmptyToolStack() +{ + // Send a cancel command to all tools until the tool stack is empty + if( m_toolManager ) + { + while( !ToolStackIsEmpty() ) + m_toolManager->DeactivateTool(); + } +} + // TODO: Implement an RAII mechanism for the stack PushTool/PopTool pairs void EDA_BASE_FRAME::PushTool( const std::string& actionName ) { diff --git a/cvpcb/cvpcb_mainframe.cpp b/cvpcb/cvpcb_mainframe.cpp index 70d8c546a..463fa5a6b 100644 --- a/cvpcb/cvpcb_mainframe.cpp +++ b/cvpcb/cvpcb_mainframe.cpp @@ -183,9 +183,8 @@ CVPCB_MAINFRAME::CVPCB_MAINFRAME( KIWAY* aKiway, wxWindow* aParent ) : CVPCB_MAINFRAME::~CVPCB_MAINFRAME() { - // Be sure a active tool (if exists) is deactivated: - if( m_toolManager ) - m_toolManager->DeactivateTool(); + // Deactivate all tools on the tool stack + EmptyToolStack(); // Clean up the tool infrastructure delete m_actions; diff --git a/cvpcb/display_footprints_frame.cpp b/cvpcb/display_footprints_frame.cpp index 727e05553..4b2382cdf 100644 --- a/cvpcb/display_footprints_frame.cpp +++ b/cvpcb/display_footprints_frame.cpp @@ -151,16 +151,15 @@ DISPLAY_FOOTPRINTS_FRAME::DISPLAY_FOOTPRINTS_FRAME( KIWAY* aKiway, wxWindow* aPa DISPLAY_FOOTPRINTS_FRAME::~DISPLAY_FOOTPRINTS_FRAME() { + // Deactivate all tools on the tool stack + EmptyToolStack(); + GetBoard()->DeleteAllModules(); GetCanvas()->StopDrawing(); GetCanvas()->GetView()->Clear(); // Be sure any event cannot be fired after frame deletion: GetCanvas()->SetEvtHandlerEnabled( false ); - // Be sure a active tool (if exists) is deactivated: - if( m_toolManager ) - m_toolManager->DeactivateTool(); - delete GetScreen(); SetScreen( NULL ); // Be sure there is no double deletion } diff --git a/eeschema/libedit/lib_edit_frame.cpp b/eeschema/libedit/lib_edit_frame.cpp index 448164ea8..b12bbc9f1 100644 --- a/eeschema/libedit/lib_edit_frame.cpp +++ b/eeschema/libedit/lib_edit_frame.cpp @@ -194,6 +194,9 @@ LIB_EDIT_FRAME::LIB_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) : LIB_EDIT_FRAME::~LIB_EDIT_FRAME() { + // Deactivate all tools on the tool stack + EmptyToolStack(); + // current screen is destroyed in EDA_DRAW_FRAME SetScreen( m_dummyScreen ); diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index 60f42e3d3..83fc69f82 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -306,6 +306,9 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ): SCH_EDIT_FRAME::~SCH_EDIT_FRAME() { + // Deactivate all tools on the tool stack + EmptyToolStack(); + delete m_item_to_repeat; // we own the cloned object, see this->SaveCopyForRepeatItem() SetScreen( NULL ); diff --git a/eeschema/viewlib_frame.cpp b/eeschema/viewlib_frame.cpp index c72d2fddc..5499515f9 100644 --- a/eeschema/viewlib_frame.cpp +++ b/eeschema/viewlib_frame.cpp @@ -204,6 +204,9 @@ LIB_VIEW_FRAME::LIB_VIEW_FRAME( KIWAY* aKiway, wxWindow* aParent, FRAME_T aFrame LIB_VIEW_FRAME::~LIB_VIEW_FRAME() { + // Deactivate all tools on the tool stack + EmptyToolStack(); + if( m_previewItem ) GetCanvas()->GetView()->Remove( m_previewItem ); } diff --git a/gerbview/gerbview_frame.cpp b/gerbview/gerbview_frame.cpp index 33b7edef9..296da68cd 100644 --- a/gerbview/gerbview_frame.cpp +++ b/gerbview/gerbview_frame.cpp @@ -231,12 +231,12 @@ GERBVIEW_FRAME::~GERBVIEW_FRAME() void GERBVIEW_FRAME::OnCloseWindow( wxCloseEvent& Event ) { + // Deactivate all tools on the tool stack + EmptyToolStack(); + GetCanvas()->StopDrawing(); GetCanvas()->GetView()->Clear(); - if( m_toolManager ) - m_toolManager->DeactivateTool(); - // Be sure any OpenGL event cannot be fired after frame deletion: GetCanvas()->SetEvtHandlerEnabled( false ); diff --git a/include/eda_base_frame.h b/include/eda_base_frame.h index 0fdac576a..9fc53b0a2 100644 --- a/include/eda_base_frame.h +++ b/include/eda_base_frame.h @@ -231,6 +231,11 @@ public: bool ToolStackIsEmpty() { return m_toolStack.empty(); } + /** + * Send cancel commands to all tools currently in the tool stack to empty it. + */ + void EmptyToolStack(); + std::string CurrentToolName() const; bool IsCurrentTool( const TOOL_ACTION& aAction ) const; @@ -328,7 +333,7 @@ public: virtual void SaveSettings( wxConfigBase* aCfg ); /** - * @return a base name prefix used in Load/Save settings to build the full name of keys + * @return a base name prefix used in Load/Save settings to build the full name of keys * used in config. * This is usually the name of the frame set by CTOR, except for frames shown in multiple * modes in which case the m_configName must be set to the base name so that a single @@ -436,14 +441,14 @@ public: /** * Update the status bar information. * - * The status bar can draw itself. This is not a drawing function per se, but rather - * updates lines of text held by the components within the status bar which is owned + * The status bar can draw itself. This is not a drawing function per se, but rather + * updates lines of text held by the components within the status bar which is owned * by the wxFrame. */ virtual void UpdateStatusBar() { } /** - * Update the toolbars (mostly settings/check buttons/checkboxes) with the current + * Update the toolbars (mostly settings/check buttons/checkboxes) with the current * controller state. */ virtual void SyncToolbars() { }; @@ -458,7 +463,7 @@ public: * Update menus, toolbars, local variables, etc. */ virtual void CommonSettingsChanged( bool aEnvVarsChanged ); - + /** * Notification to refresh the drawing canvas (if any). */ diff --git a/kicad/kicad_manager_frame.cpp b/kicad/kicad_manager_frame.cpp index 9ea194849..5f8d869ac 100644 --- a/kicad/kicad_manager_frame.cpp +++ b/kicad/kicad_manager_frame.cpp @@ -165,9 +165,8 @@ KICAD_MANAGER_FRAME::KICAD_MANAGER_FRAME( wxWindow* parent, const wxString& titl KICAD_MANAGER_FRAME::~KICAD_MANAGER_FRAME() { - // Ensure there are no active tools - if( m_toolManager ) - m_toolManager->DeactivateTool(); + // Deactivate all tools on the tool stack + EmptyToolStack(); delete m_actions; delete m_toolManager; diff --git a/pagelayout_editor/pl_editor_frame.cpp b/pagelayout_editor/pl_editor_frame.cpp index 5af58b4ec..08247ecc3 100644 --- a/pagelayout_editor/pl_editor_frame.cpp +++ b/pagelayout_editor/pl_editor_frame.cpp @@ -198,6 +198,13 @@ PL_EDITOR_FRAME::PL_EDITOR_FRAME( KIWAY* aKiway, wxWindow* aParent ) : } +PL_EDITOR_FRAME::~PL_EDITOR_FRAME() +{ + // Deactivate all tools on the tool stack + EmptyToolStack(); +} + + void PL_EDITOR_FRAME::setupTools() { // Create the manager and dispatcher & route draw panel events to the dispatcher diff --git a/pagelayout_editor/pl_editor_frame.h b/pagelayout_editor/pl_editor_frame.h index 5848b1c9d..0ee4d115a 100644 --- a/pagelayout_editor/pl_editor_frame.h +++ b/pagelayout_editor/pl_editor_frame.h @@ -66,7 +66,7 @@ private: public: PL_EDITOR_FRAME( KIWAY* aKiway, wxWindow* aParent ); - ~PL_EDITOR_FRAME() {} + ~PL_EDITOR_FRAME(); PROPERTIES_FRAME* GetPropertiesFrame() { return m_propertiesPagelayout; } diff --git a/pcbnew/footprint_edit_frame.cpp b/pcbnew/footprint_edit_frame.cpp index be839a3cf..1ff9d8ee0 100644 --- a/pcbnew/footprint_edit_frame.cpp +++ b/pcbnew/footprint_edit_frame.cpp @@ -222,6 +222,9 @@ FOOTPRINT_EDIT_FRAME::FOOTPRINT_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent, FOOTPRINT_EDIT_FRAME::~FOOTPRINT_EDIT_FRAME() { + // Deactivate all tools on the tool stack + EmptyToolStack(); + // save the footprint in the PROJECT retainLastFootprint(); diff --git a/pcbnew/footprint_viewer_frame.cpp b/pcbnew/footprint_viewer_frame.cpp index 4513c3488..5c43effab 100644 --- a/pcbnew/footprint_viewer_frame.cpp +++ b/pcbnew/footprint_viewer_frame.cpp @@ -265,6 +265,9 @@ FOOTPRINT_VIEWER_FRAME::FOOTPRINT_VIEWER_FRAME( KIWAY* aKiway, wxWindow* aParent FOOTPRINT_VIEWER_FRAME::~FOOTPRINT_VIEWER_FRAME() { + // Deactivate all tools on the tool stack + EmptyToolStack(); + GetCanvas()->StopDrawing(); GetCanvas()->GetView()->Clear(); // Be sure any event cannot be fired after frame deletion: diff --git a/pcbnew/footprint_wizard_frame.cpp b/pcbnew/footprint_wizard_frame.cpp index fe14d073e..79382dc08 100644 --- a/pcbnew/footprint_wizard_frame.cpp +++ b/pcbnew/footprint_wizard_frame.cpp @@ -224,6 +224,9 @@ FOOTPRINT_WIZARD_FRAME::FOOTPRINT_WIZARD_FRAME( KIWAY* aKiway, wxWindow* aParent FOOTPRINT_WIZARD_FRAME::~FOOTPRINT_WIZARD_FRAME() { + // Deactivate all tools on the tool stack + EmptyToolStack(); + // Delete the GRID_TRICKS. m_parameterGrid->PopEventHandler( true ); @@ -231,10 +234,6 @@ FOOTPRINT_WIZARD_FRAME::~FOOTPRINT_WIZARD_FRAME() // Be sure any event cannot be fired after frame deletion: GetCanvas()->SetEvtHandlerEnabled( false ); - // Be sure a active tool (if exists) is desactivated: - if( m_toolManager ) - m_toolManager->DeactivateTool(); - EDA_3D_VIEWER* draw3DFrame = Get3DViewerFrame(); if( draw3DFrame ) diff --git a/pcbnew/pcb_edit_frame.cpp b/pcbnew/pcb_edit_frame.cpp index 12010f796..c81514d86 100644 --- a/pcbnew/pcb_edit_frame.cpp +++ b/pcbnew/pcb_edit_frame.cpp @@ -336,6 +336,8 @@ PCB_EDIT_FRAME::PCB_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ) : PCB_EDIT_FRAME::~PCB_EDIT_FRAME() { + // Deactivate all tools on the tool stack + EmptyToolStack(); } -- 2.21.0