From 9e9c78404123797d93de47bbde28a78042565274 Mon Sep 17 00:00:00 2001 From: wmayer Date: Tue, 15 Nov 2022 11:09:12 +0100 Subject: [PATCH] TD: fix memory leaks with raw pointers of GeometryObject Replacing the use of raw pointers of GeometryObject with shared pointers fixes the observed memory leaks when running the unit tests of TechDraw --- src/Mod/TechDraw/App/AppTechDrawPy.cpp | 6 +++--- src/Mod/TechDraw/App/DrawProjectSplit.cpp | 7 +++---- src/Mod/TechDraw/App/DrawProjectSplit.h | 3 ++- src/Mod/TechDraw/App/DrawViewPart.cpp | 15 +++++---------- src/Mod/TechDraw/App/DrawViewPart.h | 11 ++++++----- src/Mod/TechDraw/App/GeometryObject.h | 3 +++ 6 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/Mod/TechDraw/App/AppTechDrawPy.cpp b/src/Mod/TechDraw/App/AppTechDrawPy.cpp index 140de23aa2..9ad5c027a0 100644 --- a/src/Mod/TechDraw/App/AppTechDrawPy.cpp +++ b/src/Mod/TechDraw/App/AppTechDrawPy.cpp @@ -411,7 +411,7 @@ private: if (PyObject_TypeCheck(viewObj, &(TechDraw::DrawViewPartPy::Type))) { obj = static_cast(viewObj)->getDocumentObjectPtr(); dvp = static_cast(obj); - TechDraw::GeometryObject* gObj = dvp->getGeometryObject(); + TechDraw::GeometryObjectPtr gObj = dvp->getGeometryObject(); TopoDS_Shape shape = TechDraw::mirrorShape(gObj->getVisHard()); ss << dxfOut.exportEdges(shape); shape = TechDraw::mirrorShape(gObj->getVisOutline()); @@ -469,7 +469,7 @@ private: if (PyObject_TypeCheck(viewObj, &(TechDraw::DrawViewPartPy::Type))) { obj = static_cast(viewObj)->getDocumentObjectPtr(); dvp = static_cast(obj); - TechDraw::GeometryObject* gObj = dvp->getGeometryObject(); + TechDraw::GeometryObjectPtr gObj = dvp->getGeometryObject(); //visible group begin "" ss << grpHead1; // double thick = dvp->LineWidth.getValue(); @@ -533,7 +533,7 @@ private: { if(!dvp->hasGeometry()) return; - TechDraw::GeometryObject* gObj = dvp->getGeometryObject(); + TechDraw::GeometryObjectPtr gObj = dvp->getGeometryObject(); TopoDS_Shape shape = TechDraw::mirrorShape(gObj->getVisHard()); double offX = 0.0; double offY = 0.0; diff --git a/src/Mod/TechDraw/App/DrawProjectSplit.cpp b/src/Mod/TechDraw/App/DrawProjectSplit.cpp index 72aaa7e602..fc60161b05 100644 --- a/src/Mod/TechDraw/App/DrawProjectSplit.cpp +++ b/src/Mod/TechDraw/App/DrawProjectSplit.cpp @@ -85,7 +85,7 @@ std::vector DrawProjectSplit::getEdgesForWalker(TopoDS_Shape shape, scaledShape = TechDraw::scaleShape(copyShape, scale); gp_Ax2 viewAxis = TechDraw::legacyViewAxis1(Base::Vector3d(0.0, 0.0, 0.0), direction, false); - TechDraw::GeometryObject* go = buildGeometryObject(scaledShape, viewAxis); + TechDraw::GeometryObjectPtr go = buildGeometryObject(scaledShape, viewAxis); const std::vector& goEdges = go->getVisibleFaceEdges(false, false); for (auto& e: goEdges){ edgesIn.push_back(e->occEdge); @@ -100,15 +100,14 @@ std::vector DrawProjectSplit::getEdgesForWalker(TopoDS_Shape shape, } } - delete go; return nonZero; } //project the shape using viewAxis (coordinate system) and return a geometry object -TechDraw::GeometryObject* DrawProjectSplit::buildGeometryObject(TopoDS_Shape shape, +TechDraw::GeometryObjectPtr DrawProjectSplit::buildGeometryObject(TopoDS_Shape shape, const gp_Ax2& viewAxis) { - TechDraw::GeometryObject* geometryObject = new TechDraw::GeometryObject("DrawProjectSplit", nullptr); + TechDraw::GeometryObjectPtr geometryObject(std::make_shared("DrawProjectSplit", nullptr)); if (geometryObject->usePolygonHLR()){ geometryObject->projectShapeWithPolygonAlgo(shape, viewAxis); diff --git a/src/Mod/TechDraw/App/DrawProjectSplit.h b/src/Mod/TechDraw/App/DrawProjectSplit.h index f23ab7e766..25f2ab4f63 100644 --- a/src/Mod/TechDraw/App/DrawProjectSplit.h +++ b/src/Mod/TechDraw/App/DrawProjectSplit.h @@ -40,6 +40,7 @@ class gp_Ax2; namespace TechDraw { class GeometryObject; +using GeometryObjectPtr = std::shared_ptr; class Vertex; class BaseGeom; } @@ -98,7 +99,7 @@ public: public: static std::vector getEdgesForWalker(TopoDS_Shape shape, double scale, Base::Vector3d direction); - static TechDraw::GeometryObject* buildGeometryObject(TopoDS_Shape shape, const gp_Ax2& viewAxis); + static TechDraw::GeometryObjectPtr buildGeometryObject(TopoDS_Shape shape, const gp_Ax2& viewAxis); static bool isOnEdge(TopoDS_Edge e, TopoDS_Vertex v, double& param, bool allowEnds = false); static std::vector splitEdges(std::vector orig, std::vector splits); diff --git a/src/Mod/TechDraw/App/DrawViewPart.cpp b/src/Mod/TechDraw/App/DrawViewPart.cpp index bcba41d982..61c488235a 100644 --- a/src/Mod/TechDraw/App/DrawViewPart.cpp +++ b/src/Mod/TechDraw/App/DrawViewPart.cpp @@ -151,7 +151,6 @@ DrawViewPart::~DrawViewPart() m_faceFuture.waitForFinished(); } removeAllReferencesFromGeom(); - delete geometryObject; } std::vector DrawViewPart::getSourceShape2d() const @@ -318,7 +317,7 @@ void DrawViewPart::partExec(TopoDS_Shape& shape) } //prepare the shape for HLR processing by centering, scaling and rotating it -GeometryObject* DrawViewPart::makeGeometryForShape(TopoDS_Shape& shape) +GeometryObjectPtr DrawViewPart::makeGeometryForShape(TopoDS_Shape& shape) { // Base::Console().Message("DVP::makeGeometryForShape() - %s\n", getNameInDocument()); gp_Pnt inputCenter; @@ -344,17 +343,17 @@ GeometryObject* DrawViewPart::makeGeometryForShape(TopoDS_Shape& shape) Rotation.getValue()); //conventional rotation } BRepTools::Write(scaledShape, "DVPScaled.brep"); //debug - GeometryObject* go = buildGeometryObject(scaledShape, viewAxis); + GeometryObjectPtr go = buildGeometryObject(scaledShape, viewAxis); return go; } //create a geometry object and trigger the HLR process in another thread -TechDraw::GeometryObject* DrawViewPart::buildGeometryObject(TopoDS_Shape& shape, const gp_Ax2 &viewAxis) +TechDraw::GeometryObjectPtr DrawViewPart::buildGeometryObject(TopoDS_Shape& shape, const gp_Ax2 &viewAxis) { // Base::Console().Message("DVP::buildGeometryObject() - %s\n", getNameInDocument()); showProgressMessage(getNameInDocument(), "is finding hidden lines"); - TechDraw::GeometryObject* go = new TechDraw::GeometryObject(getNameInDocument(), this); + TechDraw::GeometryObjectPtr go(std::make_shared(getNameInDocument(), this)); go->setIsoCount(IsoCount.getValue()); go->isPerspective(Perspective.getValue()); go->setFocus(Focus.getValue()); @@ -372,7 +371,7 @@ TechDraw::GeometryObject* DrawViewPart::buildGeometryObject(TopoDS_Shape& shape, //https://github.com/KDE/clazy/blob/1.11/docs/checks/README-connect-3arg-lambda.md connectHlrWatcher = QObject::connect(&m_hlrWatcher, &QFutureWatcherBase::finished, &m_hlrWatcher, [this] { this->onHlrFinished(); } ); - m_hlrFuture = QtConcurrent::run(go, &GeometryObject::projectShape, shape, viewAxis); + m_hlrFuture = QtConcurrent::run(go.get(), &GeometryObject::projectShape, shape, viewAxis); m_hlrWatcher.setFuture(m_hlrFuture); waitingForHlr(true); } @@ -385,10 +384,6 @@ void DrawViewPart::onHlrFinished(void) // Base::Console().Message("DVP::onHlrFinished() - %s\n", getNameInDocument()); //now that the new GeometryObject is fully populated, we can replace the old one - if (geometryObject && - m_tempGeometryObject) { - delete geometryObject; //remove the old - } if (m_tempGeometryObject) { geometryObject = m_tempGeometryObject; //replace with new m_tempGeometryObject = nullptr; //superfluous? diff --git a/src/Mod/TechDraw/App/DrawViewPart.h b/src/Mod/TechDraw/App/DrawViewPart.h index c8e82f401f..7881aa3f18 100644 --- a/src/Mod/TechDraw/App/DrawViewPart.h +++ b/src/Mod/TechDraw/App/DrawViewPart.h @@ -54,6 +54,7 @@ class Part; namespace TechDraw { class GeometryObject; +using GeometryObjectPtr = std::shared_ptr; class Vertex; class BaseGeom; class Face; @@ -124,7 +125,7 @@ public: const std::vector getFaceGeometry() const; bool hasGeometry() const; - TechDraw::GeometryObject* getGeometryObject() const { return geometryObject; } + TechDraw::GeometryObjectPtr getGeometryObject() const { return geometryObject; } TechDraw::BaseGeomPtr getGeomByIndex(int idx) const; //get existing geom for edge idx in projection TechDraw::VertexPtr getProjVertexByIndex(int idx) const; //get existing geom for vertex idx in projection @@ -221,15 +222,15 @@ public Q_SLOTS: protected: bool checkXDirection() const; - TechDraw::GeometryObject* geometryObject; - TechDraw::GeometryObject* m_tempGeometryObject; //holds the new GO until hlr is completed + TechDraw::GeometryObjectPtr geometryObject; + TechDraw::GeometryObjectPtr m_tempGeometryObject; //holds the new GO until hlr is completed Base::BoundBox3d bbox; void onChanged(const App::Property* prop) override; void unsetupObject() override; - virtual TechDraw::GeometryObject* buildGeometryObject(TopoDS_Shape& shape, const gp_Ax2& viewAxis); - virtual TechDraw::GeometryObject* makeGeometryForShape(TopoDS_Shape& shape); //const?? + virtual TechDraw::GeometryObjectPtr buildGeometryObject(TopoDS_Shape& shape, const gp_Ax2& viewAxis); + virtual TechDraw::GeometryObjectPtr makeGeometryForShape(TopoDS_Shape& shape); //const?? void partExec(TopoDS_Shape& shape); virtual void addShapes2d(void); diff --git a/src/Mod/TechDraw/App/GeometryObject.h b/src/Mod/TechDraw/App/GeometryObject.h index 8ab04671b3..78d6c69594 100644 --- a/src/Mod/TechDraw/App/GeometryObject.h +++ b/src/Mod/TechDraw/App/GeometryObject.h @@ -25,6 +25,7 @@ #include +#include #include #include @@ -222,6 +223,8 @@ protected: bool m_usePolygonHLR; }; +using GeometryObjectPtr = std::shared_ptr; + } //namespace TechDraw #endif