From 19eab62516a98c62b2cf71306cbcfa2fe56c09fb Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Wed, 30 Nov 2022 12:18:58 +0000 Subject: [PATCH] More safety around ratsnest state. Fixes https://gitlab.com/kicad/code/kicad/issues/13011 --- include/pcb_base_frame.h | 2 ++ pcbnew/board_commit.cpp | 22 +++++++++------ pcbnew/connectivity/connectivity_data.cpp | 11 ++++---- pcbnew/connectivity/connectivity_data.h | 2 +- pcbnew/drc/drc_test_provider_connectivity.cpp | 2 +- pcbnew/edit_zone_helpers.cpp | 3 +- pcbnew/files.cpp | 3 +- pcbnew/pcb_base_frame.cpp | 8 ++++++ pcbnew/pcb_edit_frame.cpp | 2 +- pcbnew/ratsnest/ratsnest_data.h | 5 ---- .../specctra_import.cpp | 2 +- pcbnew/tools/zone_filler_tool.cpp | 28 +++++++++---------- pcbnew/tools/zone_filler_tool.h | 1 + pcbnew/zone_filler.cpp | 2 +- 14 files changed, 51 insertions(+), 42 deletions(-) diff --git a/include/pcb_base_frame.h b/include/pcb_base_frame.h index 65c2bfea9b..aa6560a0e3 100644 --- a/include/pcb_base_frame.h +++ b/include/pcb_base_frame.h @@ -397,6 +397,8 @@ protected: virtual void unitsChangeRefresh() override; + void rebuildConnectivity(); + protected: BOARD* m_pcb; PCB_DISPLAY_OPTIONS m_displayOptions; diff --git a/pcbnew/board_commit.cpp b/pcbnew/board_commit.cpp index 5d0ea59563..7bf4caf3ee 100644 --- a/pcbnew/board_commit.cpp +++ b/pcbnew/board_commit.cpp @@ -175,6 +175,8 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) if( Empty() ) return; + std::shared_ptr connectivity = board->GetConnectivity(); + // Note: // frame == nullptr happens in QA tests // in this case m_isBoardEditor and m_isFootprintEditor are set to false @@ -448,8 +450,6 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) if( !( aCommitFlags & SKIP_CONNECTIVITY ) ) { - std::shared_ptr connectivity = board->GetConnectivity(); - if( ent.m_copy ) connectivity->MarkItemNetAsDirty( static_cast( ent.m_copy ) ); @@ -504,24 +504,28 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) { size_t num_changes = m_changes.size(); - if( !( aCommitFlags & SKIP_CONNECTIVITY ) ) + if( aCommitFlags & SKIP_CONNECTIVITY ) + { + connectivity->ClearRatsnest(); + connectivity->ClearLocalRatsnest(); + } + else { - std::shared_ptr connectivity = board->GetConnectivity(); - if( m_resolveNetConflicts ) connectivity->PropagateNets( this, PROPAGATE_MODE::RESOLVE_CONFLICTS ); connectivity->RecalculateRatsnest( this ); board->UpdateRatsnestExclusions(); connectivity->ClearLocalRatsnest(); + + if( frame ) + frame->GetCanvas()->RedrawRatsnest(); } - if( frame ) + if( solderMaskDirty ) { - if( solderMaskDirty ) + if( frame ) frame->HideSolderMask(); - - frame->GetCanvas()->RedrawRatsnest(); } // Log undo items for any connectivity changes diff --git a/pcbnew/connectivity/connectivity_data.cpp b/pcbnew/connectivity/connectivity_data.cpp index 8c4bce40b4..19851191f3 100644 --- a/pcbnew/connectivity/connectivity_data.cpp +++ b/pcbnew/connectivity/connectivity_data.cpp @@ -63,7 +63,10 @@ CONNECTIVITY_DATA::CONNECTIVITY_DATA( const std::vector& aItems, bo CONNECTIVITY_DATA::~CONNECTIVITY_DATA() { - Clear(); + for( RN_NET* net : m_nets ) + delete net; + + m_nets.clear(); } @@ -494,12 +497,10 @@ unsigned int CONNECTIVITY_DATA::GetUnconnectedCount( bool aVisibleOnly ) const } -void CONNECTIVITY_DATA::Clear() +void CONNECTIVITY_DATA::ClearRatsnest() { for( RN_NET* net : m_nets ) - delete net; - - m_nets.clear(); + net->Clear(); } diff --git a/pcbnew/connectivity/connectivity_data.h b/pcbnew/connectivity/connectivity_data.h index 04c827481d..3e06b2cd5f 100644 --- a/pcbnew/connectivity/connectivity_data.h +++ b/pcbnew/connectivity/connectivity_data.h @@ -154,7 +154,7 @@ public: * Function Clear() * Erases the connectivity database. */ - void Clear(); + void ClearRatsnest(); /** * Function GetNetCount() diff --git a/pcbnew/drc/drc_test_provider_connectivity.cpp b/pcbnew/drc/drc_test_provider_connectivity.cpp index 5e07297bb1..562c513337 100644 --- a/pcbnew/drc/drc_test_provider_connectivity.cpp +++ b/pcbnew/drc/drc_test_provider_connectivity.cpp @@ -83,7 +83,7 @@ bool DRC_TEST_PROVIDER_CONNECTIVITY::Run() } // Rebuild (from scratch, ignoring dirty flags) just in case. This really needs to be reliable. - connectivity->Clear(); + connectivity->ClearRatsnest(); connectivity->Build( board, m_drcEngine->GetProgressReporter() ); connectivity->FindIsolatedCopperIslands( islandsList, true ); diff --git a/pcbnew/edit_zone_helpers.cpp b/pcbnew/edit_zone_helpers.cpp index 4a7fccba8b..d17a590344 100644 --- a/pcbnew/edit_zone_helpers.cpp +++ b/pcbnew/edit_zone_helpers.cpp @@ -104,8 +104,7 @@ void PCB_EDIT_FRAME::Edit_Zone_Params( ZONE* aZone ) commit.Stage( pickedList ); commit.Push( _( "Modify zone properties" ), SKIP_CONNECTIVITY ); - GetBoard()->BuildConnectivity(); - GetToolManager()->PostEvent( EVENTS::ConnectivityChangedEvent ); + rebuildConnectivity(); pickedList.ClearItemsList(); // s_ItemsListPicker is no longer owner of picked items } diff --git a/pcbnew/files.cpp b/pcbnew/files.cpp index 756a91654f..2879acfd9a 100644 --- a/pcbnew/files.cpp +++ b/pcbnew/files.cpp @@ -987,8 +987,7 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in if( filler.Fill( toFill ) ) commit.Push( _( "Convert Zone(s)" ), SKIP_CONNECTIVITY ); - GetBoard()->BuildConnectivity( &progressReporter ); - GetToolManager()->PostEvent( EVENTS::ConnectivityChangedEvent ); + rebuildConnectivity(); } // from EDA_APPL which was first loaded BOARD only: diff --git a/pcbnew/pcb_base_frame.cpp b/pcbnew/pcb_base_frame.cpp index 7dd9ce0733..aa778ac861 100644 --- a/pcbnew/pcb_base_frame.cpp +++ b/pcbnew/pcb_base_frame.cpp @@ -1006,6 +1006,14 @@ void PCB_BASE_FRAME::OnModify() } +void PCB_BASE_FRAME::rebuildConnectivity() +{ + GetBoard()->BuildConnectivity(); + GetToolManager()->PostEvent( EVENTS::ConnectivityChangedEvent ); + GetCanvas()->RedrawRatsnest(); +} + + PCB_DRAW_PANEL_GAL* PCB_BASE_FRAME::GetCanvas() const { return static_cast( EDA_DRAW_FRAME::GetCanvas() ); diff --git a/pcbnew/pcb_edit_frame.cpp b/pcbnew/pcb_edit_frame.cpp index ea4387bee9..794259ec81 100644 --- a/pcbnew/pcb_edit_frame.cpp +++ b/pcbnew/pcb_edit_frame.cpp @@ -980,7 +980,7 @@ void PCB_EDIT_FRAME::ResolveDRCExclusions() commit.Add( marker ); } - commit.Push( wxEmptyString, SKIP_UNDO | SKIP_SET_DIRTY | SKIP_CONNECTIVITY ); + commit.Push( wxEmptyString, SKIP_UNDO | SKIP_SET_DIRTY ); for( PCB_MARKER* marker : GetBoard()->Markers() ) { diff --git a/pcbnew/ratsnest/ratsnest_data.h b/pcbnew/ratsnest/ratsnest_data.h index 9821276eac..f1d339bdf0 100644 --- a/pcbnew/ratsnest/ratsnest_data.h +++ b/pcbnew/ratsnest/ratsnest_data.h @@ -64,11 +64,6 @@ class RN_NET public: RN_NET(); - /** - * Mark ratsnest for given net as 'dirty', i.e. requiring recomputation. - */ - void MarkDirty() { m_dirty = true; } - /** * Return state of the 'dirty' flag, indicating that ratsnest for a given net is invalid * and requires an update. diff --git a/pcbnew/specctra_import_export/specctra_import.cpp b/pcbnew/specctra_import_export/specctra_import.cpp index 5296eeb4c8..73dcac2b93 100644 --- a/pcbnew/specctra_import_export/specctra_import.cpp +++ b/pcbnew/specctra_import_export/specctra_import.cpp @@ -81,7 +81,7 @@ bool PCB_EDIT_FRAME::ImportSpecctraSession( const wxString& fullFileName ) return false; } - GetBoard()->GetConnectivity()->Clear(); + GetBoard()->GetConnectivity()->ClearRatsnest(); GetBoard()->BuildConnectivity(); OnModify(); diff --git a/pcbnew/tools/zone_filler_tool.cpp b/pcbnew/tools/zone_filler_tool.cpp index ddd3eff087..e91df7fb36 100644 --- a/pcbnew/tools/zone_filler_tool.cpp +++ b/pcbnew/tools/zone_filler_tool.cpp @@ -97,9 +97,7 @@ void ZONE_FILLER_TOOL::CheckAllZones( wxWindow* aCaller, PROGRESS_REPORTER* aRep commit.Revert(); } - board()->BuildConnectivity(); - m_toolMgr->PostEvent( EVENTS::ConnectivityChangedEvent ); - + rebuildConnectivity(); refresh(); m_fillInProgress = false; @@ -174,14 +172,12 @@ void ZONE_FILLER_TOOL::FillAllZones( wxWindow* aCaller, PROGRESS_REPORTER* aRepo commit.Revert(); } - board()->BuildConnectivity( reporter.get() ); - m_toolMgr->PostEvent( EVENTS::ConnectivityChangedEvent ); + rebuildConnectivity(); + refresh(); if( filler.IsDebug() ) frame->UpdateUserInterface(); - refresh(); - m_fillInProgress = false; // wxWidgets has keyboard focus issues after the progress reporter. Re-setting the focus @@ -259,8 +255,8 @@ int ZONE_FILLER_TOOL::ZoneFillDirty( const TOOL_EVENT& aEvent ) else commit.Revert(); - board()->BuildConnectivity( reporter.get() ); - m_toolMgr->PostEvent( EVENTS::ConnectivityChangedEvent ); + rebuildConnectivity(); + refresh(); if( GetRunningMicroSecs() - startTime > 1000000 ) { @@ -286,8 +282,6 @@ int ZONE_FILLER_TOOL::ZoneFillDirty( const TOOL_EVENT& aEvent ) if( filler.IsDebug() ) frame->UpdateUserInterface(); - refresh(); - m_fillInProgress = false; // wxWidgets has keyboard focus issues after the progress reporter. Re-setting the focus @@ -340,9 +334,7 @@ int ZONE_FILLER_TOOL::ZoneFill( const TOOL_EVENT& aEvent ) commit.Revert(); } - board()->BuildConnectivity( reporter.get() ); - m_toolMgr->PostEvent( EVENTS::ConnectivityChangedEvent ); - + rebuildConnectivity(); refresh(); m_fillInProgress = false; @@ -399,6 +391,14 @@ int ZONE_FILLER_TOOL::ZoneUnfillAll( const TOOL_EVENT& aEvent ) } +void ZONE_FILLER_TOOL::rebuildConnectivity() +{ + board()->BuildConnectivity(); + m_toolMgr->PostEvent( EVENTS::ConnectivityChangedEvent ); + canvas()->RedrawRatsnest(); +} + + void ZONE_FILLER_TOOL::refresh() { canvas()->GetView()->UpdateAllItemsConditionally( KIGFX::REPAINT, diff --git a/pcbnew/tools/zone_filler_tool.h b/pcbnew/tools/zone_filler_tool.h index fc73244892..b1a56e7998 100644 --- a/pcbnew/tools/zone_filler_tool.h +++ b/pcbnew/tools/zone_filler_tool.h @@ -70,6 +70,7 @@ private: ///< Refocus on an idle event (used after the Progress Reporter messes up the focus). void singleShotRefocus( wxIdleEvent& ); + void rebuildConnectivity(); void refresh(); ///< Set up handlers for various events. diff --git a/pcbnew/zone_filler.cpp b/pcbnew/zone_filler.cpp index f3bed2d9a6..6430c85d36 100644 --- a/pcbnew/zone_filler.cpp +++ b/pcbnew/zone_filler.cpp @@ -97,7 +97,7 @@ bool ZONE_FILLER::Fill( std::vector& aZones, bool aCheck, wxWindow* aPare std::shared_ptr connectivity = m_board->GetConnectivity(); // Rebuild (from scratch, ignoring dirty flags) just in case. This really needs to be reliable. - connectivity->Clear(); + connectivity->ClearRatsnest(); connectivity->Build( m_board, m_progressReporter ); BOARD_DESIGN_SETTINGS& bds = m_board->GetDesignSettings();