From 7eb98a98631ba64dbf4333f25e61d57859c63e75 Mon Sep 17 00:00:00 2001 From: Alex Shvartzkop Date: Tue, 28 Nov 2023 20:55:37 +0300 Subject: [PATCH] Fixes for tuning patterns and router. Fixes https://gitlab.com/kicad/code/kicad/-/issues/16160 --- pcbnew/generators/pcb_tuning_pattern.cpp | 186 +++++++++++++++++----- pcbnew/router/pns_index.cpp | 2 + pcbnew/router/pns_joint.h | 13 +- pcbnew/tools/generator_tool_pns_proxy.cpp | 50 +++--- pcbnew/tools/generator_tool_pns_proxy.h | 11 +- pcbnew/tools/pcb_selection_tool.cpp | 3 +- 6 files changed, 195 insertions(+), 70 deletions(-) diff --git a/pcbnew/generators/pcb_tuning_pattern.cpp b/pcbnew/generators/pcb_tuning_pattern.cpp index a0d3be09e9..309836c483 100644 --- a/pcbnew/generators/pcb_tuning_pattern.cpp +++ b/pcbnew/generators/pcb_tuning_pattern.cpp @@ -57,6 +57,7 @@ #include #include #include +#include #include #include @@ -253,6 +254,8 @@ protected: PNS::ROUTER_MODE toPNSMode(); + bool recoverBaseline( PNS::ROUTER* aRouter ); + bool baselineValid(); bool initBaseLine( PNS::ROUTER* aRouter, int aLayer, BOARD* aBoard, VECTOR2I& aStart, @@ -534,7 +537,7 @@ void PCB_TUNING_PATTERN::EditStart( GENERATOR_TOOL* aTool, BOARD* aBoard, int layer = GetLayer(); PNS::ROUTER* router = aTool->Router(); - aTool->ClearRouterCommit(); + aTool->ClearRouterCommits(); router->SyncWorld(); PNS::RULE_RESOLVER* resolver = router->GetRuleResolver(); @@ -699,21 +702,21 @@ static std::optional getPNSLine( const VECTOR2I& aStart, const VECTOR PNS::LINKED_ITEM* startItem = pickSegment( router, aStart, layer, aStartOut ); PNS::LINKED_ITEM* endItem = pickSegment( router, aEnd, layer, aEndOut ); - wxASSERT( startItem ); - wxASSERT( endItem ); + //wxCHECK( startItem && endItem, std::nullopt ); - if( !startItem || !endItem ) - return std::nullopt; + for( PNS::LINKED_ITEM* testItem : { startItem, endItem } ) + { + if( !testItem ) + continue; - PNS::LINE line = world->AssembleLine( startItem, nullptr, false, true ); - SHAPE_LINE_CHAIN oldChain = line.CLine(); + PNS::LINE line = world->AssembleLine( testItem, nullptr, false, false ); + SHAPE_LINE_CHAIN oldChain = line.CLine(); - wxCHECK( line.ContainsLink( endItem ), std::nullopt ); + if( oldChain.PointOnEdge( aStartOut, 1 ) && oldChain.PointOnEdge( aEndOut, 1 ) ) + return line; + } - wxASSERT( oldChain.PointOnEdge( aStartOut, 1 ) ); - wxASSERT( oldChain.PointOnEdge( aEndOut, 1 ) ); - - return line; + return std::nullopt; } @@ -825,8 +828,8 @@ void PCB_TUNING_PATTERN::removeToBaseline( PNS::ROUTER* aRouter, int aLayer, } -void PCB_TUNING_PATTERN::Remove( GENERATOR_TOOL* aTool, BOARD* aBoard, - PCB_BASE_EDIT_FRAME* aFrame, BOARD_COMMIT* aCommit ) +void PCB_TUNING_PATTERN::Remove( GENERATOR_TOOL* aTool, BOARD* aBoard, PCB_BASE_EDIT_FRAME* aFrame, + BOARD_COMMIT* aCommit ) { aTool->Router()->SyncWorld(); @@ -851,7 +854,7 @@ void PCB_TUNING_PATTERN::Remove( GENERATOR_TOOL* aTool, BOARD* aBoard, aCommit->Remove( this ); - aTool->ClearRouterCommit(); + aTool->ClearRouterCommits(); if( baselineValid() ) { @@ -861,21 +864,32 @@ void PCB_TUNING_PATTERN::Remove( GENERATOR_TOOL* aTool, BOARD* aBoard, removeToBaseline( router, layer, *m_baseLineCoupled ); } - std::set clearRouterRemovedItems = aTool->GetRouterCommitRemovedItems(); - std::set clearRouterAddedItems = aTool->GetRouterCommitAddedItems(); + const std::vector& pnsCommits = aTool->GetRouterCommits(); - for( BOARD_ITEM* item : clearRouterRemovedItems ) + for( const GENERATOR_TOOL_PNS_PROXY::PNS_COMMIT& pnsCommit : pnsCommits ) { - item->ClearSelected(); - aCommit->Remove( item ); - } + const std::set routerRemovedItems = pnsCommit.removedItems; + const std::set routerAddedItems = pnsCommit.addedItems; - for( BOARD_ITEM* item : clearRouterAddedItems ) - { - aCommit->Add( item ); - } + /*std::cout << "Push commits << " << pnsCommits.size() << " routerRemovedItems " + << routerRemovedItems.size() << " routerAddedItems " << routerAddedItems.size() + << " m_removedItems " << m_removedItems.size() << std::endl;*/ - aCommit->Push( "Remove Tuning Pattern", undoFlags ); + for( BOARD_ITEM* item : routerRemovedItems ) + { + item->ClearSelected(); + aCommit->Remove( item ); + } + + for( BOARD_ITEM* item : routerAddedItems ) + { + aCommit->Add( item ); + } + + aCommit->Push( "Remove Tuning Pattern", undoFlags ); + + undoFlags |= APPEND_UNDO; + } } @@ -891,6 +905,66 @@ PNS::ROUTER_MODE PCB_TUNING_PATTERN::toPNSMode() } +bool PCB_TUNING_PATTERN::recoverBaseline( PNS::ROUTER* aRouter ) +{ + PNS::SOLID queryItem; + + SHAPE_LINE_CHAIN* chain = static_cast( getRectShape().Clone() ); + queryItem.SetShape( chain ); + queryItem.SetLayer( m_layer ); + + PNS::NODE::OBSTACLES obstacles; + PNS::COLLISION_SEARCH_OPTIONS opts; + opts.m_useClearanceEpsilon = false; + + PNS::NODE* world = aRouter->GetWorld(); + PNS::NODE* branch = world->Branch(); + + branch->QueryColliding( &queryItem, obstacles, opts ); + + for( const PNS::OBSTACLE& obs : obstacles ) + { + PNS::ITEM* item = obs.m_item; + + if( !item->OfKind( PNS::ITEM::SEGMENT_T | PNS::ITEM::ARC_T ) ) + continue; + + if( chain->PointInside( item->Anchor( 0 ), 10 ) + && chain->PointInside( item->Anchor( 1 ), 10 ) ) + { + aRouter->GetInterface()->RemoveItem( item ); + aRouter->GetWorld()->Remove( item ); + } + } + + if( baselineValid() ) + { + int lineWidth = pcbIUScale.mmToIU( 0.1 ); // TODO + + PNS::LINE recoverLine; + recoverLine.SetLayer( m_layer ); + recoverLine.SetWidth( lineWidth ); + recoverLine.Line() = *m_baseLine; + branch->Add( recoverLine, false ); + + if( m_tuningMode == DIFF_PAIR || m_tuningMode == DIFF_PAIR_SKEW ) + { + PNS::LINE recoverLineCoupled; + recoverLineCoupled.SetLayer( m_layer ); + recoverLineCoupled.SetWidth( lineWidth ); + recoverLineCoupled.Line() = *m_baseLineCoupled; + branch->Add( recoverLineCoupled, false ); + } + } + + aRouter->CommitRouting( branch ); + + //wxLogWarning( "PNS baseline recovered" ); + + return true; +} + + bool PCB_TUNING_PATTERN::resetToBaseline( PNS::ROUTER* aRouter, int aLayer, PCB_BASE_EDIT_FRAME* aFrame, SHAPE_LINE_CHAIN& aBaseLine, bool aPrimary ) @@ -902,7 +976,14 @@ bool PCB_TUNING_PATTERN::resetToBaseline( PNS::ROUTER* aRouter, int aLayer, aBaseLine.CPoint( -1 ), aRouter, aLayer, startSnapPoint, endSnapPoint ); - wxCHECK( pnsLine, false ); + if( !pnsLine ) + { + // TODO + //recoverBaseline( aRouter ); + return true; + } + + PNS::NODE* branch = world->Branch(); SHAPE_LINE_CHAIN straightChain; { @@ -920,15 +1001,15 @@ bool PCB_TUNING_PATTERN::resetToBaseline( PNS::ROUTER* aRouter, int aLayer, if( BOARD_ITEM* item = pnsItem->Parent() ) { aFrame->GetCanvas()->GetView()->Hide( item, true, true ); - m_removedItems.insert( item ); + //m_removedItems.insert( item ); } } - world->Remove( *pnsLine ); + branch->Remove( *pnsLine ); PNS::LINE straightLine( *pnsLine, straightChain ); - world->Add( straightLine, false ); + branch->Add( straightLine, false ); if( aPrimary ) { @@ -962,6 +1043,8 @@ bool PCB_TUNING_PATTERN::resetToBaseline( PNS::ROUTER* aRouter, int aLayer, } } + aRouter->CommitRouting( branch ); + return true; } @@ -993,7 +1076,7 @@ bool PCB_TUNING_PATTERN::Update( GENERATOR_TOOL* aTool, BOARD* aBoard, PCB_BASE_ } else { - initBaseLines( router, layer, aBoard ); + //initBaseLines( router, layer, aBoard ); return false; } @@ -1022,12 +1105,16 @@ bool PCB_TUNING_PATTERN::Update( GENERATOR_TOOL* aTool, BOARD* aBoard, PCB_BASE_ router->SetMode( toPNSMode() ); if( !router->StartRouting( startSnapPoint, startItem, layer ) ) + { + //recoverBaseline( router ); return false; + } PNS::MEANDER_PLACER_BASE* placer = static_cast( router->Placer() ); m_settings.m_keepEndpoints = true; // Required for re-grouping placer->UpdateSettings( m_settings ); + router->Move( m_end, nullptr ); m_trackWidth = router->Sizes().TrackWidth(); @@ -1069,17 +1156,26 @@ void PCB_TUNING_PATTERN::EditPush( GENERATOR_TOOL* aTool, BOARD* aBoard, { router->FixRoute( m_end, nullptr, true ); router->StopRouting(); + } - std::set routerRemovedItems = aTool->GetRouterCommitRemovedItems(); - std::set routerAddedItems = aTool->GetRouterCommitAddedItems(); + for( BOARD_ITEM* item : m_removedItems ) + { + aFrame->GetCanvas()->GetView()->Hide( item, false ); + aCommit->Remove( item ); + } - for( BOARD_ITEM* item : m_removedItems ) - { - aFrame->GetCanvas()->GetView()->Hide( item, false ); - aCommit->Remove( item ); - } + m_removedItems.clear(); - m_removedItems.clear(); + const std::vector& pnsCommits = aTool->GetRouterCommits(); + + for( const GENERATOR_TOOL_PNS_PROXY::PNS_COMMIT& pnsCommit : pnsCommits ) + { + const std::set routerRemovedItems = pnsCommit.removedItems; + const std::set routerAddedItems = pnsCommit.addedItems; + + //std::cout << "Push commits << " << pnsCommits.size() << " routerRemovedItems " + // << routerRemovedItems.size() << " routerAddedItems " << routerAddedItems.size() + // << " m_removedItems " << m_removedItems.size() << std::endl; for( BOARD_ITEM* item : routerRemovedItems ) { @@ -1100,12 +1196,14 @@ void PCB_TUNING_PATTERN::EditPush( GENERATOR_TOOL* aTool, BOARD* aBoard, aCommit->Add( item ); } - } - if( aCommitMsg.IsEmpty() ) - aCommit->Push( _( "Edit Tuning Pattern" ), aCommitFlags ); - else - aCommit->Push( aCommitMsg, aCommitFlags ); + if( aCommitMsg.IsEmpty() ) + aCommit->Push( _( "Edit Tuning Pattern" ), aCommitFlags ); + else + aCommit->Push( aCommitMsg, aCommitFlags ); + + aCommitFlags |= APPEND_UNDO; + } aFrame->AppendCopyToUndoList( groupUndoList, UNDO_REDO::REGROUP ); } diff --git a/pcbnew/router/pns_index.cpp b/pcbnew/router/pns_index.cpp index af2c9a9be9..e018e1e03f 100644 --- a/pcbnew/router/pns_index.cpp +++ b/pcbnew/router/pns_index.cpp @@ -28,6 +28,7 @@ namespace PNS { void INDEX::Add( ITEM* aItem ) { const LAYER_RANGE& range = aItem->Layers(); + assert( range.Start() != -1 && range.End() != -1 ); if( m_subIndices.size() <= static_cast( range.End() ) ) m_subIndices.resize( 2 * range.End() + 1 ); // +1 handles the 0 case @@ -46,6 +47,7 @@ void INDEX::Add( ITEM* aItem ) void INDEX::Remove( ITEM* aItem ) { const LAYER_RANGE& range = aItem->Layers(); + assert( range.Start() != -1 && range.End() != -1 ); if( m_subIndices.size() <= static_cast( range.End() ) ) return; diff --git a/pcbnew/router/pns_joint.h b/pcbnew/router/pns_joint.h index e81ed76377..cc281112fc 100644 --- a/pcbnew/router/pns_joint.h +++ b/pcbnew/router/pns_joint.h @@ -227,7 +227,18 @@ public: if( !IsLineCorner( aAllowLockedSegs ) ) return nullptr; - return static_cast( m_linkedItems[m_linkedItems[0] == aCurrent ? 1 : 0] ); + const std::vector& citems = m_linkedItems.CItems(); + const size_t size = citems.size(); + + for( size_t i = 0; i < size; i++ ) + { + ITEM* item = m_linkedItems[i]; + + if( item != aCurrent && item->Kind() != VIA_T ) + return static_cast( item ); + } + + return nullptr; } VIA* Via() const diff --git a/pcbnew/tools/generator_tool_pns_proxy.cpp b/pcbnew/tools/generator_tool_pns_proxy.cpp index e628e0fef2..d37bb5194d 100644 --- a/pcbnew/tools/generator_tool_pns_proxy.cpp +++ b/pcbnew/tools/generator_tool_pns_proxy.cpp @@ -36,20 +36,36 @@ class PNS_KICAD_IFACE_GENERATOR : public PNS_KICAD_IFACE { public: - void Commit() override {} - void SetHostTool( PCB_TOOL_BASE* aTool ) override { m_tool = aTool; m_commit = nullptr; - m_addedItems.clear(); - m_removedItems.clear(); + + ClearCommits(); + } + + void Commit() override + { // + m_commits.emplace_back(); + } + + void ClearCommits() + { + m_commits.clear(); + m_commits.emplace_back(); } void AddItem( PNS::ITEM* aItem ) override { BOARD_ITEM* brdItem = createBoardItem( aItem ); - m_addedItems.emplace( brdItem ); + + if( brdItem ) + { + aItem->SetParent( brdItem ); + brdItem->ClearFlags(); + + m_commits.back().addedItems.emplace( brdItem ); + } } void UpdateItem( PNS::ITEM* aItem ) override @@ -72,35 +88,27 @@ public: if( parent ) { - m_removedItems.emplace( parent ); + m_commits.back().removedItems.emplace( parent ); } } - std::set& AddedItems() { return m_addedItems; } - std::set& RemovedItems() { return m_removedItems; } + std::vector& Commits() { return m_commits; }; private: - std::set m_addedItems; - std::set m_removedItems; + std::vector m_commits; }; -void GENERATOR_TOOL_PNS_PROXY::ClearRouterCommit() +void GENERATOR_TOOL_PNS_PROXY::ClearRouterCommits() { - static_cast( GetInterface() )->AddedItems().clear(); - static_cast( GetInterface() )->RemovedItems().clear(); + static_cast( GetInterface() )->ClearCommits(); } -const std::set& GENERATOR_TOOL_PNS_PROXY::GetRouterCommitAddedItems() +const std::vector& +GENERATOR_TOOL_PNS_PROXY::GetRouterCommits() { - return static_cast( GetInterface() )->AddedItems(); -} - - -const std::set& GENERATOR_TOOL_PNS_PROXY::GetRouterCommitRemovedItems() -{ - return static_cast( GetInterface() )->RemovedItems(); + return static_cast( GetInterface() )->Commits(); } diff --git a/pcbnew/tools/generator_tool_pns_proxy.h b/pcbnew/tools/generator_tool_pns_proxy.h index e044c6ab50..957b50be28 100644 --- a/pcbnew/tools/generator_tool_pns_proxy.h +++ b/pcbnew/tools/generator_tool_pns_proxy.h @@ -36,15 +36,20 @@ class BOARD_ITEM; class GENERATOR_TOOL_PNS_PROXY : public PNS::TOOL_BASE { public: + static struct PNS_COMMIT + { + std::set addedItems; + std::set removedItems; + }; + GENERATOR_TOOL_PNS_PROXY( const std::string& aToolName ); ~GENERATOR_TOOL_PNS_PROXY(); /// @copydoc TOOL_INTERACTIVE::Reset() void Reset( RESET_REASON aReason ) override; - void ClearRouterCommit(); - const std::set& GetRouterCommitAddedItems(); - const std::set& GetRouterCommitRemovedItems(); + void ClearRouterCommits(); + const std::vector& GetRouterCommits(); }; #endif // GENERATOR_TOOL_PNS_PROXY_H diff --git a/pcbnew/tools/pcb_selection_tool.cpp b/pcbnew/tools/pcb_selection_tool.cpp index a15fed90c8..7e5526f4e1 100644 --- a/pcbnew/tools/pcb_selection_tool.cpp +++ b/pcbnew/tools/pcb_selection_tool.cpp @@ -2376,7 +2376,8 @@ bool PCB_SELECTION_TOOL::itemPassesFilter( BOARD_ITEM* aItem, bool aMultiSelect { if( static_cast( aItem )->GetItems().empty() ) { - wxASSERT_MSG( false, "Tried to select an empty generator" ); + if( !m_filter.otherItems ) + return false; } else {