← Back to team overview

kicad-developers team mailing list archive

Re: Fwd: Re: [PATCH] Fix for 3D model offset

 

Rebuilt and attached

On Thu, Nov 30, 2017 at 7:53 PM, jp charras <jp.charras@xxxxxxxxxx> wrote:

>
> -------- Message transféré --------
> Sujet : Re: [Kicad-developers] [PATCH] Fix for 3D model offset
> Date : Thu, 30 Nov 2017 09:43:05 +0100
> De : jp charras <jp.charras@xxxxxxxxxx>
> Répondre à : KiCad Developers <kicad-developers@xxxxxxxxxxxxxxxxxxx>
> Pour : Oliver Walters <oliver.henry.walters@xxxxxxxxx>
>
> Le 30/11/2017 à 08:38, Oliver Walters a écrit :
> > JP, Wayne,
> >
> > Any update on how we want to handle this?
> >
> > On Mon, Nov 27, 2017 at 9:48 PM, Oliver Walters <
> oliver.henry.walters@xxxxxxxxx
> > <mailto:oliver.henry.walters@xxxxxxxxx>> wrote:
> >
>
> Hi Oliver,
> Could you rebase and rebuild yours patches: they do not apply on current
> master.
>
> Thanks.
>
>
> --
> Jean-Pierre CHARRAS
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
From a16bf42914e78ca69c51d309b62784d9730a0973 Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@xxxxxxxxx>
Date: Mon, 27 Nov 2017 21:46:53 +1100
Subject: [PATCH 3/3] Write "offset" only if non-zero

- Use "at" if all offset dimensions are zero
- Use "offset" otherwise
---
 pcbnew/kicad_plugin.cpp | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/pcbnew/kicad_plugin.cpp b/pcbnew/kicad_plugin.cpp
index 1314864..518e1c6 100644
--- a/pcbnew/kicad_plugin.cpp
+++ b/pcbnew/kicad_plugin.cpp
@@ -1143,9 +1143,22 @@ void PCB_IO::format( MODULE* aModule, int aNestLevel ) const
             /* Write 3D model offset in mm
              * 4.0.x wrote "at" which was actually in inches
              * 5.0.x onwards, 3D model offset is written using "offset"
+             *
+             * If the offset is all zero, write "at" (fewer file changes)
+             * Otherwise, write "offset"
              */
 
-            m_out->Print( aNestLevel+2, "(offset (xyz %s %s %s))\n",
+            wxString offsetTag = "offset";
+
+            if( bs3D->m_Offset.x == 0 &&
+                bs3D->m_Offset.y == 0 &&
+                bs3D->m_Offset.z == 0 )
+            {
+                offsetTag = "at";
+            }
+
+            m_out->Print( aNestLevel+2, "(%s (xyz %s %s %s))\n",
+                          offsetTag.ToStdString().c_str(),
                           Double2Str( bs3D->m_Offset.x ).c_str(),
                           Double2Str( bs3D->m_Offset.y ).c_str(),
                           Double2Str( bs3D->m_Offset.z ).c_str() );
-- 
2.7.4

From be3002200847e49f7670099a0acacc9c98673e7f Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@xxxxxxxxx>
Date: Mon, 27 Nov 2017 08:20:02 +1100
Subject: [PATCH 2/3] Decode "offset" in STEP export tool

- Read "at" as inches
- Read "offset" as mm
---
 pcbnew/kicad_plugin.cpp             |  2 ++
 utils/kicad2step/pcb/kicadmodel.cpp | 25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/pcbnew/kicad_plugin.cpp b/pcbnew/kicad_plugin.cpp
index f42dcff..1314864 100644
--- a/pcbnew/kicad_plugin.cpp
+++ b/pcbnew/kicad_plugin.cpp
@@ -1142,7 +1142,9 @@ void PCB_IO::format( MODULE* aModule, int aNestLevel ) const
 
             /* Write 3D model offset in mm
              * 4.0.x wrote "at" which was actually in inches
+             * 5.0.x onwards, 3D model offset is written using "offset"
              */
+
             m_out->Print( aNestLevel+2, "(offset (xyz %s %s %s))\n",
                           Double2Str( bs3D->m_Offset.x ).c_str(),
                           Double2Str( bs3D->m_Offset.y ).c_str(),
diff --git a/utils/kicad2step/pcb/kicadmodel.cpp b/utils/kicad2step/pcb/kicadmodel.cpp
index 6f680e0..61395ba 100644
--- a/utils/kicad2step/pcb/kicadmodel.cpp
+++ b/utils/kicad2step/pcb/kicadmodel.cpp
@@ -80,12 +80,37 @@ bool KICADMODEL::Read( SEXPR::SEXPR* aEntry )
         std::string name = child->GetChild( 0 )->GetSymbol();
         bool ret = true;
 
+        /*
+         * Version 4.x and prior used 'at' parameter,
+         * which was specified in inches.
+         */
         if( name == "at" )
+        {
             ret = Get3DCoordinate( child->GetChild( 1 ), m_offset );
+
+            if( ret )
+            {
+                m_offset.x *= 25.4f;
+                m_offset.y *= 25.4f;
+                m_offset.z *= 25.4f;
+            }
+        }
+        /*
+         * From 5.x onwards, 3D model is provided in 'offset',
+         * which is in millimetres
+         */
+        else if( name == "offset" )
+        {
+            ret = Get3DCoordinate( child->GetChild( 1 ), m_offset );
+        }
         else if( name == "scale" )
+        {
             ret = Get3DCoordinate( child->GetChild( 1 ), m_scale );
+        }
         else if( name == "rotate" )
+        {
             ret = GetXYZRotation( child->GetChild( 1 ), m_rotation );
+        }
 
         if( !ret )
             return false;
-- 
2.7.4

From 36a7f04524689ec52ba076af6a34971b9ad9720e Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@xxxxxxxxx>
Date: Thu, 23 Nov 2017 21:21:48 +1100
Subject: [PATCH 1/3] CHANGE file format for 3D model offset

- Reverts behaviour of 3e71ed2421f5df433c11a554e0756f6898e25b06
- 3D model offset now explicitly written in mm, using "offset" tag
- Any read "at" tag is converted from inches to mm
- This will not break any 3D model offset when reading files
- It will however render files incompatible with old versions
---
 pcbnew/kicad_plugin.cpp |  6 ++++--
 pcbnew/kicad_plugin.h   |  3 ++-
 pcbnew/pcb_parser.cpp   | 35 ++++++++++++++++++++---------------
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/pcbnew/kicad_plugin.cpp b/pcbnew/kicad_plugin.cpp
index 61a1cd0..f42dcff 100644
--- a/pcbnew/kicad_plugin.cpp
+++ b/pcbnew/kicad_plugin.cpp
@@ -1140,8 +1140,10 @@ void PCB_IO::format( MODULE* aModule, int aNestLevel ) const
             m_out->Print( aNestLevel+1, "(model %s\n",
                           m_out->Quotew( bs3D->m_Filename ).c_str() );
 
-            // Model offset is in mm
-            m_out->Print( aNestLevel+2, "(at (xyz %s %s %s))\n",
+            /* Write 3D model offset in mm
+             * 4.0.x wrote "at" which was actually in inches
+             */
+            m_out->Print( aNestLevel+2, "(offset (xyz %s %s %s))\n",
                           Double2Str( bs3D->m_Offset.x ).c_str(),
                           Double2Str( bs3D->m_Offset.y ).c_str(),
                           Double2Str( bs3D->m_Offset.z ).c_str() );
diff --git a/pcbnew/kicad_plugin.h b/pcbnew/kicad_plugin.h
index 8cbd10d..0c0792a 100644
--- a/pcbnew/kicad_plugin.h
+++ b/pcbnew/kicad_plugin.h
@@ -46,7 +46,8 @@ class NETINFO_MAPPING;
 //#define SEXPR_BOARD_FILE_VERSION    20170920  // long pad names and custom pad shape
 //#define SEXPR_BOARD_FILE_VERSION    20170922  // Keepout zones can exist on multiple layers
 //#define SEXPR_BOARD_FILE_VERSION    20171114  // Save 3D model offset in mm, instead of inches
-#define SEXPR_BOARD_FILE_VERSION      20171125  // Locked/unlocked TEXTE_MODULE
+//#define SEXPR_BOARD_FILE_VERSION    20171125  // Locked/unlocked TEXTE_MODULE
+#define SEXPR_BOARD_FILE_VERSION      20171130  // 3D model offset written using "offset" parameter
 
 #define CTL_STD_LAYER_NAMES         (1 << 0)    ///< Use English Standard layer names
 #define CTL_OMIT_NETS               (1 << 1)    ///< Omit pads net names (useless in library)
diff --git a/pcbnew/pcb_parser.cpp b/pcbnew/pcb_parser.cpp
index cea3766..4d9faa8 100644
--- a/pcbnew/pcb_parser.cpp
+++ b/pcbnew/pcb_parser.cpp
@@ -360,25 +360,30 @@ MODULE_3D_SETTINGS* PCB_PARSER::parse3DModel()
                 Expecting( T_xyz );
 
             /* Note:
-             * Prior to SEXPR_BOARD_FILE_VERSION 20171114,
-             * 3D model offset was read and written in inches
-             *
-             * Now, the offset is explicitly written in mm.
-             * If a board is read with an older version,
-             * the offset is converted from inches to mm
+             * Prior to KiCad v5, model offset was designated by "at",
+             * and the units were in inches.
+             * Now we use mm, but support reading of legacy files
              */
 
+            n3D->m_Offset.x = parseDouble( "x value" ) * 25.4f;
+            n3D->m_Offset.y = parseDouble( "y value" ) * 25.4f;
+            n3D->m_Offset.z = parseDouble( "z value" ) * 25.4f;
+            NeedRIGHT();
+            break;
+
+        case T_offset:
+            NeedLEFT();
+            token = NextTok();
+
+            if( token != T_xyz )
+                Expecting( T_xyz );
+
+            /*
+             * 3D model offset is in mm
+             */
             n3D->m_Offset.x = parseDouble( "x value" );
             n3D->m_Offset.y = parseDouble( "y value" );
             n3D->m_Offset.z = parseDouble( "z value" );
-
-            if(m_requiredVersion < 20171114L)
-            {
-                n3D->m_Offset.x *= 25.4f;
-                n3D->m_Offset.y *= 25.4f;
-                n3D->m_Offset.z *= 25.4f;
-            }
-
             NeedRIGHT();
             break;
 
@@ -409,7 +414,7 @@ MODULE_3D_SETTINGS* PCB_PARSER::parse3DModel()
             break;
 
         default:
-            Expecting( "at, scale, or rotate" );
+            Expecting( "at, offset, scale, or rotate" );
         }
 
         NeedRIGHT();
-- 
2.7.4


Follow ups

References