From 51d7fea934f56dcf6760bd9de7e06bfdf2d8ef5d Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Thu, 30 Jul 2020 21:21:25 -0700 Subject: [PATCH] Separate Cleanup Via from Cleanup Track There are cases where you will want to keep vias that only connect to one layer but still want to cleanup tracks. As distinct entities, they are given their own checkboxes and actions Fixes https://gitlab.com/kicad/code/kicad/issues/5019 --- pcbnew/cleanup_item.cpp | 1 + .../dialog_cleanup_tracks_and_vias.cpp | 10 ++- .../dialog_cleanup_tracks_and_vias_base.cpp | 11 +++- .../dialog_cleanup_tracks_and_vias_base.fbp | 65 +++++++++++++++++++ .../dialog_cleanup_tracks_and_vias_base.h | 3 +- pcbnew/pcbnew_settings.cpp | 3 + pcbnew/pcbnew_settings.h | 1 + pcbnew/tracks_cleaner.cpp | 32 +++++---- pcbnew/tracks_cleaner.h | 10 ++- 9 files changed, 115 insertions(+), 21 deletions(-) diff --git a/pcbnew/cleanup_item.cpp b/pcbnew/cleanup_item.cpp index 815785f5e6..d70e89d339 100644 --- a/pcbnew/cleanup_item.cpp +++ b/pcbnew/cleanup_item.cpp @@ -30,6 +30,7 @@ CLEANUP_ITEM::CLEANUP_ITEM( int aErrorCode ) { m_errorCode = aErrorCode; + m_errorTitle = GetErrorText( aErrorCode ); } diff --git a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp index f96b6b8077..4e743156c6 100644 --- a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp +++ b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias.cpp @@ -42,6 +42,7 @@ DIALOG_CLEANUP_TRACKS_AND_VIAS::DIALOG_CLEANUP_TRACKS_AND_VIAS( PCB_EDIT_FRAME* m_deleteUnconnectedOpt->SetValue( cfg->m_Cleanup.cleanup_unconnected ); m_cleanShortCircuitOpt->SetValue( cfg->m_Cleanup.cleanup_short_circuits ); m_deleteTracksInPadsOpt->SetValue( cfg->m_Cleanup.cleanup_tracks_in_pad ); + m_deleteDanglingViasOpt->SetValue( cfg->m_Cleanup.delete_dangling_vias ); m_changesTreeModel = new RC_TREE_MODEL( m_parentFrame, m_changesDataView ); m_changesDataView->AssociateModel( m_changesTreeModel ); @@ -53,8 +54,9 @@ DIALOG_CLEANUP_TRACKS_AND_VIAS::DIALOG_CLEANUP_TRACKS_AND_VIAS( PCB_EDIT_FRAME* m_sdbSizerOK->SetLabel( _( "Update PCB" ) ); m_sdbSizerOK->SetDefault(); - GetSizer()->SetSizeHints(this); - Centre(); + m_sdbSizer->SetSizeHints( this ); + + FinishDialogSettings(); } @@ -67,6 +69,7 @@ DIALOG_CLEANUP_TRACKS_AND_VIAS::~DIALOG_CLEANUP_TRACKS_AND_VIAS() cfg->m_Cleanup.cleanup_unconnected = m_deleteUnconnectedOpt->GetValue(); cfg->m_Cleanup.cleanup_short_circuits = m_cleanShortCircuitOpt->GetValue(); cfg->m_Cleanup.cleanup_tracks_in_pad = m_deleteTracksInPadsOpt->GetValue(); + cfg->m_Cleanup.delete_dangling_vias = m_deleteDanglingViasOpt->GetValue(); for( CLEANUP_ITEM* item : m_items ) delete item; @@ -124,7 +127,8 @@ void DIALOG_CLEANUP_TRACKS_AND_VIAS::doCleanup( bool aDryRun ) m_cleanViasOpt->GetValue(), m_mergeSegmOpt->GetValue(), m_deleteUnconnectedOpt->GetValue(), - m_deleteTracksInPadsOpt->GetValue() ); + m_deleteTracksInPadsOpt->GetValue(), + m_deleteDanglingViasOpt->GetValue() ); if( aDryRun ) { diff --git a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.cpp b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.cpp index 9e6b2b0e49..66a3dff9d2 100644 --- a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.cpp +++ b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.cpp @@ -1,5 +1,5 @@ /////////////////////////////////////////////////////////////////////////// -// C++ code generated with wxFormBuilder (version Oct 26 2018) +// C++ code generated with wxFormBuilder (version 3.9.0 Jul 27 2020) // http://www.wxformbuilder.org/ // // PLEASE DO *NOT* EDIT THIS FILE! @@ -29,17 +29,20 @@ DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE( wxWind bSizerUpper->Add( m_cleanViasOpt, 0, wxBOTTOM|wxLEFT|wxRIGHT, 5 ); + m_deleteDanglingViasOpt = new wxCheckBox( this, wxID_ANY, _("Delete vias connected only on one layer"), wxDefaultPosition, wxDefaultSize, 0 ); + bSizerUpper->Add( m_deleteDanglingViasOpt, 0, wxALL, 5 ); + m_mergeSegmOpt = new wxCheckBox( this, wxID_ANY, _("&Merge co-linear tracks"), wxDefaultPosition, wxDefaultSize, 0 ); m_mergeSegmOpt->SetToolTip( _("merge aligned track segments, and remove null segments") ); bSizerUpper->Add( m_mergeSegmOpt, 0, wxBOTTOM|wxLEFT|wxRIGHT, 5 ); - m_deleteUnconnectedOpt = new wxCheckBox( this, wxID_ANY, _("Delete &dangling tracks"), wxDefaultPosition, wxDefaultSize, 0 ); + m_deleteUnconnectedOpt = new wxCheckBox( this, wxID_ANY, _("Delete tracks unconnected at one end"), wxDefaultPosition, wxDefaultSize, 0 ); m_deleteUnconnectedOpt->SetToolTip( _("delete tracks having at least one dangling end") ); bSizerUpper->Add( m_deleteUnconnectedOpt, 0, wxBOTTOM|wxLEFT|wxRIGHT, 5 ); - m_deleteTracksInPadsOpt = new wxCheckBox( this, wxID_ANY, _("Delete Tracks in Pads"), wxDefaultPosition, wxDefaultSize, 0 ); + m_deleteTracksInPadsOpt = new wxCheckBox( this, wxID_ANY, _("Delete tracks fully inside pads"), wxDefaultPosition, wxDefaultSize, 0 ); m_deleteTracksInPadsOpt->SetToolTip( _("Delete tracks that have both start and end positions inside of a pad") ); bSizerUpper->Add( m_deleteTracksInPadsOpt, 0, wxBOTTOM|wxLEFT|wxRIGHT, 5 ); @@ -80,6 +83,7 @@ DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE( wxWind // Connect Events m_cleanShortCircuitOpt->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); m_cleanViasOpt->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); + m_deleteDanglingViasOpt->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); m_mergeSegmOpt->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); m_deleteUnconnectedOpt->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); m_deleteTracksInPadsOpt->Connect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); @@ -92,6 +96,7 @@ DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::~DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE() // Disconnect Events m_cleanShortCircuitOpt->Disconnect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); m_cleanViasOpt->Disconnect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); + m_deleteDanglingViasOpt->Disconnect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); m_mergeSegmOpt->Disconnect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); m_deleteUnconnectedOpt->Disconnect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); m_deleteTracksInPadsOpt->Disconnect( wxEVT_COMMAND_CHECKBOX_CLICKED, wxCommandEventHandler( DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE::OnCheckBox ), NULL, this ); diff --git a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.fbp b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.fbp index c4ab2c52f4..9b102c0011 100644 --- a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.fbp +++ b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.fbp @@ -197,6 +197,71 @@ OnCheckBox + + 5 + wxALL + 0 + + 1 + 1 + 1 + 1 + + + + + + + + 1 + 0 + 0 + 1 + + 1 + 0 + Dock + 0 + Left + 1 + + 1 + + 0 + 0 + wxID_ANY + Delete vias connected only on one layer + + 0 + + + 0 + + 1 + m_deleteDanglingViasOpt + 1 + + + protected + 1 + + Resizable + 1 + + + ; ; forward_declare + 0 + + + wxFILTER_NONE + wxDefaultValidator + + + + + OnCheckBox + + 5 wxBOTTOM|wxLEFT|wxRIGHT diff --git a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.h b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.h index feef49c841..b6466a4fa2 100644 --- a/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.h +++ b/pcbnew/dialogs/dialog_cleanup_tracks_and_vias_base.h @@ -1,5 +1,5 @@ /////////////////////////////////////////////////////////////////////////// -// C++ code generated with wxFormBuilder (version Oct 26 2018) +// C++ code generated with wxFormBuilder (version 3.9.0 Jul 27 2020) // http://www.wxformbuilder.org/ // // PLEASE DO *NOT* EDIT THIS FILE! @@ -36,6 +36,7 @@ class DIALOG_CLEANUP_TRACKS_AND_VIAS_BASE : public DIALOG_SHIM protected: wxCheckBox* m_cleanShortCircuitOpt; wxCheckBox* m_cleanViasOpt; + wxCheckBox* m_deleteDanglingViasOpt; wxCheckBox* m_mergeSegmOpt; wxCheckBox* m_deleteUnconnectedOpt; wxCheckBox* m_deleteTracksInPadsOpt; diff --git a/pcbnew/pcbnew_settings.cpp b/pcbnew/pcbnew_settings.cpp index 5f7a7ce96b..61be6f28f3 100644 --- a/pcbnew/pcbnew_settings.cpp +++ b/pcbnew/pcbnew_settings.cpp @@ -174,6 +174,9 @@ PCBNEW_SETTINGS::PCBNEW_SETTINGS() m_params.emplace_back( new PARAM( "cleanup.cleanup_vias", &m_Cleanup.cleanup_vias, true ) ); + m_params.emplace_back( new PARAM( "cleanup.delete_dangling_vias", + &m_Cleanup.delete_dangling_vias, true ) ); + m_params.emplace_back( new PARAM( "cleanup.merge_segments", &m_Cleanup.merge_segments, true ) ); diff --git a/pcbnew/pcbnew_settings.h b/pcbnew/pcbnew_settings.h index 96bc073d96..ea5f45819c 100644 --- a/pcbnew/pcbnew_settings.h +++ b/pcbnew/pcbnew_settings.h @@ -74,6 +74,7 @@ public: struct DIALOG_CLEANUP { bool cleanup_vias; + bool delete_dangling_vias; bool cleanup_tracks_in_pad; bool cleanup_unconnected; bool cleanup_short_circuits; diff --git a/pcbnew/tracks_cleaner.cpp b/pcbnew/tracks_cleaner.cpp index 4889bd1264..80713d3e18 100644 --- a/pcbnew/tracks_cleaner.cpp +++ b/pcbnew/tracks_cleaner.cpp @@ -52,8 +52,10 @@ TRACKS_CLEANER::TRACKS_CLEANER( BOARD* aPcb, BOARD_COMMIT& aCommit ) : */ void TRACKS_CLEANER::CleanupBoard( bool aDryRun, std::vector* aItemsList, bool aRemoveMisConnected, bool aCleanVias, bool aMergeSegments, - bool aDeleteUnconnected, bool aDeleteTracksinPad ) + bool aDeleteUnconnected, bool aDeleteTracksinPad, bool aDeleteDanglingVias ) { + bool has_deleted = false; + m_dryRun = aDryRun; m_itemsList = aItemsList; @@ -80,15 +82,14 @@ void TRACKS_CLEANER::CleanupBoard( bool aDryRun, std::vector* aIt // Delete dangling tracks if( aDeleteUnconnected ) - { - if( deleteDanglingTracks() ) - { - // Removed tracks can leave aligned segments - // (when a T was formed by tracks and the "vertical" segment is removed) - if( aMergeSegments ) - cleanupSegments(); - } - } + has_deleted = deleteDanglingTracks( false ); + + // Delete dangling vias + if( aDeleteDanglingVias ) + has_deleted |= deleteDanglingTracks( true ); + + if( has_deleted && aMergeSegments ) + cleanupSegments(); // Clear the flag used to mark some segments: for( TRACK* segment : m_brd->Tracks() ) @@ -234,7 +235,7 @@ bool TRACKS_CLEANER::testTrackEndpointIsNode( TRACK* aTrack, bool aTstStart ) } -bool TRACKS_CLEANER::deleteDanglingTracks() +bool TRACKS_CLEANER::deleteDanglingTracks( bool aVia ) { bool item_erased = false; bool modified = false; @@ -253,13 +254,20 @@ bool TRACKS_CLEANER::deleteDanglingTracks() bool flag_erase = false; // Start without a good reason to erase it wxPoint pos; + if( aVia && track->Type() != PCB_VIA_T ) + continue; + else if( !aVia && track->Type() == PCB_VIA_T ) + continue; + // Tst if a track (or a via) endpoint is not connected to another track or to a zone. if( m_brd->GetConnectivity()->TestTrackEndpointDangling( track, &pos ) ) flag_erase = true; if( flag_erase ) { - int errorCode = track->IsTrack() ? CLEANUP_DANGLING_TRACK : CLEANUP_DANGLING_VIA; + int errorCode = + ( track->Type() != PCB_VIA_T ) ? + CLEANUP_DANGLING_TRACK : CLEANUP_DANGLING_VIA; CLEANUP_ITEM* item = new CLEANUP_ITEM( errorCode ); item->SetItems( track ); m_itemsList->push_back( item ); diff --git a/pcbnew/tracks_cleaner.h b/pcbnew/tracks_cleaner.h index b2b89f813f..3d41ee38ac 100644 --- a/pcbnew/tracks_cleaner.h +++ b/pcbnew/tracks_cleaner.h @@ -45,10 +45,11 @@ public: * @param aMergeSegments = true to merge collinear segmenst and remove 0 len segm * @param aDeleteUnconnected = true to remove dangling tracks * @param aDeleteTracksinPad = true to remove tracks fully inside pads + * @param aDeleteDanglingVias = true to remove a via that is only connected to a single layer */ void CleanupBoard( bool aDryRun, std::vector* aItemsList, bool aCleanVias, bool aRemoveMisConnected, bool aMergeSegments, bool aDeleteUnconnected, - bool aDeleteTracksinPad ); + bool aDeleteTracksinPad, bool aDeleteDanglingVias ); private: /* @@ -61,7 +62,12 @@ private: */ void cleanupVias(); - bool deleteDanglingTracks(); + /** + * Removes tracks or vias only connected on one end + * @param aVia if true, clean vias, if false clean tracks + * @return true if any items were deleted + */ + bool deleteDanglingTracks( bool aVia ); void deleteTracksInPads();