From b31e97cfeda9b76a165854b98974d253e221e93b Mon Sep 17 00:00:00 2001
From: Ian McInerney <ian.s.mcinerney@ieee.org>
Date: Sun, 15 Nov 2020 21:28:27 +0000
Subject: [PATCH] Fix FP/PCB_SHAPE fill sexpr syntax to be backwards compatible
Previously the fill for shapes inside a footprint was not being
properly imported from older versions becase the default of no
flag changed from representing filled to representing no fill.
Introduce explicit filling information, and continue to treat no
flag on a polygon as meaning it is filled (which is the legacy
behavior).
Fixes https://gitlab.com/kicad/code/kicad/-/issues/6393
Fixes https://gitlab.com/kicad/code/kicad/-/issues/6390
---
common/pcb.keywords | 1 +
pcbnew/plugins/kicad/kicad_plugin.cpp | 10 ++++--
pcbnew/plugins/kicad/kicad_plugin.h | 2 +-
pcbnew/plugins/kicad/pcb_parser.cpp | 44 ++++++++++++++++++++++-----
4 files changed, 47 insertions(+), 10 deletions(-)
diff --git a/common/pcb.keywords b/common/pcb.keywords
index b3ee52a484..a597b5f516 100644
--- a/common/pcb.keywords
+++ b/common/pcb.keywords
@@ -245,6 +245,7 @@ solder_mask_min_width
solder_paste_margin
solder_paste_margin_ratio
solder_paste_ratio
+solid
stackup
start
status
diff --git a/pcbnew/plugins/kicad/kicad_plugin.cpp b/pcbnew/plugins/kicad/kicad_plugin.cpp
index 0fcf792c9d..9ca6eb455a 100644
--- a/pcbnew/plugins/kicad/kicad_plugin.cpp
+++ b/pcbnew/plugins/kicad/kicad_plugin.cpp
@@ -841,8 +841,11 @@ void PCB_IO::format( PCB_SHAPE* aShape, int aNestLevel ) const
m_out->Print( 0, " (width %s)", FormatInternalUnits( aShape->GetWidth() ).c_str() );
+ // The filled flag represents if a solid fill is present
if( aShape->IsFilled() )
- m_out->Print( 0, " (fill yes)" );
+ m_out->Print( 0, " (fill solid)" );
+ else
+ m_out->Print( 0, " (fill none)" );
m_out->Print( 0, " (tstamp %s)", TO_UTF8( aShape->m_Uuid.AsString() ) );
@@ -929,8 +932,11 @@ void PCB_IO::format( FP_SHAPE* aFPShape, int aNestLevel ) const
m_out->Print( 0, " (width %s)", FormatInternalUnits( aFPShape->GetWidth() ).c_str() );
+ // The filled flag represents if a solid fill is present
if( aFPShape->IsFilled() )
- m_out->Print( 0, " (fill yes)" );
+ m_out->Print( 0, " (fill solid)" );
+ else
+ m_out->Print( 0, " (fill none)" );
m_out->Print( 0, " (tstamp %s)", TO_UTF8( aFPShape->m_Uuid.AsString() ) );
diff --git a/pcbnew/plugins/kicad/kicad_plugin.h b/pcbnew/plugins/kicad/kicad_plugin.h
index 06bfd3b8c8..ca05511425 100644
--- a/pcbnew/plugins/kicad/kicad_plugin.h
+++ b/pcbnew/plugins/kicad/kicad_plugin.h
@@ -90,7 +90,7 @@ class PCB_TEXT;
//#define SEXPR_BOARD_FILE_VERSION 20200922 // Add user name to layer definition.
//#define SEXPR_BOARD_FILE_VERSION 20201002 // Add groups in footprints (for footprint editor).
//#define SEXPR_BOARD_FILE_VERSION 20201114 // Add first-class support for filled shapes.
-#define SEXPR_BOARD_FILE_VERSION 20201115 // module -> footprint.
+#define SEXPR_BOARD_FILE_VERSION 20201115 // module -> footprint and change fill syntax.
#define BOARD_FILE_HOST_VERSION 20200825 ///< Earlier files than this include the host tag
diff --git a/pcbnew/plugins/kicad/pcb_parser.cpp b/pcbnew/plugins/kicad/pcb_parser.cpp
index 20e4f1ecd1..ce7f49b044 100644
--- a/pcbnew/plugins/kicad/pcb_parser.cpp
+++ b/pcbnew/plugins/kicad/pcb_parser.cpp
@@ -2231,6 +2231,8 @@ PCB_SHAPE* PCB_PARSER::parsePCB_SHAPE()
Expecting( "gr_arc, gr_circle, gr_curve, gr_line, gr_poly, or gp_rect" );
}
+ bool foundFill = false;
+
for( token = NextTok(); token != T_RIGHT; token = NextTok() )
{
if( token != T_LEFT )
@@ -2262,6 +2264,8 @@ PCB_SHAPE* PCB_PARSER::parsePCB_SHAPE()
break;
case T_fill:
+ foundFill = true;
+
for( token = NextTok(); token != T_RIGHT; token = NextTok() )
{
if( token == T_LEFT )
@@ -2269,12 +2273,19 @@ PCB_SHAPE* PCB_PARSER::parsePCB_SHAPE()
switch( token )
{
+ // T_yes was used to indicate filling when first introduced,
+ // so treat it like a solid fill since that was the only fill available
case T_yes:
+ case T_solid:
shape->SetFilled( true );
break;
+ case T_none:
+ shape->SetFilled( false );
+ break;
+
default:
- Expecting( "yes" );
+ Expecting( "yes, none, solid" );
}
}
break;
@@ -2290,6 +2301,12 @@ PCB_SHAPE* PCB_PARSER::parsePCB_SHAPE()
}
}
+ // In legacy versions there was no option for polygon filling and all polygons
+ // were considered filled if they were on a layer othar than the edge cuts.
+ // So if there was no (fill ) section in the sexpr for a polygon, assume the polygon is filled.
+ if( !foundFill && ( shape->GetShape() == S_POLYGON ) && ( shape->GetLayer() != Edge_Cuts ) )
+ shape->SetFilled( true );
+
// Legacy versions didn't have a filled flag but allowed some shapes to indicate they
// should be filled by specifying a 0 stroke-width.
if( m_requiredVersion <= 20201002 )
@@ -3386,6 +3403,8 @@ FP_SHAPE* PCB_PARSER::parseFP_SHAPE()
Expecting( "fp_arc, fp_circle, fp_curve, fp_line, fp_poly, or fp_rect" );
}
+ bool foundFill = false;
+
for( token = NextTok(); token != T_RIGHT; token = NextTok() )
{
if( token != T_LEFT )
@@ -3412,6 +3431,8 @@ FP_SHAPE* PCB_PARSER::parseFP_SHAPE()
break;
case T_fill:
+ foundFill = true;
+
for( token = NextTok(); token != T_RIGHT; token = NextTok() )
{
if( token == T_LEFT )
@@ -3419,14 +3440,22 @@ FP_SHAPE* PCB_PARSER::parseFP_SHAPE()
switch( token )
{
+ // T_yes was used to indicate filling when first introduced,
+ // so treat it like a solid fill since that was the only fill available
case T_yes:
+ case T_solid:
shape->SetFilled( true );
break;
+ case T_none:
+ shape->SetFilled( false );
+ break;
+
default:
- Expecting( "yes" );
+ Expecting( "yes, none, solid" );
}
}
+
break;
/// We continue to parse the status field but it is no longer written
@@ -3440,6 +3469,12 @@ FP_SHAPE* PCB_PARSER::parseFP_SHAPE()
}
}
+ // In legacy versions there was no option for polygon filling and all polygons
+ // were considered filled if they were on a layer othar than the edge cuts.
+ // So if there was no (fill ) section in the sexpr for a polygon, assume the polygon is filled.
+ if( !foundFill && ( shape->GetShape() == S_POLYGON ) && ( shape->GetLayer() != Edge_Cuts ) )
+ shape->SetFilled( true );
+
if( m_requiredVersion <= 20201002 )
{
// Legacy versions didn't have a filled flag but allowed some shapes to indicate they
@@ -3449,11 +3484,6 @@ FP_SHAPE* PCB_PARSER::parseFP_SHAPE()
{
shape->SetFilled( true );
}
- // Polygons on non-Edge_Cuts layers were always filled
- else if( shape->GetShape() == S_POLYGON && shape->GetLayer() != Edge_Cuts )
- {
- shape->SetFilled( true );
- }
}
// Only filled shapes may have a zero line-width. This is not permitted in KiCad but some