From b6027e081556aa179a17ccc1f5ad750cbad1eec5 Mon Sep 17 00:00:00 2001 From: jean-pierre charras Date: Wed, 13 Feb 2019 11:41:52 +0100 Subject: [PATCH] Drill dialog: fix incorrect parent window when called from the Plot dialog. The parent window was always the board edit frame, but this is incorrect when this dialog is called from the Plot dialog. Fixes: lp:1815530 https://bugs.launchpad.net/kicad/+bug/1815530 --- pcbnew/dialogs/dialog_gendrill.cpp | 47 +++++++++++---------- pcbnew/dialogs/dialog_gendrill.h | 32 +++++++++++++-- pcbnew/dialogs/dialog_gendrill_base.cpp | 30 ++++++++------ pcbnew/dialogs/dialog_gendrill_base.fbp | 54 +++++++++++++------------ pcbnew/dialogs/dialog_gendrill_base.h | 10 +++-- pcbnew/dialogs/dialog_plot.cpp | 7 +++- 6 files changed, 107 insertions(+), 73 deletions(-) diff --git a/pcbnew/dialogs/dialog_gendrill.cpp b/pcbnew/dialogs/dialog_gendrill.cpp index 09c45a6349..a431bface8 100644 --- a/pcbnew/dialogs/dialog_gendrill.cpp +++ b/pcbnew/dialogs/dialog_gendrill.cpp @@ -69,28 +69,28 @@ static DRILL_PRECISION precisionListForMetric( 3, 3 ); */ void PCB_EDIT_FRAME::InstallDrillFrame( wxCommandEvent& event ) { - DIALOG_GENDRILL dlg( this ); + DIALOG_GENDRILL dlg( this, this ); dlg.ShowModal(); } -DIALOG_GENDRILL::DIALOG_GENDRILL( PCB_EDIT_FRAME* parent ) : - DIALOG_GENDRILL_BASE( parent ) +DIALOG_GENDRILL::DIALOG_GENDRILL( PCB_EDIT_FRAME* aPcbEditFrame, wxWindow* aParent ) : + DIALOG_GENDRILL_BASE( aParent ) { - m_parent = parent; - m_board = parent->GetBoard(); + m_pcbEditFrame = aPcbEditFrame; + m_board = m_pcbEditFrame->GetBoard(); m_config = Kiface().KifaceSettings(); - m_plotOpts = m_parent->GetPlotSettings(); + m_plotOpts = m_pcbEditFrame->GetPlotSettings(); // We use a sdbSizer to get platform-dependent ordering of the action buttons, but // that requires us to correct the button labels here. - m_sdbSizer1OK->SetLabel( _( "Generate Drill File" ) ); - m_sdbSizer1Apply->SetLabel( _( "Generate Map File" ) ); - m_sdbSizer1Cancel->SetLabel( _( "Close" ) ); + m_sdbSizerOK->SetLabel( _( "Generate Drill File" ) ); + m_sdbSizerApply->SetLabel( _( "Generate Map File" ) ); + m_sdbSizerCancel->SetLabel( _( "Close" ) ); m_buttonsSizer->Layout(); - m_sdbSizer1OK->SetDefault(); + m_sdbSizerOK->SetDefault(); SetReturnCode( 1 ); initDialog(); GetSizer()->SetSizeHints( this ); @@ -109,7 +109,6 @@ bool DIALOG_GENDRILL::m_UseRouteModeForOvalHoles = true; // Use G00 route mod DIALOG_GENDRILL::~DIALOG_GENDRILL() { - UpdateConfig(); } @@ -154,7 +153,7 @@ void DIALOG_GENDRILL::InitDisplayParams() m_microViasCount = 0; m_blindOrBuriedViasCount = 0; - for( MODULE* module = m_parent->GetBoard()->m_Modules; module; module = module->Next() ) + for( MODULE* module = m_board->m_Modules; module; module = module->Next() ) { for( D_PAD* pad = module->PadsList(); pad != NULL; pad = pad->Next() ) { @@ -181,7 +180,7 @@ void DIALOG_GENDRILL::InitDisplayParams() } } - for( TRACK* track = m_parent->GetBoard()->m_Track; track != NULL; track = track->Next() ) + for( TRACK* track = m_board->m_Track; track != NULL; track = track->Next() ) { const VIA *via = dynamic_cast( track ); if( via ) @@ -246,7 +245,7 @@ void DIALOG_GENDRILL::onFileFormatSelection( wxCommandEvent& event ) void DIALOG_GENDRILL::UpdateConfig() { - SetParams(); + UpdateDrillParams(); m_config->Write( ZerosFormatKey, m_ZerosFormat ); m_config->Write( MirrorKey, m_Mirror ); @@ -314,7 +313,7 @@ void DIALOG_GENDRILL::OnOutputDirectoryBrowseClicked( wxCommandEvent& event ) wxFileName dirName = wxFileName::DirName( dirDialog.GetPath() ); - fn = Prj().AbsolutePath( m_parent->GetBoard()->GetFileName() ); + fn = Prj().AbsolutePath( m_board->GetFileName() ); wxString defaultPath = fn.GetPathWithSep(); wxString msg; msg.Printf( _( "Do you want to use a path relative to\n\"%s\"" ), GetChars( defaultPath ) ); @@ -333,7 +332,7 @@ void DIALOG_GENDRILL::OnOutputDirectoryBrowseClicked( wxCommandEvent& event ) } -void DIALOG_GENDRILL::SetParams() +void DIALOG_GENDRILL::UpdateDrillParams() { wxString msg; @@ -357,7 +356,7 @@ void DIALOG_GENDRILL::SetParams() if( m_Choice_Drill_Offset->GetSelection() == 0 ) m_FileDrillOffset = wxPoint( 0, 0 ); else - m_FileDrillOffset = m_parent->GetAuxOrigin(); + m_FileDrillOffset = m_pcbEditFrame->GetAuxOrigin(); if( m_UnitDrillIsInch ) m_Precision = precisionListForInches; @@ -372,7 +371,7 @@ void DIALOG_GENDRILL::GenDrillAndMapFiles( bool aGenDrill, bool aGenMap ) { UpdateConfig(); // set params and Save drill options - m_parent->ClearMsgPanel(); + m_pcbEditFrame->ClearMsgPanel(); WX_TEXT_CTRL_REPORTER reporter( m_messagesBox ); const PlotFormat filefmt[6] = @@ -389,7 +388,7 @@ void DIALOG_GENDRILL::GenDrillAndMapFiles( bool aGenDrill, bool aGenMap ) // Create output directory if it does not exist (also transform it in // absolute form). Bail if it fails wxFileName outputDir = wxFileName::DirName( m_plotOpts.GetOutputDirectory() ); - wxString boardFilename = m_parent->GetBoard()->GetFileName(); + wxString boardFilename = m_board->GetFileName(); if( !EnsureFileDirectoryExists( &outputDir, boardFilename, &reporter ) ) { @@ -402,7 +401,7 @@ void DIALOG_GENDRILL::GenDrillAndMapFiles( bool aGenDrill, bool aGenMap ) if( m_drillFileType == 0 ) { - EXCELLON_WRITER excellonWriter( m_parent->GetBoard() ); + EXCELLON_WRITER excellonWriter( m_board ); excellonWriter.SetFormat( !m_UnitDrillIsInch, (EXCELLON_WRITER::ZEROS_FMT) m_ZerosFormat, m_Precision.m_lhs, m_Precision.m_rhs ); excellonWriter.SetOptions( m_Mirror, m_MinimalHeader, m_FileDrillOffset, m_Merge_PTH_NPTH ); @@ -414,7 +413,7 @@ void DIALOG_GENDRILL::GenDrillAndMapFiles( bool aGenDrill, bool aGenMap ) } else { - GERBER_WRITER gerberWriter( m_parent->GetBoard() ); + GERBER_WRITER gerberWriter( m_board ); // Set gerber precision: only 5 or 6 digits for mantissa are allowed // (SetFormat() accept 5 or 6, and any other value set the precision to 5) // the integer part precision is always 4, and units always mm @@ -432,7 +431,7 @@ void DIALOG_GENDRILL::OnGenReportFile( wxCommandEvent& event ) { UpdateConfig(); // set params and Save drill options - wxFileName fn = m_parent->GetBoard()->GetFileName(); + wxFileName fn = m_board->GetFileName(); fn.SetName( fn.GetName() + wxT( "-drl" ) ); fn.SetExt( ReportFileExtension ); @@ -454,13 +453,13 @@ void DIALOG_GENDRILL::OnGenReportFile( wxCommandEvent& event ) // (file ext, Merge PTH/NPTH option) if( m_drillFileType == 0 ) { - EXCELLON_WRITER excellonWriter( m_parent->GetBoard() ); + EXCELLON_WRITER excellonWriter( m_board ); excellonWriter.SetMergeOption( m_Merge_PTH_NPTH ); success = excellonWriter.GenDrillReportFile( dlg.GetPath() ); } else { - GERBER_WRITER gerberWriter( m_parent->GetBoard() ); + GERBER_WRITER gerberWriter( m_board ); success = gerberWriter.GenDrillReportFile( dlg.GetPath() ); } diff --git a/pcbnew/dialogs/dialog_gendrill.h b/pcbnew/dialogs/dialog_gendrill.h index b1cc9dcb81..2b45526fac 100644 --- a/pcbnew/dialogs/dialog_gendrill.h +++ b/pcbnew/dialogs/dialog_gendrill.h @@ -29,14 +29,26 @@ #ifndef DIALOG_GENDRILL_H_ #define DIALOG_GENDRILL_H_ +#include // for DRILL_PRECISION definition #include class DIALOG_GENDRILL : public DIALOG_GENDRILL_BASE { public: - DIALOG_GENDRILL( PCB_EDIT_FRAME* parent ); + /** + * Ctor + * @param aPcbEditFrame is the board edit frame + * @param aParent is the parent window caller ( the board edit frame or a dialog ) + */ + DIALOG_GENDRILL( PCB_EDIT_FRAME* aPcbEditFrame, wxWindow* aParent ); ~DIALOG_GENDRILL(); + /** + * Update board drill/plot parameters + */ + void UpdateDrillParams( void ); + + static int m_UnitDrillIsInch; static int m_ZerosFormat; static bool m_MinimalHeader; @@ -50,7 +62,7 @@ public: private: - PCB_EDIT_FRAME* m_parent; + PCB_EDIT_FRAME* m_pcbEditFrame; wxConfigBase* m_config; BOARD* m_board; PCB_PLOT_PARAMS m_plotOpts; @@ -76,6 +88,20 @@ private: void OnGenMapFile( wxCommandEvent& event ) override; void onFileFormatSelection( wxCommandEvent& event ) override; + // Called when closing the dialog: Update config. + // This is not done in Dtor, because the dtor call is often delayed + // and the update could happen too late for the caller. + void onCloseDlg( wxCloseEvent& event ) override + { + UpdateConfig(); + event.Skip(); + } + void onQuitDlg( wxCommandEvent& event ) override + { + UpdateConfig(); + event.Skip(); + } + /* * Create a plain text report file giving a list of drill values and drill count * for through holes, oblong holes, and for buried vias, @@ -86,8 +112,6 @@ private: void OnOutputDirectoryBrowseClicked( wxCommandEvent& event ) override; // Specific functions: - void SetParams( void ); - /** * Function GenDrillAndMapFiles * Calls the functions to create EXCELLON drill files and/or drill map files diff --git a/pcbnew/dialogs/dialog_gendrill_base.cpp b/pcbnew/dialogs/dialog_gendrill_base.cpp index 8a53c06161..33a32f8e7e 100644 --- a/pcbnew/dialogs/dialog_gendrill_base.cpp +++ b/pcbnew/dialogs/dialog_gendrill_base.cpp @@ -218,16 +218,16 @@ DIALOG_GENDRILL_BASE::DIALOG_GENDRILL_BASE( wxWindow* parent, wxWindowID id, con m_buttonReport = new wxButton( this, wxID_ANY, _("Generate Report File"), wxDefaultPosition, wxDefaultSize, 0 ); m_buttonsSizer->Add( m_buttonReport, 0, wxEXPAND|wxRIGHT|wxLEFT, 10 ); - m_sdbSizer1 = new wxStdDialogButtonSizer(); - m_sdbSizer1OK = new wxButton( this, wxID_OK ); - m_sdbSizer1->AddButton( m_sdbSizer1OK ); - m_sdbSizer1Apply = new wxButton( this, wxID_APPLY ); - m_sdbSizer1->AddButton( m_sdbSizer1Apply ); - m_sdbSizer1Cancel = new wxButton( this, wxID_CANCEL ); - m_sdbSizer1->AddButton( m_sdbSizer1Cancel ); - m_sdbSizer1->Realize(); + m_sdbSizer = new wxStdDialogButtonSizer(); + m_sdbSizerOK = new wxButton( this, wxID_OK ); + m_sdbSizer->AddButton( m_sdbSizerOK ); + m_sdbSizerApply = new wxButton( this, wxID_APPLY ); + m_sdbSizer->AddButton( m_sdbSizerApply ); + m_sdbSizerCancel = new wxButton( this, wxID_CANCEL ); + m_sdbSizer->AddButton( m_sdbSizerCancel ); + m_sdbSizer->Realize(); - m_buttonsSizer->Add( m_sdbSizer1, 1, wxEXPAND, 5 ); + m_buttonsSizer->Add( m_sdbSizer, 1, wxEXPAND, 5 ); bMainSizer->Add( m_buttonsSizer, 0, wxALL|wxEXPAND, 5 ); @@ -239,26 +239,30 @@ DIALOG_GENDRILL_BASE::DIALOG_GENDRILL_BASE( wxWindow* parent, wxWindowID id, con this->Centre( wxBOTH ); // Connect Events + this->Connect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler( DIALOG_GENDRILL_BASE::onCloseDlg ) ); m_browseButton->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnOutputDirectoryBrowseClicked ), NULL, this ); m_rbExcellon->Connect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::onFileFormatSelection ), NULL, this ); m_rbGerberX2->Connect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::onFileFormatSelection ), NULL, this ); m_Choice_Unit->Connect( wxEVT_COMMAND_RADIOBOX_SELECTED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnSelDrillUnitsSelected ), NULL, this ); m_Choice_Zeros_Format->Connect( wxEVT_COMMAND_RADIOBOX_SELECTED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnSelZerosFmtSelected ), NULL, this ); m_buttonReport->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnGenReportFile ), NULL, this ); - m_sdbSizer1Apply->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnGenMapFile ), NULL, this ); - m_sdbSizer1OK->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnGenDrillFile ), NULL, this ); + m_sdbSizerApply->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnGenMapFile ), NULL, this ); + m_sdbSizerCancel->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::onQuitDlg ), NULL, this ); + m_sdbSizerOK->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnGenDrillFile ), NULL, this ); } DIALOG_GENDRILL_BASE::~DIALOG_GENDRILL_BASE() { // Disconnect Events + this->Disconnect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler( DIALOG_GENDRILL_BASE::onCloseDlg ) ); m_browseButton->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnOutputDirectoryBrowseClicked ), NULL, this ); m_rbExcellon->Disconnect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::onFileFormatSelection ), NULL, this ); m_rbGerberX2->Disconnect( wxEVT_COMMAND_RADIOBUTTON_SELECTED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::onFileFormatSelection ), NULL, this ); m_Choice_Unit->Disconnect( wxEVT_COMMAND_RADIOBOX_SELECTED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnSelDrillUnitsSelected ), NULL, this ); m_Choice_Zeros_Format->Disconnect( wxEVT_COMMAND_RADIOBOX_SELECTED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnSelZerosFmtSelected ), NULL, this ); m_buttonReport->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnGenReportFile ), NULL, this ); - m_sdbSizer1Apply->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnGenMapFile ), NULL, this ); - m_sdbSizer1OK->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnGenDrillFile ), NULL, this ); + m_sdbSizerApply->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnGenMapFile ), NULL, this ); + m_sdbSizerCancel->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::onQuitDlg ), NULL, this ); + m_sdbSizerOK->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( DIALOG_GENDRILL_BASE::OnGenDrillFile ), NULL, this ); } diff --git a/pcbnew/dialogs/dialog_gendrill_base.fbp b/pcbnew/dialogs/dialog_gendrill_base.fbp index 181818ca84..677b9fdc18 100644 --- a/pcbnew/dialogs/dialog_gendrill_base.fbp +++ b/pcbnew/dialogs/dialog_gendrill_base.fbp @@ -53,6 +53,7 @@ + onCloseDlg bMainSizer @@ -1121,16 +1122,16 @@ 5 wxEXPAND|wxTOP 1 - + bRightBoxSizer wxVERTICAL none - + 5 wxBOTTOM|wxEXPAND|wxLEFT|wxRIGHT 1 - + wxID_ANY Hole Counts @@ -1138,11 +1139,11 @@ wxVERTICAL 1 none - + 5 wxEXPAND 1 - + 2 wxBOTH @@ -1154,11 +1155,11 @@ none 0 0 - + 5 wxLEFT|wxRIGHT 0 - + 1 1 1 @@ -1215,11 +1216,11 @@ -1 - + 5 wxALIGN_RIGHT|wxLEFT|wxRIGHT 0 - + 1 1 1 @@ -1276,11 +1277,11 @@ -1 - + 5 wxLEFT|wxRIGHT|wxTOP 0 - + 1 1 1 @@ -1337,11 +1338,11 @@ -1 - + 5 wxALIGN_RIGHT|wxLEFT|wxRIGHT|wxTOP 0 - + 1 1 1 @@ -1398,11 +1399,11 @@ -1 - + 5 wxLEFT|wxRIGHT|wxTOP 0 - + 1 1 1 @@ -1459,11 +1460,11 @@ -1 - + 5 wxALIGN_RIGHT|wxLEFT|wxRIGHT|wxTOP 0 - + 1 1 1 @@ -1520,11 +1521,11 @@ -1 - + 5 wxLEFT|wxRIGHT|wxTOP 0 - + 1 1 1 @@ -1581,11 +1582,11 @@ -1 - + 5 wxALIGN_RIGHT|wxLEFT|wxRIGHT|wxTOP 0 - + 1 1 1 @@ -1642,11 +1643,11 @@ -1 - + 5 wxALL 0 - + 1 1 1 @@ -1703,11 +1704,11 @@ -1 - + 5 wxALIGN_RIGHT|wxALL 0 - + 1 1 1 @@ -1946,9 +1947,10 @@ 0 0 - m_sdbSizer1 + m_sdbSizer protected OnGenMapFile + onQuitDlg OnGenDrillFile diff --git a/pcbnew/dialogs/dialog_gendrill_base.h b/pcbnew/dialogs/dialog_gendrill_base.h index 4518200491..f6c7e6fea1 100644 --- a/pcbnew/dialogs/dialog_gendrill_base.h +++ b/pcbnew/dialogs/dialog_gendrill_base.h @@ -69,18 +69,20 @@ class DIALOG_GENDRILL_BASE : public DIALOG_SHIM wxTextCtrl* m_messagesBox; wxBoxSizer* m_buttonsSizer; wxButton* m_buttonReport; - wxStdDialogButtonSizer* m_sdbSizer1; - wxButton* m_sdbSizer1OK; - wxButton* m_sdbSizer1Apply; - wxButton* m_sdbSizer1Cancel; + wxStdDialogButtonSizer* m_sdbSizer; + wxButton* m_sdbSizerOK; + wxButton* m_sdbSizerApply; + wxButton* m_sdbSizerCancel; // Virtual event handlers, overide them in your derived class + virtual void onCloseDlg( wxCloseEvent& event ) { event.Skip(); } virtual void OnOutputDirectoryBrowseClicked( wxCommandEvent& event ) { event.Skip(); } virtual void onFileFormatSelection( wxCommandEvent& event ) { event.Skip(); } virtual void OnSelDrillUnitsSelected( wxCommandEvent& event ) { event.Skip(); } virtual void OnSelZerosFmtSelected( wxCommandEvent& event ) { event.Skip(); } virtual void OnGenReportFile( wxCommandEvent& event ) { event.Skip(); } virtual void OnGenMapFile( wxCommandEvent& event ) { event.Skip(); } + virtual void onQuitDlg( wxCommandEvent& event ) { event.Skip(); } virtual void OnGenDrillFile( wxCommandEvent& event ) { event.Skip(); } diff --git a/pcbnew/dialogs/dialog_plot.cpp b/pcbnew/dialogs/dialog_plot.cpp index 6ee97bea14..2daea00623 100644 --- a/pcbnew/dialogs/dialog_plot.cpp +++ b/pcbnew/dialogs/dialog_plot.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -276,9 +277,11 @@ void DIALOG_PLOT::OnPopUpLayers( wxCommandEvent& event ) void DIALOG_PLOT::CreateDrillFile( wxCommandEvent& event ) { // Be sure drill file use the same settings (axis option, plot directory) - // than plot files: + // as plot files: applyPlotSettings(); - m_parent->InstallDrillFrame( event ); + + DIALOG_GENDRILL dlg( m_parent, this ); + dlg.ShowModal(); // a few plot settings can be modified: take them in account m_plotOpts = m_parent->GetPlotSettings();