altium: parse Regions6 to fill zone on import

pcbnew/plugins/altium/altium_parser.ksy

@@ -578,7 +578,8 @@ types:
       type: u1  # KEEPOUT = 2
     - id: net
       type: u2
-    - size: 2
+    - id: subpolyindex
+      type: u2
     - id: component
       type: u2
     - size: 5
@@ -590,16 +591,29 @@ types:
     - id: properties
       size: propterties_len
       type: str
+    # region1 type
+    - id: outline
+      type: region1_part
+    - id: holes
+      type: region1_part
+      repeat-expr: holecount
+      repeat: expr
+    # TODO
+    #- id: vertices_num
+    #  type: u4
+    #- id: vertices2 # region2 type
+    #  repeat: expr
+    #  repeat-expr: vertices_num+1
+    #  type: xyf2
+
+  region1_part:
+    seq:
     - id: vertices_num
       type: u4
     - id: vertices  # region1 type
       repeat: expr
       repeat-expr: vertices_num
       type: xyf
-    #- id: vertices2 # region2 type
-    #  repeat: expr
-    #  repeat-expr: vertices_num+1
-    #  type: xyf2
 
   componentbody:
     seq:

pcbnew/plugins/altium/altium_parser_pcb.cpp

@@ -990,9 +990,11 @@ AREGION6::AREGION6( ALTIUM_PARSER& aReader, bool aExtendedVertices )
     is_keepout     = flags2 == 2;
 
     net = aReader.Read<uint16_t>();
-    aReader.Skip( 2 );
+    subpolyindex = aReader.Read<uint16_t>();
     component = aReader.Read<uint16_t>();
-    aReader.Skip( 9 );
+    aReader.Skip( 5 );
+    holecount = aReader.Read<uint16_t>();
+    aReader.Skip( 2 );
 
     std::map<wxString, wxString> properties = aReader.ReadProperties();
     if( properties.empty() )
@@ -1005,8 +1007,9 @@ AREGION6::AREGION6( ALTIUM_PARSER& aReader, bool aExtendedVertices )
 
     is_shapebased = ALTIUM_PARSER::PropertiesReadBool( properties, "ISSHAPEBASED", false );
 
-    subpolyindex = static_cast<uint16_t>(
+    // TODO: this can differ from the other subpolyindex?!
-            ALTIUM_PARSER::PropertiesReadInt( properties, "SUBPOLYINDEX", ALTIUM_POLYGON_NONE ) );
+    //subpolyindex = static_cast<uint16_t>(
+    //        ALTIUM_PARSER::PropertiesReadInt( properties, "SUBPOLYINDEX", ALTIUM_POLYGON_NONE ) );
 
     switch( pkind )
     {
@@ -1037,9 +1040,9 @@ AREGION6::AREGION6( ALTIUM_PARSER& aReader, bool aExtendedVertices )
         break;
     }
 
-    uint32_t num_vertices = aReader.Read<uint32_t>();
+    uint32_t num_outline_vertices = aReader.Read<uint32_t>();
 
-    for( uint32_t i = 0; i < num_vertices; i++ )
+    for( uint32_t i = 0; i < num_outline_vertices; i++ )
     {
         if( aExtendedVertices )
         {
@@ -1049,14 +1052,32 @@ AREGION6::AREGION6( ALTIUM_PARSER& aReader, bool aExtendedVertices )
             int32_t radius   = aReader.ReadKicadUnit();
             double  angle1   = aReader.Read<double>();
             double  angle2   = aReader.Read<double>();
-            vertices.emplace_back( isRound, radius, angle1, angle2, position, center );
+            outline.emplace_back( isRound, radius, angle1, angle2, position, center );
         }
         else
         {
             // For some regions the coordinates are stored as double and not as int32_t
             int32_t x = ALTIUM_PARSER::ConvertToKicadUnit( aReader.Read<double>() );
             int32_t y = ALTIUM_PARSER::ConvertToKicadUnit( -aReader.Read<double>() );
-            vertices.emplace_back( wxPoint( x, y ) );
+            outline.emplace_back( wxPoint( x, y ) );
+        }
+    }
+
+    // TODO: for now we only support holes in regions where there are stored as double
+    if( !aExtendedVertices )
+    {
+        holes.resize( holecount );
+        for( uint16_t k = 0; k < holecount; k++ )
+        {
+            uint32_t num_hole_vertices = aReader.Read<uint32_t>();
+            holes.at( k ).reserve( num_hole_vertices );
+
+            for( uint32_t i = 0; i < num_hole_vertices; i++ )
+            {
+                int32_t x = ALTIUM_PARSER::ConvertToKicadUnit( aReader.Read<double>() );
+                int32_t y = ALTIUM_PARSER::ConvertToKicadUnit( -aReader.Read<double>() );
+                holes.at( k ).emplace_back( wxPoint( x, y ) );
+            }
         }
     }
 

pcbnew/plugins/altium/altium_parser_pcb.h

@@ -493,10 +493,12 @@ struct AREGION6
     uint16_t     net;
     uint16_t     component;
     uint16_t     subpolyindex;
+    uint16_t     holecount;
 
     ALTIUM_REGION_KIND kind; // I assume this means if normal or keepout?
 
-    std::vector<ALTIUM_VERTICE> vertices;
+    std::vector<ALTIUM_VERTICE>              outline;
+    std::vector<std::vector<ALTIUM_VERTICE>> holes;
 
     explicit AREGION6( ALTIUM_PARSER& aReader, bool aExtendedVertices );
 };

pcbnew/plugins/altium/altium_pcb.cpp

@@ -1432,18 +1432,18 @@ void ALTIUM_PCB::ParseShapeBasedRegions6Data( const CFB::CompoundFileReader& aRe
 
         if( elem.kind == ALTIUM_REGION_KIND::BOARD_CUTOUT )
         {
-            HelperCreateBoardOutline( elem.vertices );
+            HelperCreateBoardOutline( elem.outline );
         }
         else if( elem.kind == ALTIUM_REGION_KIND::POLYGON_CUTOUT || elem.is_keepout )
         {
             SHAPE_LINE_CHAIN linechain;
-            HelperShapeLineChainFromAltiumVertices( linechain, elem.vertices );
+            HelperShapeLineChainFromAltiumVertices( linechain, elem.outline );
 
             if( linechain.PointCount() < 2 )
             {
                 wxLogError( wxString::Format(
                         _( "ShapeBasedRegion has only %d point extracted from %ld vertices. At least 2 points are required." ),
-                        linechain.PointCount(), elem.vertices.size() ) );
+                        linechain.PointCount(), elem.outline.size() ) );
                 continue;
             }
 
@@ -1457,7 +1457,7 @@ void ALTIUM_PCB::ParseShapeBasedRegions6Data( const CFB::CompoundFileReader& aRe
             zone->SetDoNotAllowFootprints( false );
             zone->SetDoNotAllowCopperPour( true );
 
-            zone->SetPosition( elem.vertices.at( 0 ).position );
+            zone->SetPosition( elem.outline.at( 0 ).position );
             zone->Outline()->AddOutline( linechain );
 
             if( elem.layer == ALTIUM_LAYER::MULTI_LAYER )
@@ -1495,14 +1495,13 @@ void ALTIUM_PCB::ParseShapeBasedRegions6Data( const CFB::CompoundFileReader& aRe
                 }
 
                 SHAPE_LINE_CHAIN linechain;
-                HelperShapeLineChainFromAltiumVertices( linechain, elem.vertices );
+                HelperShapeLineChainFromAltiumVertices( linechain, elem.outline );
 
                 if( linechain.PointCount() < 2 )
                 {
                     wxLogError( wxString::Format( _( "Polygon has only %d point extracted from %ld "
                                                      "vertices. At least 2 points are required." ),
-                                                  linechain.PointCount(),
+                                                  linechain.PointCount(), elem.outline.size() ) );
-                                                  elem.vertices.size() ) );
                     continue;
                 }
 
@@ -1546,7 +1545,6 @@ void ALTIUM_PCB::ParseRegions6Data( const CFB::CompoundFileReader& aReader,
     {
         AREGION6 elem( reader, false );
 
-#if 0 // TODO: it seems this code has multiple issues right now, and we can manually fill anyways
         if( elem.subpolyindex != ALTIUM_POLYGON_NONE )
         {
             if( m_polygons.size() <= elem.subpolyindex )
@@ -1563,22 +1561,50 @@ void ALTIUM_PCB::ParseRegions6Data( const CFB::CompoundFileReader& aReader,
                 continue; // we know the zone id, but because we do not know the layer we did not add it!
             }
 
+            PCB_LAYER_ID klayer = GetKicadLayer( elem.layer );
+            if( klayer == UNDEFINED_LAYER )
+            {
+                continue; // Just skip it for now. Users cann fill it themself.
+            }
+
             SHAPE_LINE_CHAIN linechain;
-            for( auto& vertice : elem.vertices )
+            for( const ALTIUM_VERTICE& vertice : elem.outline )
             {
                 linechain.Append( vertice.position );
             }
-            linechain.Append( elem.vertices.at( 0 ).position );
+            linechain.Append( elem.outline.at( 0 ).position );
             linechain.SetClosed( true );
 
-            SHAPE_POLY_SET polyset;
+            SHAPE_POLY_SET rawPolys;
-            polyset.AddOutline( linechain );
+            rawPolys.AddOutline( linechain );
-            polyset.BooleanAdd( zone->GetFilledPolysList(), SHAPE_POLY_SET::POLYGON_MODE::PM_STRICTLY_SIMPLE );
 
-            zone->SetFilledPolysList( polyset );
+            for( const std::vector<ALTIUM_VERTICE>& hole : elem.holes )
-            zone->SetIsFilled( true );
+            {
+                SHAPE_LINE_CHAIN hole_linechain;
+                for( const ALTIUM_VERTICE& vertice : hole )
+                {
+                    hole_linechain.Append( vertice.position );
+                }
+                hole_linechain.Append( hole.at( 0 ).position );
+                hole_linechain.SetClosed( true );
+                rawPolys.AddHole( hole_linechain );
+            }
+
+            // The calculation -2 ensures we do not accidentially remove thermal spokes for now.
+            rawPolys.Deflate( zone->GetMinThickness() / 2 - 2, 32 );
+
+            if( zone->HasFilledPolysForLayer( klayer ) )
+                rawPolys.BooleanAdd( zone->RawPolysList( klayer ),
+                                     SHAPE_POLY_SET::PM_STRICTLY_SIMPLE );
+
+            SHAPE_POLY_SET finalPolys = rawPolys;
+            finalPolys.Fracture( SHAPE_POLY_SET::PM_STRICTLY_SIMPLE );
+
+            zone->SetRawPolysList( klayer, rawPolys );
+            zone->SetFilledPolysList( klayer, finalPolys );
+            zone->SetIsFilled( true );
+            zone->SetNeedRefill( false );
         }
-#endif
     }
 
     if( reader.GetRemainingBytes() != 0 )