diff --git a/src/Mod/Import/App/dxf/ImpExpDxf.cpp b/src/Mod/Import/App/dxf/ImpExpDxf.cpp index 1fd4c144fa..9ade2e2d2e 100644 --- a/src/Mod/Import/App/dxf/ImpExpDxf.cpp +++ b/src/Mod/Import/App/dxf/ImpExpDxf.cpp @@ -78,10 +78,12 @@ using BRepAdaptor_HCurve = BRepAdaptor_Curve; //****************************************************************************** // reading -ImpExpDxfRead::ImpExpDxfRead(std::string filepath, App::Document* pcDoc) - : CDxfRead(filepath.c_str()) +ImpExpDxfRead::ImpExpDxfRead(const std::string& filepath, App::Document* pcDoc) + : CDxfRead(filepath) + , document(pcDoc) + , optionGroupLayers(false) + , optionImportAnnotations(true) { - document = pcDoc; setOptionSource("User parameter:BaseApp/Preferences/Mod/Draft"); setOptions(); } @@ -92,22 +94,17 @@ void ImpExpDxfRead::setOptions() App::GetApplication().GetParameterGroupByPath(getOptionSource().c_str()); optionGroupLayers = hGrp->GetBool("groupLayers", false); optionImportAnnotations = hGrp->GetBool("dxftext", false); - optionScaling = hGrp->GetFloat("dxfScaling", 1.0); + SetAdditionalScaling(hGrp->GetFloat("dxfScaling", 1.0)); } -gp_Pnt ImpExpDxfRead::makePoint(const double point3d[3]) const +void ImpExpDxfRead::OnReadLine(const Base::Vector3d& start, + const Base::Vector3d& end, + bool /*hidden*/) { - if (optionScaling != 1.0) { - return {point3d[0] * optionScaling, point3d[1] * optionScaling, point3d[2] * optionScaling}; - } - return {point3d[0], point3d[1], point3d[2]}; -} - -void ImpExpDxfRead::OnReadLine(const double* s, const double* e, bool /*hidden*/) -{ - gp_Pnt p0 = makePoint(s); - gp_Pnt p1 = makePoint(e); + gp_Pnt p0 = makePoint(start); + gp_Pnt p1 = makePoint(end); if (p0.IsEqual(p1, 0.00000001)) { + // TODO: Really?? What about the people designing integrated circuits? return; } BRepBuilderAPI_MakeEdge makeEdge(p0, p1); @@ -116,27 +113,27 @@ void ImpExpDxfRead::OnReadLine(const double* s, const double* e, bool /*hidden*/ } -void ImpExpDxfRead::OnReadPoint(const double* s) +void ImpExpDxfRead::OnReadPoint(const Base::Vector3d& start) { - BRepBuilderAPI_MakeVertex makeVertex(makePoint(s)); + BRepBuilderAPI_MakeVertex makeVertex(makePoint(start)); TopoDS_Vertex vertex = makeVertex.Vertex(); AddObject(new Part::TopoShape(vertex)); } -void ImpExpDxfRead::OnReadArc(const double* s, - const double* e, - const double* c, +void ImpExpDxfRead::OnReadArc(const Base::Vector3d& start, + const Base::Vector3d& end, + const Base::Vector3d& center, bool dir, bool /*hidden*/) { - gp_Pnt p0 = makePoint(s); - gp_Pnt p1 = makePoint(e); + gp_Pnt p0 = makePoint(start); + gp_Pnt p1 = makePoint(end); gp_Dir up(0, 0, 1); if (!dir) { up = -up; } - gp_Pnt pc = makePoint(c); + gp_Pnt pc = makePoint(center); gp_Circ circle(gp_Ax2(pc, up), p0.Distance(pc)); if (circle.Radius() > 0) { BRepBuilderAPI_MakeEdge makeEdge(circle, p0, p1); @@ -149,14 +146,17 @@ void ImpExpDxfRead::OnReadArc(const double* s, } -void ImpExpDxfRead::OnReadCircle(const double* s, const double* c, bool dir, bool /*hidden*/) +void ImpExpDxfRead::OnReadCircle(const Base::Vector3d& start, + const Base::Vector3d& center, + bool dir, + bool /*hidden*/) { - gp_Pnt p0 = makePoint(s); + gp_Pnt p0 = makePoint(start); gp_Dir up(0, 0, 1); if (!dir) { up = -up; } - gp_Pnt pc = makePoint(c); + gp_Pnt pc = makePoint(center); gp_Circ circle(gp_Ax2(pc, up), p0.Distance(pc)); if (circle.Radius() > 0) { BRepBuilderAPI_MakeEdge makeEdge(circle); @@ -180,18 +180,18 @@ Handle(Geom_BSplineCurve) getSplineFromPolesAndKnots(struct SplineData& sd) // handle the poles TColgp_Array1OfPnt occpoles(1, sd.control_points); int index = 1; - for (auto x : sd.controlx) { - occpoles(index++).SetX(x); + for (auto coordinate : sd.controlx) { + occpoles(index++).SetX(coordinate); } index = 1; - for (auto y : sd.controly) { - occpoles(index++).SetY(y); + for (auto coordinate : sd.controly) { + occpoles(index++).SetY(coordinate); } index = 1; - for (auto z : sd.controlz) { - occpoles(index++).SetZ(z); + for (auto coordinate : sd.controlz) { + occpoles(index++).SetZ(coordinate); } // handle knots and mults @@ -202,10 +202,9 @@ Handle(Geom_BSplineCurve) getSplineFromPolesAndKnots(struct SplineData& sd) TColStd_Array1OfInteger occmults(1, numKnots); TColStd_Array1OfReal occknots(1, numKnots); index = 1; - for (auto k : unique) { - size_t m = std::count(sd.knot.begin(), sd.knot.end(), k); - occknots(index) = k; - occmults(index) = m; + for (auto knot : unique) { + occknots(index) = knot; + occmults(index) = (int)std::count(sd.knot.begin(), sd.knot.end(), knot); index++; } @@ -213,8 +212,8 @@ Handle(Geom_BSplineCurve) getSplineFromPolesAndKnots(struct SplineData& sd) TColStd_Array1OfReal occweights(1, sd.control_points); if (sd.weight.size() == std::size_t(sd.control_points)) { index = 1; - for (auto w : sd.weight) { - occweights(index++) = w; + for (auto weight : sd.weight) { + occweights(index++) = weight; } } else { @@ -240,18 +239,18 @@ Handle(Geom_BSplineCurve) getInterpolationSpline(struct SplineData& sd) // handle the poles Handle(TColgp_HArray1OfPnt) fitpoints = new TColgp_HArray1OfPnt(1, sd.fit_points); int index = 1; - for (auto x : sd.fitx) { - fitpoints->ChangeValue(index++).SetX(x); + for (auto coordinate : sd.fitx) { + fitpoints->ChangeValue(index++).SetX(coordinate); } index = 1; - for (auto y : sd.fity) { - fitpoints->ChangeValue(index++).SetY(y); + for (auto coordinate : sd.fity) { + fitpoints->ChangeValue(index++).SetY(coordinate); } index = 1; - for (auto z : sd.fitz) { - fitpoints->ChangeValue(index++).SetZ(z); + for (auto coordinate : sd.fitz) { + fitpoints->ChangeValue(index++).SetZ(coordinate); } Standard_Boolean periodic = sd.flag == 2; @@ -288,21 +287,22 @@ void ImpExpDxfRead::OnReadSpline(struct SplineData& sd) } } - -void ImpExpDxfRead::OnReadEllipse(const double* c, +// NOLINTBEGIN(bugprone-easily-swappable-parameters) +void ImpExpDxfRead::OnReadEllipse(const Base::Vector3d& center, double major_radius, double minor_radius, double rotation, double /*start_angle*/, double /*end_angle*/, bool dir) +// NOLINTEND(bugprone-easily-swappable-parameters) { gp_Dir up(0, 0, 1); if (!dir) { up = -up; } - gp_Pnt pc = makePoint(c); - gp_Elips ellipse(gp_Ax2(pc, up), major_radius * optionScaling, minor_radius * optionScaling); + gp_Pnt pc = makePoint(center); + gp_Elips ellipse(gp_Ax2(pc, up), major_radius, minor_radius); ellipse.Rotate(gp_Ax1(pc, up), rotation); if (ellipse.MinorRadius() > 0) { BRepBuilderAPI_MakeEdge makeEdge(ellipse); @@ -315,34 +315,34 @@ void ImpExpDxfRead::OnReadEllipse(const double* c, } -void ImpExpDxfRead::OnReadText(const double* point, +void ImpExpDxfRead::OnReadText(const Base::Vector3d& point, const double height, - const char* text, + const std::string& text, const double rotation) { // Note that our parameters do not contain all the information needed to properly orient the // text. As a result the text will always appear on the XY plane if (optionImportAnnotations) { - if (LayerName().substr(0, 6) != "BLOCKS") { + if (LayerName().rfind("BLOCKS", 0) != 0) { PyObject* draftModule = nullptr; - Base::Vector3d insertionPoint(point[0], point[1], point[2]); - insertionPoint *= optionScaling; Base::Rotation rot(Base::Vector3d(0, 0, 1), rotation); - PyObject* placement = new Base::PlacementPy(Base::Placement(insertionPoint, rot)); + PyObject* placement = new Base::PlacementPy(Base::Placement(point, rot)); draftModule = PyImport_ImportModule("Draft"); if (draftModule != nullptr) { // returns a wrapped App::FeaturePython - auto builtText = static_cast*>( - PyObject_CallMethod(draftModule, - "make_text", - "sOif", - text, - placement, - 0, - height)); + auto builtText = dynamic_cast*>( + // NOLINTNEXTLINE(readability/nolint) + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) + (Base::PyObjectBase*)PyObject_CallMethod(draftModule, + "make_text", + "sOif", + text.c_str(), + placement, + 0, + height)); if (builtText != nullptr) { ApplyGuiStyles( - static_cast(builtText->getDocumentObjectPtr())); + dynamic_cast(builtText->getDocumentObjectPtr())); } } // We own all the return values so we must release them. @@ -356,9 +356,9 @@ void ImpExpDxfRead::OnReadText(const double* point, } -void ImpExpDxfRead::OnReadInsert(const double* point, - const double* scale, - const char* name, +void ImpExpDxfRead::OnReadInsert(const Base::Vector3d& point, + const Base::Vector3d& scale, + const std::string& name, double rotation) { // std::cout << "Inserting block " << name << " rotation " << rotation << " pos " << point[0] << @@ -367,32 +367,26 @@ void ImpExpDxfRead::OnReadInsert(const double* point, std::string prefix = "BLOCKS "; prefix += name; prefix += " "; - auto checkScale = [=](double v) { - return v != 0.0 ? v : 1.0; + auto checkScale = [=](double scale) { + return scale != 0.0 ? scale : 1.0; }; - for (std::map>::const_iterator i = layers.begin(); - i != layers.end(); - ++i) { - std::string k = i->first; - if (k.substr(0, prefix.size()) == prefix) { + for (const auto& layer : layers) { + if (layer.first.substr(0, prefix.size()) == prefix) { BRep_Builder builder; TopoDS_Compound comp; builder.MakeCompound(comp); - std::vector v = i->second; - for (std::vector::const_iterator j = v.begin(); j != v.end(); ++j) { - const TopoDS_Shape& sh = (*j)->getShape(); + for (const auto& shape : layer.second) { + const TopoDS_Shape& sh = shape->getShape(); if (!sh.IsNull()) { builder.Add(comp, sh); } } if (!comp.IsNull()) { - Part::TopoShape* pcomp = new Part::TopoShape(comp); + auto pcomp = new Part::TopoShape(comp); Base::Matrix4D mat; mat.scale(checkScale(scale[0]), checkScale(scale[1]), checkScale(scale[2])); mat.rotZ(rotation); - mat.move(point[0] * optionScaling, - point[1] * optionScaling, - point[2] * optionScaling); + mat.move(point[0], point[1], point[2]); pcomp->transformShape(mat, true); AddObject(pcomp); } @@ -401,35 +395,36 @@ void ImpExpDxfRead::OnReadInsert(const double* point, } -void ImpExpDxfRead::OnReadDimension(const double* s, - const double* e, - const double* point, +void ImpExpDxfRead::OnReadDimension(const Base::Vector3d& start, + const Base::Vector3d& end, + const Base::Vector3d& point, double /*rotation*/) { if (optionImportAnnotations) { PyObject* draftModule = nullptr; - PyObject* start = new Base::VectorPy(Base::Vector3d(s[0], s[1], s[2]) * optionScaling); - PyObject* end = new Base::VectorPy(Base::Vector3d(e[0], e[1], e[2]) * optionScaling); - PyObject* lineLocation = - new Base::VectorPy(Base::Vector3d(point[0], point[1], point[2]) * optionScaling); + PyObject* startPy = new Base::VectorPy(start); + PyObject* endPy = new Base::VectorPy(end); + PyObject* lineLocationPy = new Base::VectorPy(point); draftModule = PyImport_ImportModule("Draft"); if (draftModule != nullptr) { // returns a wrapped App::FeaturePython - auto builtDim = static_cast*>( - PyObject_CallMethod(draftModule, - "make_linear_dimension", - "OOO", - start, - end, - lineLocation)); + auto builtDim = dynamic_cast*>( + // NOLINTNEXTLINE(readability/nolint) + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast) + (Base::PyObjectBase*)PyObject_CallMethod(draftModule, + "make_linear_dimension", + "OOO", + startPy, + endPy, + lineLocationPy)); if (builtDim != nullptr) { - ApplyGuiStyles(static_cast(builtDim->getDocumentObjectPtr())); + ApplyGuiStyles(dynamic_cast(builtDim->getDocumentObjectPtr())); } } // We own all the return values so we must release them. - Py_DECREF(start); - Py_DECREF(end); - Py_DECREF(lineLocation); + Py_DECREF(startPy); + Py_DECREF(endPy); + Py_DECREF(lineLocationPy); Py_XDECREF(draftModule); } } @@ -438,15 +433,16 @@ void ImpExpDxfRead::OnReadDimension(const double* s, void ImpExpDxfRead::AddObject(Part::TopoShape* shape) { std::vector vec; - if (layers.count(LayerName())) { - vec = layers[LayerName()]; + std::string destinationLayerName(LayerName()); + if (layers.count(destinationLayerName) != 0) { + vec = layers[destinationLayerName]; } vec.push_back(shape); - layers[LayerName()] = vec; + layers[destinationLayerName] = vec; if (!optionGroupLayers) { - if (LayerName().substr(0, 6) != "BLOCKS") { - Part::Feature* pcFeature = - static_cast(document->addObject("Part::Feature", "Shape")); + if (destinationLayerName.rfind("BLOCKS", 0) != 0) { + auto pcFeature = + dynamic_cast(document->addObject("Part::Feature", "Shape")); pcFeature->Shape.setValue(shape->getShape()); ApplyGuiStyles(pcFeature); } @@ -461,33 +457,31 @@ std::string ImpExpDxfRead::Deformat(const char* text) bool escape = false; // turned on when finding an escape character bool longescape = false; // turned on for certain escape codes that expect additional chars for (unsigned int i = 0; i < strlen(text); i++) { - if (text[i] == '\\') { + char ch = text[i]; + if (ch == '\\') { escape = true; } else if (escape) { if (longescape) { - if (text[i] == ';') { + if (ch == ';') { escape = false; longescape = false; } } + else if ((ch == 'H') || (ch == 'h') || (ch == 'Q') || (ch == 'q') || (ch == 'W') + || (ch == 'w') || (ch == 'F') || (ch == 'f') || (ch == 'A') || (ch == 'a') + || (ch == 'C') || (ch == 'c') || (ch == 'T') || (ch == 't')) { + longescape = true; + } else { - if ((text[i] == 'H') || (text[i] == 'h') || (text[i] == 'Q') || (text[i] == 'q') - || (text[i] == 'W') || (text[i] == 'w') || (text[i] == 'F') || (text[i] == 'f') - || (text[i] == 'A') || (text[i] == 'a') || (text[i] == 'C') || (text[i] == 'c') - || (text[i] == 'T') || (text[i] == 't')) { - longescape = true; - } - else { - if ((text[i] == 'P') || (text[i] == 'p')) { - ss << "\n"; - } - escape = false; + if ((ch == 'P') || (ch == 'p')) { + ss << "\n"; } + escape = false; } } - else if ((text[i] != '{') && (text[i] != '}')) { - ss << text[i]; + else if ((ch != '{') && (ch != '}')) { + ss << ch; } } return ss.str(); @@ -497,28 +491,23 @@ std::string ImpExpDxfRead::Deformat(const char* text) void ImpExpDxfRead::AddGraphics() const { if (optionGroupLayers) { - for (std::map>::const_iterator i = - layers.begin(); - i != layers.end(); - ++i) { + for (const auto& layer : layers) { BRep_Builder builder; TopoDS_Compound comp; builder.MakeCompound(comp); - std::string k = i->first; + std::string k = layer.first; if (k == "0") { // FreeCAD doesn't like an object name being '0'... k = "LAYER_0"; } - std::vector v = i->second; - if (k.substr(0, 6) != "BLOCKS") { - for (std::vector::const_iterator j = v.begin(); j != v.end(); - ++j) { - const TopoDS_Shape& sh = (*j)->getShape(); + if (k.rfind("BLOCKS", 0) != 0) { + for (const auto& shape : layer.second) { + const TopoDS_Shape& sh = shape->getShape(); if (!sh.IsNull()) { builder.Add(comp, sh); } } if (!comp.IsNull()) { - Part::Feature* pcFeature = static_cast( + auto pcFeature = dynamic_cast( document->addObject("Part::Feature", k.c_str())); pcFeature->Shape.setValue(comp); } @@ -530,7 +519,7 @@ void ImpExpDxfRead::AddGraphics() const //****************************************************************************** // writing -void gPntToTuple(double* result, gp_Pnt& p) +void gPntToTuple(double result[3], gp_Pnt& p) { result[0] = p.X(); result[1] = p.Y(); @@ -574,9 +563,9 @@ void ImpExpDxfWrite::exportShape(const TopoDS_Shape input) if (adapt.GetType() == GeomAbs_Circle) { double f = adapt.FirstParameter(); double l = adapt.LastParameter(); - gp_Pnt s = adapt.Value(f); + gp_Pnt start = adapt.Value(f); gp_Pnt e = adapt.Value(l); - if (fabs(l - f) > 1.0 && s.SquareDistance(e) < 0.001) { + if (fabs(l - f) > 1.0 && start.SquareDistance(e) < 0.001) { exportCircle(adapt); } else { @@ -586,9 +575,9 @@ void ImpExpDxfWrite::exportShape(const TopoDS_Shape input) else if (adapt.GetType() == GeomAbs_Ellipse) { double f = adapt.FirstParameter(); double l = adapt.LastParameter(); - gp_Pnt s = adapt.Value(f); + gp_Pnt start = adapt.Value(f); gp_Pnt e = adapt.Value(l); - if (fabs(l - f) > 1.0 && s.SquareDistance(e) < 0.001) { + if (fabs(l - f) > 1.0 && start.SquareDistance(e) < 0.001) { if (m_polyOverride) { if (m_version >= 14) { exportLWPoly(adapt); diff --git a/src/Mod/Import/App/dxf/ImpExpDxf.h b/src/Mod/Import/App/dxf/ImpExpDxf.h index fff54e0ef6..349c0d77c7 100644 --- a/src/Mod/Import/App/dxf/ImpExpDxf.h +++ b/src/Mod/Import/App/dxf/ImpExpDxf.h @@ -39,19 +39,25 @@ namespace Import class ImportExport ImpExpDxfRead: public CDxfRead { public: - ImpExpDxfRead(std::string filepath, App::Document* pcDoc); + ImpExpDxfRead(const std::string& filepath, App::Document* pcDoc); // CDxfRead's virtual functions - void OnReadLine(const double* s, const double* e, bool hidden) override; - void OnReadPoint(const double* s) override; - void OnReadText(const double* point, - const double height, - const char* text, - const double rotation) override; - void - OnReadArc(const double* s, const double* e, const double* c, bool dir, bool hidden) override; - void OnReadCircle(const double* s, const double* c, bool dir, bool hidden) override; - void OnReadEllipse(const double* c, + void OnReadLine(const Base::Vector3d& start, const Base::Vector3d& end, bool hidden) override; + void OnReadPoint(const Base::Vector3d& start) override; + void OnReadText(const Base::Vector3d& point, + double height, + const std::string& text, + double rotation) override; + void OnReadArc(const Base::Vector3d& start, + const Base::Vector3d& end, + const Base::Vector3d& center, + bool dir, + bool hidden) override; + void OnReadCircle(const Base::Vector3d& start, + const Base::Vector3d& center, + bool dir, + bool hidden) override; + void OnReadEllipse(const Base::Vector3d& center, double major_radius, double minor_radius, double rotation, @@ -59,13 +65,13 @@ public: double end_angle, bool dir) override; void OnReadSpline(struct SplineData& sd) override; - void OnReadInsert(const double* point, - const double* scale, - const char* name, + void OnReadInsert(const Base::Vector3d& point, + const Base::Vector3d& scale, + const std::string& name, double rotation) override; - void OnReadDimension(const double* s, - const double* e, - const double* point, + void OnReadDimension(const Base::Vector3d& start, + const Base::Vector3d& end, + const Base::Vector3d& point, double rotation) override; void AddGraphics() const override; @@ -77,24 +83,26 @@ public: { return m_optionSource; } - void setOptionSource(std::string s) + void setOptionSource(const std::string& sourceName) { - m_optionSource = s; + m_optionSource = sourceName; } void setOptions(); private: - gp_Pnt makePoint(const double point3d[3]) const; + static gp_Pnt makePoint(const Base::Vector3d& point3d) + { + return {point3d.x, point3d.y, point3d.z}; + } protected: - virtual void ApplyGuiStyles(Part::Feature*) + virtual void ApplyGuiStyles(Part::Feature* /*object*/) {} - virtual void ApplyGuiStyles(App::FeaturePython*) + virtual void ApplyGuiStyles(App::FeaturePython* /*object*/) {} App::Document* document; bool optionGroupLayers; bool optionImportAnnotations; - double optionScaling; std::map> layers; std::string m_optionSource; }; @@ -103,9 +111,13 @@ class ImportExport ImpExpDxfWrite: public CDxfWrite { public: explicit ImpExpDxfWrite(std::string filepath); + ImpExpDxfWrite(const ImpExpDxfWrite&) = delete; + ImpExpDxfWrite(const ImpExpDxfWrite&&) = delete; + ImpExpDxfWrite& operator=(const ImpExpDxfWrite&) = delete; + ImpExpDxfWrite& operator=(const ImpExpDxfWrite&&) = delete; ~ImpExpDxfWrite(); - void exportShape(const TopoDS_Shape input); + void exportShape(TopoDS_Shape input); std::string getOptionSource() { return m_optionSource; @@ -129,8 +141,8 @@ public: int type); void exportAngularDim(Base::Vector3d textLocn, Base::Vector3d lineLocn, - Base::Vector3d extLine1Start, - Base::Vector3d extLine2Start, + Base::Vector3d extLine1End, + Base::Vector3d extLine2End, Base::Vector3d apexPoint, char* dimText); void exportRadialDim(Base::Vector3d centerPoint, diff --git a/src/Mod/Import/App/dxf/dxf.cpp b/src/Mod/Import/App/dxf/dxf.cpp index e6abf227b3..7aab753225 100644 --- a/src/Mod/Import/App/dxf/dxf.cpp +++ b/src/Mod/Import/App/dxf/dxf.cpp @@ -20,18 +20,16 @@ #include #include #include +#include #include using namespace std; - -Base::Vector3d toVector3d(const double* a) +static Base::Vector3d MakeVector3d(const double coordinates[3]) { - Base::Vector3d result; - result.x = a[0]; - result.y = a[1]; - result.z = a[2]; - return result; + // NOLINTNEXTLINE(readability/nolint) + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + return Base::Vector3d(coordinates[0], coordinates[1], coordinates[2]); } CDxfWrite::CDxfWrite(const char* filepath) @@ -41,7 +39,13 @@ CDxfWrite::CDxfWrite(const char* filepath) // used by dxf.cpp A01 - FFFE // ACAD HANDSEED FFFF - m_handle(0xA00) + m_fail(false) + , m_ssBlock(new std::ostringstream()) + , m_ssBlkRecord(new std::ostringstream()) + , m_ssEntity(new std::ostringstream()) + , m_ssLayer(new std::ostringstream()) + , m_version(12) + , m_handle(0xA00) , // room for 2560 handles in boilerplate files // m_entityHandle(0x300), //don't need special ranges for handles // m_layerHandle(0x30), @@ -51,14 +55,8 @@ CDxfWrite::CDxfWrite(const char* filepath) , m_layerName("none") { // start the file - m_fail = false; - m_version = 12; Base::FileInfo fi(filepath); m_ofs = new Base::ofstream(fi, ios::out); - m_ssBlock = new std::ostringstream(); - m_ssBlkRecord = new std::ostringstream(); - m_ssEntity = new std::ostringstream(); - m_ssLayer = new std::ostringstream(); if (!(*m_ofs)) { m_fail = true; @@ -438,7 +436,7 @@ std::string CDxfWrite::getPlateFile(std::string fileSpec) } else { string line; - ifstream inFile(fi.filePath().c_str()); + ifstream inFile(fi.filePath()); while (!inFile.eof()) { getline(inFile, line); @@ -499,28 +497,32 @@ std::string CDxfWrite::getBlkRecordHandle() // return ss.str(); } -void CDxfWrite::addBlockName(std::string b, std::string h) +void CDxfWrite::addBlockName(const std::string& name, const std::string& blkRecordHandle) { - m_blockList.push_back(b); - m_blkRecordList.push_back(h); + m_blockList.push_back(name); + m_blkRecordList.push_back(blkRecordHandle); } -void CDxfWrite::setLayerName(std::string s) +void CDxfWrite::setLayerName(std::string name) { - m_layerName = s; - m_layerList.push_back(s); + m_layerName = name; + m_layerList.push_back(name); } -void CDxfWrite::writeLine(const double* s, const double* e) +void CDxfWrite::writeLine(const double* start, const double* end) { - putLine(toVector3d(s), toVector3d(e), m_ssEntity, getEntityHandle(), m_saveModelSpaceHandle); + putLine(toVector3d(start), + toVector3d(end), + m_ssEntity, + getEntityHandle(), + m_saveModelSpaceHandle); } -void CDxfWrite::putLine(const Base::Vector3d s, - const Base::Vector3d e, +void CDxfWrite::putLine(const Base::Vector3d& start, + const Base::Vector3d& end, std::ostringstream* outStream, - const std::string handle, - const std::string ownerHandle) + const std::string& handle, + const std::string& ownerHandle) { (*outStream) << " 0" << endl; (*outStream) << "LINE" << endl; @@ -538,18 +540,18 @@ void CDxfWrite::putLine(const Base::Vector3d s, (*outStream) << "100" << endl; (*outStream) << "AcDbLine" << endl; } - (*outStream) << " 10" << endl; // Start point of line - (*outStream) << s.x << endl; // X in WCS coordinates + (*outStream) << " 10" << endl; // Start point of line + (*outStream) << start.x << endl; // X in WCS coordinates (*outStream) << " 20" << endl; - (*outStream) << s.y << endl; // Y in WCS coordinates + (*outStream) << start.y << endl; // Y in WCS coordinates (*outStream) << " 30" << endl; - (*outStream) << s.z << endl; // Z in WCS coordinates - (*outStream) << " 11" << endl; // End point of line - (*outStream) << e.x << endl; // X in WCS coordinates + (*outStream) << start.z << endl; // Z in WCS coordinates + (*outStream) << " 11" << endl; // End point of line + (*outStream) << end.x << endl; // X in WCS coordinates (*outStream) << " 21" << endl; - (*outStream) << e.y << endl; // Y in WCS coordinates + (*outStream) << end.y << endl; // Y in WCS coordinates (*outStream) << " 31" << endl; - (*outStream) << e.z << endl; // Z in WCS coordinates + (*outStream) << end.z << endl; // Z in WCS coordinates } @@ -664,7 +666,7 @@ void CDxfWrite::writePolyline(const LWPolyDataOut& pd) (*m_ssEntity) << getLayerName() << endl; } -void CDxfWrite::writePoint(const double* s) +void CDxfWrite::writePoint(const double* point) { (*m_ssEntity) << " 0" << endl; (*m_ssEntity) << "POINT" << endl; @@ -683,22 +685,22 @@ void CDxfWrite::writePoint(const double* s) (*m_ssEntity) << "AcDbPoint" << endl; } (*m_ssEntity) << " 10" << endl; - (*m_ssEntity) << s[0] << endl; // X in WCS coordinates + (*m_ssEntity) << point[0] << endl; // X in WCS coordinates (*m_ssEntity) << " 20" << endl; - (*m_ssEntity) << s[1] << endl; // Y in WCS coordinates + (*m_ssEntity) << point[1] << endl; // Y in WCS coordinates (*m_ssEntity) << " 30" << endl; - (*m_ssEntity) << s[2] << endl; // Z in WCS coordinates + (*m_ssEntity) << point[2] << endl; // Z in WCS coordinates } -void CDxfWrite::writeArc(const double* s, const double* e, const double* c, bool dir) +void CDxfWrite::writeArc(const double* start, const double* end, const double* center, bool dir) { - double ax = s[0] - c[0]; - double ay = s[1] - c[1]; - double bx = e[0] - c[0]; - double by = e[1] - c[1]; + double ax = start[0] - center[0]; + double ay = start[1] - center[1]; + double bx = end[0] - center[0]; + double by = end[1] - center[1]; - double start_angle = atan2(ay, ax) * 180 / M_PI; - double end_angle = atan2(by, bx) * 180 / M_PI; + double start_angle = Base::toDegrees(atan2(ay, ax)); + double end_angle = Base::toDegrees(atan2(by, bx)); double radius = sqrt(ax * ax + ay * ay); if (!dir) { double temp = start_angle; @@ -723,14 +725,14 @@ void CDxfWrite::writeArc(const double* s, const double* e, const double* c, bool (*m_ssEntity) << "100" << endl; (*m_ssEntity) << "AcDbCircle" << endl; } - (*m_ssEntity) << " 10" << endl; // Centre X - (*m_ssEntity) << c[0] << endl; // X in WCS coordinates + (*m_ssEntity) << " 10" << endl; // Centre X + (*m_ssEntity) << center[0] << endl; // X in WCS coordinates (*m_ssEntity) << " 20" << endl; - (*m_ssEntity) << c[1] << endl; // Y in WCS coordinates + (*m_ssEntity) << center[1] << endl; // Y in WCS coordinates (*m_ssEntity) << " 30" << endl; - (*m_ssEntity) << c[2] << endl; // Z in WCS coordinates - (*m_ssEntity) << " 40" << endl; // - (*m_ssEntity) << radius << endl; // Radius + (*m_ssEntity) << center[2] << endl; // Z in WCS coordinates + (*m_ssEntity) << " 40" << endl; // + (*m_ssEntity) << radius << endl; // Radius if (m_version > 12) { (*m_ssEntity) << "100" << endl; @@ -742,7 +744,7 @@ void CDxfWrite::writeArc(const double* s, const double* e, const double* c, bool (*m_ssEntity) << end_angle << endl; // End angle } -void CDxfWrite::writeCircle(const double* c, double radius) +void CDxfWrite::writeCircle(const double* center, double radius) { (*m_ssEntity) << " 0" << endl; (*m_ssEntity) << "CIRCLE" << endl; @@ -760,17 +762,17 @@ void CDxfWrite::writeCircle(const double* c, double radius) (*m_ssEntity) << "100" << endl; (*m_ssEntity) << "AcDbCircle" << endl; } - (*m_ssEntity) << " 10" << endl; // Centre X - (*m_ssEntity) << c[0] << endl; // X in WCS coordinates + (*m_ssEntity) << " 10" << endl; // Centre X + (*m_ssEntity) << center[0] << endl; // X in WCS coordinates (*m_ssEntity) << " 20" << endl; - (*m_ssEntity) << c[1] << endl; // Y in WCS coordinates - // (*m_ssEntity) << " 30" << endl; - // (*m_ssEntity) << c[2] << endl; // Z in WCS coordinates + (*m_ssEntity) << center[1] << endl; // Y in WCS coordinates + // (*m_ssEntity) << " 30" << endl; + // (*m_ssEntity) << center[2] << endl; // Z in WCS coordinates (*m_ssEntity) << " 40" << endl; // (*m_ssEntity) << radius << endl; // Radius } -void CDxfWrite::writeEllipse(const double* c, +void CDxfWrite::writeEllipse(const double* center, double major_radius, double minor_radius, double rotation, @@ -778,7 +780,7 @@ void CDxfWrite::writeEllipse(const double* c, double end_angle, bool endIsCW) { - double m[3] = {major_radius * sin(rotation), major_radius * cos(rotation), 0}; + Base::Vector3d m(major_radius * sin(rotation), major_radius * cos(rotation), 0); double ratio = minor_radius / major_radius; if (!endIsCW) { // end is NOT CW from start @@ -802,18 +804,18 @@ void CDxfWrite::writeEllipse(const double* c, (*m_ssEntity) << "100" << endl; (*m_ssEntity) << "AcDbEllipse" << endl; } - (*m_ssEntity) << " 10" << endl; // Centre X - (*m_ssEntity) << c[0] << endl; // X in WCS coordinates + (*m_ssEntity) << " 10" << endl; // Centre X + (*m_ssEntity) << center[0] << endl; // X in WCS coordinates (*m_ssEntity) << " 20" << endl; - (*m_ssEntity) << c[1] << endl; // Y in WCS coordinates + (*m_ssEntity) << center[1] << endl; // Y in WCS coordinates (*m_ssEntity) << " 30" << endl; - (*m_ssEntity) << c[2] << endl; // Z in WCS coordinates - (*m_ssEntity) << " 11" << endl; // - (*m_ssEntity) << m[0] << endl; // Major X + (*m_ssEntity) << center[2] << endl; // Z in WCS coordinates + (*m_ssEntity) << " 11" << endl; // + (*m_ssEntity) << m.x << endl; // Major X (*m_ssEntity) << " 21" << endl; - (*m_ssEntity) << m[1] << endl; // Major Y + (*m_ssEntity) << m.y << endl; // Major Y (*m_ssEntity) << " 31" << endl; - (*m_ssEntity) << m[2] << endl; // Major Z + (*m_ssEntity) << m.z << endl; // Major Z (*m_ssEntity) << " 40" << endl; // (*m_ssEntity) << ratio << endl; // Ratio @@ -960,13 +962,13 @@ void CDxfWrite::writeText(const char* text, // putText // added by Wandererfan 2018 (wandererfan@gmail.com) for FreeCAD project void CDxfWrite::putText(const char* text, - const Base::Vector3d location1, - const Base::Vector3d location2, + const Base::Vector3d& location1, + const Base::Vector3d& location2, const double height, const int horizJust, std::ostringstream* outStream, - const std::string handle, - const std::string ownerHandle) + const std::string& handle, + const std::string& ownerHandle) { (void)location2; @@ -1031,12 +1033,12 @@ void CDxfWrite::putText(const char* text, } } -void CDxfWrite::putArrow(Base::Vector3d arrowPos, - Base::Vector3d barb1Pos, - Base::Vector3d barb2Pos, +void CDxfWrite::putArrow(Base::Vector3d& arrowPos, + Base::Vector3d& barb1Pos, + Base::Vector3d& barb2Pos, std::ostringstream* outStream, - const std::string handle, - const std::string ownerHandle) + const std::string& handle, + const std::string& ownerHandle) { (*outStream) << " 0" << endl; (*outStream) << "SOLID" << endl; @@ -1472,56 +1474,52 @@ void CDxfWrite::writeBlockTrailer() // writeLinearDimBlock // added by Wandererfan 2018 (wandererfan@gmail.com) for FreeCAD project void CDxfWrite::writeLinearDimBlock(const double* textMidPoint, - const double* dimLine, - const double* e1Start, - const double* e2Start, + const double* lineDefPoint, + const double* extLine1, + const double* extLine2, const char* dimText, int type) { - Base::Vector3d e1S(e1Start[0], e1Start[1], e1Start[2]); - Base::Vector3d e2S(e2Start[0], e2Start[1], e2Start[2]); - Base::Vector3d dl(dimLine[0], dimLine[1], dimLine[2]); // point on DimLine (somewhere!) + Base::Vector3d e1S(MakeVector3d(extLine1)); + Base::Vector3d e2S(MakeVector3d(extLine2)); + // point on DimLine (somewhere!) + Base::Vector3d dl(MakeVector3d(lineDefPoint)); Base::Vector3d perp = dl.DistanceToLineSegment(e2S, e1S); Base::Vector3d e1E = e1S - perp; Base::Vector3d e2E = e2S - perp; Base::Vector3d para = e1E - e2E; - Base::Vector3d X(1.0, 0.0, 0.0); - double angle = para.GetAngle(X); - angle = angle * 180.0 / M_PI; if (type == ALIGNED) { // NOP } else if (type == HORIZONTAL) { - double x = e1Start[0]; - double y = dimLine[1]; + double x = extLine1[0]; + double y = lineDefPoint[1]; e1E = Base::Vector3d(x, y, 0.0); - x = e2Start[0]; + x = extLine2[0]; e2E = Base::Vector3d(x, y, 0.0); perp = Base::Vector3d(0, -1, 0); // down para = Base::Vector3d(1, 0, 0); // right - if (dimLine[1] > e1Start[1]) { + if (lineDefPoint[1] > extLine1[1]) { perp = Base::Vector3d(0, 1, 0); // up } - if (e1Start[0] > e2Start[0]) { + if (extLine1[0] > extLine2[0]) { para = Base::Vector3d(-1, 0, 0); // left } - angle = 0; } else if (type == VERTICAL) { - double x = dimLine[0]; - double y = e1Start[1]; + double x = lineDefPoint[0]; + double y = extLine1[1]; e1E = Base::Vector3d(x, y, 0.0); - y = e2Start[1]; + y = extLine2[1]; e2E = Base::Vector3d(x, y, 0.0); perp = Base::Vector3d(1, 0, 0); para = Base::Vector3d(0, 1, 0); - if (dimLine[0] < e1Start[0]) { + if (lineDefPoint[0] < extLine1[0]) { perp = Base::Vector3d(-1, 0, 0); } - if (e1Start[1] > e2Start[1]) { + if (extLine1[1] > extLine2[1]) { para = Base::Vector3d(0, -1, 0); } - angle = 90; } double arrowLen = 5.0; // magic number @@ -1535,7 +1533,7 @@ void CDxfWrite::writeLinearDimBlock(const double* textMidPoint, putText(dimText, toVector3d(textMidPoint), - toVector3d(dimLine), + toVector3d(lineDefPoint), 3.5, 1, m_ssBlock, @@ -1567,10 +1565,10 @@ void CDxfWrite::writeAngularDimBlock(const double* textMidPoint, const double* endExt2, const char* dimText) { - Base::Vector3d e1S(startExt1[0], startExt1[1], startExt1[2]); // apex - Base::Vector3d e2S(startExt2[0], startExt2[1], startExt2[2]); - Base::Vector3d e1E(endExt1[0], endExt1[1], endExt1[2]); - Base::Vector3d e2E(endExt2[0], endExt2[1], endExt2[2]); + Base::Vector3d e1S(MakeVector3d(startExt1)); // apex + Base::Vector3d e2S(MakeVector3d(startExt2)); + Base::Vector3d e1E(MakeVector3d(endExt1)); + Base::Vector3d e2E(MakeVector3d(endExt2)); Base::Vector3d e1 = e1E - e1S; Base::Vector3d e2 = e2E - e2S; @@ -1579,17 +1577,17 @@ void CDxfWrite::writeAngularDimBlock(const double* textMidPoint, double span = fabs(endAngle - startAngle); double offset = span * 0.10; if (startAngle < 0) { - startAngle += 2.0 * M_PI; + startAngle += 2 * M_PI; } if (endAngle < 0) { - endAngle += 2.0 * M_PI; + endAngle += 2 * M_PI; } Base::Vector3d startOff(cos(startAngle + offset), sin(startAngle + offset), 0.0); Base::Vector3d endOff(cos(endAngle - offset), sin(endAngle - offset), 0.0); - startAngle = startAngle * 180.0 / M_PI; - endAngle = endAngle * 180.0 / M_PI; + startAngle = Base::toDegrees(startAngle); + endAngle = Base::toDegrees(endAngle); - Base::Vector3d linePt(lineDefPoint[0], lineDefPoint[1], lineDefPoint[2]); + Base::Vector3d linePt(MakeVector3d(lineDefPoint)); double radius = (e2S - linePt).Length(); (*m_ssBlock) << " 0" << endl; @@ -1685,8 +1683,8 @@ void CDxfWrite::writeRadialDimBlock(const double* centerPoint, getBlockHandle(), m_saveBlkRecordHandle); - Base::Vector3d center(centerPoint[0], centerPoint[1], centerPoint[2]); - Base::Vector3d a(arcPoint[0], arcPoint[1], arcPoint[2]); + Base::Vector3d center(MakeVector3d(centerPoint)); + Base::Vector3d a(MakeVector3d(arcPoint)); Base::Vector3d para = a - center; double arrowLen = 5.0; // magic number double arrowWidth = arrowLen / 6.0 / 2.0; // magic number calc! @@ -1722,8 +1720,8 @@ void CDxfWrite::writeDiametricDimBlock(const double* textMidPoint, getBlockHandle(), m_saveBlkRecordHandle); - Base::Vector3d a1(arcPoint1[0], arcPoint1[1], arcPoint1[2]); - Base::Vector3d a2(arcPoint2[0], arcPoint2[1], arcPoint2[2]); + Base::Vector3d a1(MakeVector3d(arcPoint1)); + Base::Vector3d a2(MakeVector3d(arcPoint2)); Base::Vector3d para = a2 - a1; double arrowLen = 5.0; // magic number double arrowWidth = arrowLen / 6.0 / 2.0; // magic number calc! @@ -1792,10 +1790,11 @@ void CDxfWrite::writeObjectsSection() (*m_ofs) << getPlateFile(fileSpec); } -CDxfRead::CDxfRead(const char* filepath) +const DxfUnits DxfUnits::Instance; + +CDxfRead::CDxfRead(const std::string& filepath) + : m_ifs(new ifstream(filepath)) { - // start the file - m_ifs = new ifstream(filepath); if (!(*m_ifs)) { m_fail = true; ImportError("DXF file didn't load\n"); @@ -1807,106 +1806,41 @@ CDxfRead::CDxfRead(const char* filepath) CDxfRead::~CDxfRead() { delete m_ifs; - delete m_CodePage; - delete m_encoding; } -double CDxfRead::mm(double value) const -{ - // re #6461 - // this if handles situation of malformed DXF file where - // MEASUREMENT specifies English units, but - // INSUNITS specifies millimeters or is not specified - //(millimeters is our default) - if (m_measurement_inch && (m_eUnits == eMillimeters)) { - value *= 25.4; - } - - switch (m_eUnits) { - case eUnspecified: - return (value * 1.0); // We don't know any better. - case eInches: - return (value * 25.4); - case eFeet: - return (value * 25.4 * 12); - case eMiles: - return (value * 1609344.0); - case eMillimeters: - return (value * 1.0); - case eCentimeters: - return (value * 10.0); - case eMeters: - return (value * 1000.0); - case eKilometers: - return (value * 1000000.0); - case eMicroinches: - return (value * 25.4 / 1000.0); - case eMils: - return (value * 25.4 / 1000.0); - case eYards: - return (value * 3 * 12 * 25.4); - case eAngstroms: - return (value * 0.0000001); - case eNanometers: - return (value * 0.000001); - case eMicrons: - return (value * 0.001); - case eDecimeters: - return (value * 100.0); - case eDekameters: - return (value * 10000.0); - case eHectometers: - return (value * 100000.0); - case eGigameters: - return (value * 1000000000000.0); - case eAstronomicalUnits: - return (value * 149597870690000.0); - case eLightYears: - return (value * 9454254955500000000.0); - case eParsecs: - return (value * 30856774879000000000.0); - default: - return (value * 1.0); // We don't know any better. - } // End switch -} // End mm() method // // Setup for ProcessCommonEntityAttribute -void CDxfRead::Setup3DVectorAttribute(int x_record_type, double destination[3]) +void CDxfRead::Setup3DVectorAttribute(eDXFGroupCode_t x_record_type, Base::Vector3d& destination) { - SetupScaledDoubleAttribute(x_record_type, destination[0]); - SetupScaledDoubleAttribute(x_record_type + 10, destination[1]); - SetupScaledDoubleAttribute(x_record_type + 20, destination[2]); - destination[0] = destination[1] = destination[2] = 0.0; + SetupScaledDoubleAttribute((eDXFGroupCode_t)(x_record_type + eXOffset), destination.x); + SetupScaledDoubleAttribute((eDXFGroupCode_t)(x_record_type + eYOffset), destination.y); + SetupScaledDoubleAttribute((eDXFGroupCode_t)(x_record_type + eZOffset), destination.z); } -void CDxfRead::Setup3DCoordinatesIntoLists(int x_record_type, +void CDxfRead::Setup3DCoordinatesIntoLists(eDXFGroupCode_t x_record_type, list& x_destination, list& y_destination, list& z_destination) { - SetupScaledDoubleIntoList(x_record_type, x_destination); - SetupScaledDoubleIntoList(x_record_type + 10, y_destination); - SetupScaledDoubleIntoList(x_record_type + 20, z_destination); + SetupScaledDoubleIntoList((eDXFGroupCode_t)(x_record_type + eXOffset), x_destination); + SetupScaledDoubleIntoList((eDXFGroupCode_t)(x_record_type + eYOffset), y_destination); + SetupScaledDoubleIntoList((eDXFGroupCode_t)(x_record_type + eZOffset), z_destination); } -void CDxfRead::SetupScaledDoubleAttribute(int x_record_type, double& destination) +void CDxfRead::SetupScaledDoubleAttribute(eDXFGroupCode_t x_record_type, double& destination) { m_coordinate_attributes.emplace(x_record_type, std::pair(&ProcessScaledDouble, &destination)); } -void CDxfRead::SetupScaledDoubleIntoList(int x_record_type, list& destination) +void CDxfRead::SetupScaledDoubleIntoList(eDXFGroupCode_t x_record_type, list& destination) { m_coordinate_attributes.emplace(x_record_type, std::pair(&ProcessScaledDoubleIntoList, &destination)); } -void CDxfRead::SetupStringAttribute(int x_record_type, char* destination) -{ - m_coordinate_attributes.emplace(x_record_type, std::pair(&ProcessString, destination)); -} -void CDxfRead::SetupStringAttribute(int x_record_type, std::string& destination) +void CDxfRead::SetupStringAttribute(eDXFGroupCode_t x_record_type, std::string& destination) { m_coordinate_attributes.emplace(x_record_type, std::pair(&ProcessStdString, &destination)); } template -void CDxfRead::SetupValueAttribute(int record_type, T& destination) +void CDxfRead::SetupValueAttribute(eDXFGroupCode_t record_type, T& destination) { m_coordinate_attributes.emplace(record_type, std::pair(&ProcessValue, &destination)); } @@ -1919,12 +1853,11 @@ void CDxfRead::ProcessScaledDouble(CDxfRead* object, void* target) ss.imbue(std::locale("C")); ss.str(object->m_record_data); - double value; + double value = 0; ss >> value; if (ss.fail()) { object->ImportError("Unable to parse value '%s', using zero as its value\n", object->m_record_data); - value = 0; } *static_cast(target) = object->mm(value); } @@ -1934,17 +1867,16 @@ void CDxfRead::ProcessScaledDoubleIntoList(CDxfRead* object, void* target) ss.imbue(std::locale("C")); ss.str(object->m_record_data); - double value; + double value = 0; ss >> value; if (ss.fail()) { object->ImportError("Unable to parse value '%s', using zero as its value\n", object->m_record_data); - value = 0; } static_cast*>(target)->push_back(object->mm(value)); } template -void CDxfRead::ProcessValue(CDxfRead* object, void* target) +bool CDxfRead::ParseValue(CDxfRead* object, void* target) { std::istringstream ss; ss.imbue(std::locale("C")); @@ -1955,16 +1887,15 @@ void CDxfRead::ProcessValue(CDxfRead* object, void* target) object->ImportError("Unable to parse value '%s', using zero as its value\n", object->m_record_data); *static_cast(target) = 0; + return false; } + // TODO: Verify nothing it left but whitespace in ss. + return true; } void CDxfRead::ProcessStdString(CDxfRead* object, void* target) { *static_cast(target) = object->m_record_data; } -void CDxfRead::ProcessString(CDxfRead* object, void* target) -{ - strcpy(static_cast(target), object->m_record_data); -} void CDxfRead::InitializeAttributes() { @@ -1973,12 +1904,12 @@ void CDxfRead::InitializeAttributes() void CDxfRead::InitializeCommonEntityAttributes() { InitializeAttributes(); - strcpy(m_layer_name, "0"); - m_LineType[0] = '\0'; + m_layer_name.clear(); + m_LineType.clear(); m_ColorIndex = ColorBylayer; - SetupStringAttribute(6, m_LineType); - SetupStringAttribute(8, m_layer_name); - SetupValueAttribute(62, m_ColorIndex); + SetupStringAttribute(eLinetypeName, m_LineType); + SetupStringAttribute(eLayerName, m_layer_name); + SetupValueAttribute(eColor, m_ColorIndex); } bool CDxfRead::ProcessAttribute() { @@ -1991,7 +1922,7 @@ bool CDxfRead::ProcessAttribute() } void CDxfRead::ProcessAllAttributes() { - while (get_next_record() && m_record_type != 0) { + while (get_next_record() && m_record_type != eObjectType) { ProcessAttribute(); } repeat_last_record(); @@ -2002,9 +1933,10 @@ void CDxfRead::ProcessAllAttributes() // These return false if they catch an exception and ignore it because of ignore_errors. bool CDxfRead::ReadLine() { - double start[3], end[3]; - Setup3DVectorAttribute(10, start); - Setup3DVectorAttribute(11, end); + Base::Vector3d start; + Base::Vector3d end; + Setup3DVectorAttribute(ePrimaryPoint, start); + Setup3DVectorAttribute(ePoint2, end); ProcessAllAttributes(); OnReadLine(start, end, LineTypeIsHidden()); @@ -2013,8 +1945,8 @@ bool CDxfRead::ReadLine() bool CDxfRead::ReadPoint() { - double location[3]; - Setup3DVectorAttribute(10, location); + Base::Vector3d location; + Setup3DVectorAttribute(ePrimaryPoint, location); ProcessAllAttributes(); OnReadPoint(location); @@ -2026,21 +1958,21 @@ bool CDxfRead::ReadArc() double start_angle_degrees = 0; double end_angle_degrees = 0; double radius = 0; - double centre[3]; - double z_extrusion_dir = 1.0; + Base::Vector3d centre; + Base::Vector3d extrusionDirection(0, 0, 1); - Setup3DVectorAttribute(10, centre); - SetupScaledDoubleAttribute(40, radius); - SetupValueAttribute(50, start_angle_degrees); - SetupValueAttribute(51, end_angle_degrees); - SetupValueAttribute(230, z_extrusion_dir); + Setup3DVectorAttribute(ePrimaryPoint, centre); + SetupScaledDoubleAttribute(eFloat1, radius); + SetupValueAttribute(eAngleDegrees1, start_angle_degrees); + SetupValueAttribute(eAngleDegrees2, end_angle_degrees); + Setup3DVectorAttribute(eExtrusionDirection, extrusionDirection); ProcessAllAttributes(); OnReadArc(start_angle_degrees, end_angle_degrees, radius, centre, - z_extrusion_dir, + extrusionDirection.z, LineTypeIsHidden()); return true; } @@ -2054,18 +1986,18 @@ bool CDxfRead::ReadSpline() sd.control_points = 0; sd.fit_points = 0; - Setup3DVectorAttribute(210, sd.norm); - SetupValueAttribute(70, sd.flag); - SetupValueAttribute(71, sd.degree); - SetupValueAttribute(72, sd.knots); - SetupValueAttribute(73, sd.control_points); - SetupValueAttribute(74, sd.fit_points); - SetupScaledDoubleIntoList(40, sd.knot); - SetupScaledDoubleIntoList(41, sd.weight); - Setup3DCoordinatesIntoLists(10, sd.controlx, sd.controly, sd.controlz); - Setup3DCoordinatesIntoLists(11, sd.fitx, sd.fity, sd.fitz); - Setup3DCoordinatesIntoLists(12, sd.starttanx, sd.starttany, sd.starttanz); - Setup3DCoordinatesIntoLists(12, sd.endtanx, sd.endtany, sd.endtanz); + Setup3DVectorAttribute(eExtrusionDirection, sd.norm); + SetupValueAttribute(eInteger1, sd.flag); + SetupValueAttribute(eInteger2, sd.degree); + SetupValueAttribute(eInteger3, sd.knots); + SetupValueAttribute(eInteger4, sd.control_points); + SetupValueAttribute(eInteger5, sd.fit_points); + SetupScaledDoubleIntoList(eFloat1, sd.knot); + SetupScaledDoubleIntoList(eFloat2, sd.weight); + Setup3DCoordinatesIntoLists(ePrimaryPoint, sd.controlx, sd.controly, sd.controlz); + Setup3DCoordinatesIntoLists(ePoint2, sd.fitx, sd.fity, sd.fitz); + Setup3DCoordinatesIntoLists(ePoint3, sd.starttanx, sd.starttany, sd.starttanz); + Setup3DCoordinatesIntoLists(ePoint4, sd.endtanx, sd.endtany, sd.endtanz); ProcessAllAttributes(); OnReadSpline(sd); @@ -2075,10 +2007,10 @@ bool CDxfRead::ReadSpline() bool CDxfRead::ReadCircle() { double radius = 0.0; - double centre[3]; + Base::Vector3d centre; - Setup3DVectorAttribute(10, centre); - SetupScaledDoubleAttribute(40, radius); + Setup3DVectorAttribute(ePrimaryPoint, centre); + SetupScaledDoubleAttribute(eFloat1, radius); ProcessAllAttributes(); OnReadCircle(centre, radius, LineTypeIsHidden()); @@ -2087,42 +2019,41 @@ bool CDxfRead::ReadCircle() bool CDxfRead::ReadText() { - double insertionPoint[3]; + Base::Vector3d insertionPoint; + // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers) double height = 0.03082; double rotation = 0; std::string textPrefix; - Setup3DVectorAttribute(10, insertionPoint); - SetupScaledDoubleAttribute(40, height); - SetupValueAttribute(50, rotation); - while (get_next_record() && m_record_type != 0) { + Setup3DVectorAttribute(ePrimaryPoint, insertionPoint); + SetupScaledDoubleAttribute(eFloat1, height); + SetupValueAttribute(eAngleDegrees1, rotation); + while (get_next_record() && m_record_type != eObjectType) { if (!ProcessAttribute()) { switch (m_record_type) { - case 3: + case eExtraText: // Additional text that goes before the type 1 text // Note that if breaking the text into type-3 records splits a UFT-8 encoding we // do the decoding after splicing the lines together. I'm not sure if this // actually occurs, but handling the text this way will treat this condition // properly. + case ePrimaryText: + // final text is treated the same. + // ORDER: We are asusming the type 1 record follows all the type 3's. textPrefix.append(m_record_data); break; - case 1: - // final text - textPrefix.append(m_record_data); + default: break; } } } - const char* utfStr = (this->*stringToUTF8)(textPrefix.c_str()); - if (utfStr != nullptr) { - OnReadText(insertionPoint, height * 25.4 / 72.0, utfStr, rotation); - if (utfStr != textPrefix.c_str()) { - delete utfStr; - } + if ((this->*stringToUTF8)(textPrefix)) { + OnReadText(insertionPoint, height * 25.4 / 72.0, textPrefix, rotation); + // NOLINTEND(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers) } else { - ImportError("Unable to process encoding for TEXT/MTEXT '%s'", textPrefix.c_str()); + ImportError("Unable to process encoding for TEXT/MTEXT '%s'", textPrefix); } repeat_last_record(); return true; @@ -2130,17 +2061,17 @@ bool CDxfRead::ReadText() bool CDxfRead::ReadEllipse() { - double centre[3]; - double majorAxisEnd[3]; // relative to centre + Base::Vector3d centre; + Base::Vector3d majorAxisEnd; // relative to centre double eccentricity = 0; double startAngleRadians = 0; double endAngleRadians = 2 * M_PI; - Setup3DVectorAttribute(10, centre); - Setup3DVectorAttribute(11, majorAxisEnd); - SetupValueAttribute(40, eccentricity); - SetupValueAttribute(41, startAngleRadians); - SetupValueAttribute(42, endAngleRadians); + Setup3DVectorAttribute(ePrimaryPoint, centre); + Setup3DVectorAttribute(ePoint2, majorAxisEnd); + SetupValueAttribute(eFloat1, eccentricity); + SetupValueAttribute(eFloat2, startAngleRadians); + SetupValueAttribute(eFloat3, endAngleRadians); ProcessAllAttributes(); OnReadEllipse(centre, majorAxisEnd, eccentricity, startAngleRadians, endAngleRadians); @@ -2149,7 +2080,7 @@ bool CDxfRead::ReadEllipse() bool CDxfRead::ReadLwPolyLine() { - VertexInfo currentVertex = {{0, 0, 0}, 0}; + VertexInfo currentVertex; list vertices; int flags = 0; @@ -2165,24 +2096,23 @@ bool CDxfRead::ReadLwPolyLine() // stroke and the X/Y coordinates for the vertex that ends the stroke or the end of the entity. // In the latter case the stroke attributes apply to the closure stroke (if any) which ends at // the first vertex. - Setup3DVectorAttribute(10, currentVertex.location); - SetupScaledDoubleAttribute(42, currentVertex.bulge); - SetupValueAttribute(70, flags); - while (get_next_record() && m_record_type != 0) { - if ((m_record_type == 10 && have_x) || (m_record_type == 21 && have_y)) { + Setup3DVectorAttribute(ePrimaryPoint, currentVertex.location); + SetupScaledDoubleAttribute(eFloat3, currentVertex.bulge); + SetupValueAttribute(eInteger1, flags); + while (get_next_record() && m_record_type != eObjectType) { + if ((m_record_type == ePrimaryPoint + eXOffset && have_x) + || (m_record_type == ePrimaryPoint + eYOffset && have_y)) { // Starting a new vertex and there is a previous vertex. Save it and init a new one. vertices.push_back(currentVertex); - currentVertex.location[0] = 0.0; - currentVertex.location[1] = 0.0; - currentVertex.location[2] = 0.0; + currentVertex.location = Base::Vector3d(); currentVertex.bulge = 0.0; - have_x = m_record_type == 10; - have_y = m_record_type == 20; + have_x = m_record_type == ePrimaryPoint + eXOffset; + have_y = m_record_type == ePrimaryPoint + eYOffset; } - else if (m_record_type == 10) { + else if (m_record_type == ePrimaryPoint + eXOffset) { have_x = true; } - else if (m_record_type == 10) { + else if (m_record_type == ePrimaryPoint + eYOffset) { have_y = true; } ProcessAttribute(); @@ -2200,28 +2130,26 @@ bool CDxfRead::ReadLwPolyLine() bool CDxfRead::ReadPolyLine() { - VertexInfo currentVertex = {{0, 0, 0}, 0}; + VertexInfo currentVertex; list vertices; int flags = 0; - SetupValueAttribute(70, flags); + SetupValueAttribute(eInteger1, flags); ProcessAllAttributes(); // We are now followed by a series of VERTEX entities followed by ENDSEQ. // To avoid eating and discarding the rest of the entieies if ENDSEQ is missing, // we quit on any unknown type-0 record. - Setup3DVectorAttribute(10, currentVertex.location); - SetupScaledDoubleAttribute(42, currentVertex.bulge); - while (get_next_record() && m_record_type == 0 && !strcmp(m_record_data, "VERTEX")) { + Setup3DVectorAttribute(ePrimaryPoint, currentVertex.location); + SetupScaledDoubleAttribute(eFloat3, currentVertex.bulge); + while (get_next_record() && m_record_type == eObjectType && IsObjectName("VERTEX")) { // Set vertex defaults - currentVertex.location[0] = 0.0; - currentVertex.location[1] = 0.0; - currentVertex.location[2] = 0.0; + currentVertex.location = Base::Vector3d(); currentVertex.bulge = 0.0; ProcessAllAttributes(); vertices.push_back(currentVertex); } - if (strcmp(m_record_data, "SEQEND")) { + if (!IsObjectName("SEQEND")) { ImportError("POLYLINE ends with '%s' record rather than 'SEQEND'\n", m_record_data); repeat_last_record(); } @@ -2232,28 +2160,28 @@ bool CDxfRead::ReadPolyLine() bool CDxfRead::ReadInsert() { - double center[3]; - double scale[3] = {1, 1, 1}; + Base::Vector3d center; + Base::Vector3d scale(1, 1, 1); double rotationDegrees = 0.0; - char blockName[1024] = {0}; + std::string blockName; - Setup3DVectorAttribute(10, center); - SetupValueAttribute(41, scale[0]); - SetupValueAttribute(42, scale[1]); - SetupValueAttribute(43, scale[2]); - SetupValueAttribute(50, rotationDegrees); - SetupStringAttribute(2, blockName); + Setup3DVectorAttribute(ePrimaryPoint, center); + SetupValueAttribute(eFloat2, scale.x); + SetupValueAttribute(eFloat3, scale.y); + SetupValueAttribute(eFloat4, scale.z); + SetupValueAttribute(eAngleDegrees1, rotationDegrees); + SetupStringAttribute(eName, blockName); ProcessAllAttributes(); - OnReadInsert(center, scale, blockName, rotationDegrees * M_PI / 180); + OnReadInsert(center, scale, blockName, Base::toRadians(rotationDegrees)); return (true); } bool CDxfRead::ReadDimension() { - double start[3]; - double end[3]; - double linePosition[3]; - double textPosition[3]; + Base::Vector3d start; + Base::Vector3d end; + Base::Vector3d linePosition; + Base::Vector3d textPosition; double rotation = 0; int dimensionType = 0; // This appears to default to zero @@ -2265,25 +2193,25 @@ bool CDxfRead::ReadDimension() // 50 is the rotation of the dimension (the direction of the dimension line) // 52 (if present) is the angle relative to the dimension line for the extension lines; default // 90 degrees - Setup3DVectorAttribute(13, start); // WCS - Setup3DVectorAttribute(14, end); // WCS - Setup3DVectorAttribute(10, linePosition); // WCS - Setup3DVectorAttribute(11, textPosition); // OCS - SetupValueAttribute(50, rotation); - SetupValueAttribute(70, dimensionType); + Setup3DVectorAttribute(ePoint4, start); // WCS + Setup3DVectorAttribute(ePoint5, end); // WCS + Setup3DVectorAttribute(ePrimaryPoint, linePosition); // WCS + Setup3DVectorAttribute(ePoint2, textPosition); // OCS + SetupValueAttribute(eAngleDegrees1, rotation); + SetupValueAttribute(eInteger1, dimensionType); ProcessAllAttributes(); - dimensionType &= ~0xE0; // Remove flags - switch (dimensionType) { - case 0: - case 1: - OnReadDimension(start, end, linePosition, rotation * M_PI / 180); + dimensionType &= eTypeMask; // Remove flags + switch ((eDimensionType_t)dimensionType) { + case eLinear: + case eAligned: + OnReadDimension(start, end, linePosition, Base::toRadians(rotation)); break; default: UnsupportedFeature("Dimension type '%d'", dimensionType); break; } - return (true); + return true; } bool CDxfRead::ReadUnknownEntity() @@ -2298,9 +2226,9 @@ bool CDxfRead::ReadBlockInfo() int blockType = 0; InitializeAttributes(); // Both 2 and 3 are the block name. - SetupStringAttribute(2, m_block_name); - SetupStringAttribute(3, m_block_name); - SetupValueAttribute(70, blockType); + SetupStringAttribute(eName, m_block_name); + SetupStringAttribute(eExtraText, m_block_name); + SetupValueAttribute(eInteger1, blockType); ProcessAllAttributes(); // Read the entities in the block definition. @@ -2310,17 +2238,17 @@ bool CDxfRead::ReadBlockInfo() // Note that the Layer Name in the block entities is ignored even though it // should be used to resolve BYLAYER attributes and also for placing the entity // when the block is inserted. - if (blockType & 0x04) { + if ((blockType & 0x04) != 0) { // Note that this doesn't mean there are not entities in the block. I don't // know if the external reference can be cached because there are two other bits // here, 0x10 and 0x20, that seem to handle "resolved" external references. UnsupportedFeature("External (xref) BLOCK"); } - while (get_next_record() && m_record_type != 0 && strcmp(m_record_data, "ENDBLK")) { - if (blockType & 0x01) { + while (get_next_record() && m_record_type != eObjectType && !IsObjectName("ENDBLK")) { + if ((blockType & 0x01) != 0) { // It is an anonymous block used to build dimensions, hatches, etc so we don't need it // and don't want to complaining about unhandled entity types. - while (get_next_record() && m_record_type != 0) {} + while (get_next_record() && m_record_type != eObjectType) {} repeat_last_record(); } else if (IgnoreErrors()) { @@ -2341,22 +2269,11 @@ bool CDxfRead::ReadBlockInfo() return true; } -void CDxfRead::UnsupportedFeature(const char* format, ...) +template +void CDxfRead::UnsupportedFeature(const char* format, args&&... argValuess) { - // Upcoming C++20 std::format might simplify some of this code - va_list args; - std::string formattedMessage; - - va_start(args, format); - // Make room in formattedMessage for the formatted text and its terminating null byte - formattedMessage.resize((size_t)vsnprintf(nullptr, 0, format, args) + 1); - va_end(args); - va_start(args, format); - vsprintf(formattedMessage.data(), format, args); - va_end(args); - // Remove the null byte - formattedMessage.pop_back(); - + // NOLINTNEXTLINE(runtime/printf) + std::string formattedMessage = fmt::sprintf(format, std::forward(argValuess)...); // We place these formatted messages in a map, count their occurrences and not their first // occurrence. if (m_unsupportedFeaturesNoted[formattedMessage].first++ == 0) { @@ -2376,24 +2293,26 @@ bool CDxfRead::get_next_record() return false; } - m_ifs->getline(m_record_data, 1024); + std::getline(*m_ifs, m_record_data); ++m_line; - if (sscanf(m_record_data, "%d", &m_record_type) != 1) { + int temp = 0; + if (!ParseValue(this, &temp)) { ImportError("CDxfRead::get_next_record() Failed to get integer record type from '%s'\n", m_record_data); - return (false); + return false; } + m_record_type = (eDXFGroupCode_t)temp; if ((*m_ifs).eof()) { return false; } - m_ifs->getline(m_record_data, 1024); + std::getline(*m_ifs, m_record_data); ++m_line; // Remove any carriage return at the end of m_str which may occur because of inconsistent // handling of LF vs. CRLF line termination. - size_t len = strlen(m_record_data); - if (len > 0 && m_record_data[len - 1] == '\r') { - m_record_data[len - 1] = '\0'; + auto last = m_record_data.rbegin(); + if (last != m_record_data.rend() && *last == '\r') { + m_record_data.pop_back(); } // The code that was here just blindly trimmed leading white space, but if you have, for // instance, a TEXT entity whose text starts with spaces, or, more plausibly, a long TEXT entity @@ -2407,23 +2326,6 @@ void CDxfRead::repeat_last_record() m_repeat_last_record = true; } -bool CDxfRead::ReadUnits() -{ - get_next_record(); // Get the value for the variable - int n = 0; - if (sscanf(m_record_data, "%d", &n) == 1) { - m_eUnits = eDxfUnits_t(n); - if (m_eUnits != eUnspecified) { - m_measurement_inch = false; // prioritize INSUNITS over MEASUREMENT variable - } - return (true); - } // End if - then - else { - ImportError("CDxfRead::ReadUnits() Failed to get integer from '%s'\n", m_record_data); - return (false); - } -} - // // Intercepts for On... calls to derived class // (These have distinct signatures from the ones they call) @@ -2445,14 +2347,14 @@ bool CDxfRead::OnReadPolyline(std::list& vertices, int flags) if (startVertex != vertices.end()) { if (startVertex->bulge != 0.0) { // Bulge is 1/4 tan(arc angle), positive for CCW arc. - double cot = 0.5 * ((1.0 / startVertex->bulge) - startVertex->bulge); - double cx = ((startVertex->location[0] + endVertex->location[0]) - - ((endVertex->location[1] - startVertex->location[1]) * cot)) - / 2.0; - double cy = ((startVertex->location[1] + endVertex->location[1]) - + ((endVertex->location[0] - startVertex->location[0]) * cot)) - / 2.0; - double pc[3] = {cx, cy, (startVertex->location[2] + endVertex->location[2]) / 2.0}; + double cot = ((1.0 / startVertex->bulge) - startVertex->bulge) / 2; + double cx = ((startVertex->location.x + endVertex->location.x) + - ((endVertex->location.y - startVertex->location.y) * cot)) + / 2; + double cy = ((startVertex->location.y + endVertex->location.y) + + ((endVertex->location.x - startVertex->location.x) * cot)) + / 2; + Base::Vector3d pc(cx, cy, (startVertex->location.z + endVertex->location.z) / 2); OnReadArc(startVertex->location, endVertex->location, pc, @@ -2470,61 +2372,59 @@ bool CDxfRead::OnReadPolyline(std::list& vertices, int flags) void CDxfRead::OnReadArc(double start_angle, double end_angle, double radius, - const double* center, + const Base::Vector3d& center, double z_extrusion_dir, bool hidden) { - double s[3] = {0, 0, 0}, e[3] = {0, 0, 0}, temp[3] = {0, 0, 0}; - if (z_extrusion_dir == 1.0) { - temp[0] = center[0]; - temp[1] = center[1]; - temp[2] = center[2]; - s[0] = center[0] + radius * cos(start_angle * M_PI / 180); - s[1] = center[1] + radius * sin(start_angle * M_PI / 180); - s[2] = center[2]; - e[0] = center[0] + radius * cos(end_angle * M_PI / 180); - e[1] = center[1] + radius * sin(end_angle * M_PI / 180); - e[2] = center[2]; + Base::Vector3d temp(center); + // Calculate the start and end points of the arc + Base::Vector3d start(center); + start.x += radius * cos(Base::toRadians(start_angle)); + start.y += radius * sin(Base::toRadians(start_angle)); + Base::Vector3d end(center); + end.x += radius * cos(Base::toRadians(end_angle)); + end.y += radius * sin(Base::toRadians(end_angle)); + if (z_extrusion_dir < 0) { + // This is a dumbed-down handling of general OCS. This only works + // for arcs drawn exactly upside down (i.e. with the extrusion vector + // being (0, 0, <0). + // We treat this as 180-degree mirroring through the YZ plane + // TODO: I don't even think this is correct, but it is functionally what the + // code did before. + temp.x = -temp.x; + start.x = -start.x; + end.x = -end.x; } - else { - temp[0] = -center[0]; - temp[1] = center[1]; - temp[2] = center[2]; - - e[0] = -(center[0] + radius * cos(start_angle * M_PI / 180)); - e[1] = (center[1] + radius * sin(start_angle * M_PI / 180)); - e[2] = center[2]; - s[0] = -(center[0] + radius * cos(end_angle * M_PI / 180)); - s[1] = (center[1] + radius * sin(end_angle * M_PI / 180)); - s[2] = center[2]; - } - OnReadArc(s, e, temp, true, hidden); + OnReadArc(start, end, temp, true, hidden); } -void CDxfRead::OnReadCircle(const double* center, double radius, bool hidden) +void CDxfRead::OnReadCircle(const Base::Vector3d& center, double radius, bool hidden) { // OnReadCircle wants a start point, so we pick an arbitrary point on the circumference - double s[3] = {center[0] + radius, center[1], center[2]}; + Base::Vector3d start(center); + start.x += radius; - OnReadCircle(s, + OnReadCircle(start, center, false, hidden); // false to change direction because otherwise the arc length is zero } -void CDxfRead::OnReadEllipse(const double* center, - const double* m, +// NOLINTBEGIN(bugprone-easily-swappable-parameters) +void CDxfRead::OnReadEllipse(const Base::Vector3d& center, + const Base::Vector3d& majorAxisEnd, double ratio, double start_angle, double end_angle) +// NOLINTEND(bugprone-easily-swappable-parameters) { - double major_radius = sqrt(m[0] * m[0] + m[1] * m[1] + m[2] * m[2]); + double major_radius = majorAxisEnd.Length(); double minor_radius = major_radius * ratio; // Since we only support 2d stuff, we can calculate the rotation from the major axis x and y // value only, since z is zero, major_radius is the vector length - double rotation = atan2(m[1] / major_radius, m[0] / major_radius); + double rotation = atan2(majorAxisEnd.y, majorAxisEnd.x); OnReadEllipse(center, major_radius, minor_radius, rotation, start_angle, end_angle, true); @@ -2547,9 +2447,9 @@ bool CDxfRead::ReadVersion() assert(VersionNames.size() == RNewer - ROlder - 1); get_next_record(); // Get the value for the variable - std::vector::const_iterator first = VersionNames.cbegin(); - std::vector::const_iterator last = VersionNames.cend(); - std::vector::const_iterator found = std::lower_bound(first, last, m_record_data); + auto first = VersionNames.cbegin(); + auto last = VersionNames.cend(); + auto found = std::lower_bound(first, last, m_record_data); if (found == last) { m_version = RNewer; } @@ -2568,39 +2468,36 @@ bool CDxfRead::ReadVersion() bool CDxfRead::ReadDWGCodePage() { - get_next_record(); // Get the value for the variable - assert(m_CodePage == nullptr); // If not, we have found two DWGCODEPAGE variables or DoRead was - // called twice on the same CDxfRead object. - m_CodePage = new std::string(m_record_data); + get_next_record(); // Get the value for the variable + assert(m_CodePage.empty()); // If not, we have found two DWGCODEPAGE variables or DoRead + // was called twice on the same CDxfRead object. + m_CodePage = m_record_data; return ResolveEncoding(); } bool CDxfRead::ResolveEncoding() { - if (m_encoding != nullptr) { - delete m_encoding; - m_encoding = nullptr; - } if (m_version >= R2007) { // Note this does not include RUnknown, but does include RLater - m_encoding = new std::string("utf_8"); + m_encoding = "utf_8"; stringToUTF8 = &CDxfRead::UTF8ToUTF8; } - else if (m_CodePage == nullptr) { + else if (m_CodePage.empty()) { // cp1252 - m_encoding = new std::string("cp1252"); + m_encoding = "cp1252"; stringToUTF8 = &CDxfRead::GeneralToUTF8; } else { // Codepage names may be of the form "ansi_1252" which we map to "cp1252" but we don't map // "ansi_x3xxxx" (which happens to mean "ascii") // Also some DXF files have the codepage name in uppercase so we lowercase it. - std::string* p = new std::string(*m_CodePage); - std::transform(p->begin(), p->end(), p->begin(), ::tolower); - if (strncmp(p->c_str(), "ansi_", 5) == 0 && strncmp(p->c_str(), "ansi_x3", 7) != 0) { - p->replace(0, 5, "cp"); + m_encoding = m_CodePage; + std::transform(m_encoding.begin(), m_encoding.end(), m_encoding.begin(), ::tolower); + // NOLINTNEXTLINE(readability/nolint) +#define ANSI_ENCODING_PREFIX "ansi_" // NOLINT(cppcoreguidelines-macro-usage) + if (m_encoding.rfind(ANSI_ENCODING_PREFIX, 0) == 0 && m_encoding.rfind("ansi_x3", 0) != 0) { + m_encoding.replace(0, (sizeof ANSI_ENCODING_PREFIX) - 1, "cp"); } - m_encoding = p; // At this point we want to recognize synonyms for "utf_8" and use the custom decoder // function. This is because this is one of the common cases and our decoder function is a // fast no-op. We don't actually use the decoder function we get from PyCodec_Decoder @@ -2609,7 +2506,7 @@ bool CDxfRead::ResolveEncoding() // Python string, which we then decode back to UTF-8. It is simpler to call // PyUnicode_DecodeXxxx which takes a (const char *) and is just a direct c++ callable. Base::PyGILStateLocker lock; - PyObject* pyDecoder = PyCodec_Decoder(m_encoding->c_str()); + PyObject* pyDecoder = PyCodec_Decoder(m_encoding.c_str()); if (pyDecoder == nullptr) { return false; // A key error exception will have been placed. } @@ -2624,36 +2521,33 @@ bool CDxfRead::ResolveEncoding() Py_DECREF(pyDecoder); Py_DECREF(pyUTF8Decoder); } - return m_encoding != nullptr; + return !m_encoding.empty(); } -const char* CDxfRead::UTF8ToUTF8(const char* encoded) const +// NOLINTNEXTLINE(readability/nolint) +// NOLINTNEXTLINE(readability-convert-member-functions-to-static) +bool CDxfRead::UTF8ToUTF8(std::string& /*encoded*/) const { - return encoded; + return true; } -const char* CDxfRead::GeneralToUTF8(const char* encoded) const +bool CDxfRead::GeneralToUTF8(std::string& encoded) const { Base::PyGILStateLocker lock; - PyObject* decoded = PyUnicode_Decode(encoded, strlen(encoded), m_encoding->c_str(), "strict"); + PyObject* decoded = PyUnicode_Decode(encoded.c_str(), + (Py_ssize_t)encoded.length(), + m_encoding.c_str(), + "strict"); if (decoded == nullptr) { - return nullptr; + return false; } - Py_ssize_t len; - const char* converted = PyUnicode_AsUTF8AndSize(decoded, &len); - char* result = nullptr; + const char* converted = PyUnicode_AsUTF8(decoded); + // converted has the same lifetime as decoded so we don't have to delete it. if (converted != nullptr) { - // converted only has lifetime of decoded so we must save a copy. - result = (char*)malloc(len + 1); - if (result == nullptr) { - PyErr_SetString(PyExc_MemoryError, "Out of memory"); - } - else { - memcpy(result, converted, len + 1); - } + encoded = converted; } Py_DECREF(decoded); - return result; + return converted != nullptr; } void CDxfRead::DoRead(const bool ignore_errors /* = false */) @@ -2663,115 +2557,105 @@ void CDxfRead::DoRead(const bool ignore_errors /* = false */) return; } - // Loop reading and identifying the sections. + // Loop reading the sections. while (get_next_record()) { - if (m_record_type != 0) { + if (m_record_type != eObjectType) { ImportError("Found type %d record when expecting start of a SECTION or EOF\n", - m_record_type); + (int)m_record_type); continue; } - if (!strcmp(m_record_data, "EOF")) { // TODO: Check for drivel beyond EOF record + if (IsObjectName("EOF")) { // TODO: Check for drivel beyond EOF record break; } - if (strcmp(m_record_data, "SECTION")) { - ImportError("Found %s record when expecting start of a SECTION\n", m_record_data); + if (!IsObjectName("SECTION")) { + ImportError("Found %s record when expecting start of a SECTION\n", + m_record_data.c_str()); continue; } - - if (!get_next_record()) { - ImportError("Unclosed SECTION at end of file\n"); + if (!ReadSection()) { return; } - if (m_record_type != 2) { - ImportError("Ignored SECTION with no name record\n"); - if (!ReadIgnoredSection()) { - return; - } - } - else if (!strcmp(m_record_data, "HEADER")) { - if (!ReadHeaderSection()) { - return; - } - } - else if (!strcmp(m_record_data, "TABLES")) { - if (!ReadTablesSection()) { - return; - } - } - else if (!strcmp(m_record_data, "BLOCKS")) { - if (!ReadBlocksSection()) { - return; - } - } - else if (!strcmp(m_record_data, "ENTITIES")) { - if (!ReadEntitiesSection()) { - return; - } - } - else { - if (!ReadIgnoredSection()) { - return; - } - } } AddGraphics(); // FLush out any unsupported features messages - auto i = m_unsupportedFeaturesNoted.begin(); - if (i != m_unsupportedFeaturesNoted.end()) { + if (!m_unsupportedFeaturesNoted.empty()) { ImportError("Unsupported DXF features:\n"); - do { + for (auto& featureInfo : m_unsupportedFeaturesNoted) { ImportError("%s: %d time(s) first at line %d\n", - i->first, - i->second.first, - i->second.second); - } while (++i != m_unsupportedFeaturesNoted.end()); + featureInfo.first, + featureInfo.second.first, + featureInfo.second.second); + } } } +bool CDxfRead::ReadSection() +{ + if (!get_next_record()) { + ImportError("Unclosed SECTION at end of file\n"); + return false; + } + if (m_record_type != eName) { + ImportError("Ignored SECTION with no name record\n"); + return ReadIgnoredSection(); + } + if (IsObjectName("HEADER")) { + return ReadHeaderSection(); + } + if (IsObjectName("TABLES")) { + return ReadTablesSection(); + } + if (IsObjectName("BLOCKS")) { + return ReadBlocksSection(); + } + if (IsObjectName("ENTITIES")) { + return ReadEntitiesSection(); + } + return ReadIgnoredSection(); +} + bool CDxfRead::ReadEntity() { InitializeCommonEntityAttributes(); // The entity record is already the current record and is already checked as a type 0 record - if (!strcmp(m_record_data, "LINE")) { + if (IsObjectName("LINE")) { return ReadLine(); } - else if (!strcmp(m_record_data, "ARC")) { + if (IsObjectName("ARC")) { return ReadArc(); } - else if (!strcmp(m_record_data, "CIRCLE")) { + if (IsObjectName("CIRCLE")) { return ReadCircle(); } - else if (!strcmp(m_record_data, "MTEXT")) { + if (IsObjectName("MTEXT")) { return ReadText(); } - else if (!strcmp(m_record_data, "TEXT")) { + if (IsObjectName("TEXT")) { return ReadText(); } - else if (!strcmp(m_record_data, "ELLIPSE")) { + if (IsObjectName("ELLIPSE")) { return ReadEllipse(); } - else if (!strcmp(m_record_data, "SPLINE")) { + if (IsObjectName("SPLINE")) { return ReadSpline(); } - else if (!strcmp(m_record_data, "LWPOLYLINE")) { + if (IsObjectName("LWPOLYLINE")) { return ReadLwPolyLine(); } - else if (!strcmp(m_record_data, "POLYLINE")) { + if (IsObjectName("POLYLINE")) { return ReadPolyLine(); } - else if (!strcmp(m_record_data, "POINT")) { + if (IsObjectName("POINT")) { return ReadPoint(); } - else if (!strcmp(m_record_data, "INSERT")) { + if (IsObjectName("INSERT")) { return ReadInsert(); } - else if (!strcmp(m_record_data, "DIMENSION")) { + if (IsObjectName("DIMENSION")) { return ReadDimension(); } - else { - return ReadUnknownEntity(); - } + return ReadUnknownEntity(); } bool CDxfRead::ReadHeaderSection() @@ -2781,46 +2665,70 @@ bool CDxfRead::ReadHeaderSection() // the variable name, followed by a single record giving the value; the record type depends on // the variable's data type. while (get_next_record()) { - if (m_record_type == 0 && !strcmp(m_record_data, "ENDSEC")) { + if (m_record_type == eObjectType && IsObjectName("ENDSEC")) { + if (m_unitScalingFactor == 0.0) { + // Neither INSUNITS nor MEASUREMENT found, assume 1 DXF unit = 1mm + // TODO: Perhaps this default should depend on the current measuring units of the + // app. + m_unitScalingFactor = m_additionalScaling; + ImportObservation("No INSUNITS or MEASUREMENT; setting scaling to 1 DXF unit = " + "%gmm based on DXF scaling option\n", + m_unitScalingFactor); + } return true; } - else if (m_record_type != 9) { + if (m_record_type != eVariableName) { continue; // Quietly ignore unknown record types } - if (!strcmp(m_record_data, "$INSUNITS")) { - if (!ReadUnits()) { - return false; - } - } - else if (!strcmp(m_record_data, "$MEASUREMENT")) { - get_next_record(); - int n = 1; - if (sscanf(m_record_data, "%d", &n) == 1) { - if (n == 0) { - m_measurement_inch = true; - } - } - } - else if (!strcmp(m_record_data, "$ACADVER")) { - if (!ReadVersion()) { - return false; - } - } - else if (!strcmp(m_record_data, "$DWGCODEPAGE")) { - if (!ReadDWGCodePage()) { - return false; - } - } - else { - // any other variable, skip its value - if (!get_next_record()) { - return false; - } + if (!ReadVariable()) { + return false; } } return false; } +bool CDxfRead::ReadVariable() +{ + if (IsVariableName("$INSUNITS")) { + get_next_record(); // Get the value for the variable + int varValue = 0; + if (!ParseValue(this, &varValue)) { + ImportError("Failed to get integer from INSUNITS value '%s'\n", m_record_data); + } + else if (auto units = DxfUnits::eDxfUnits_t(varValue); !DxfUnits::IsValid(units)) { + ImportError("Unknown value '%d' for INSUNITS\n", varValue); + } + else { + m_unitScalingFactor = DxfUnits::Factor(units) * m_additionalScaling; + ImportObservation("Setting scaling to 1 DXF unit = %gmm based on INSUNITS and " + "DXF scaling option\n", + m_unitScalingFactor); + } + return true; + } + if (IsVariableName("$MEASUREMENT")) { + get_next_record(); + int varValue = 1; + if (m_unitScalingFactor == 0.0 && ParseValue(this, &varValue)) { + m_unitScalingFactor = + DxfUnits::Factor(varValue != 0 ? DxfUnits::eMillimeters : DxfUnits::eInches) + * m_additionalScaling; + ImportObservation("Setting scaling to 1 DXF unit = %gmm based on MEASUREMENT and " + "DXF scaling option\n", + m_unitScalingFactor); + } + return true; + } + if (IsVariableName("$ACADVER")) { + return ReadVersion(); + } + if (IsVariableName("$DWGCODEPAGE")) { + return ReadDWGCodePage(); + } + // any other variable, skip its value + return get_next_record(); +} + bool CDxfRead::ReadTablesSection() { // Read to the next ENDSEC record marking the end of the section. @@ -2828,20 +2736,21 @@ bool CDxfRead::ReadTablesSection() // record followed by a type-2 (name) record giving the table name, followed by the table // contents. Each set of contents is terminates by the next type-0 TABLE or ENDSEC directive. while (get_next_record()) { - if (m_record_type != 0) { + if (m_record_type != eObjectType) { continue; // Ignore any non-type-0 contents in the section. } - if (!strcmp(m_record_data, "ENDSEC")) { + if (IsObjectName("ENDSEC")) { return true; } - if (strcmp(m_record_data, "TABLE")) { + if (!IsObjectName("TABLE")) { continue; // Ignore any type-0 non-TABLE contents in the section } get_next_record(); - if (m_record_type != 2) { - ImportError("Found unexpected type %d record instead of table name\n", m_record_type); + if (m_record_type != eName) { + ImportError("Found unexpected type %d record instead of table name\n", + (int)m_record_type); } - else if (!strcmp(m_record_data, "LAYER")) { + else if (IsObjectName("LAYER")) { if (!ReadLayerTable()) { return false; } @@ -2859,7 +2768,7 @@ bool CDxfRead::ReadIgnoredSection() { // Read to the next ENDSEC record marking the end of the section. while (get_next_record()) { - if (m_record_type == 0 && !strcmp(m_record_data, "ENDSEC")) { + if (m_record_type == eObjectType && IsObjectName("ENDSEC")) { return true; } } @@ -2871,20 +2780,20 @@ bool CDxfRead::ReadBlocksSection() // Read to the next ENDSEC record marking the end of the section. // Within this section we should find type-0 BLOCK groups while (get_next_record()) { - if (m_record_type != 0) { + if (m_record_type != eObjectType) { continue; // quietly ignore non-type-0 records; } - if (!strcmp(m_record_data, "ENDSEC")) { + if (IsObjectName("ENDSEC")) { // End of section return true; } - if (strcmp(m_record_data, "BLOCK")) { + if (!IsObjectName("BLOCK")) { continue; // quietly ignore non-BLOCK records } if (!ReadBlockInfo()) { ImportError("CDxfRead::DoRead() Failed to read block\n"); } - m_block_name[0] = '\0'; + m_block_name.clear(); } return false; @@ -2895,10 +2804,10 @@ bool CDxfRead::ReadEntitiesSection() // Read to the next ENDSEC record marking the end of the section. // Within this section we should find type-0 BLOCK groups while (get_next_record()) { - if (m_record_type != 0) { + if (m_record_type != eObjectType) { continue; // quietly ignore non-type-0 records; } - if (!strcmp(m_record_data, "ENDSEC")) { + if (IsObjectName("ENDSEC")) { // End of section return true; } @@ -2928,15 +2837,15 @@ bool CDxfRead::ReadLayer() int layerFlags = 0; InitializeAttributes(); - SetupStringAttribute(2, layername); - SetupValueAttribute(62, layerColor); - SetupValueAttribute(70, layerFlags); + SetupStringAttribute(eName, layername); + SetupValueAttribute(eColor, layerColor); + SetupValueAttribute(eInteger1, layerFlags); ProcessAllAttributes(); if (layername.empty()) { ImportError("CDxfRead::ReadLayer() - no layer name\n"); return false; } - if (layerFlags & 0x01) { + if ((layerFlags & 0x01) != 0) { // Frozen layers are implicitly hidden which we don't do yet. // TODO: Should have an import option to omit frozen layers. UnsupportedFeature("Frozen layers"); @@ -2954,21 +2863,20 @@ bool CDxfRead::ReadLayerTable() // ENDSEC record marking the end of the TABLES section. This table contains a series of type-0 // LAYER groups while (get_next_record()) { - if (m_record_type != 0) { + if (m_record_type != eObjectType) { continue; // quietly ignore non-type-0 records; this table has some preamble } - if (!strcmp(m_record_data, "TABLE") || !strcmp(m_record_data, "ENDSEC")) { + if (IsObjectName("TABLE") || IsObjectName("ENDSEC")) { // End of table repeat_last_record(); return true; } - if (strcmp(m_record_data, "LAYER")) { + if (!IsObjectName("LAYER")) { continue; // quietly ignore non-LAYER records } if (!ReadLayer()) { ImportError("CDxfRead::DoRead() Failed to read layer\n"); } - continue; } return false; @@ -2979,8 +2887,7 @@ bool CDxfRead::ReadIgnoredTable() // Read to the next TABLE record indicating another table in the TABLES section, or to the // ENDSEC record marking the end of the TABLES section. while (get_next_record()) { - if (m_record_type == 0 - && (!strcmp(m_record_data, "TABLE") || !strcmp(m_record_data, "ENDSEC"))) { + if (m_record_type == eObjectType && (IsObjectName("TABLE") || IsObjectName("ENDSEC"))) { repeat_last_record(); return true; } @@ -2992,13 +2899,13 @@ std::string CDxfRead::LayerName() const { std::string result; - if (strlen(m_block_name) > 0) { + if (!m_block_name.empty()) { result.append("BLOCKS "); result.append(m_block_name); result.append(" "); } - else if (strlen(m_layer_name) > 0) { + else if (!m_layer_name.empty()) { result.append("ENTITIES "); result.append(m_layer_name); } @@ -3006,6 +2913,7 @@ std::string CDxfRead::LayerName() const return (result); } +// NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers) inline static double level(int distance, double blackLevel) { // Distance is the number of 24ths around the color wheel between the desired hue and @@ -3023,28 +2931,32 @@ inline static double level(int distance, double blackLevel) // A distance 4 or less givs full intensity of the primary color return 1.0; } - else if (distance < 8) { + if (distance < 8) { // Between 4 and 8 gives a blend of the full primary and the black level return ((8 - distance) + blackLevel * (distance - 4)) / 4; } - else { - // 8 and beyond yield the black level - return blackLevel; - } + // 8 and beyond yield the black level + return blackLevel; } inline static App::Color wheel(int hue, double blackLevel, double multiplier = 1.0) { - return App::Color(level(hue - 0, blackLevel) * multiplier, - level(hue - 8, blackLevel) * multiplier, - level(hue - 16, blackLevel) * multiplier); + return App::Color((float)(level(hue - 0, blackLevel) * multiplier), + (float)(level(hue - 8, blackLevel) * multiplier), + (float)(level(hue - 16, blackLevel) * multiplier)); } App::Color CDxfRead::ObjectColor() const { int index = m_ColorIndex; if (index == ColorBylayer) // if color = layer color, replace by color from layer { - auto key = std::string(m_layer_name); - index = m_layer_ColorIndex_map.count(key) > 0 ? m_layer_ColorIndex_map.at(key) : 0; + // We could just use [] to find the color except this will make an entry with value 0 if + // there is none (rather than just returning 0 on each such call), and this my cause a + // problem if we try to evaluate color reading BLOCK definitions, since these appear before + // the LAYER table. Note that we shouldn't be evaluating color during block definition but + // this might be happening already. + index = m_layer_ColorIndex_map.count(m_layer_name) > 0 + ? m_layer_ColorIndex_map.at(m_layer_name) + : 0; } // TODO: If it is ColorByBlock we need to use the color of the INSERT entity. // This is tricky because a block can itself contain INSERT entities and we don't currently @@ -3081,16 +2993,16 @@ App::Color CDxfRead::ObjectColor() const result = App::Color(0.75, 0.75, 0.75); } else if (index >= 250) { - int brightness = (index - 250 + (255 - index) * 0.2) / 5; + auto brightness = (float)((index - 250 + (255 - index) * 0.2) / 5); result = App::Color(brightness, brightness, brightness); } else { - int fade = (index / 2) % 5; - static double fades[5] = {1.00, 0.74, 0.50, 0.40, 0.30}; - return wheel(index / 10 - 1, (index & 1) ? 0.69 : 0, fades[fade]); + static const std::array fades = {1.00, 0.74, 0.50, 0.40, 0.30}; + return wheel(index / 10 - 1, (index & 1) != 0 ? 0.69 : 0, fades[(index / 2) % 5]); } // TODO: These colors are modified to contrast with the background. In the original program // this is just a rendering feature, but FreeCAD does not support this so the user has the // option of modifying the colors to contrast with the background at time of import. return result; } +// NOLINTEND(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers) diff --git a/src/Mod/Import/App/dxf/dxf.h b/src/Mod/Import/App/dxf/dxf.h index 40479be575..d82119df08 100644 --- a/src/Mod/Import/App/dxf/dxf.h +++ b/src/Mod/Import/App/dxf/dxf.h @@ -3,8 +3,8 @@ // This program is released under the BSD license. See the file COPYING for details. // modified 2018 wandererfan -#ifndef _dxf_h_ -#define _dxf_h_ +#ifndef Included_dxf_h_ +#define Included_dxf_h_ #ifdef _MSC_VER #pragma warning(disable : 4251) @@ -26,39 +26,93 @@ #include #include +// For some reason Cpplint complains about some of the categories used by Clang-tidy +// However, cpplint also does not seem to use NOLINTE BEGIN and NOLINT END so we must use +// NOLINT NEXT LINE on each occurrence. [spaces added to avoid being seen by lint] +using ColorIndex_t = int; // DXF color index -typedef int ColorIndex_t; // DXF color index - -typedef enum +// The C++ version we use does not support designated initiailzers, so we have a class to set this +// up +class DxfUnits { - eUnspecified = 0, // Unspecified (No units) - eInches, - eFeet, - eMiles, - eMillimeters, - eCentimeters, - eMeters, - eKilometers, - eMicroinches, - eMils, - eYards, - eAngstroms, - eNanometers, - eMicrons, - eDecimeters, - eDekameters, - eHectometers, - eGigameters, - eAstronomicalUnits, - eLightYears, - eParsecs -} eDxfUnits_t; +public: + using eDxfUnits_t = enum { + eUnspecified = 0, // Unspecified (No units) + eInches, + eFeet, + eMiles, + eMillimeters, + eCentimeters, + eMeters, + eKilometers, + eMicroinches, + eMils, + eYards, + eAngstroms, + eNanometers, + eMicrons, + eDecimeters, + eDekameters, + eHectometers, + eGigameters, + eAstronomicalUnits, + eLightYears, + eParsecs, + kMaxUnit + }; +private: + // NOLINTNEXTLINE(readability/nolint) + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init) + DxfUnits() + { + // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers) + m_factors[eInches] = 25.4; + m_factors[eFeet] = 25.4 * 12; + m_factors[eMiles] = 1609344.0; + m_factors[eMillimeters] = 1.0; + m_factors[eCentimeters] = 10.0; + m_factors[eMeters] = 1000.0; + m_factors[eKilometers] = 1000000.0; + m_factors[eMicroinches] = 25.4 / 1000.0; + m_factors[eMils] = 25.4 / 1000.0; + m_factors[eYards] = 3 * 12 * 25.4; + m_factors[eAngstroms] = 0.0000001; + m_factors[eNanometers] = 0.000001; + m_factors[eMicrons] = 0.001; + m_factors[eDecimeters] = 100.0; + m_factors[eDekameters] = 10000.0; + m_factors[eHectometers] = 100000.0; + m_factors[eGigameters] = 1000000000000.0; + m_factors[eAstronomicalUnits] = 149597870690000.0; + m_factors[eLightYears] = 9454254955500000000.0; + m_factors[eParsecs] = 30856774879000000000.0; + // NOLINTEND(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers) + } + +public: + static double Factor(eDxfUnits_t enumValue) + { + // NOLINTNEXTLINE(readability/nolint) + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) + return Instance.m_factors[enumValue]; + } + static bool IsValid(eDxfUnits_t enumValue) + { + return enumValue > eUnspecified && enumValue <= eParsecs; + } + +private: + static const DxfUnits Instance; + // NOLINTNEXTLINE(readability/nolint) + // NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays, modernize-avoid-c-arrays) + double m_factors[kMaxUnit]; +}; // spline data for reading struct SplineData { - double norm[3] = {0, 0, 0}; + Base::Vector3d norm; int degree = 0; int knots = 0; int control_points = 0; @@ -119,8 +173,45 @@ struct LWPolyDataOut std::vector Bulge; point3D Extr; }; -typedef enum -{ +using eDXFGroupCode_t = enum { + eObjectType = 0, + ePrimaryText = 1, + eName = 2, + eExtraText = 3, + eLinetypeName = 6, + eTextStyleName = 7, + eLayerName = 8, + eVariableName = 9, + ePrimaryPoint = 10, + ePoint2 = 11, + ePoint3 = 12, + ePoint4 = 13, + ePoint5 = 14, + eFloat1 = 40, + eFloat2 = 41, + eFloat3 = 42, + eFloat4 = 43, + eAngleDegrees1 = 50, + eAngleDegrees2 = 51, + eColor = 62, + eCoordinateSpace = 67, + eInteger1 = 70, + eInteger2 = 71, + eInteger3 = 72, + eInteger4 = 73, + eInteger5 = 74, + eUCSOrigin = 110, + eUCSXDirection = 111, + eUCSYDirection = 112, + eExtrusionDirection = 210, + + // The following apply to points and directions in text DXF files to identify the three + // coordinates + eXOffset = 0, + eYOffset = 10, + eZOffset = 20 +}; +using eDXFVersion_t = enum { RUnknown, ROlder, R10, @@ -134,7 +225,20 @@ typedef enum R2013, R2018, RNewer, -} eDXFVersion_t; +}; +using eDimensionType_t = enum { + eLinear = 0, // Rotated, Horizontal, or Vertical + eAligned = 1, + eAngular = 2, + eDiameter = 3, + eRadius = 4, + eAngular3Point = 5, + eOrdinate = 6, + eTypeMask = 0xF, + eOnlyBlockReference = 32, + eOrdianetIsXType = 64, + eUserTextLocation = 128 +}; //******************** class ImportExport CDxfWrite @@ -148,38 +252,45 @@ private: std::ostringstream* m_ssLayer; protected: - void putLine(const Base::Vector3d s, - const Base::Vector3d e, + static Base::Vector3d toVector3d(const double* coordinatesXYZ) + { + // NOLINTNEXTLINE(readability/nolint) + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) + return Base::Vector3d(coordinatesXYZ[0], coordinatesXYZ[1], coordinatesXYZ[2]); + } + + void putLine(const Base::Vector3d& start, + const Base::Vector3d& end, std::ostringstream* outStream, - const std::string handle, - const std::string ownerHandle); + const std::string& handle, + const std::string& ownerHandle); void putText(const char* text, - const Base::Vector3d location1, - const Base::Vector3d location2, - const double height, - const int horizJust, + const Base::Vector3d& location1, + const Base::Vector3d& location2, + double height, + int horizJust, std::ostringstream* outStream, - const std::string handle, - const std::string ownerHandle); - void putArrow(Base::Vector3d arrowPos, - Base::Vector3d barb1Pos, - Base::Vector3d barb2Pos, + const std::string& handle, + const std::string& ownerHandle); + void putArrow(Base::Vector3d& arrowPos, + Base::Vector3d& barb1Pos, + Base::Vector3d& barb2Pos, std::ostringstream* outStream, - const std::string handle, - const std::string ownerHandle); + const std::string& handle, + const std::string& ownerHandle); //! copy boiler plate file std::string getPlateFile(std::string fileSpec); - void setDataDir(std::string s) + void setDataDir(const std::string& dirName) { - m_dataDir = s; + m_dataDir = dirName; } std::string getHandle(); std::string getEntityHandle(); std::string getLayerHandle(); std::string getBlockHandle(); std::string getBlkRecordHandle(); - + // NOLINTBEGIN(cppcoreguidelines-non-private-member-variables-in-classes) std::string m_optionSource; int m_version; int m_handle; @@ -199,15 +310,20 @@ protected: std::vector m_layerList; std::vector m_blockList; std::vector m_blkRecordList; + // NOLINTEND(cppcoreguidelines-non-private-member-variables-in-classes) public: explicit CDxfWrite(const char* filepath); + CDxfWrite(const CDxfWrite&) = delete; + CDxfWrite(const CDxfWrite&&) = delete; + CDxfWrite& operator=(const CDxfWrite&) = delete; + CDxfWrite& operator=(const CDxfWrite&&) = delete; ~CDxfWrite(); void init(); void endRun(); - bool Failed() + bool Failed() const { return m_fail; } @@ -217,37 +333,39 @@ public: { return m_layerName; } - void setLayerName(std::string s); - void setVersion(int v) + void setLayerName(std::string name); + void setVersion(int version) { - m_version = v; + m_version = version; } - void setPolyOverride(bool b) + void setPolyOverride(bool setting) { - m_polyOverride = b; + m_polyOverride = setting; } - void addBlockName(std::string s, std::string blkRecordHandle); + void addBlockName(const std::string& name, const std::string& blkRecordHandle); - void writeLine(const double* s, const double* e); + void writeLine(const double* start, const double* end); void writePoint(const double*); - void writeArc(const double* s, const double* e, const double* c, bool dir); - void writeEllipse(const double* c, + void writeArc(const double* start, const double* end, const double* center, bool dir); + void writeEllipse(const double* center, double major_radius, double minor_radius, double rotation, double start_angle, double end_angle, bool endIsCW); - void writeCircle(const double* c, double radius); + void writeCircle(const double* center, double radius); void writeSpline(const SplineDataOut& sd); void writeLWPolyLine(const LWPolyDataOut& pd); void writePolyline(const LWPolyDataOut& pd); + // NOLINTNEXTLINE(readability/nolint) + // NOLINTNEXTLINE(readability-identifier-length) void writeVertex(double x, double y, double z); void writeText(const char* text, const double* location1, const double* location2, - const double height, - const int horizJust); + double height, + int horizJust); void writeLinearDim(const double* textMidPoint, const double* lineDefPoint, const double* extLine1, @@ -312,36 +430,50 @@ class ImportExport CDxfRead { private: // Low-level reader members - std::ifstream* m_ifs; - int m_record_type = 0; - char m_record_data[1024] = ""; + std::ifstream* m_ifs; // TODO: gsl::owner + eDXFGroupCode_t m_record_type = eObjectType; + std::string m_record_data; bool m_not_eof = true; int m_line = 0; bool m_repeat_last_record = false; - // file-level options/properties - eDxfUnits_t m_eUnits = eMillimeters; - bool m_measurement_inch = false; + // The scaling from DXF units to millimetres. + // This does not include the dxfScaling option + // This has the value 0.0 if no units have been specified. + // If it is still 0 after reading the HEADER section, it iw set to comething sensible. + double m_unitScalingFactor = 0.0; + +protected: + // An additional scaling factor which can be modified before readDXF is called, and will be + // incorporated into m_unitScalingFactor. + void SetAdditionalScaling(double scaling) + { + m_additionalScaling = scaling <= 0.0 ? 1.0 : scaling; + } + +private: + double m_additionalScaling = 1.0; // The following provide a state when reading any entity: If m_block_name is not empty the // entity is in a BLOCK being defined, and LayerName() will return "BLOCKS xxxx" where xxxx is // the block name. Otherwise m_layer_name will be the layer name from the entity records // (default to "0") and LayerName() will return "ENTITIES xxxx" where xxxx is the layer name. // This is clunky but it is a non-private interface and so difficult to change. - char m_layer_name[1024] = "0"; - char m_block_name[1024] = ""; + std::string m_layer_name; + std::string m_block_name; // Error-handling control bool m_ignore_errors = true; bool m_fail = false; - std::map - m_layer_ColorIndex_map; // Mapping from layer name -> layer color index + // Mapping from layer name -> layer color index + std::map m_layer_ColorIndex_map; const ColorIndex_t ColorBylayer = 256; const ColorIndex_t ColorByBlock = 0; // Readers for various parts of the DXF file. + bool ReadSection(); // Section readers (sections are identified by the type-2 (name) record they start with and each // has its own reader function bool ReadHeaderSection(); @@ -351,6 +483,12 @@ private: bool ReadIgnoredSection(); + // The Header section consists of multipel variables, only a few of which we give special + // handling. + bool ReadVariable(); + bool ReadVersion(); + bool ReadDWGCodePage(); + // The Tables section consists of several tables (again identified by their type-2 record asfter // th 0-TABLE record) each with its own reader bool ReadLayerTable(); @@ -359,7 +497,6 @@ private: // inline-defined to. bool ReadIgnoredTable(); - bool ReadUnits(); bool ReadLayer(); bool ReadEntity(); // Identify entity type and read it @@ -373,134 +510,181 @@ private: bool ReadSpline(); bool ReadLwPolyLine(); bool ReadPolyLine(); - typedef struct + struct VertexInfo { - double location[3]; - double bulge; - } VertexInfo; + Base::Vector3d location; + double bulge = 0; + }; bool OnReadPolyline(std::list&, int flags); void OnReadArc(double start_angle, double end_angle, double radius, - const double* c, + const Base::Vector3d& center, double z_extrusion_dir, bool hidden); - void OnReadCircle(const double* c, double radius, bool hidden); - void OnReadEllipse(const double* c, - const double* m, + void OnReadCircle(const Base::Vector3d& center, double radius, bool hidden); + void OnReadEllipse(const Base::Vector3d& center, + const Base::Vector3d& majorAxisEnd, double ratio, double start_angle, double end_angle); bool ReadInsert(); bool ReadDimension(); bool ReadUnknownEntity(); + // Helper for reading common attributes for entities void InitializeAttributes(); void InitializeCommonEntityAttributes(); - void Setup3DVectorAttribute(int x_record_type, double destination[3]); - void SetupScaledDoubleAttribute(int record_type, double& destination); - void SetupScaledDoubleIntoList(int record_type, std::list& destination); - void Setup3DCoordinatesIntoLists(int x_record_type, + void Setup3DVectorAttribute(eDXFGroupCode_t x_record_type, Base::Vector3d& destination); + void SetupScaledDoubleAttribute(eDXFGroupCode_t record_type, double& destination); + void SetupScaledDoubleIntoList(eDXFGroupCode_t record_type, std::list& destination); + void Setup3DCoordinatesIntoLists(eDXFGroupCode_t x_record_type, std::list& x_destination, std::list& y_destination, std::list& z_destination); - void SetupStringAttribute(int record_type, char* destination); - void SetupStringAttribute(int record_type, std::string& destination); + void SetupStringAttribute(eDXFGroupCode_t record_type, std::string& destination); std::map> m_coordinate_attributes; static void ProcessScaledDouble(CDxfRead* object, void* target); static void ProcessScaledDoubleIntoList(CDxfRead* object, void* target); - static void ProcessString(CDxfRead* object, void* target); static void ProcessStdString(CDxfRead* object, void* target); // For all types T where strean >> x and x = 0 works template - void SetupValueAttribute(int record_type, T& target); + void SetupValueAttribute(eDXFGroupCode_t record_type, T& destination); // TODO: Once all compilers used for FreeCAD support class-level template specializations, // SetupValueAttribute could have specializations and replace SetupStringAttribute etc. // The template specialization is required to handle the (char *) case, which would // otherwise try to read the actual pointer from the stream, or... what? // The specialization would also handle the default value when it cannot be zero. template - static void ProcessValue(CDxfRead* object, void* target); + static void ProcessValue(CDxfRead* object, void* target) + { + ParseValue(object, target); + } + template + static bool ParseValue(CDxfRead* object, void* target); bool ProcessAttribute(); void ProcessAllAttributes(); bool ReadBlockInfo(); - bool ReadVersion(); - bool ReadDWGCodePage(); bool ResolveEncoding(); bool get_next_record(); void repeat_last_record(); + bool (CDxfRead::*stringToUTF8)(std::string&) const = &CDxfRead::UTF8ToUTF8; + protected: // common entity properties. Some properties are accumulated local to the reader method and // passed to the ReadXxxx virtual method. Others are collected here as private values and also // passed to ReadXxxx. Finally some of the attributes are accessed using references to // public/protected fields or methods (such as LayerName()). Altogether a bit of a mishmash. + // NOLINTBEGIN(cppcoreguidelines-non-private-member-variables-in-classes) ColorIndex_t m_ColorIndex = 0; - char m_LineType[1024] = ""; + std::string m_LineType; eDXFVersion_t m_version = RUnknown; // Version from $ACADVER variable in DXF - const char* (CDxfRead::*stringToUTF8)(const char*) const = &CDxfRead::UTF8ToUTF8; - // Although this is called "ImportWarning" it is just a wrapper to write a warning eithout any + // NOLINTEND(cppcoreguidelines-non-private-member-variables-in-classes) + + // Although this is called "ImportError" it is just a wrapper to write a warning eithout any // additional information such as a line number and as such, may be split into a basic // message-writer and something that adds a line number. + // + // The "Developer" methods stick a line break into the output window. + // The "User" methods show up in the notification popup window. + // "Critical" causes a popup messagebox + // "Warnings" show up in yellow in the output window and with a warning icon in the notification + // popup "Error" show up in red in the output window and with an error icon in the notification + // popup "Notification" show up in black in the output window and an information icon in the + // notification popup "Log" goes to a log somewhere and not to the screen/user at all + template - void ImportError(const char* format, args... argValues) const + void ImportError(const char* format, args&&... argValues) const { - Base::ConsoleSingleton::Instance().Warning(format, argValues...); + Base::ConsoleSingleton::Instance().Warning(format, std::forward(argValues)...); } - void UnsupportedFeature(const char* format, ...); + template + void ImportObservation(const char* format, args&&... argValues) const + { + Base::ConsoleSingleton::Instance().Message(format, std::forward(argValues)...); + } + template + void UnsupportedFeature(const char* format, args&&... argValues); private: std::map> m_unsupportedFeaturesNoted; - const std::string* m_CodePage = - nullptr; // Code Page name from $DWGCODEPAGE or null if none/not read yet + std::string m_CodePage; // Code Page name from $DWGCODEPAGE or null if none/not read yet // The following was going to be python's canonical name for the encoding, but this is (a) not // easily found and (b) does not speed up finding the encoding object. - const std::string* m_encoding = - nullptr; // A name for the encoding implied by m_version and m_CodePage - const char* UTF8ToUTF8(const char* encoded) const; - const char* GeneralToUTF8(const char* encoded) const; + std::string m_encoding; // A name for the encoding implied by m_version and m_CodePage + bool UTF8ToUTF8(std::string& encoded) const; + bool GeneralToUTF8(std::string& encoded) const; + + // Compare with specific object name for eObjectType records + bool IsObjectName(const char* testName) const + { + return m_record_data == testName; + } + // Compare with specific variable name for eVariableName records + bool IsVariableName(const char* testName) const + { + return m_record_data == testName; + } public: - explicit CDxfRead(const char* filepath); // this opens the file - virtual ~CDxfRead(); // this closes the file + explicit CDxfRead(const std::string& filepath); // this opens the file + CDxfRead(const CDxfRead&) = delete; + CDxfRead(const CDxfRead&&) = delete; + CDxfRead& operator=(const CDxfRead&) = delete; + CDxfRead& operator=(const CDxfRead&&) = delete; + virtual ~CDxfRead(); // this closes the file - bool Failed() + bool Failed() const { return m_fail; } - void DoRead( - const bool ignore_errors = false); // this reads the file and calls the following functions + void + DoRead(bool ignore_errors = false); // this reads the file and calls the following functions - double mm(double value) const; +private: + double mm(double value) const + { + if (m_unitScalingFactor == 0.0) { + // No scaling factor has been specified. + // TODO: Resolve this once we know the HEADER is complete + return value; + } + return m_unitScalingFactor * value; + } +public: bool IgnoreErrors() const { return (m_ignore_errors); } - virtual void OnReadLine(const double* /*s*/, const double* /*e*/, bool /*hidden*/) + virtual void + OnReadLine(const Base::Vector3d& /*start*/, const Base::Vector3d& /*end*/, bool /*hidden*/) {} - virtual void OnReadPoint(const double* /*s*/) + virtual void OnReadPoint(const Base::Vector3d& /*start*/) {} - virtual void OnReadText(const double* /*point*/, + virtual void OnReadText(const Base::Vector3d& /*point*/, const double /*height*/, - const char* /*text*/, + const std::string& /*text*/, const double /*rotation*/) {} - virtual void OnReadArc(const double* /*s*/, - const double* /*e*/, - const double* /*c*/, + virtual void OnReadArc(const Base::Vector3d& /*start*/, + const Base::Vector3d& /*end*/, + const Base::Vector3d& /*center*/, bool /*dir*/, bool /*hidden*/) {} - virtual void - OnReadCircle(const double* /*s*/, const double* /*c*/, bool /*dir*/, bool /*hidden*/) + virtual void OnReadCircle(const Base::Vector3d& /*start*/, + const Base::Vector3d& /*center*/, + bool /*dir*/, + bool /*hidden*/) {} - virtual void OnReadEllipse(const double* /*c*/, + virtual void OnReadEllipse(const Base::Vector3d& /*center*/, double /*major_radius*/, double /*minor_radius*/, double /*rotation*/, @@ -510,14 +694,14 @@ public: {} virtual void OnReadSpline(struct SplineData& /*sd*/) {} - virtual void OnReadInsert(const double* /*point*/, - const double* /*scale*/, - const char* /*name*/, + virtual void OnReadInsert(const Base::Vector3d& /*point*/, + const Base::Vector3d& /*scale*/, + const std::string& /*name*/, double /*rotation*/) {} - virtual void OnReadDimension(const double* /*s*/, - const double* /*e*/, - const double* /*point*/, + virtual void OnReadDimension(const Base::Vector3d& /*start*/, + const Base::Vector3d& /*end*/, + const Base::Vector3d& /*point*/, double /*rotation*/) {} virtual void AddGraphics() const diff --git a/src/Mod/Import/Gui/dxf/ImpExpDxfGui.h b/src/Mod/Import/Gui/dxf/ImpExpDxfGui.h index 24a1c9acea..294b31797a 100644 --- a/src/Mod/Import/Gui/dxf/ImpExpDxfGui.h +++ b/src/Mod/Import/Gui/dxf/ImpExpDxfGui.h @@ -40,8 +40,10 @@ public: ImpExpDxfReadGui(std::string filepath, App::Document* pcDoc); protected: - void ApplyGuiStyles(Part::Feature*); - void ApplyGuiStyles(App::FeaturePython*); + void ApplyGuiStyles(Part::Feature* object) override; + void ApplyGuiStyles(App::FeaturePython* object) override; + +private: Gui::Document* GuiDocument; }; } // namespace ImportGui diff --git a/src/Mod/Path/libarea/AreaDxf.cpp b/src/Mod/Path/libarea/AreaDxf.cpp index a8eb33f703..8d05a1850e 100644 --- a/src/Mod/Path/libarea/AreaDxf.cpp +++ b/src/Mod/Path/libarea/AreaDxf.cpp @@ -7,25 +7,31 @@ AreaDxfRead::AreaDxfRead(CArea* area, const char* filepath):CDxfRead(filepath), m_area(area){} -void AreaDxfRead::StartCurveIfNecessary(const double* s) +void AreaDxfRead::StartCurveIfNecessary(const Base::Vector3d& startPoint) const { - Point ps(s); - if((m_area->m_curves.size() == 0) || (m_area->m_curves.back().m_vertices.size() == 0) || (m_area->m_curves.back().m_vertices.back().m_p != ps)) + Point ps(startPoint.x, startPoint.y); + if(m_area->m_curves.empty() || m_area->m_curves.back().m_vertices.empty() || m_area->m_curves.back().m_vertices.back().m_p != ps) { // start a new curve m_area->m_curves.emplace_back(); - m_area->m_curves.back().m_vertices.push_back(ps); + m_area->m_curves.back().m_vertices.emplace_back(ps); } } -void AreaDxfRead::OnReadLine(const double* s, const double* e, bool /*hidden*/) +void AreaDxfRead::OnReadLine(const Base::Vector3d& start, const Base::Vector3d& end, bool /*hidden*/) { - StartCurveIfNecessary(s); - m_area->m_curves.back().m_vertices.push_back(Point(e)); + StartCurveIfNecessary(start); + m_area->m_curves.back().m_vertices.emplace_back(Point(end.x, end.y)); } -void AreaDxfRead::OnReadArc(const double* s, const double* e, const double* c, bool dir, bool /*hidden*/) +void AreaDxfRead::OnReadArc(const Base::Vector3d& start, + const Base::Vector3d& end, + const Base::Vector3d& center, + bool dir, + bool /*hidden*/) { - StartCurveIfNecessary(s); - m_area->m_curves.back().m_vertices.emplace_back(dir?1:0, Point(e), Point(c)); + StartCurveIfNecessary(start); + m_area->m_curves.back().m_vertices.emplace_back(dir ? 1 : 0, + Point(end.x, end.y), + Point(center.x, center.y)); } diff --git a/src/Mod/Path/libarea/AreaDxf.h b/src/Mod/Path/libarea/AreaDxf.h index 7dc5a74bde..b3b815697a 100644 --- a/src/Mod/Path/libarea/AreaDxf.h +++ b/src/Mod/Path/libarea/AreaDxf.h @@ -11,13 +11,13 @@ class CArea; class CCurve; class AreaDxfRead : public CDxfRead{ - void StartCurveIfNecessary(const double* s); + void StartCurveIfNecessary(const Base::Vector3d& startPoint) const; public: CArea* m_area; AreaDxfRead(CArea* area, const char* filepath); // AreaDxfRead's virtual functions - void OnReadLine(const double* s, const double* e, bool /*hidden*/) override; - void OnReadArc(const double* s, const double* e, const double* c, bool dir, bool /*hidden*/) override; + void OnReadLine(const Base::Vector3d& start, const Base::Vector3d& end, bool /*hidden*/) override; + void OnReadArc(const Base::Vector3d& start, const Base::Vector3d& end, const Base::Vector3d& center, bool dir, bool /*hidden*/) override; };