From 2ee65b2d83923acb71aa77ce0efab09a3f2a8f44 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Sat, 13 Aug 2022 18:51:14 -0700 Subject: [PATCH] Force removal of zero-sized pads on load. Pads with zero width or height cause issues when rendering and selecting. KiCad has never allowed these elements but hasn't prevented importing systems where they exist. This prevents their import and cleans existing designs where the pads are placed Fixes https://gitlab.com/kicad/code/kicad/issues/12200 --- .../cadstar/cadstar_pcb_archive_loader.cpp | 36 +++++++++++++------ pcbnew/plugins/eagle/eagle_plugin.cpp | 17 +++++++-- pcbnew/plugins/fabmaster/import_fabmaster.cpp | 16 ++++++--- pcbnew/plugins/geda/gpcb_plugin.cpp | 13 +++++-- pcbnew/plugins/kicad/pcb_parser.cpp | 20 ++++++++--- pcbnew/plugins/legacy/legacy_plugin.cpp | 11 +++++- 6 files changed, 87 insertions(+), 26 deletions(-) diff --git a/pcbnew/plugins/cadstar/cadstar_pcb_archive_loader.cpp b/pcbnew/plugins/cadstar/cadstar_pcb_archive_loader.cpp index 1e94683d78..823c94dc53 100644 --- a/pcbnew/plugins/cadstar/cadstar_pcb_archive_loader.cpp +++ b/pcbnew/plugins/cadstar/cadstar_pcb_archive_loader.cpp @@ -43,6 +43,7 @@ #include #include #include +#include #include #include // std::numeric_limits @@ -807,7 +808,7 @@ void CADSTAR_PCB_ARCHIVE_LOADER::loadLibraryCoppers( const SYMDEF_PCB& aComponen if( !found ) anchorPad = aComponent.ComponentPads.at( compCopper.AssociatedPadIDs.front() ); - PAD* pad = new PAD( aFootprint ); + std::unique_ptr pad = std::make_unique( aFootprint ); pad->SetAttribute( PAD_ATTRIB::SMD ); pad->SetLayerSet( LSET( 1, copperLayer ) ); pad->SetNumber( anchorPad.Identifier.IsEmpty() @@ -821,6 +822,9 @@ void CADSTAR_PCB_ARCHIVE_LOADER::loadLibraryCoppers( const SYMDEF_PCB& aComponen int anchorSize = getKiCadLength( anchorpadcode.Shape.Size ); VECTOR2I anchorPos = getKiCadPoint( anchorPad.Position ); + if( anchorSize <= 0 ) + anchorSize = 1; + pad->SetShape( PAD_SHAPE::CUSTOM ); pad->SetAnchorPadShape( PAD_SHAPE::CIRCLE ); pad->SetSize( { anchorSize, anchorSize } ); @@ -834,7 +838,7 @@ void CADSTAR_PCB_ARCHIVE_LOADER::loadLibraryCoppers( const SYMDEF_PCB& aComponen shapePolys.Move( aFootprint->GetPosition() - anchorPos ); pad->AddPrimitivePoly( shapePolys, 0, true ); - aFootprint->Add( pad, ADD_MODE::APPEND ); // Append so that we get the correct behaviour + aFootprint->Add( pad.release(), ADD_MODE::APPEND ); // Append so that we get the correct behaviour // when finding pads by PAD_ID. See loadNets() m_librarycopperpads[aComponent.ID][anchorPad.ID].push_back( aFootprint->Pads().size() ); @@ -911,9 +915,9 @@ void CADSTAR_PCB_ARCHIVE_LOADER::loadLibraryPads( const SYMDEF_PCB& aComponent, { for( std::pair padPair : aComponent.ComponentPads ) { - PAD* pad = getKiCadPad( padPair.second, aFootprint ); - aFootprint->Add( pad, ADD_MODE::APPEND ); // Append so that we get correct behaviour - // when finding pads by PAD_ID - see loadNets() + if( PAD* pad = getKiCadPad( padPair.second, aFootprint ) ) + aFootprint->Add( pad, ADD_MODE::APPEND ); // Append so that we get correct behaviour + // when finding pads by PAD_ID - see loadNets() } } @@ -923,7 +927,14 @@ PAD* CADSTAR_PCB_ARCHIVE_LOADER::getKiCadPad( const COMPONENT_PAD& aCadstarPad, PADCODE csPadcode = getPadCode( aCadstarPad.PadCodeID ); wxString errorMSG; - PAD* pad = new PAD( aParent ); + if( csPadcode.Shape.Size <= 0) + { + wxLogError( _( "Invalid zero-sized pad ignored in\nfile: %s" ), m_board->GetFileName() ); + + return nullptr; + } + + std::unique_ptr pad = std::make_unique( aParent ); LSET padLayerSet; switch( aCadstarPad.Side ) @@ -1243,7 +1254,7 @@ PAD* CADSTAR_PCB_ARCHIVE_LOADER::getKiCadPad( const COMPONENT_PAD& aCadstarPad, m_padcodesTested.insert( csPadcode.ID ); } - return pad; + return pad.release(); } @@ -1719,11 +1730,14 @@ void CADSTAR_PCB_ARCHIVE_LOADER::loadComponents() wxString padNumber = kiPad->GetNumber(); delete kiPad; - kiPad = getKiCadPad( csPad, footprint ); - kiPad->SetNumber( padNumber ); - // Change the pointer in the footprint to the newly created pad - getPadReference( footprint, padEx.ID ) = kiPad; + if( kiPad = getKiCadPad( csPad, footprint ) ) + { + kiPad->SetNumber( padNumber ); + + // Change the pointer in the footprint to the newly created pad + getPadReference( footprint, padEx.ID ) = kiPad; + } } } diff --git a/pcbnew/plugins/eagle/eagle_plugin.cpp b/pcbnew/plugins/eagle/eagle_plugin.cpp index 2448dd4801..60c4e55caf 100644 --- a/pcbnew/plugins/eagle/eagle_plugin.cpp +++ b/pcbnew/plugins/eagle/eagle_plugin.cpp @@ -1929,9 +1929,8 @@ void EAGLE_PLUGIN::packagePad( FOOTPRINT* aFootprint, wxXmlNode* aTree ) int shape = EPAD::UNDEF; int eagleDrillz = e.drill.ToPcbUnits(); - PAD* pad = new PAD( aFootprint ); - aFootprint->Add( pad ); - transferPad( e, pad ); + std::unique_ptr pad = std::make_unique( aFootprint ); + transferPad( e, pad.get() ); if( e.first && *e.first && m_rules->psFirst != EPAD::UNDEF ) shape = m_rules->psFirst; @@ -2015,6 +2014,18 @@ void EAGLE_PLUGIN::packagePad( FOOTPRINT* aFootprint, wxXmlNode* aTree ) if( e.rot ) pad->SetOrientation( EDA_ANGLE( e.rot->degrees, DEGREES_T ) ); + + + VECTOR2I sz = pad->GetSize(); + + if( pad->GetSizeX() > 0 && pad->GetSizeY() > 0 ) + { + aFootprint->Add( pad.release() ); + } + else + { + wxLogError( _( "Invalid zero-sized pad ignored in\nfile: %s" ), m_board->GetFileName() ); + } } diff --git a/pcbnew/plugins/fabmaster/import_fabmaster.cpp b/pcbnew/plugins/fabmaster/import_fabmaster.cpp index 9b110d4676..63e94a0f63 100644 --- a/pcbnew/plugins/fabmaster/import_fabmaster.cpp +++ b/pcbnew/plugins/fabmaster/import_fabmaster.cpp @@ -2241,7 +2241,7 @@ bool FABMASTER::loadFootprints( BOARD* aBoard ) auto net_it = netinfo.find( netname ); - PAD* newpad = new PAD( fp ); + std::unique_ptr newpad = std::make_unique( fp ); if( net_it != netinfo.end() ) newpad->SetNet( net_it->second ); @@ -2259,8 +2259,8 @@ bool FABMASTER::loadFootprints( BOARD* aBoard ) if( padstack == pads.end() ) { - ///TODO:Warning - delete newpad; + wxLogError( _( "Unable to locate padstack %s in file %s\n" ), + pin->padstack.c_str(), aBoard->GetFileName().wc_str() ); continue; } else @@ -2412,7 +2412,15 @@ bool FABMASTER::loadFootprints( BOARD* aBoard ) else newpad->SetOrientation( EDA_ANGLE( src->rotate - pin->rotation, DEGREES_T ) ); - fp->Add( newpad, ADD_MODE::APPEND ); + if( newpad->GetSizeX() > 0 || newpad->GetSizeY() > 0 ) + { + fp->Add( newpad.release(), ADD_MODE::APPEND ); + } + else + { + wxLogError( _( "Invalid zero-sized pad ignored in\nfile: %s" ), + aBoard->GetFileName().wc_str() ); + } } } diff --git a/pcbnew/plugins/geda/gpcb_plugin.cpp b/pcbnew/plugins/geda/gpcb_plugin.cpp index 921f88004f..76a734ea76 100644 --- a/pcbnew/plugins/geda/gpcb_plugin.cpp +++ b/pcbnew/plugins/geda/gpcb_plugin.cpp @@ -539,7 +539,7 @@ FOOTPRINT* GPCB_FPL_CACHE::parseFOOTPRINT( LINE_READER* aLineReader ) aLineReader->LineNumber(), 0 ); } - PAD* pad = new PAD( footprint.get() ); + std::unique_ptr pad = std::make_unique( footprint.get() ); static const LSET pad_front( 3, F_Cu, F_Mask, F_Paste ); static const LSET pad_back( 3, B_Cu, B_Mask, B_Paste ); @@ -607,7 +607,16 @@ FOOTPRINT* GPCB_FPL_CACHE::parseFOOTPRINT( LINE_READER* aLineReader ) pad->SetShape( PAD_SHAPE::OVAL ); } - footprint->Add( pad ); + if( pad->GetSizeX() > 0 && pad->GetSizeY() > 0 ) + { + footprint->Add( pad.release() ); + } + else + { + wxLogError( _( "Invalid zero-sized pad ignored in\nfile: %s" ), + aLineReader->GetSource() ); + } + continue; } diff --git a/pcbnew/plugins/kicad/pcb_parser.cpp b/pcbnew/plugins/kicad/pcb_parser.cpp index 60b1027407..4de1795e32 100644 --- a/pcbnew/plugins/kicad/pcb_parser.cpp +++ b/pcbnew/plugins/kicad/pcb_parser.cpp @@ -3893,12 +3893,14 @@ FOOTPRINT* PCB_PARSER::parseFOOTPRINT_unchecked( wxArrayString* aInitialComments case T_pad: { - PAD* pad = parsePAD( footprint.get() ); - pt = pad->GetPos0(); + if( PAD* pad = parsePAD( footprint.get() ) ) + { + pt = pad->GetPos0(); + RotatePoint( pt, footprint->GetOrientation() ); + pad->SetPosition( pt + footprint->GetPosition() ); + footprint->Add( pad, ADD_MODE::APPEND, true ); + } - RotatePoint( pt, footprint->GetOrientation() ); - pad->SetPosition( pt + footprint->GetPosition() ); - footprint->Add( pad, ADD_MODE::APPEND, true ); break; } @@ -5036,6 +5038,14 @@ PAD* PCB_PARSER::parsePAD( FOOTPRINT* aParent ) pad->SetNumber( wxEmptyString ); } + // Zero-sized pads are not really selectable and likely cause crashes. + // They are not supported by KiCad and are removed on loading + if( pad->GetSizeX() <= 0 || pad->GetSizeY() <= 0 ) + { + wxLogError( _( "Invalid zero-sized pad ignored in\nfile: %s" ), CurSource() ); + return nullptr; + } + return pad.release(); } diff --git a/pcbnew/plugins/legacy/legacy_plugin.cpp b/pcbnew/plugins/legacy/legacy_plugin.cpp index c589636043..2d9df869f4 100644 --- a/pcbnew/plugins/legacy/legacy_plugin.cpp +++ b/pcbnew/plugins/legacy/legacy_plugin.cpp @@ -1514,7 +1514,16 @@ void LEGACY_PLUGIN::loadPAD( FOOTPRINT* aFootprint ) pad->SetPosition( padpos + aFootprint->GetPosition() ); - aFootprint->Add( pad.release() ); + if( pad->GetSizeX() > 0 && pad->GetSizeY() > 0 ) + { + aFootprint->Add( pad.release() ); + } + else + { + wxLogError( _( "Invalid zero-sized pad ignored in\nfile: %s" ), + m_reader->GetSource() ); + } + return; // preferred exit } }