From ba517db96c9bd7e5c1a8a409b18c19b2d2765676 Mon Sep 17 00:00:00 2001
From: jean-pierre charras <jp.charras@wanadoo.fr>
Date: Sat, 8 Apr 2017 12:53:40 +0200
Subject: [PATCH] Gerbview: fix a subtle issue when reading parameter values in
Gerber files.
In Gerber files the char 'X' is used as separator.
But when reading parameter values, the sequence "0xnnn" is a number in hexadecimal format, and the 'X' char is not seen as separator by usual strtod or strtol C functions.
This is now fixed.
---
gerbview/class_am_param.cpp | 5 ++++
gerbview/class_am_param.h | 6 +++++
gerbview/class_aperture_macro.cpp | 7 ++++-
.../aperture_macro-with_param-test.gbr | 1 +
gerbview/readgerb.cpp | 1 +
gerbview/rs274_read_XY_and_IJ_coordinates.cpp | 26 +++++++++++++++++--
gerbview/rs274x.cpp | 26 ++++++++++++++-----
7 files changed, 63 insertions(+), 9 deletions(-)
diff --git a/gerbview/class_am_param.cpp b/gerbview/class_am_param.cpp
index 63306a4251..d7d9469a85 100644
--- a/gerbview/class_am_param.cpp
+++ b/gerbview/class_am_param.cpp
@@ -75,6 +75,7 @@ double AM_PARAM::GetValue( const D_CODE* aDcode ) const
for( unsigned ii = 0; ii < m_paramStack.size(); ii++ )
{
AM_PARAM_ITEM item = m_paramStack[ii];
+
switch( item.GetType() )
{
case ADD:
@@ -89,7 +90,9 @@ double AM_PARAM::GetValue( const D_CODE* aDcode ) const
if( aDcode ) // should be always true here
{
if( item.GetIndex() <= aDcode->GetParamCount() )
+ {
curr_value = aDcode->GetParam( item.GetIndex() );
+ }
else // Get parameter from local param definition
{
const APERTURE_MACRO * am_parent = aDcode->GetMacro();
@@ -104,6 +107,7 @@ double AM_PARAM::GetValue( const D_CODE* aDcode ) const
case PUSHVALUE: // a value is on the stack:
if( item.GetType() == PUSHVALUE )
curr_value = item.GetValue();
+
switch( state )
{
case POPVALUE:
@@ -137,6 +141,7 @@ double AM_PARAM::GetValue( const D_CODE* aDcode ) const
break;
}
}
+
return paramvalue;
}
diff --git a/gerbview/class_am_param.h b/gerbview/class_am_param.h
index 7ce2c8416e..5be6a0d935 100644
--- a/gerbview/class_am_param.h
+++ b/gerbview/class_am_param.h
@@ -162,6 +162,7 @@ public:
m_dvalue = aValue;
m_ivalue = 0;
}
+
AM_PARAM_ITEM( parm_item_type aType, int aValue )
{
m_type = aType;
@@ -173,18 +174,22 @@ public:
{
m_dvalue = aValue;
}
+
double GetValue( ) const
{
return m_dvalue;
}
+
parm_item_type GetType() const
{
return m_type;
}
+
unsigned GetIndex() const
{
return (unsigned) m_ivalue;
}
+
bool IsOperator() const
{
return m_type == ADD || m_type == SUB || m_type == MUL || m_type == DIV;
@@ -193,6 +198,7 @@ public:
{
return m_type == PUSHVALUE || m_type == PUSHPARM;
}
+
bool IsDefered() const
{
return m_type == PUSHPARM;
diff --git a/gerbview/class_aperture_macro.cpp b/gerbview/class_aperture_macro.cpp
index 41fb469e2a..3bc52fd4fe 100644
--- a/gerbview/class_aperture_macro.cpp
+++ b/gerbview/class_aperture_macro.cpp
@@ -143,6 +143,7 @@ void AM_PRIMITIVE::DrawBasicShape( GERBER_DRAW_ITEM* aParent,
// shape rotation:
rotation = params[6].GetValue( tool ) * 10.0;
+
if( rotation != 0)
{
for( unsigned ii = 0; ii < polybuffer.size(); ii++ )
@@ -454,7 +455,7 @@ void AM_PRIMITIVE::ConvertShapeToPolygon( GERBER_DRAW_ITEM* aParent,
aBuffer[ii] += start;
}
}
- break;
+ break;
case AMP_LINE_CENTER:
{
@@ -793,6 +794,7 @@ double APERTURE_MACRO::GetLocalParam( const D_CODE* aDcode, unsigned aParamId )
{
// find parameter descr.
const AM_PARAM * param = NULL;
+
for( unsigned ii = 0; ii < m_localparamStack.size(); ii ++ )
{
if( m_localparamStack[ii].GetIndex() == aParamId )
@@ -801,9 +803,12 @@ double APERTURE_MACRO::GetLocalParam( const D_CODE* aDcode, unsigned aParamId )
break;
}
}
+
if ( param == NULL ) // not found
return 0.0;
+
// Evaluate parameter
double value = param->GetValue( aDcode );
+
return value;
}
diff --git a/gerbview/gerber_test_files/aperture_macro-with_param-test.gbr b/gerbview/gerber_test_files/aperture_macro-with_param-test.gbr
index 774d97a46c..42acb66155 100644
--- a/gerbview/gerber_test_files/aperture_macro-with_param-test.gbr
+++ b/gerbview/gerber_test_files/aperture_macro-with_param-test.gbr
@@ -5,6 +5,7 @@ G04 Handcoded by Stefan Petersen *
%OFA0.0000B0.0000*%
G90*
%AMCIRCLE*
+0 this is a comment*
1,1,$1,0,0*
%
%AMVECTOR*
diff --git a/gerbview/readgerb.cpp b/gerbview/readgerb.cpp
index d9b2493b04..1edf17b384 100644
--- a/gerbview/readgerb.cpp
+++ b/gerbview/readgerb.cpp
@@ -173,6 +173,7 @@ bool GERBER_FILE_IMAGE::LoadGerberFile( const wxString& aFullFileName )
case 'I':
case 'J': /* Auxiliary Move command */
m_IJPos = ReadIJCoord( text );
+
if( *text == '*' ) // command like X35142Y15945J504*
{
Execute_DCODE_Command( text,
diff --git a/gerbview/rs274_read_XY_and_IJ_coordinates.cpp b/gerbview/rs274_read_XY_and_IJ_coordinates.cpp
index e5e197e79d..089f36a2e8 100644
--- a/gerbview/rs274_read_XY_and_IJ_coordinates.cpp
+++ b/gerbview/rs274_read_XY_and_IJ_coordinates.cpp
@@ -267,7 +267,18 @@ wxPoint GERBER_FILE_IMAGE::ReadIJCoord( char*& Text )
*/
int ReadInt( char*& text, bool aSkipSeparator = true )
{
- int ret = (int) strtol( text, &text, 10 );
+ int ret;
+
+ // For strtol, a string starting by 0X or 0x is a valid number in hexadecimal or octal.
+ // However, 'X' is a separator in Gerber strings with numbers.
+ // We need to detect that
+ if( strnicmp( text, "0X", 2 ) == 0 )
+ {
+ text++;
+ ret = 0;
+ }
+ else
+ ret = (int) strtol( text, &text, 10 );
if( *text == ',' || isspace( *text ) )
{
@@ -290,7 +301,18 @@ int ReadInt( char*& text, bool aSkipSeparator = true )
*/
double ReadDouble( char*& text, bool aSkipSeparator = true )
{
- double ret = strtod( text, &text );
+ double ret;
+
+ // For strtod, a string starting by 0X or 0x is a valid number in hexadecimal or octal.
+ // However, 'X' is a separator in Gerber strings with numbers.
+ // We need to detect that
+ if( strnicmp( text, "0X", 2 ) == 0 )
+ {
+ text++;
+ ret = 0.0;
+ }
+ else
+ ret = strtod( text, &text );
if( *text == ',' || isspace( *text ) )
{
diff --git a/gerbview/rs274x.cpp b/gerbview/rs274x.cpp
index e7e3a20d07..2617383430 100644
--- a/gerbview/rs274x.cpp
+++ b/gerbview/rs274x.cpp
@@ -733,7 +733,6 @@ bool GERBER_FILE_IMAGE::ExecuteRS274XCommand( int command, char* buff, char*& te
break;
case AP_DEFINITION:
-
/* input example: %ADD30R,0.081800X0.101500*%
* Aperture definition has 4 options: C, R, O, P
* (Circle, Rect, Oval, regular Polygon)
@@ -898,18 +897,24 @@ bool GERBER_FILE_IMAGE::ExecuteRS274XCommand( int command, char* buff, char*& te
if( *text == ',' )
{ // Read aperture macro parameters and store them
text++; // text points the first parameter
+
while( *text && *text != '*' )
{
double param = ReadDouble( text );
dcode->AppendParam( param );
- while( isspace( *text ) ) text++;
- if( *text == 'X' )
- ++text;
+
+ while( isspace( *text ) )
+ text++;
+
+ // Skip 'X' separator:
+ if( *text == 'X' || *text == 'x' )
+ text++;
}
}
// lookup the aperture macro here.
APERTURE_MACRO* pam = FindApertureMacro( am_lookup );
+
if( !pam )
{
msg.Printf( wxT( "RS274X: aperture macro %s not found\n" ),
@@ -922,6 +927,7 @@ bool GERBER_FILE_IMAGE::ExecuteRS274XCommand( int command, char* buff, char*& te
dcode->m_Shape = APT_MACRO;
dcode->SetMacro( pam );
}
+
break;
default:
@@ -1024,7 +1030,8 @@ bool GERBER_FILE_IMAGE::ReadApertureMacro( char *buff,
if( *text == '*' )
++text;
- text = GetNextLine( buff, text, gerber_file ); // Get next line
+ text = GetNextLine( buff, text, gerber_file );
+
if( text == NULL ) // End of File
return false;
@@ -1034,6 +1041,7 @@ bool GERBER_FILE_IMAGE::ReadApertureMacro( char *buff,
// last line is % or *% sometime found.
if( *text == '*' )
++text;
+
if( *text == '%' )
break; // exit with text still pointing at %
@@ -1063,12 +1071,15 @@ bool GERBER_FILE_IMAGE::ReadApertureMacro( char *buff,
else
primitive_type = ReadInt( text );
+ bool is_comment = false;
+
switch( primitive_type )
{
case AMP_COMMENT: // lines starting by 0 are a comment
paramCount = 0;
+ is_comment = true;
// Skip comment
- while( *text && (*text != '*') )
+ while( *text && ( *text != '*' ) )
text++;
break;
@@ -1114,6 +1125,9 @@ bool GERBER_FILE_IMAGE::ReadApertureMacro( char *buff,
return false;
}
+ if( is_comment )
+ continue;
+
AM_PRIMITIVE prim( m_GerbMetric );
prim.primitive_id = (AM_PRIMITIVE_ID) primitive_type;
int ii;