← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH v2] eeschema: Allow hierarchy navigator to stay open

 

Hi Franck,

There is a memory leak with your code.  The Destroy() method is never
called when the hierarchy navigation dialog is closed.  When using
modeless dialogs, you are responsible for cleaning up the dialog created
on the heap.  The schematic editor Find/Replace dialog is modeless so
you can use that as an example of how to handle modeless dialogs.

Cheers,

Wayne

On 11/4/19 6:48 AM, franck.jullien@xxxxxxxxx wrote:
> From: Franck Jullien <franck.jullien@xxxxxxxxx>
> 
> Signed-off-by: Franck Jullien <franck.jullien@xxxxxxxxx>
> ---
>  eeschema/dialogs/panel_eeschema_settings.cpp  |  2 +
>  .../dialogs/panel_eeschema_settings_base.cpp  |  2 +
>  .../dialogs/panel_eeschema_settings_base.fbp  | 88 +++++++++++++++++++
>  .../dialogs/panel_eeschema_settings_base.h    |  3 +-
>  eeschema/eeschema_config.cpp                  |  3 +
>  eeschema/hierarch.cpp                         |  6 +-
>  eeschema/sch_edit_frame.h                     |  4 +
>  7 files changed, 104 insertions(+), 4 deletions(-)
> 
> diff --git a/eeschema/dialogs/panel_eeschema_settings.cpp b/eeschema/dialogs/panel_eeschema_settings.cpp
> index d50b8a373..3ad650d18 100644
> --- a/eeschema/dialogs/panel_eeschema_settings.cpp
> +++ b/eeschema/dialogs/panel_eeschema_settings.cpp
> @@ -46,6 +46,7 @@ bool PANEL_EESCHEMA_SETTINGS::TransferDataToWindow()
>  
>      m_checkHVOrientation->SetValue( m_frame->GetForceHVLines() );
>      m_footprintPreview->SetValue( m_frame->GetShowFootprintPreviews() );
> +    m_navigatorStaysOpen->SetValue( m_frame->GetNavigatorStaysOpen() );
>  
>      m_checkAutoplaceFields->SetValue( m_frame->GetAutoplaceFields() );
>      m_checkAutoplaceJustify->SetValue( m_frame->GetAutoplaceJustify() );
> @@ -75,6 +76,7 @@ bool PANEL_EESCHEMA_SETTINGS::TransferDataFromWindow()
>  
>      m_frame->SetForceHVLines( m_checkHVOrientation->GetValue() );
>      m_frame->SetShowFootprintPreviews( m_footprintPreview->GetValue() );
> +    m_frame->SetNavigatorStaysOpen( m_navigatorStaysOpen->GetValue() );
>  
>      m_frame->SetAutoplaceFields( m_checkAutoplaceFields->GetValue() );
>      m_frame->SetAutoplaceJustify( m_checkAutoplaceJustify->GetValue() );
> diff --git a/eeschema/dialogs/panel_eeschema_settings_base.cpp b/eeschema/dialogs/panel_eeschema_settings_base.cpp
> index 84c6470b1..7553170b0 100644
> --- a/eeschema/dialogs/panel_eeschema_settings_base.cpp
> +++ b/eeschema/dialogs/panel_eeschema_settings_base.cpp
> @@ -124,6 +124,8 @@ PANEL_EESCHEMA_SETTINGS_BASE::PANEL_EESCHEMA_SETTINGS_BASE( wxWindow* parent, wx
>  	m_footprintPreview = new wxCheckBox( this, wxID_ANY, _("Show footprint previews in symbol chooser"), wxDefaultPosition, wxDefaultSize, 0 );
>  	bSizer9->Add( m_footprintPreview, 0, wxBOTTOM|wxLEFT|wxRIGHT, 5 );
>  	
> +	m_navigatorStaysOpen = new wxCheckBox( this, wxID_ANY, _("Allow hierarchy navigator to stay open"), wxDefaultPosition, wxDefaultSize, 0 );
> +	bSizer9->Add( m_navigatorStaysOpen, 0, wxBOTTOM|wxLEFT|wxRIGHT, 5 );
>  	
>  	bLeftColumn->Add( bSizer9, 0, wxTOP|wxBOTTOM|wxRIGHT, 5 );
>  	
> diff --git a/eeschema/dialogs/panel_eeschema_settings_base.fbp b/eeschema/dialogs/panel_eeschema_settings_base.fbp
> index 6bc9b6da1..a8996f56f 100644
> --- a/eeschema/dialogs/panel_eeschema_settings_base.fbp
> +++ b/eeschema/dialogs/panel_eeschema_settings_base.fbp
> @@ -1810,6 +1810,94 @@
>                                          <event name="OnUpdateUI"></event>
>                                      </object>
>                                  </object>
> +                                <object class="sizeritem" expanded="1">
> +                                    <property name="border">5</property>
> +                                    <property name="flag">wxALL</property>
> +                                    <property name="proportion">0</property>
> +                                    <object class="wxCheckBox" expanded="1">
> +                                        <property name="BottomDockable">1</property>
> +                                        <property name="LeftDockable">1</property>
> +                                        <property name="RightDockable">1</property>
> +                                        <property name="TopDockable">1</property>
> +                                        <property name="aui_layer"></property>
> +                                        <property name="aui_name"></property>
> +                                        <property name="aui_position"></property>
> +                                        <property name="aui_row"></property>
> +                                        <property name="best_size"></property>
> +                                        <property name="bg"></property>
> +                                        <property name="caption"></property>
> +                                        <property name="caption_visible">1</property>
> +                                        <property name="center_pane">0</property>
> +                                        <property name="checked">1</property>
> +                                        <property name="close_button">1</property>
> +                                        <property name="context_help"></property>
> +                                        <property name="context_menu">1</property>
> +                                        <property name="default_pane">0</property>
> +                                        <property name="dock">Dock</property>
> +                                        <property name="dock_fixed">0</property>
> +                                        <property name="docking">Left</property>
> +                                        <property name="enabled">1</property>
> +                                        <property name="fg"></property>
> +                                        <property name="floatable">1</property>
> +                                        <property name="font"></property>
> +                                        <property name="gripper">0</property>
> +                                        <property name="hidden">0</property>
> +                                        <property name="id">wxID_ANY</property>
> +                                        <property name="label">Allow hierarchy navigator to stay open</property>
> +                                        <property name="max_size"></property>
> +                                        <property name="maximize_button">0</property>
> +                                        <property name="maximum_size"></property>
> +                                        <property name="min_size"></property>
> +                                        <property name="minimize_button">0</property>
> +                                        <property name="minimum_size"></property>
> +                                        <property name="moveable">1</property>
> +                                        <property name="name">m_navigatorStaysOpen</property>
> +                                        <property name="pane_border">1</property>
> +                                        <property name="pane_position"></property>
> +                                        <property name="pane_size"></property>
> +                                        <property name="permission">protected</property>
> +                                        <property name="pin_button">1</property>
> +                                        <property name="pos"></property>
> +                                        <property name="resize">Resizable</property>
> +                                        <property name="show">1</property>
> +                                        <property name="size"></property>
> +                                        <property name="style"></property>
> +                                        <property name="subclass">; forward_declare</property>
> +                                        <property name="toolbar_pane">0</property>
> +                                        <property name="tooltip"></property>
> +                                        <property name="validator_data_type"></property>
> +                                        <property name="validator_style">wxFILTER_NONE</property>
> +                                        <property name="validator_type">wxDefaultValidator</property>
> +                                        <property name="validator_variable"></property>
> +                                        <property name="window_extra_style"></property>
> +                                        <property name="window_name"></property>
> +                                        <property name="window_style"></property>
> +                                        <event name="OnChar"></event>
> +                                        <event name="OnCheckBox"></event>
> +                                        <event name="OnEnterWindow"></event>
> +                                        <event name="OnEraseBackground"></event>
> +                                        <event name="OnKeyDown"></event>
> +                                        <event name="OnKeyUp"></event>
> +                                        <event name="OnKillFocus"></event>
> +                                        <event name="OnLeaveWindow"></event>
> +                                        <event name="OnLeftDClick"></event>
> +                                        <event name="OnLeftDown"></event>
> +                                        <event name="OnLeftUp"></event>
> +                                        <event name="OnMiddleDClick"></event>
> +                                        <event name="OnMiddleDown"></event>
> +                                        <event name="OnMiddleUp"></event>
> +                                        <event name="OnMotion"></event>
> +                                        <event name="OnMouseEvents"></event>
> +                                        <event name="OnMouseWheel"></event>
> +                                        <event name="OnPaint"></event>
> +                                        <event name="OnRightDClick"></event>
> +                                        <event name="OnRightDown"></event>
> +                                        <event name="OnRightUp"></event>
> +                                        <event name="OnSetFocus"></event>
> +                                        <event name="OnSize"></event>
> +                                        <event name="OnUpdateUI"></event>
> +                                    </object>
> +                                </object>
>                              </object>
>                          </object>
>                      </object>
> diff --git a/eeschema/dialogs/panel_eeschema_settings_base.h b/eeschema/dialogs/panel_eeschema_settings_base.h
> index 9a0fb8394..623271490 100644
> --- a/eeschema/dialogs/panel_eeschema_settings_base.h
> +++ b/eeschema/dialogs/panel_eeschema_settings_base.h
> @@ -59,7 +59,8 @@ class PANEL_EESCHEMA_SETTINGS_BASE : public wxPanel
>  		wxCheckBox* m_checkAutoplaceJustify;
>  		wxCheckBox* m_checkAutoplaceAlign;
>  		wxCheckBox* m_footprintPreview;
> -		
> +		wxCheckBox* m_navigatorStaysOpen;
> +
>  		// Virtual event handlers, overide them in your derived class
>  		virtual void OnChooseUnits( wxCommandEvent& event ) { event.Skip(); }
>  		
> diff --git a/eeschema/eeschema_config.cpp b/eeschema/eeschema_config.cpp
> index 527a40262..0c4d35b3b 100644
> --- a/eeschema/eeschema_config.cpp
> +++ b/eeschema/eeschema_config.cpp
> @@ -242,6 +242,7 @@ const wxChar AutoplaceJustifyEntry[] =              wxT( "AutoplaceJustify" );
>  const wxChar AutoplaceAlignEntry[] =                wxT( "AutoplaceAlign" );
>  static const wxChar DragActionIsMoveEntry[] =       wxT( "DragActionIsMove" );
>  static const wxChar FootprintPreviewEntry[] =       wxT( "FootprintPreview" );
> +static const wxChar NavigatorStaysOpenEntry[] =     wxT( "NavigatorStaysOpen" );
>  static const wxChar DefaultBusWidthEntry[] =        wxT( "DefaultBusWidth" );
>  static const wxChar DefaultWireWidthEntry[] =       wxT( "DefaultWireWidth" );
>  static const wxChar DefaultDrawLineWidthEntry[] =   wxT( "DefaultDrawLineWidth" );
> @@ -335,6 +336,7 @@ void SCH_EDIT_FRAME::LoadSettings( wxConfigBase* aCfg )
>      aCfg->Read( AutoplaceJustifyEntry, &m_autoplaceJustify, true );
>      aCfg->Read( AutoplaceAlignEntry, &m_autoplaceAlign, false );
>      aCfg->Read( FootprintPreviewEntry, &m_footprintPreview, false );
> +    aCfg->Read( NavigatorStaysOpenEntry, &m_navigatorStaysOpen, false );
>  
>      wxString templateFieldNames = aCfg->Read( FieldNamesEntry, wxEmptyString );
>  
> @@ -379,6 +381,7 @@ void SCH_EDIT_FRAME::SaveSettings( wxConfigBase* aCfg )
>      aCfg->Write( AutoplaceJustifyEntry, m_autoplaceJustify );
>      aCfg->Write( AutoplaceAlignEntry, m_autoplaceAlign );
>      aCfg->Write( FootprintPreviewEntry, m_footprintPreview );
> +    aCfg->Write( NavigatorStaysOpenEntry, m_navigatorStaysOpen );
>  
>      // Save template fieldnames
>      STRING_FORMATTER sf;
> diff --git a/eeschema/hierarch.cpp b/eeschema/hierarch.cpp
> index f3d25a825..7ed69f992 100644
> --- a/eeschema/hierarch.cpp
> +++ b/eeschema/hierarch.cpp
> @@ -133,8 +133,7 @@ int SCH_EDITOR_CONTROL::NavigateHierarchy( const TOOL_EVENT& aEvent )
>  {
>      HIERARCHY_NAVIG_DLG* treeframe = new HIERARCHY_NAVIG_DLG( m_frame );
>  
> -    treeframe->ShowQuasiModal();
> -    treeframe->Destroy();
> +    treeframe->Show( true );
>  
>      return 0;
>  }
> @@ -254,7 +253,8 @@ void HIERARCHY_NAVIG_DLG::onSelectSheetPath( wxTreeEvent& event )
>      wxTreeItemId ItemSel = m_Tree->GetSelection();
>      m_SchFrameEditor->SetCurrentSheet(( (TreeItemData*) m_Tree->GetItemData( ItemSel ) )->m_SheetPath );
>      m_SchFrameEditor->DisplayCurrentSheet();
> -    Close( true );
> +    if( m_SchFrameEditor->GetNavigatorStaysOpen() == false )
> +        Close( true );
>  }
>  
>  
> diff --git a/eeschema/sch_edit_frame.h b/eeschema/sch_edit_frame.h
> index c7982050c..af3c01694 100644
> --- a/eeschema/sch_edit_frame.h
> +++ b/eeschema/sch_edit_frame.h
> @@ -123,6 +123,7 @@ private:
>      bool                    m_autoplaceJustify;   ///< allow autoplace to change justification
>      bool                    m_autoplaceAlign;     ///< align autoplaced fields to the grid
>      bool                    m_footprintPreview;   ///< whether to show footprint previews
> +    bool                    m_navigatorStaysOpen; ///< whether to keep Navigator open
>      bool                    m_showIllegalSymbolLibDialog;
>  
>      DIALOG_SCH_FIND*        m_findReplaceDialog;
> @@ -199,6 +200,9 @@ public:
>      bool GetShowFootprintPreviews() const { return m_footprintPreview; }
>      void SetShowFootprintPreviews( bool aEnable ) { m_footprintPreview = aEnable; }
>  
> +    bool GetNavigatorStaysOpen() const { return m_navigatorStaysOpen; }
> +    void SetNavigatorStaysOpen( bool aEnable ) { m_navigatorStaysOpen = aEnable; }
> +
>      bool GetAutoplaceFields() const { return m_autoplaceFields; }
>      void SetAutoplaceFields( bool aEnable ) { m_autoplaceFields = aEnable; }
>  
> 


Follow ups

References