From 7d8e31041c729bfa47504c56ef8bee0c596f5c5a Mon Sep 17 00:00:00 2001 From: Abdullah Tahiri Date: Sun, 11 Dec 2022 17:30:36 +0100 Subject: [PATCH] Sketcher: Split functionality fixes =================================== Changes from naked pointers to smart pointers are motivated to the use of functions that can reasonably throw under certain circumnstances (such as trim). When introducing smart pointers, it is not necessary to explicitly delete the new geometry array at the end of the function. When using the new facility to add a smart pointer geometry (previous commit), the copies generated in the split algorithm can be reused, which renders keeping track of the new geometry for memory management unnecessary. As geometry is added to the property which each call to addGeometry, the stored newIds can be reused if access is necessary to geometry pointers afterwards (e.g. for constraint management). --- src/Mod/Sketcher/App/SketchObject.cpp | 36 ++++++++++++--------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index f6be46daf5..e62b4f02f3 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -2978,7 +2978,6 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) } const Part::Geometry *geo = getGeometry(GeoId); - std::vector newGeometries; std::vector newIds; std::vector newConstraints; @@ -2999,8 +2998,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) // create new arc and restrict it auto newCurve = getCurveWithLimitParams(curve, startParam, endParam); int newId(GeoEnum::GeoUndef); - newGeometries.push_back(newCurve); - newId = addGeometry(newCurve); + newId = addGeometry(std::move(newCurve)); // after here newCurve is a shell if (newId >= 0) { newIds.push_back(newId); setConstruction(newId, GeometryFacade::getConstruction(curve)); @@ -3034,8 +3032,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) // create new curves auto newCurve = getCurveWithLimitParams(curve, startParam, splitParam); int newId(GeoEnum::GeoUndef); - newGeometries.push_back(newCurve); - newId = addGeometry(newCurve); + newId = addGeometry(std::move(newCurve)); if (newId >= 0) { newIds.push_back(newId); setConstruction(newId, GeometryFacade::getConstruction(curve)); @@ -3043,8 +3040,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) // the "second" half newCurve = getCurveWithLimitParams(curve, splitParam, endParam); - newGeometries.push_back(newCurve); - newId = addGeometry(newCurve); + newId = addGeometry(std::move(newCurve)); if (newId >= 0) { newIds.push_back(newId); setConstruction(newId, GeometryFacade::getConstruction(curve)); @@ -3067,7 +3063,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) ok = createGeosFromNonPeriodic( static_cast(geo), [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newArc = static_cast(curve->clone()); + auto newArc = std::unique_ptr(static_cast(curve->copy())); newArc->setRange(startParam, endParam); return newArc; }, @@ -3088,7 +3084,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) ok = createGeosFromPeriodic( static_cast(geo), [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newArc = new Part::GeomArcOfCircle(Handle(Geom_Circle)::DownCast(curve->handle()->Copy())); + auto newArc = std::make_unique(Handle(Geom_Circle)::DownCast(curve->handle()->Copy())); newArc->setRange(startParam, endParam, false); return newArc; }, @@ -3100,7 +3096,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) ok = createGeosFromPeriodic( static_cast(geo), [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newArc = new Part::GeomArcOfEllipse(Handle(Geom_Ellipse)::DownCast(curve->handle()->Copy())); + auto newArc = std::make_unique(Handle(Geom_Ellipse)::DownCast(curve->handle()->Copy())); newArc->setRange(startParam, endParam, false); return newArc; }, @@ -3112,7 +3108,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) ok = createGeosFromNonPeriodic( static_cast(geo), [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newArc = static_cast(curve->clone()); + auto newArc = std::unique_ptr(static_cast(curve->copy())); newArc->setRange(startParam, endParam, false); return newArc; }, @@ -3142,7 +3138,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) ok = createGeosFromNonPeriodic( static_cast(geo), [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newArc = static_cast(curve->clone()); + auto newArc = std::unique_ptr(static_cast(curve->copy())); newArc->setRange(startParam, endParam); return newArc; }, @@ -3171,7 +3167,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) ok = createGeosFromPeriodic( static_cast(geo), [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newBsp = static_cast(curve->clone()); + auto newBsp = std::unique_ptr(static_cast(curve->copy())); newBsp->Trim(startParam, endParam); return newBsp; }, @@ -3183,7 +3179,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) ok = createGeosFromNonPeriodic( static_cast(geo), [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newBsp = static_cast(curve->clone()); + auto newBsp = std::unique_ptr(static_cast(curve->copy())); newBsp->Trim(startParam, endParam); return newBsp; }, @@ -3243,13 +3239,16 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) if (geo->isDerivedFrom(Part::GeomArcOfConic::getClassTypeId())) { const Part::Geometry *conGeo = getGeometry(conId); - if (conGeo && conGeo->isDerivedFrom(Part::GeomCurve::getClassTypeId())) { + + if (conGeo && conGeo->isDerivedFrom(Part::GeomCurve::getClassTypeId())){ std::vector> intersections; bool intersects[2]; + auto *geo1 = getGeometry(newIds[0]); + auto *geo2 = getGeometry(newIds[1]); - intersects[0] = static_cast(newGeometries[0])-> + intersects[0] = static_cast(geo1)-> intersect(static_cast(conGeo), intersections); - intersects[1] = static_cast(newGeometries[1])-> + intersects[1] = static_cast(geo2)-> intersect(static_cast(conGeo), intersections); initial = longestPart; @@ -3336,9 +3335,6 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) addConstraints(newConstraints); } - for (auto& geom: newGeometries) { - delete geom; - } for (auto& cons: newConstraints) { delete cons; }