diff --git a/common/jobs/job_export_pcb_step.h b/common/jobs/job_export_pcb_step.h index 8ce7bd9037..99e8a583ae 100644 --- a/common/jobs/job_export_pcb_step.h +++ b/common/jobs/job_export_pcb_step.h @@ -39,7 +39,7 @@ public: m_outputFile(), m_xOrigin( 0.0 ), m_yOrigin( 0.0 ), - m_minDistance( 0.01 ) // 0.01 mm is a good value to connect 2 items of the board outlines + m_boardOutlinesChainingEpsilon( 0.01 ) // 0.01 mm is a good value to connect 2 items of the board outlines { } @@ -53,7 +53,7 @@ public: wxString m_outputFile; double m_xOrigin; double m_yOrigin; - double m_minDistance; + double m_boardOutlinesChainingEpsilon; }; #endif \ No newline at end of file diff --git a/kicad/cli/command_export_pcb_step.cpp b/kicad/cli/command_export_pcb_step.cpp index 430c8bdd22..f64182e330 100644 --- a/kicad/cli/command_export_pcb_step.cpp +++ b/kicad/cli/command_export_pcb_step.cpp @@ -154,7 +154,7 @@ int CLI::EXPORT_PCB_STEP_COMMAND::doPerform( KIWAY& aKiway ) std::smatch sm; std::string str( minDistance.ToUTF8() ); std::regex_search( str, sm, re_pattern ); - step->m_minDistance = atof( sm.str( 1 ).c_str() ); + step->m_boardOutlinesChainingEpsilon = atof( sm.str( 1 ).c_str() ); std::string tunit( sm[2] ); @@ -162,7 +162,7 @@ int CLI::EXPORT_PCB_STEP_COMMAND::doPerform( KIWAY& aKiway ) { if( !tunit.compare( "in" ) || !tunit.compare( "inch" ) ) { - step->m_minDistance *= 25.4; + step->m_boardOutlinesChainingEpsilon *= 25.4; } else if( tunit.compare( "mm" ) ) { diff --git a/pcbnew/exporters/step/exporter_step.cpp b/pcbnew/exporters/step/exporter_step.cpp index 534c7f2125..3ee841083f 100644 --- a/pcbnew/exporters/step/exporter_step.cpp +++ b/pcbnew/exporters/step/exporter_step.cpp @@ -115,8 +115,7 @@ EXPORTER_STEP::EXPORTER_STEP( BOARD* aBoard, const EXPORTER_STEP_PARAMS& aParams m_hasGridOrigin( false ), m_board( aBoard ), m_pcbModel( nullptr ), - m_minDistance( STEPEXPORT_MIN_DISTANCE ), - m_boardThickness( DEFAULT_BOARD_THICKNESS ) + m_boardThickness( DEFAULT_BOARD_THICKNESS_MM ) { m_solderMaskColor = COLOR4D( 0.08, 0.20, 0.14, 0.83 ); @@ -270,9 +269,14 @@ bool EXPORTER_STEP::composePCB() m_pcbModel->SetPCBThickness( m_boardThickness ); - // Set the min distance betewenn 2 points for OCC to see these 2 points as merged - double minDistmm = std::max( m_params.m_minDistance, STEPEXPORT_MIN_ACCEPTABLE_DISTANCE ); - m_pcbModel->SetMinDistance( minDistmm ); + // Note: m_params.m_BoardOutlinesChainingEpsilon is used only to build the board outlines, + // not to set OCC chaining epsilon (much smaller) + // + // Set the min distance between 2 points for OCC to see these 2 points as merged + // OCC_MAX_DISTANCE_TO_MERGE_POINTS is acceptable for OCC, otherwise there are issues + // to handle the shapes chaining on copper layers, because the Z dist is 0.035 mm and the + // min dist must be much smaller (we use 0.001 mm giving good results) + m_pcbModel->OCCSetMergeMaxDistance( OCC_MAX_DISTANCE_TO_MERGE_POINTS ); m_pcbModel->SetMaxError( m_board->GetDesignSettings().m_MaxError ); diff --git a/pcbnew/exporters/step/exporter_step.h b/pcbnew/exporters/step/exporter_step.h index b46cd4b490..2d95786cb5 100644 --- a/pcbnew/exporters/step/exporter_step.h +++ b/pcbnew/exporters/step/exporter_step.h @@ -30,6 +30,10 @@ #include #include +// Default value to chain 2 shapes when creating the board outlines +// from shapes on Edges.Cut layer +#define BOARD_DEFAULT_CHAINING_EPSILON 0.01 + class PCBMODEL; class BOARD; class FOOTPRINT; @@ -45,7 +49,7 @@ public: m_useDrillOrigin( false ), m_includeExcludedBom( true ), m_substModels( true ), - m_minDistance( STEPEXPORT_MIN_DISTANCE ), + m_boardOutlinesChainingEpsilon( BOARD_DEFAULT_CHAINING_EPSILON ), m_boardOnly( false ) {}; wxString m_outputFile; @@ -57,7 +61,7 @@ public: bool m_useDrillOrigin; bool m_includeExcludedBom; bool m_substModels; - double m_minDistance; + double m_boardOutlinesChainingEpsilon; bool m_boardOnly; }; diff --git a/pcbnew/exporters/step/step_pcb_model.cpp b/pcbnew/exporters/step/step_pcb_model.cpp index cec2b37e81..6dea0be1f3 100644 --- a/pcbnew/exporters/step/step_pcb_model.cpp +++ b/pcbnew/exporters/step/step_pcb_model.cpp @@ -94,19 +94,9 @@ static constexpr double USER_PREC = 1e-4; static constexpr double USER_ANGLE_PREC = 1e-6; -// minimum PCB thickness in mm (2 microns assumes a very thin polyimide film) -static constexpr double THICKNESS_MIN = 0.002; - -// default PCB thickness in mm -static constexpr double THICKNESS_DEFAULT = 1.6; - // nominal offset from the board static constexpr double BOARD_OFFSET = 0.05; -// min. length**2 below which 2 points are considered coincident -static constexpr double MIN_LENGTH2 = STEPEXPORT_MIN_DISTANCE * STEPEXPORT_MIN_DISTANCE; - - // supported file types enum FormatType { @@ -195,12 +185,12 @@ STEP_PCB_MODEL::STEP_PCB_MODEL( const wxString& aPcbName ) m_components = 0; m_precision = USER_PREC; m_angleprec = USER_ANGLE_PREC; - m_thickness = THICKNESS_DEFAULT; - m_minDistance2 = MIN_LENGTH2; + m_boardThickness = BOARD_THICKNESS_DEFAULT_MM; + m_mergeOCCMaxDist = OCC_MAX_DISTANCE_TO_MERGE_POINTS; m_minx = 1.0e10; // absurdly large number; any valid PCB X value will be smaller m_pcbName = aPcbName; - BRepBuilderAPI::Precision( STEPEXPORT_MIN_DISTANCE ); - m_maxError = 5000; // 5 microns + BRepBuilderAPI::Precision( m_mergeOCCMaxDist ); + m_maxError = pcbIUScale.mmToIU( ARC_TO_SEGMENT_MAX_ERROR_MM ); } @@ -215,16 +205,21 @@ bool STEP_PCB_MODEL::AddPadHole( const PAD* aPad, const VECTOR2D& aOrigin ) if( NULL == aPad || !aPad->GetDrillSize().x ) return false; - VECTOR2I pos = aPad->GetPosition(); + VECTOR2I pos = aPad->GetPosition(); + const double margin = 0.01; // a small margin on the Z axix to be sure the hole + // is bigget than the board with copper + // must be > OCC_MAX_DISTANCE_TO_MERGE_POINTS + double holeZsize = m_boardThickness + ( margin * 2 ); if( aPad->GetDrillShape() == PAD_DRILL_SHAPE_CIRCLE ) { TopoDS_Shape s = - BRepPrimAPI_MakeCylinder( pcbIUScale.IUTomm( aPad->GetDrillSize().x ) * 0.5, m_thickness * 2.0 ).Shape(); + BRepPrimAPI_MakeCylinder( pcbIUScale.IUTomm( aPad->GetDrillSize().x ) * 0.5, holeZsize ) + .Shape(); gp_Trsf shift; shift.SetTranslation( gp_Vec( pcbIUScale.IUTomm( pos.x - aOrigin.x ), -pcbIUScale.IUTomm( pos.y - aOrigin.y ), - -m_thickness * 0.5 ) ); + -m_boardThickness * 0.5 ) ); BRepBuilderAPI_Transform hole( s, shift ); m_cutouts.push_back( hole.Shape() ); return true; @@ -241,7 +236,7 @@ bool STEP_PCB_MODEL::AddPadHole( const PAD* aPad, const VECTOR2D& aOrigin ) if( holeOutlines.OutlineCount() > 0 ) { - if( MakeShape( hole, holeOutlines.COutline( 0 ), m_thickness, 0.0, aOrigin ) ) + if( MakeShape( hole, holeOutlines.COutline( 0 ), holeZsize, 0.0, aOrigin ) ) { m_cutouts.push_back( hole ); } @@ -313,11 +308,11 @@ bool STEP_PCB_MODEL::AddComponent( const std::string& aFileNameUTF8, const std:: void STEP_PCB_MODEL::SetPCBThickness( double aThickness ) { if( aThickness < 0.0 ) - m_thickness = THICKNESS_DEFAULT; - else if( aThickness < THICKNESS_MIN ) - m_thickness = THICKNESS_MIN; + m_boardThickness = BOARD_THICKNESS_DEFAULT_MM; + else if( aThickness < BOARD_THICKNESS_MIN_MM ) + m_boardThickness = BOARD_THICKNESS_MIN_MM; else - m_thickness = aThickness; + m_boardThickness = aThickness; } @@ -329,21 +324,20 @@ void STEP_PCB_MODEL::SetBoardColor( double r, double g, double b ) } -void STEP_PCB_MODEL::SetMinDistance( double aDistance ) +void STEP_PCB_MODEL::OCCSetMergeMaxDistance( double aDistance ) { // Ensure a minimal value (in mm) - aDistance = std::max( aDistance, STEPEXPORT_MIN_ACCEPTABLE_DISTANCE ); - - // m_minDistance2 keeps a squared distance value - m_minDistance2 = aDistance * aDistance; - BRepBuilderAPI::Precision( aDistance ); + m_mergeOCCMaxDist = aDistance; + BRepBuilderAPI::Precision( m_mergeOCCMaxDist ); } + bool STEP_PCB_MODEL::isBoardOutlineValid() { return m_pcb_labels.size() > 0; } + // A helper function to know if a SHAPE_LINE_CHAIN is encoding a circle static bool IsChainCircle( const SHAPE_LINE_CHAIN& aChain ) { @@ -434,9 +428,8 @@ bool STEP_PCB_MODEL::MakeShape( TopoDS_Shape& aShape, const SHAPE_LINE_CHAIN& aC // Do not export very small segments: they can create broken outlines double seg_len = std::abs( end.X() - start.X()) + std::abs(start.Y() - end.Y() ); - double min_len = 0.0001; // In mm, i.e. 0.1 micron - if( seg_len < min_len ) + if( seg_len <= m_mergeOCCMaxDist ) continue; try @@ -515,7 +508,7 @@ bool STEP_PCB_MODEL::CreatePCB( SHAPE_POLY_SET& aOutline, VECTOR2D aOrigin ) TopoDS_Shape curr_brd; - if( !MakeShape( curr_brd, outline, m_thickness, 0.0, aOrigin ) ) + if( !MakeShape( curr_brd, outline, m_boardThickness, 0.0, aOrigin ) ) { // Error ReportMessage( wxString::Format( @@ -537,7 +530,7 @@ bool STEP_PCB_MODEL::CreatePCB( SHAPE_POLY_SET& aOutline, VECTOR2D aOrigin ) const SHAPE_LINE_CHAIN& holeOutline = aOutline.Hole( cnt, ii ); TopoDS_Shape hole; - if( MakeShape( hole, holeOutline, m_thickness, 0.0, aOrigin ) ) + if( MakeShape( hole, holeOutline, m_boardThickness, 0.0, aOrigin ) ) { m_cutouts.push_back( hole ); } @@ -1006,7 +999,7 @@ bool STEP_PCB_MODEL::getModelLocation( bool aBottom, VECTOR2D aPosition, double } else { - aOffset.z += m_thickness; + aOffset.z += m_boardThickness; lRot.SetRotation( gp_Ax1( gp_Pnt( 0.0, 0.0, 0.0 ), gp_Dir( 0.0, 0.0, 1.0 ) ), aRotation ); lPos.Multiply( lRot ); } diff --git a/pcbnew/exporters/step/step_pcb_model.h b/pcbnew/exporters/step/step_pcb_model.h index 1f2834de3d..b0d9e2db78 100644 --- a/pcbnew/exporters/step/step_pcb_model.h +++ b/pcbnew/exporters/step/step_pcb_model.h @@ -42,9 +42,24 @@ #include #include -///< Default minimum distance between points to treat them as separate ones (mm) -static constexpr double STEPEXPORT_MIN_DISTANCE = 0.01; -static constexpr double STEPEXPORT_MIN_ACCEPTABLE_DISTANCE = 0.001; +/** + * Default distance between points to treat them as separate ones (mm) + * 0.001 mm is a reasonable value. A too large value creates issues by + * merging points that should be different. + * Remember we are a 3D space, so a thin line can be broken if 2 points + * are merged (in X, Y, Z coords) when they should not. + * round shapes converted to polygon can also be not good with a to large value + */ +static constexpr double OCC_MAX_DISTANCE_TO_MERGE_POINTS = 0.001; + +// default PCB thickness in mm +static constexpr double BOARD_THICKNESS_DEFAULT_MM = 1.6; + +// minimum PCB thickness in mm (10 microns assumes a very thin polyimide film) +static constexpr double BOARD_THICKNESS_MIN_MM = 0.01; + +// Max error to approximate an arc by segments (in mm) +static constexpr double ARC_TO_SEGMENT_MAX_ERROR_MM = 0.005; class PAD; @@ -75,8 +90,9 @@ public: // aThickness > THICKNESS_MIN == use aThickness void SetPCBThickness( double aThickness ); - // Set the minimum distance (in mm) to consider 2 points have the same coordinates - void SetMinDistance( double aDistance ); + // Set the max distance (in mm) to consider 2 points have the same coordinates + // and can be merged + void OCCSetMergeMaxDistance( double aDistance = OCC_MAX_DISTANCE_TO_MERGE_POINTS ); void SetMaxError( int aMaxError ) { m_maxError = aMaxError; } @@ -163,10 +179,11 @@ private: double m_precision; // model (length unit) numeric precision double m_angleprec; // angle numeric precision double m_boardColor[3];// RGB values - double m_thickness; // PCB thickness, mm + double m_boardThickness; // PCB thickness, mm - double m_minx; // leftmost curve point - double m_minDistance2; // minimum squared distance between items (mm) + double m_minx; // leftmost curve point + double m_mergeOCCMaxDist; // minimum distance (mm) below which two + // points are considered coincident by OCC std::vector m_cutouts; diff --git a/pcbnew/pcbnew_jobs_handler.cpp b/pcbnew/pcbnew_jobs_handler.cpp index 7b7c76d0e5..e2f21d3348 100644 --- a/pcbnew/pcbnew_jobs_handler.cpp +++ b/pcbnew/pcbnew_jobs_handler.cpp @@ -97,7 +97,7 @@ int PCBNEW_JOBS_HANDLER::JobExportStep( JOB* aJob ) EXPORTER_STEP_PARAMS params; params.m_includeExcludedBom = aStepJob->m_includeExcludedBom; - params.m_minDistance = aStepJob->m_minDistance; + params.m_boardOutlinesChainingEpsilon = aStepJob->m_boardOutlinesChainingEpsilon; params.m_overwrite = aStepJob->m_overwrite; params.m_substModels = aStepJob->m_substModels; params.m_origin = VECTOR2D( aStepJob->m_xOrigin, aStepJob->m_yOrigin );