From 2c0bc89f6237148debdcb591455265e50b9ae724 Mon Sep 17 00:00:00 2001 From: Kevin Martin Date: Thu, 14 Dec 2023 07:02:04 -0500 Subject: [PATCH] Streamline scaling for DXF import Eliminate m_measurement_inch to clean up logic for priority of MEASUREMENT and INSUNITS. Save the actual scaling factor rather than the scaling enum so a switch statement is not executed for each call to mm() Add to CDxfRead the work to handle dxfScaling option, ImpExpDxfRead just has to set it up now. Get the scaling factor from a lookup table rather than a switch statement Display a message explaining what the scaling factor is and where it comes from Remove large amount of Lint. --- src/Mod/Import/App/dxf/ImpExpDxf.cpp | 259 +++---- src/Mod/Import/App/dxf/ImpExpDxf.h | 64 +- src/Mod/Import/App/dxf/dxf.cpp | 1036 +++++++++++-------------- src/Mod/Import/App/dxf/dxf.h | 436 ++++++++--- src/Mod/Import/Gui/dxf/ImpExpDxfGui.h | 6 +- src/Mod/Path/libarea/AreaDxf.cpp | 26 +- src/Mod/Path/libarea/AreaDxf.h | 6 +- 7 files changed, 969 insertions(+), 864 deletions(-) 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; };