From 120ab06db42abed9d82d6f5d4e7d3bb517d148fd Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Wed, 3 Apr 2019 11:29:17 -0400 Subject: [PATCH] Pcbnew: fix DXF import line width bug. There is a circular dependency between the GRAPHICS_IMPORTER object and the GRAPHICS_IMPORT_PLUGIN object which makes sharing the settings in the GRAPHICS_IMPORTER unwieldy. This fix is a ugly hack that resolves the issue. The underlying issue will require some major refactoring. Fixes lp:1821234 https://bugs.launchpad.net/kicad/+bug/1821234 --- pcbnew/import_gfx/dxf_import_plugin.cpp | 35 +++++++---- pcbnew/import_gfx/dxf_import_plugin.h | 68 +++++++++++++------- pcbnew/import_gfx/graphics_import_plugin.h | 29 ++++----- pcbnew/import_gfx/graphics_importer.h | 73 +++++++++++++--------- 4 files changed, 128 insertions(+), 77 deletions(-) diff --git a/pcbnew/import_gfx/dxf_import_plugin.cpp b/pcbnew/import_gfx/dxf_import_plugin.cpp index d1c8b2b24c..0fb7a3bfb4 100644 --- a/pcbnew/import_gfx/dxf_import_plugin.cpp +++ b/pcbnew/import_gfx/dxf_import_plugin.cpp @@ -24,7 +24,7 @@ // The DXF reader lib (libdxfrw) comes from dxflib project used in QCAD // See http://www.ribbonsoft.com -// Each time a dxf entity is read, a "call back" fuction is called +// Each time a dxf entity is read, a "call back" function is called // like void DXF_IMPORT_PLUGIN::addLine( const DL_LineData& data ) when a line is read. // this function just add the BOARD entity from dxf parameters (start and end point ...) @@ -101,7 +101,16 @@ double DXF_IMPORT_PLUGIN::GetImageHeight() const return m_maxY - m_minY; } -// coordinate conversions from dxf to internal units + +void DXF_IMPORT_PLUGIN::SetImporter( GRAPHICS_IMPORTER* aImporter ) +{ + GRAPHICS_IMPORT_PLUGIN::SetImporter( aImporter ); + + if( m_importer ) + SetDefaultLineWidthMM( m_importer->GetLineWidthMM() ); +} + + double DXF_IMPORT_PLUGIN::mapX( double aDxfCoordX ) { return SCALE_FACTOR( m_xOffset + ( aDxfCoordX * m_DXF2mm ) ); @@ -131,6 +140,7 @@ double DXF_IMPORT_PLUGIN::mapWidth( double aDxfWidth ) return SCALE_FACTOR( m_defaultThickness ); } + bool DXF_IMPORT_PLUGIN::ImportDxfFile( const wxString& aFile ) { DL_Dxf dxf_reader; @@ -178,9 +188,11 @@ void DXF_IMPORT_PLUGIN::addSpline( const DL_SplineData& aData ) void DXF_IMPORT_PLUGIN::addControlPoint( const DL_ControlPointData& aData ) { // Called for every spline control point, when reading a spline entity - m_curr_entity.m_SplineControlPointList.push_back( SPLINE_CTRL_POINT( aData.x , aData.y, aData.w ) ); + m_curr_entity.m_SplineControlPointList.push_back( SPLINE_CTRL_POINT( aData.x , aData.y, + aData.w ) ); } + void DXF_IMPORT_PLUGIN::addFitPoint( const DL_FitPointData& aData ) { // Called for every spline fit point, when reading a spline entity @@ -259,7 +271,8 @@ void DXF_IMPORT_PLUGIN::addVertex( const DL_VertexData& aData ) if( std::abs( m_curr_entity.m_BulgeVertex ) < MIN_BULGE ) insertLine( m_curr_entity.m_LastCoordinate, seg_end, lineWidth ); else - insertArc( m_curr_entity.m_LastCoordinate, seg_end, m_curr_entity.m_BulgeVertex, lineWidth ); + insertArc( m_curr_entity.m_LastCoordinate, seg_end, m_curr_entity.m_BulgeVertex, + lineWidth ); m_curr_entity.m_LastCoordinate = seg_end; m_curr_entity.m_BulgeVertex = vertex->bulge; @@ -277,7 +290,8 @@ void DXF_IMPORT_PLUGIN::endEntity() double lineWidth = mapWidth( attributes.getWidth() ); if( std::abs( m_curr_entity.m_BulgeVertex ) < MIN_BULGE ) - insertLine( m_curr_entity.m_LastCoordinate, m_curr_entity.m_PolylineStart, lineWidth ); + insertLine( m_curr_entity.m_LastCoordinate, m_curr_entity.m_PolylineStart, + lineWidth ); else insertArc( m_curr_entity.m_LastCoordinate, m_curr_entity.m_PolylineStart, m_curr_entity.m_BulgeVertex, lineWidth ); @@ -307,9 +321,6 @@ void DXF_IMPORT_PLUGIN::addCircle( const DL_CircleData& aData ) } -/* - * Import Arc entities. - */ void DXF_IMPORT_PLUGIN::addArc( const DL_ArcData& aData ) { // Init arc centre: @@ -496,7 +507,7 @@ void DXF_IMPORT_PLUGIN::addMText( const DL_MTextData& aData ) VECTOR2D topLeft(0.0, 0.0); VECTOR2D topRight(0.0, 0.0); - /* Some texts start by '\' and have formating chars (font name, font option...) + /* Some texts start by '\' and have formatting chars (font name, font option...) * ending with ';' * Here are some mtext formatting codes: * Format code Purpose @@ -510,7 +521,7 @@ void DXF_IMPORT_PLUGIN::addMText( const DL_MTextData& aData ) \\ \Hvaluex; Changes the text height to a multiple of the current text height \\ \S...^...; Stacks the subsequent text at the \, #, or ^ symbol \\ \Tvalue; Adjusts the space between characters, from.75 to 4 times - \\ \Qangle; Changes obliquing angle + \\ \Qangle; Changes oblique angle \\ \Wvalue; Changes width factor to produce wide text \\ \A Sets the alignment value; valid values: 0, 1, 2 (bottom, center, top) while( text.StartsWith( wxT("\\") ) ) */ @@ -575,7 +586,7 @@ void DXF_IMPORT_PLUGIN::addMText( const DL_MTextData& aData ) topLeft.x = -textWidth; } -#if 0 // These setting have no mening in Pcbnew +#if 0 // These setting have no meaning in Pcbnew if( data.alignH == 1 ) { // Text is left to right; @@ -968,7 +979,7 @@ void DXF_IMPORT_PLUGIN::insertSpline( int aWidth ) #if 0 // set to 1 to approximate the spline by segments between 2 control points VECTOR2D startpoint( mapX( m_curr_entity.m_SplineControlPointList[0].m_x ), - mapY( m_curr_entity.m_SplineControlPointList[0].m_y ) ); + mapY( m_curr_entity.m_SplineControlPointList[0].m_y ) ); for( unsigned int ii = 1; ii < imax; ++ii ) { diff --git a/pcbnew/import_gfx/dxf_import_plugin.h b/pcbnew/import_gfx/dxf_import_plugin.h index 05be1a071d..c13234b034 100644 --- a/pcbnew/import_gfx/dxf_import_plugin.h +++ b/pcbnew/import_gfx/dxf_import_plugin.h @@ -1,4 +1,4 @@ -/**************************************************************************** +/* * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2019 Jean-Pierre Charras, jp.charras at wanadoo.fr @@ -20,8 +20,7 @@ * or you may search the http://www.gnu.org website for the version 2 license, * or you may write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA -**********************************************************************/ - + */ #ifndef DXF2BRD_ITEMS_H #define DXF2BRD_ITEMS_H @@ -58,14 +57,15 @@ class DXF2BRD_ENTITY_DATA { public: int m_EntityType; // the DXF type of entity - int m_EntityParseStatus; // Inside a entity: status od parsing: + int m_EntityParseStatus; // Inside a entity: status of parsing: // 0 = no entity // 1 = first item of entity // 2 = entity in progress int m_EntityFlag; // a info flag to parse entities VECTOR2D m_LastCoordinate; // the last vertex coordinate read (unit = mm) - VECTOR2D m_PolylineStart; // The first point of the polyline entity, when reading a polyline (unit = mm) + VECTOR2D m_PolylineStart; // The first point of the polyline entity, when reading a + // polyline (unit = mm) double m_BulgeVertex; // the last vertex bulge value read // for spline parsing: parameters @@ -73,10 +73,10 @@ public: unsigned int m_SplineKnotsCount; unsigned int m_SplineControlCount; unsigned int m_SplineFitCount; - double m_SplineTangentStartX; // tangeant dir X for the start point - double m_SplineTangentStartY; // tangeant dir Y for the start point - double m_SplineTangentEndX; // tangeant dir X for the end point - double m_SplineTangentEndY; // tangeant dir Y for the end point + double m_SplineTangentStartX; // tangent dir X for the start point + double m_SplineTangentStartY; // tangent dir Y for the start point + double m_SplineTangentEndX; // tangent dir X for the end point + double m_SplineTangentEndY; // tangent dir Y for the end point // for spline parsing: buffers to store control points, fit points and knot std::vector m_SplineKnotsList; // knots list, code 40 @@ -159,6 +159,8 @@ public: void updateImageLimits( const VECTOR2D& aPoint ); + virtual void SetImporter( GRAPHICS_IMPORTER* aImporter ) override; + /** * Allows the import DXF items converted to board graphic items or footprint * graphic items. @@ -181,9 +183,10 @@ public: m_defaultThickness = aWidth; } + void SetLineWidthMM( double aWidth ) override { SetDefaultLineWidthMM( aWidth ); } + /** - * Set the coordinate offset between the imported dxf items - * and Pcbnew. + * Set the coordinate offset between the imported dxf items and Pcbnew. * because dxf files have the Y axis from bottom to top; * aOffsetX = 0, and aOffsetY = - vertical page size to import a full page * @param aOffsetX = the X offset in mm @@ -197,7 +200,7 @@ public: /** * Set the layer number to import dxf items. - * the layer should be a techicanl layer, not a copper layer + * the layer should be a technical layer, not a copper layer */ void SetBrdLayer( int aBrdLayer ) { m_brdLayer = aBrdLayer; } @@ -217,7 +220,6 @@ public: return m_messages; } - private: // report message to keep trace of not supported dxf entities: void reportMsg( const char* aMessage ); @@ -238,7 +240,7 @@ private: void insertSpline( int aWidth ); // Methods from DL_CreationAdapter: - // They are something like"call back" fonctions, + // They are something like"call back" functions, // called when the corresponding object is read in dxf file /** @@ -299,24 +301,46 @@ private: const DL_DimAngular3PData& ) override { reportMsg( "DL_Dimension not managed" ); } virtual void addDimOrdinate( const DL_DimensionData&, const DL_DimOrdinateData& ) override { reportMsg( "DL_Dimension not managed" ); } - virtual void addLeader( const DL_LeaderData& ) override { reportMsg( "DL_Leader not managed" ); } - virtual void addLeaderVertex( const DL_LeaderVertexData& ) override { reportMsg( "DL_LeaderVertex not managed" ); } + virtual void addLeader( const DL_LeaderData& ) override + { + reportMsg( "DL_Leader not managed" ); + } + + virtual void addLeaderVertex( const DL_LeaderVertexData& ) override + { + reportMsg( "DL_LeaderVertex not managed" ); + } virtual void addHatch( const DL_HatchData& ) override { reportMsg( "DL_Hatch not managed" ); } virtual void addTrace( const DL_TraceData& ) override { reportMsg( "DL_Trace not managed" ); } - virtual void add3dFace( const DL_3dFaceData& ) override { reportMsg( "DL_3dFace not managed" ); } + virtual void add3dFace( const DL_3dFaceData& ) override + { + reportMsg( "DL_3dFace not managed" ); + } + virtual void addSolid( const DL_SolidData& ) override { reportMsg( "DL_Solid not managed" ); } virtual void addImage( const DL_ImageData& ) override { reportMsg( "DL_ImageDa not managed" ); } - virtual void linkImage( const DL_ImageDefData& ) override { reportMsg( "DL_ImageDef not managed" ); } - virtual void addHatchLoop( const DL_HatchLoopData& ) override { reportMsg( "DL_HatchLoop not managed" ); } - virtual void addHatchEdge( const DL_HatchEdgeData& ) override { reportMsg( "DL_HatchEdge not managed" ); } + virtual void linkImage( const DL_ImageDefData& ) override + { + reportMsg( "DL_ImageDef not managed" ); + } + + virtual void addHatchLoop( const DL_HatchLoopData& ) override + { + reportMsg( "DL_HatchLoop not managed" ); + } + + virtual void addHatchEdge( const DL_HatchEdgeData& ) override + { + reportMsg( "DL_HatchEdge not managed" ); + } /** - * Converts a native unicode string into a DXF encoded string. + * Convert a native unicode string into a DXF encoded string. * - * DXF endoding includes the following special sequences: + * DXF encoding includes the following special sequences: * - %%%c for a diameter sign * - %%%d for a degree sign * - %%%p for a plus/minus sign diff --git a/pcbnew/import_gfx/graphics_import_plugin.h b/pcbnew/import_gfx/graphics_import_plugin.h index 529afa1039..523dead39d 100644 --- a/pcbnew/import_gfx/graphics_import_plugin.h +++ b/pcbnew/import_gfx/graphics_import_plugin.h @@ -2,6 +2,7 @@ * This program source code file is part of KICAD, a free EDA CAD application. * * Copyright (C) 2016 CERN + * Copyright (C) 2019 KiCad Developers, see AUTHORS.txt for contributors. * @author Maciej Suminski * * This program is free software; you can redistribute it and/or @@ -30,7 +31,7 @@ class GRAPHICS_IMPORTER; /** - * @brief Interface for vector graphics import plugins. + * Interface for vector graphics import plugins. */ class GRAPHICS_IMPORT_PLUGIN { @@ -40,27 +41,27 @@ public: } /** - * @brief Sets the receiver of the imported shapes. + * Set the receiver of the imported shapes. */ - void SetImporter( GRAPHICS_IMPORTER* aImporter ) + virtual void SetImporter( GRAPHICS_IMPORTER* aImporter ) { m_importer = aImporter; } /** - * @breif Returns the plugin name. + * Return the plugin name. * * This string will be used as the description in the file dialog. */ virtual const wxString GetName() const = 0; /** - * @brief Returns a string array of the file extensions handled by this plugin. + * Return a string array of the file extensions handled by this plugin. */ virtual const wxArrayString GetFileExtensions() const = 0; /** - * @brief Returns a list of wildcards that contains the file extensions + * Return a list of wildcards that contains the file extensions * handled by this plugin, separated with a coma. */ wxString GetWildcards() const @@ -82,41 +83,42 @@ public: } /** - * @brief Loads file for import. + * Load file for import. * * It is necessary to have the GRAPHICS_IMPORTER object set before. */ virtual bool Load( const wxString& aFileName ) = 0; /** - * @brief Return image height from original imported file. + * Return image height from original imported file. * * @return Original Image height in mm. */ virtual double GetImageHeight() const = 0; /** - * @brief Return image width from original imported file. + * Return image width from original imported file. * * @return Original Image width in mm. */ virtual double GetImageWidth() const = 0; /** - * @brief Actually imports the file. + * Actually imports the file. * * It is necessary to have loaded the file beforehand. */ virtual bool Import() = 0; + virtual void SetLineWidthMM( double aLineWidth ) {} + /** - * @brief collect warning and error messages after loading/importing. + * Collect warning and error messages after loading/importing. + * * @return the list of messages in one string. Each message ends by '\n' */ const virtual std::string& GetMessages() const = 0; - - protected: ///> Importer used to create objects representing the imported shapes. GRAPHICS_IMPORTER* m_importer; @@ -124,4 +126,3 @@ protected: #endif /* GRAPHICS_IMPORT_PLUGIN_H */ - diff --git a/pcbnew/import_gfx/graphics_importer.h b/pcbnew/import_gfx/graphics_importer.h index 4ae6bba834..308a69b3ad 100644 --- a/pcbnew/import_gfx/graphics_importer.h +++ b/pcbnew/import_gfx/graphics_importer.h @@ -2,6 +2,7 @@ * This program source code file is part of KICAD, a free EDA CAD application. * * Copyright (C) 2016 CERN + * Copyright (C) 2019 KiCad Developers, see AUTHORS.txt for contributors. * @author Maciej Suminski * * This program is free software; you can redistribute it and/or @@ -38,7 +39,7 @@ class EDA_ITEM; /** - * @brief Interface that creates objects representing shapes for a given data model. + * Interface that creates objects representing shapes for a given data model. */ class GRAPHICS_IMPORTER { @@ -50,22 +51,25 @@ public: } /** - * @brief Sets the import plugin used to obtain shapes from a file. + * Set the import plugin used to obtain shapes from a file. */ void SetPlugin( std::unique_ptr aPlugin ) { m_plugin = std::move( aPlugin ); + + if( m_plugin ) + m_plugin->SetImporter( this ); } /** - * @brief Load file and get its basic data + * Load file and get its basic data * */ bool Load( const wxString &aFileName ); /** - * @brief Imports shapes from loaded file. + * Import shapes from loaded file. * * It is important to have the file loaded before importing. * @@ -75,7 +79,8 @@ public: bool Import( double aScale = 1.0 ); /** - * @brief collect warning and error messages after loading/importing. + * Collect warning and error messages after loading/importing. + * * @return the list of messages in one string. Each message ends by '\n' */ const std::string& GetMessages() const @@ -83,9 +88,8 @@ public: return m_plugin->GetMessages(); } - /** - * @brief Get original image Wigth. + * Get original image Width. * * @return Width of the loaded image in mm. */ @@ -95,7 +99,7 @@ public: } /** - * @brief Get original image Height + * Get original image Height * * @return Height of the loaded image in mm. */ @@ -104,9 +108,8 @@ public: return m_originalHeight; } - /** - * @brief Sets the line width for the imported outlines (in mm). + * Sets the line width for the imported outlines (in mm). */ void SetLineWidthMM( double aWidth ) { @@ -114,21 +117,23 @@ public: } /** - * @brief Returns the line width used for importing the outlines (in mm). + * Returns the line width used for importing the outlines (in mm). */ double GetLineWidthMM() const { return m_lineWidth; } - /** @return the scale factor affecting the imported shapes. + /** + * @return the scale factor affecting the imported shapes. */ double GetScale() const { return m_scale; } - /** @return the offset to add to coordinates when importing graphic items. + /** + * @return the offset to add to coordinates when importing graphic items. * The offset is always in mm */ const VECTOR2D& GetImportOffsetMM() const @@ -136,7 +141,8 @@ public: return m_offsetCoordmm; } - /** Set the offset to add to coordinates when importing graphic items. + /** + * Set the offset to add to coordinates when importing graphic items. * The offset is always in mm */ void SetImportOffsetMM( const VECTOR2D& aOffset ) @@ -144,7 +150,8 @@ public: m_offsetCoordmm = aOffset; } - /** Set the scale factor affecting the imported shapes. + /** + * Set the scale factor affecting the imported shapes. * it allows conversion between imported shapes units and mm */ void SetScale( double aScale ) @@ -152,14 +159,14 @@ public: m_scale = aScale; } - /** @return the conversion factor from mm to internal unit + /** + * @return the conversion factor from mm to internal unit */ double GetMillimeterToIuFactor() { return m_millimeterToIu; } - /** * @return the overall scale factor to convert the imported shapes dimension to mm. */ @@ -169,7 +176,7 @@ public: } /** - * @breif Returns the list of objects representing the imported shapes. + * Return the list of objects representing the imported shapes. */ std::list>& GetItems() { @@ -182,7 +189,8 @@ public: // Methods to be implemented by derived graphics importers /** - * @brief Creates an object representing a line segment. + * Create an object representing a line segment. + * * @param aOrigin is the segment origin point expressed in mm. * @param aEnd is the segment end point expressed in mm. * @param aWidth is the segment thickness in mm. Use -1 for default line thickness @@ -190,7 +198,8 @@ public: virtual void AddLine( const VECTOR2D& aOrigin, const VECTOR2D& aEnd, double aWidth ) = 0; /** - * @brief Creates an object representing a circle. + * Create an object representing a circle. + * * @param aCenter is the circle center point expressed in mm. * @param aRadius is the circle radius expressed in mm. * @param aWidth is the segment thickness in mm. Use -1 for default line thickness @@ -198,19 +207,22 @@ public: virtual void AddCircle( const VECTOR2D& aCenter, double aRadius, double aWidth ) = 0; /** - * @brief Creates an object representing an arc. + * Create an object representing an arc. + * * @param aCenter is the arc center point expressed in mm. * @param aStart is the arc arm end point expressed in mm. * Its length is the arc radius. * @param aAngle is the arc angle expressed in degrees. * @param aWidth is the segment thickness in mm. Use -1 for default line thickness */ - virtual void AddArc( const VECTOR2D& aCenter, const VECTOR2D& aStart, double aAngle, double aWidth ) = 0; + virtual void AddArc( const VECTOR2D& aCenter, const VECTOR2D& aStart, double aAngle, + double aWidth ) = 0; virtual void AddPolygon( const std::vector< VECTOR2D >& aVertices, double aWidth ) = 0; /** - * @brief Creates an object representing a text. + * Create an object representing a text. + * * @param aOrigin is the text position. * @param aText is the displayed text. * @param aHeight is the text height expressed in mm. @@ -225,7 +237,8 @@ public: EDA_TEXT_HJUSTIFY_T aHJustify, EDA_TEXT_VJUSTIFY_T aVJustify ) = 0; /** - * @brief Creates an object representing an arc. + * Create an object representing an arc. + * * @param aStart is the curve start point expressed in mm. * @param aBezierControl1 is the first Bezier control point expressed in mm. * @param aBezierControl2 is the second Bezier control point expressed in mm. @@ -233,7 +246,8 @@ public: * @param aWidth is the segment thickness in mm. Use -1 for default line thickness */ virtual void AddSpline( const VECTOR2D& aStart, const VECTOR2D& aBezierControl1, - const VECTOR2D& aBezierControl2, const VECTOR2D& aEnd, double aWidth ) = 0; + const VECTOR2D& aBezierControl2, const VECTOR2D& aEnd, + double aWidth ) = 0; protected: ///> Adds an item to the imported shapes list. @@ -255,15 +269,16 @@ private: ///> Total image Height; double m_originalHeight; - ///> Default line thickness for the imported graphics - double m_lineWidth; - - /** Scale factor applied to the imported graphics. + /** + * Scale factor applied to the imported graphics. * 1.0 does not change the size of imported items * scale < 1.0 reduce the size of imported items */ double m_scale; + ///> Default line thickness for the imported graphics + double m_lineWidth; + protected: ///> factor to convert millimeters to Internal Units double m_millimeterToIu;