Fix Altium custom pads import

- Add missing parsing to AREGION6 properties
- Link imported regions to pads (replace pad shape)

common/io/altium/altium_props_utils.cpp

@@ -46,11 +46,19 @@ int32_t ALTIUM_PROPS_UTILS::ConvertToKicadUnit( const double aValue )
 }
 
 
+std::optional<int> ALTIUM_PROPS_UTILS::ReadOptInt( const std::map<wxString, wxString>& aProps,
+                                                   const wxString& aKey )
+{
+    const std::map<wxString, wxString>::const_iterator& value = aProps.find( aKey );
+    return value == aProps.end() ? std::optional<int>{}  : wxAtoi( value->second );
+}
+
+
 int ALTIUM_PROPS_UTILS::ReadInt( const std::map<wxString, wxString>& aProps, const wxString& aKey,
                             int aDefault )
 {
-    const std::map<wxString, wxString>::const_iterator& value = aProps.find( aKey );
+    std::optional<int> opt = ReadOptInt(aProps, aKey);
-    return value == aProps.end() ? aDefault : wxAtoi( value->second );
+    return opt.has_value() ? opt.value() : aDefault;
 }
 
 

common/io/altium/altium_props_utils.h

@@ -27,6 +27,7 @@
 
 #include <stdint.h>
 #include <map>
+#include <optional>
 
 #include <wx/string.h>
 
@@ -36,6 +37,9 @@ class ALTIUM_PROPS_UTILS
 public:
     static int32_t ConvertToKicadUnit( const double aValue );
 
+    static std::optional<int> ReadOptInt( const std::map<wxString, wxString>& aProps,
+                                          const wxString& aKey );
+
     static int ReadInt( const std::map<wxString, wxString>& aProps, const wxString& aKey,
                         int aDefault );
 

pcbnew/pcb_io/altium/altium_parser_pcb.cpp

@@ -1191,6 +1191,8 @@ AREGION6::AREGION6( ALTIUM_BINARY_PARSER& aReader, bool aExtendedVertices )
     int  pkind     = ALTIUM_PROPS_UTILS::ReadInt( properties, wxT( "KIND" ), 0 );
     bool is_cutout = ALTIUM_PROPS_UTILS::ReadBool( properties, wxT( "ISBOARDCUTOUT" ), false );
 
+    v7layer = ALTIUM_PROPS_UTILS::ReadString( properties, wxT( "V7_LAYER" ), wxEmptyString );
+    name = ALTIUM_PROPS_UTILS::ReadString( properties, wxT( "NAME" ), wxEmptyString );
     is_shapebased = ALTIUM_PROPS_UTILS::ReadBool( properties, wxT( "ISSHAPEBASED" ), false );
     keepoutrestrictions = static_cast<uint8_t>(
             ALTIUM_PROPS_UTILS::ReadInt( properties, wxT( "KEEPOUTRESTRIC" ), 0x1F ) );
@@ -1200,6 +1202,25 @@ AREGION6::AREGION6( ALTIUM_BINARY_PARSER& aReader, bool aExtendedVertices )
     subpolyindex = static_cast<uint16_t>(
             ALTIUM_PROPS_UTILS::ReadInt( properties, "SUBPOLYINDEX", ALTIUM_POLYGON_NONE ) );
 
+    unionindex = ALTIUM_PROPS_UTILS::ReadOptInt( properties, "UNIONINDEX" );
+    padindex = ALTIUM_PROPS_UTILS::ReadOptInt( properties, "PADINDEX" );
+    arc_resolution = ALTIUM_PROPS_UTILS::ReadKicadUnit( properties, "ARCRESOLUTION", "0.1mil");
+    cavity_height = ALTIUM_PROPS_UTILS::ReadKicadUnit( properties, "CAVITYHEIGHT", "0mil");
+
+    // Note RFB:
+    // Sometimes the format could have vertices in the properties region, e.g. like below. In these
+    // cases a property "MAINCONTOURVERTEXCOUNT" will also be present containing the number of
+    // vertices. For now, I decided it is not worth it to parse it since the same data is present in
+    // the binary stream in all samples I have seen.
+    //  KIND0=0
+    //  VX0=-27mil
+    //  VY0=-15mil
+    //  CX0=0mil
+    //  CY0=0mil
+    //  SA0= 0.00000000000000E+0000
+    //  EA0= 0.00000000000000E+0000
+    //  R0=0mil
+
     switch( pkind )
     {
     case 0:

pcbnew/pcb_io/altium/altium_parser_pcb.h

@@ -562,8 +562,16 @@ struct AREGION6
     uint8_t      keepoutrestrictions;
     uint16_t     holecount;
 
+    wxString name;
+    wxString v7layer; ///< duplicate data? seems identical to 'layer' in the binary stream
+
+    std::optional<int> unionindex; ///< Unclear what unionindex refers to
+    std::optional<int> padindex;
+
     ALTIUM_REGION_KIND kind; // I assume this means if normal or keepout?
 
+    std::optional<int32_t>                   cavity_height; ///< Unclear what cavity height is
+    std::optional<int32_t>                   arc_resolution;
     std::vector<ALTIUM_VERTICE>              outline;
     std::vector<std::vector<ALTIUM_VERTICE>> holes;
 

pcbnew/pcb_io/altium/altium_pcb.cpp

@@ -75,6 +75,29 @@ bool IsAltiumLayerAPlane( ALTIUM_LAYER aLayer )
     return aLayer >= ALTIUM_LAYER::INTERNAL_PLANE_1 && aLayer <= ALTIUM_LAYER::INTERNAL_PLANE_16;
 }
 
+
+PAD* ALTIUM_PCB::HelperGetPad( int aPadIndex ) const
+{
+    if( !m_padIndexMap.contains( aPadIndex ) )
+    {
+        THROW_IO_ERROR( wxString::Format( wxT( "Component creator tries to access pad id "
+                                               "%d, which does not exist" ),
+                                          aPadIndex ) );
+    }
+
+    BOARD_CONNECTED_ITEM* padBoardItem = m_padIndexMap.at( aPadIndex );
+
+    if( padBoardItem->Type() != PCB_PAD_T )
+    {
+        // Todo: graceful handling?
+        THROW_IO_ERROR( wxString::Format( wxT( "Pad %d is not on a copper layer, unexpected" ),
+                                          aPadIndex ) );
+    }
+
+    return static_cast<PAD*>( padBoardItem );
+}
+
+
 FOOTPRINT* ALTIUM_PCB::HelperGetFootprint( uint16_t aComponent ) const
 {
     if( aComponent == ALTIUM_COMPONENT_NONE || m_components.size() <= aComponent )
@@ -792,7 +815,7 @@ FOOTPRINT* ALTIUM_PCB::ParseFootprint( ALTIUM_PCB_COMPOUND_FILE& altiumLibFile,
         case ALTIUM_RECORD::PAD:
         {
             APAD6 pad( parser );
-            ConvertPads6ToFootprintItem( footprint.get(), pad );
+            ConvertPads6ToFootprintItem( footprint.get(), pad, primitiveIndex );
             break;
         }
         case ALTIUM_RECORD::VIA:
@@ -2662,6 +2685,11 @@ void ALTIUM_PCB::ConvertShapeBasedRegions6ToFootprintItem( FOOTPRINT*      aFoot
 void ALTIUM_PCB::ConvertShapeBasedRegions6ToBoardItemOnLayer( const AREGION6& aElem,
                                                               PCB_LAYER_ID    aLayer )
 {
+    wxASSERT_MSG( !aElem.padindex.has_value(),
+                  wxT( "ConvertShapeBasedRegions6ToBoardItemOnLayer was called for a Region with a "
+                       "pad index! ConvertShapeBasedRegions6ToFootprintItemOnLayer should be "
+                       "called instead" ) );
+
     SHAPE_LINE_CHAIN linechain;
     HelperShapeLineChainFromAltiumVertices( linechain, aElem.outline );
 
@@ -2735,9 +2763,24 @@ void ALTIUM_PCB::ConvertShapeBasedRegions6ToFootprintItemOnLayer( FOOTPRINT*
         polySet.AddHole( hole_linechain );
     }
 
-    if( aLayer == F_Cu || aLayer == B_Cu )
+    if( aElem.padindex.has_value() && LSET::AllCuMask().Contains( aLayer ) )
+    {
+        // Replace shape on layer of existing pad
+        int actualPadindex=aElem.padindex.value() - 1; // altium seems to index starting at 1?
+        PAD* pad = HelperGetPad( actualPadindex ); // throws if pad doesn't exist
+
+        pad->SetShape( aLayer, PAD_SHAPE::CUSTOM );
+        polySet.Move( -pad->GetPosition() );
+        polySet.Rotate( -pad->Padstack().GetOrientation() );
+        pad->AddPrimitivePoly( aLayer, polySet, 0, true );
+
+        // Ignore extended primitive information from the region - this appears to mean nothing
+        // for regions used in pads. It should have already been set correctly when the pad was
+        // loaded
+
+    }
+    else if( aLayer == F_Cu || aLayer == B_Cu )
     {
-        // TODO(JE) padstacks -- not sure what should happen here yet
         std::unique_ptr<PAD> pad = std::make_unique<PAD>( aFootprint );
 
         LSET padLayers;
@@ -3137,6 +3180,8 @@ void ALTIUM_PCB::ParsePads6Data( const ALTIUM_PCB_COMPOUND_FILE&     aAltiumPcbF
 
     ALTIUM_BINARY_PARSER reader( aAltiumPcbFile, aEntry );
 
+    int padindex = 0;
+
     while( reader.GetRemainingBytes() >= 4 /* TODO: use Header section of file */ )
     {
         checkpoint();
@@ -3144,12 +3189,12 @@ void ALTIUM_PCB::ParsePads6Data( const ALTIUM_PCB_COMPOUND_FILE&     aAltiumPcbF
 
         if( elem.component == ALTIUM_COMPONENT_NONE )
         {
-            ConvertPads6ToBoardItem( elem );
+            ConvertPads6ToBoardItem( elem, padindex++ );
         }
         else
         {
             FOOTPRINT* footprint = HelperGetFootprint( elem.component );
-            ConvertPads6ToFootprintItem( footprint, elem );
+            ConvertPads6ToFootprintItem( footprint, elem, padindex++ );
         }
     }
 
@@ -3158,13 +3203,13 @@ void ALTIUM_PCB::ParsePads6Data( const ALTIUM_PCB_COMPOUND_FILE&     aAltiumPcbF
 }
 
 
-void ALTIUM_PCB::ConvertPads6ToBoardItem( const APAD6& aElem )
+void ALTIUM_PCB::ConvertPads6ToBoardItem( const APAD6& aElem, const int aPadIndex )
 {
     // It is possible to place altium pads on non-copper layers -> we need to interpolate them using drawings!
     if( !IsAltiumLayerCopper( aElem.layer ) && !IsAltiumLayerAPlane( aElem.layer )
         && aElem.layer != ALTIUM_LAYER::MULTI_LAYER )
     {
-        ConvertPads6ToBoardItemOnNonCopper( aElem );
+        ConvertPads6ToBoardItemOnNonCopper( aElem, aPadIndex );
     }
     else
     {
@@ -3172,7 +3217,7 @@ void ALTIUM_PCB::ConvertPads6ToBoardItem( const APAD6& aElem )
         std::unique_ptr<FOOTPRINT> footprint = std::make_unique<FOOTPRINT>( m_board );
         footprint->SetPosition( aElem.position );
 
-        ConvertPads6ToFootprintItemOnCopper( footprint.get(), aElem );
+        ConvertPads6ToFootprintItemOnCopper( footprint.get(), aElem, aPadIndex );
 
         m_board->Add( footprint.release(), ADD_MODE::APPEND );
     }
@@ -3253,22 +3298,24 @@ void ALTIUM_PCB::ConvertVias6ToFootprintItem( FOOTPRINT* aFootprint, const AVIA6
 }
 
 
-void ALTIUM_PCB::ConvertPads6ToFootprintItem( FOOTPRINT* aFootprint, const APAD6& aElem )
+void ALTIUM_PCB::ConvertPads6ToFootprintItem( FOOTPRINT* aFootprint, const APAD6& aElem,
+                                              const int aPadIndex )
 {
     // It is possible to place altium pads on non-copper layers -> we need to interpolate them using drawings!
     if( !IsAltiumLayerCopper( aElem.layer ) && !IsAltiumLayerAPlane( aElem.layer )
         && aElem.layer != ALTIUM_LAYER::MULTI_LAYER )
     {
-        ConvertPads6ToFootprintItemOnNonCopper( aFootprint, aElem );
+        ConvertPads6ToFootprintItemOnNonCopper( aFootprint, aElem, aPadIndex );
     }
     else
     {
-        ConvertPads6ToFootprintItemOnCopper( aFootprint, aElem );
+        ConvertPads6ToFootprintItemOnCopper( aFootprint, aElem, aPadIndex );
     }
 }
 
 
-void ALTIUM_PCB::ConvertPads6ToFootprintItemOnCopper( FOOTPRINT* aFootprint, const APAD6& aElem )
+void ALTIUM_PCB::ConvertPads6ToFootprintItemOnCopper( FOOTPRINT* aFootprint, const APAD6& aElem,
+                                                      const int aPadIndex )
 {
     std::unique_ptr<PAD> pad = std::make_unique<PAD>( aFootprint );
 
@@ -3596,11 +3643,13 @@ void ALTIUM_PCB::ConvertPads6ToFootprintItemOnCopper( FOOTPRINT* aFootprint, con
     if( aElem.is_tent_bottom )
         pad->SetLayerSet( pad->GetLayerSet().reset( B_Mask ) );
 
-    aFootprint->Add( pad.release(), ADD_MODE::APPEND );
+    PAD* pad_ptr = pad.release();
+    m_padIndexMap.emplace( aPadIndex, pad_ptr );
+    aFootprint->Add( pad_ptr, ADD_MODE::APPEND );
 }
 
 
-void ALTIUM_PCB::ConvertPads6ToBoardItemOnNonCopper( const APAD6& aElem )
+void ALTIUM_PCB::ConvertPads6ToBoardItemOnNonCopper( const APAD6& aElem, const int aPadIndex )
 {
     PCB_LAYER_ID klayer = GetKicadLayer( aElem.layer );
 
@@ -3622,11 +3671,14 @@ void ALTIUM_PCB::ConvertPads6ToBoardItemOnNonCopper( const APAD6& aElem )
 
     HelperParsePad6NonCopper( aElem, klayer, pad.get() );
 
-    m_board->Add( pad.release(), ADD_MODE::APPEND );
+    PCB_SHAPE* pad_ptr = pad.release();
+    m_padIndexMap.emplace( aPadIndex, pad_ptr );
+    m_board->Add( pad_ptr, ADD_MODE::APPEND );
 }
 
 
-void ALTIUM_PCB::ConvertPads6ToFootprintItemOnNonCopper( FOOTPRINT* aFootprint, const APAD6& aElem )
+void ALTIUM_PCB::ConvertPads6ToFootprintItemOnNonCopper( FOOTPRINT* aFootprint, const APAD6& aElem,
+                                                         const int aPadIndex )
 {
     PCB_LAYER_ID klayer = GetKicadLayer( aElem.layer );
 
@@ -3668,7 +3720,9 @@ void ALTIUM_PCB::ConvertPads6ToFootprintItemOnNonCopper( FOOTPRINT* aFootprint,
 
     HelperParsePad6NonCopper( aElem, klayer, pad.get() );
 
-    aFootprint->Add( pad.release(), ADD_MODE::APPEND );
+    PCB_SHAPE* pad_ptr = pad.release();
+    m_padIndexMap.emplace( aPadIndex, pad_ptr );
+    aFootprint->Add( pad_ptr, ADD_MODE::APPEND );
 }
 
 

pcbnew/pcb_io/altium/altium_pcb.h

@@ -85,6 +85,8 @@ enum class ALTIUM_PCB_DIR
 
 
 class BOARD;
+class BOARD_CONNECTED_ITEM;
+class PAD;
 class FP_SHAPE;
 class PCB_SHAPE;
 class PCB_TEXTBOX;
@@ -182,11 +184,14 @@ private:
                                               const ACOMPONENTBODY6& aElem );
     void ParsePads6Data( const ALTIUM_PCB_COMPOUND_FILE&     aAltiumPcbFile,
                          const CFB::COMPOUND_FILE_ENTRY* aEntry );
-    void ConvertPads6ToBoardItem( const APAD6& aElem );
+    void ConvertPads6ToBoardItem( const APAD6& aElem, const int aPadIndex );
-    void ConvertPads6ToFootprintItem( FOOTPRINT* aFootprint, const APAD6& aElem );
+    void ConvertPads6ToFootprintItem( FOOTPRINT* aFootprint, const APAD6& aElem,
-    void ConvertPads6ToBoardItemOnNonCopper( const APAD6& aElem );
+                                      const int aPadIndex );
-    void ConvertPads6ToFootprintItemOnCopper( FOOTPRINT* aFootprint, const APAD6& aElem );
+    void ConvertPads6ToBoardItemOnNonCopper( const APAD6& aElem, const int aPadIndex );
-    void ConvertPads6ToFootprintItemOnNonCopper( FOOTPRINT* aFootprint, const APAD6& aElem );
+    void ConvertPads6ToFootprintItemOnCopper( FOOTPRINT* aFootprint, const APAD6& aElem,
+                                              const int aPadIndex );
+    void ConvertPads6ToFootprintItemOnNonCopper( FOOTPRINT* aFootprint, const APAD6& aElem,
+                                                 const int aPadIndex );
     void ParseVias6Data( const ALTIUM_PCB_COMPOUND_FILE&     aAltiumPcbFile,
                          const CFB::COMPOUND_FILE_ENTRY* aEntry );
     void ConvertVias6ToFootprintItem( FOOTPRINT* aFootprint, const AVIA6& aElem );
@@ -261,6 +266,7 @@ private:
                                            const ALTIUM_LAYER aAltiumLayer );
 
     FOOTPRINT* HelperGetFootprint( uint16_t aComponent ) const;
+    PAD* HelperGetPad( int aPadIndex ) const;
 
     void remapUnsureLayers( std::vector<ABOARD6_LAYER_STACKUP>& aStackup );
 
@@ -277,6 +283,7 @@ private:
     std::map<ALTIUM_RULE_KIND, std::vector<ARULE6>> m_rules;
     std::map<ALTIUM_RECORD, std::multimap<int, const AEXTENDED_PRIMITIVE_INFORMATION>>
             m_extendedPrimitiveInformationMaps;
+    std::map<int, BOARD_CONNECTED_ITEM*> m_padIndexMap;
 
     std::map<ALTIUM_LAYER, ZONE*>        m_outer_plane;