From 088abe7353fc8de93452278a5a634ceb82de1999 Mon Sep 17 00:00:00 2001 From: Abdullah Tahiri Date: Sun, 21 Jun 2020 07:22:53 +0200 Subject: [PATCH] Sketcher: Use move semantics wherever sensible ============================================== Take advantage of PropertyGeometryList setValues() move overload in order to make code more readable and prevent memory leaks (mostly by inadvertedly not deleting cloned geometry and constraints). PropertyGeometryList and PropertyConstraintList are vectors of heap allocated pointers. Copying a vector makes a shallow copy, not a deep copy (the pointers are the same in the copy). For property management, setValues() function taking a const reference effectively make a deep copy of all pointed objects. This means that heap allocated pointers of the client class passed to these functions must be released by the client. While this sounds sensible, forgetting to is easy. In the cases where the developer remembered to release these pointers, extra code is needed just for memory management. This commit does not seek a substantial performance increase that would justify rewritting the code, although code may be slightly faster sometimes. Functions where setValues() is conditional are not changed to move semantics, as it makes no sense to make a deep copy to sometimes perform a second deep copy later on. This code still uses const ref setValues(). CHECKS performed to refactored functions with this commit: 1) That the vector is NOT used after moving its content. 2) That whereever there is a clone(), there must be EITHER -a std::move if using rvalue setValues() OR - a delete to free the heap memory after setValues if using the const ref setValues() 3) That memory is released if an exception occurred. N.B.: A couple of memory leaks are fixed in this commit too. --- src/Mod/Sketcher/App/SketchObject.cpp | 417 ++++++++++++++++---------- 1 file changed, 262 insertions(+), 155 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index e04352df98..45b78faad2 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -320,14 +320,19 @@ int SketchObject::setDatum(int ConstrId, double Datum) // copy the list std::vector newVals(vals); - // clone the changed Constraint - Constraint *constNew = vals[ConstrId]->clone(); - constNew->setValue(Datum); - newVals[ConstrId] = constNew; - this->Constraints.setValues(newVals); - delete constNew; + + for(size_t i=0; iclone(); + + if((int)i == ConstrId) { + newVals[i]->setValue(Datum); + } + } + + this->Constraints.setValues(std::move(newVals)); int err = solve(); + if (err) this->Constraints.setValues(vals); @@ -345,14 +350,20 @@ int SketchObject::setDriving(int ConstrId, bool isdriving) // copy the list std::vector newVals(vals); + // clone the changed Constraint - Constraint *constNew = vals[ConstrId]->clone(); - constNew->isDriving = isdriving; - newVals[ConstrId] = constNew; - this->Constraints.setValues(newVals); + for(size_t i=0; iclone(); + + if((int)i == ConstrId) { + newVals[i]->isDriving = isdriving; + } + } + + this->Constraints.setValues(std::move(newVals)); + if (!isdriving) setExpression(Constraints.createPath(ConstrId), boost::shared_ptr()); - delete constNew; if(noRecomputes) // if we do not have a recompute, the sketch must be solved to update the DoF of the solver solve(); @@ -396,14 +407,23 @@ int SketchObject::toggleDriving(int ConstrId) // copy the list std::vector newVals(vals); - // clone the changed Constraint - Constraint *constNew = vals[ConstrId]->clone(); - constNew->isDriving = !constNew->isDriving; - newVals[ConstrId] = constNew; - this->Constraints.setValues(newVals); - if (!constNew->isDriving) + + bool isdriving = newVals[ConstrId]->isDriving; + + // deep copy + for(size_t i=0; iclone(); + + if((int)i == ConstrId) { + newVals[i]->isDriving = !newVals[i]->isDriving; + } + } + + this->Constraints.setValues(std::move(newVals)); + + if (!isdriving) setExpression(Constraints.createPath(ConstrId), boost::shared_ptr()); - delete constNew; + if(noRecomputes) // if we do not have a recompute, the sketch must be solved to update the DoF of the solver solve(); @@ -436,13 +456,17 @@ int SketchObject::setActive(int ConstrId, bool isactive) // copy the list std::vector newVals(vals); - // clone the changed Constraint - Constraint *constNew = vals[ConstrId]->clone(); - constNew->isActive = isactive; - newVals[ConstrId] = constNew; - this->Constraints.setValues(newVals); - delete constNew; + // deep copy + for(size_t i=0; iclone(); + + if((int)i == ConstrId) { + newVals[i]->isActive = isactive; + } + } + + this->Constraints.setValues(std::move(newVals)); if(noRecomputes) // if we do not have a recompute, the sketch must be solved to update the DoF of the solver solve(); @@ -471,13 +495,17 @@ int SketchObject::toggleActive(int ConstrId) // copy the list std::vector newVals(vals); - // clone the changed Constraint - Constraint *constNew = vals[ConstrId]->clone(); - constNew->isActive = !constNew->isActive; - newVals[ConstrId] = constNew; - this->Constraints.setValues(newVals); - delete constNew; + // deep copy + for(size_t i=0; iclone(); + + if((int)i == ConstrId) { + newVals[i]->isActive = !newVals[i]->isActive;; + } + } + + this->Constraints.setValues(std::move(newVals)); if(noRecomputes) // if we do not have a recompute, the sketch must be solved to update the DoF of the solver solve(); @@ -492,27 +520,22 @@ int SketchObject::setDatumsDriving(bool isdriving) const std::vector &vals = this->Constraints.getValues(); std::vector newVals(vals); - std::vector< Constraint * > tbd; // list of dynamically allocated memory that need to be deleted; + for(size_t i=0; iclone(); - for (size_t i=0; iclone(); - constNew->isDriving = isdriving; - newVals[i] = constNew; - tbd.push_back(constNew); - } + if (!testDrivingChange(i, isdriving)) + newVals[i]->isDriving = isdriving; } - this->Constraints.setValues(newVals); - for (size_t i = 0; i < newVals.size(); i++) { - if (!isdriving && newVals[i]->isDimensional()) + this->Constraints.setValues(std::move(newVals)); // we already did a deep copy of all pointers, so avoid one in setValues + + const std::vector &uvals = this->Constraints.getValues(); // newVals is a shell now + + for (size_t i = 0; i < uvals.size(); i++) { + if (!isdriving && uvals[i]->isDimensional()) setExpression(Constraints.createPath(i), boost::shared_ptr()); } - for (auto &t : tbd) - delete t; - if (noRecomputes) // if we do not have a recompute, the sketch must be solved to update the DoF of the solver solve(); @@ -562,14 +585,16 @@ int SketchObject::setVirtualSpace(int ConstrId, bool isinvirtualspace) // copy the list std::vector newVals(vals); - // clone the changed Constraint - Constraint *constNew = vals[ConstrId]->clone(); - constNew->isInVirtualSpace = isinvirtualspace; - newVals[ConstrId] = constNew; + // deep copy + for(size_t i=0; iclone(); - this->Constraints.setValues(newVals); + if((int)i == ConstrId) { + newVals[i]->isInVirtualSpace = isinvirtualspace; + } + } - delete constNew; + this->Constraints.setValues(std::move(newVals)); return 0; } @@ -595,14 +620,16 @@ int SketchObject::toggleVirtualSpace(int ConstrId) // copy the list std::vector newVals(vals); - // clone the changed Constraint - Constraint *constNew = vals[ConstrId]->clone(); - constNew->isInVirtualSpace = !constNew->isInVirtualSpace; - newVals[ConstrId] = constNew; + // deep copy + for(size_t i=0; iclone(); - this->Constraints.setValues(newVals); + if((int)i == ConstrId) { + newVals[i]->isInVirtualSpace = !newVals[i]->isInVirtualSpace; + } + } - delete constNew; + this->Constraints.setValues(std::move(newVals)); return 0; } @@ -824,24 +851,25 @@ int SketchObject::addGeometry(const std::vector &geoList, bool { const std::vector< Part::Geometry * > &vals = getInternalGeometry(); - std::vector< Part::Geometry * > newVals(vals); - std::vector< Part::Geometry * > copies; - copies.reserve(geoList.size()); - for (std::vector::const_iterator it = geoList.begin(); it != geoList.end(); ++it) { - Part::Geometry* copy = (*it)->copy(); + std::vector< Part::Geometry * > newVals; + + newVals.reserve(vals.size()+geoList.size()); + + for( auto & v : vals) + newVals.push_back(v->clone()); + + for( auto & v : geoList) { + Part::Geometry* copy = v->copy(); + if(construction && copy->getTypeId() != Part::GeomPoint::getClassTypeId()) { copy->Construction = construction; } - copies.push_back(copy); + newVals.push_back(copy); } - newVals.insert(newVals.end(), copies.begin(), copies.end()); // On setting geometry the onChanged method will call acceptGeometry(), thereby updating constraint geometry indices and rebuilding the vertex index - Geometry.setValues(newVals); - for (std::vector::iterator it = copies.begin(); it != copies.end(); ++it) - delete *it; - + Geometry.setValues(std::move(newVals)); return Geometry.getSize()-1; } @@ -850,16 +878,22 @@ int SketchObject::addGeometry(const Part::Geometry *geo, bool construction/*=fal { const std::vector< Part::Geometry * > &vals = getInternalGeometry(); - std::vector< Part::Geometry * > newVals(vals); + std::vector< Part::Geometry * > newVals; + + newVals.reserve(vals.size()+1); + + for( auto & v : vals) + newVals.push_back(v->clone()); + Part::Geometry *geoNew = geo->copy(); if(geoNew->getTypeId() != Part::GeomPoint::getClassTypeId()) geoNew->Construction = construction; newVals.push_back(geoNew); + // On setting geometry the onChanged method will call acceptGeometry(), thereby updating constraint geometry indices and rebuilding the vertex index - Geometry.setValues(newVals); - delete geoNew; + Geometry.setValues(std::move(newVals)); return Geometry.getSize()-1; } @@ -973,15 +1007,20 @@ int SketchObject::toggleConstruction(int GeoId) std::vector< Part::Geometry * > newVals(vals); - Part::Geometry *geoNew = newVals[GeoId]->clone(); - geoNew->Construction = !geoNew->Construction; - newVals[GeoId]=geoNew; + // deep copy + for(size_t i=0; iclone(); + + if((int)i == GeoId) { + newVals[i]->Construction = !newVals[i]->Construction; + } + } // There is not actual intertransaction going on here, however for a toggle neither the geometry indices nor the vertices need to be updated // so this is a convenient way of preventing it. { Base::StateLocker lock(internaltransaction, true); - this->Geometry.setValues(newVals); + this->Geometry.setValues(std::move(newVals)); } solverNeedsUpdate=true; @@ -999,15 +1038,20 @@ int SketchObject::setConstruction(int GeoId, bool on) std::vector< Part::Geometry * > newVals(vals); - Part::Geometry *geoNew = newVals[GeoId]->clone(); - geoNew->Construction = on; - newVals[GeoId]=geoNew; + // deep copy + for(size_t i=0; iclone(); + + if((int)i == GeoId) { + newVals[i]->Construction = on; + } + } // There is not actual intertransaction going on here, however for a toggle neither the geometry indices nor the vertices need to be updated // so this is a convenient way of preventing it. { Base::StateLocker lock(internaltransaction, true); - this->Geometry.setValues(newVals); + this->Geometry.setValues(std::move(newVals)); } solverNeedsUpdate=true; @@ -1019,26 +1063,27 @@ int SketchObject::addConstraints(const std::vector &ConstraintList { const std::vector< Constraint * > &vals = this->Constraints.getValues(); - std::vector< Constraint * > newVals(vals); - newVals.insert(newVals.end(), ConstraintList.begin(), ConstraintList.end()); + std::vector< Constraint * > newVals; - //test if tangent constraints have been added; AutoLockTangency. - std::vector< Constraint * > tbd;//list of temporary copies that need to be deleted - for(std::size_t i = newVals.size()-ConstraintList.size(); iType == Tangent || newVals[i]->Type == Perpendicular ){ - Constraint *constNew = newVals[i]->clone(); - AutoLockTangencyAndPerpty(constNew); - tbd.push_back(constNew); - newVals[i] = constNew; + newVals.reserve(vals.size() + ConstraintList.size()); + + // deep copy originals + for(auto &v : vals) { + newVals.push_back(v->clone()); + } + + // deep copy new ones + for(auto &v : ConstraintList) { + + auto & cnew = v; + newVals.push_back(v->clone()); + + if( cnew->Type == Tangent || cnew->Type == Perpendicular ){ + AutoLockTangencyAndPerpty(cnew); } } - this->Constraints.setValues(newVals); - - //clean up - delete temporary copies of constraints that were made to affect the constraints - for(std::size_t i=0; iConstraints.setValues(std::move(newVals)); return this->Constraints.getSize()-1; } @@ -1049,17 +1094,24 @@ int SketchObject::addCopyOfConstraints(const SketchObject &orig) const std::vector< Constraint * > &origvals = orig.Constraints.getValues(); - std::vector< Constraint * > newVals(vals); + std::vector< Constraint * > newVals; - for(std::size_t j = 0; jcopy()); + newVals.reserve(vals.size()+origvals.size()); - std::size_t valssize = vals.size(); + for( auto & v : vals ) + newVals.push_back(v->clone()); - this->Constraints.setValues(newVals); + for( auto & v : origvals ) + newVals.push_back(v->copy()); - for(std::size_t i = valssize, j = 0; iisDriving && newVals[i]->isDimensional()) { + this->Constraints.setValues(std::move(newVals)); + + auto & uvals = this->Constraints.getValues(); + + std::size_t uvalssize = uvals.size(); + + for(std::size_t i = uvalssize, j = 0; iisDriving && uvals[i]->isDimensional()) { App::ObjectIdentifier spath = orig.Constraints.createPath(j); @@ -1069,7 +1121,6 @@ int SketchObject::addCopyOfConstraints(const SketchObject &orig) App::ObjectIdentifier dpath = this->Constraints.createPath(i); setExpression(dpath, boost::shared_ptr(expr_info.expression->copy())); } - } } @@ -1080,15 +1131,24 @@ int SketchObject::addConstraint(const Constraint *constraint) { const std::vector< Constraint * > &vals = this->Constraints.getValues(); - std::vector< Constraint * > newVals(vals); + std::vector< Constraint * > newVals; + + newVals.reserve(vals.size()+1); + + // deep copy originals + for(auto &v : vals) { + newVals.push_back(v->clone()); + } + Constraint *constNew = constraint->clone(); if (constNew->Type == Tangent || constNew->Type == Perpendicular) AutoLockTangencyAndPerpty(constNew); - newVals.push_back(constNew); - this->Constraints.setValues(newVals); - delete constNew; + newVals.push_back(constNew); // add new constraint at the back + + this->Constraints.setValues(std::move(newVals)); + return this->Constraints.getSize()-1; } @@ -3050,10 +3110,19 @@ bool SketchObject::isCarbonCopyAllowed(App::Document *pDoc, App::DocumentObject int SketchObject::addSymmetric(const std::vector &geoIdList, int refGeoId, Sketcher::PointPos refPosId/*=Sketcher::none*/) { const std::vector< Part::Geometry * > &geovals = getInternalGeometry(); - std::vector< Part::Geometry * > newgeoVals(geovals); - const std::vector< Constraint * > &constrvals = this->Constraints.getValues(); - std::vector< Constraint * > newconstrVals(constrvals); + + std::vector< Part::Geometry * > newgeoVals; + std::vector< Constraint * > newconstrVals; + + newgeoVals.reserve(geovals.size()+geoIdList.size()); + newconstrVals.reserve(constrvals.size()); // At least these, probably more. + + for( auto & v : geovals ) + newgeoVals.push_back(v->clone()); + + for( auto & v : constrvals ) + newconstrVals.push_back(v->clone()); int cgeoid = getHighestCurveIndex()+1; @@ -3477,7 +3546,7 @@ int SketchObject::addSymmetric(const std::vector &geoIdList, int refGeoId, // Block acceptGeometry in OnChanged to avoid unnecessary checks and updates { Base::StateLocker lock(internaltransaction, true); - Geometry.setValues(newgeoVals); + Geometry.setValues(std::move(newgeoVals)); for (std::vector::const_iterator it = constrvals.begin(); it != constrvals.end(); ++it) { @@ -3574,7 +3643,7 @@ int SketchObject::addSymmetric(const std::vector &geoIdList, int refGeoId, } if( newconstrVals.size() > constrvals.size() ) - Constraints.setValues(newconstrVals); + Constraints.setValues(std::move(newconstrVals)); } @@ -3585,14 +3654,32 @@ int SketchObject::addSymmetric(const std::vector &geoIdList, int refGeoId, return Geometry.getSize()-1; } -int SketchObject::addCopy(const std::vector &geoIdList, const Base::Vector3d& displacement, bool moveonly /*=false*/, bool clone /*=false*/, int csize/*=2*/, int rsize/*=1*/, - bool constraindisplacement /*= false*/, double perpscale /*= 1.0*/) +int SketchObject::addCopy(const std::vector &geoIdList, const Base::Vector3d& displacement, bool moveonly /*=false*/, + bool clone /*=false*/, int csize/*=2*/, int rsize/*=1*/, bool constraindisplacement /*= false*/, + double perpscale /*= 1.0*/) { const std::vector< Part::Geometry * > &geovals = getInternalGeometry(); - std::vector< Part::Geometry * > newgeoVals(geovals); const std::vector< Constraint * > &constrvals = this->Constraints.getValues(); - std::vector< Constraint * > newconstrVals(constrvals); + + std::vector< Part::Geometry * > newgeoVals; + + std::vector< Constraint * > newconstrVals; + + if(moveonly) { + newgeoVals.reserve(geovals.size()); + newconstrVals.reserve(constrvals.size()); + } + else { + newgeoVals.reserve(geovals.size()+geoIdList.size()); + newconstrVals.reserve(constrvals.size()); // At least the same (moving), probably more (copying) but not easy to estimate. + } + + for(auto & v : geovals) + newgeoVals.push_back(v->clone()); + + for(auto & v : constrvals) + newconstrVals.push_back(v->clone()); std::vector newgeoIdList(geoIdList); @@ -3652,7 +3739,14 @@ int SketchObject::addCopy(const std::vector &geoIdList, const Base::Vector3 int index = 0; for (std::vector::const_iterator it = newgeoIdList.begin(); it != newgeoIdList.end(); ++it, index++) { const Part::Geometry *geo = getGeometry(*it); - Part::Geometry *geocopy = geo->copy(); // make a copy of the pointer for undo even if moving + + Part::Geometry *geocopy; + + // We have already cloned all geometry and constraints, we only need a copy if not moving + if(!moveonly) + geocopy = geo->copy(); // make a copy of the pointer for undo even if moving + else + geocopy = newgeoVals[*it]; // Handle Geometry if(geocopy->getTypeId() == Part::GeomLineSegment::getClassTypeId()){ @@ -3755,13 +3849,13 @@ int SketchObject::addCopy(const std::vector &geoIdList, const Base::Vector3 continue; } - if(!moveonly) { + if(!moveonly) { // we are copying newgeoVals.push_back(geocopy); geoIdMap.insert(std::make_pair(*it, cgeoid)); cgeoid++; } - else { - newgeoVals[index] = geocopy; + else { // We are moving + newgeoVals[*it] = geocopy; } } @@ -3998,10 +4092,10 @@ int SketchObject::addCopy(const std::vector &geoIdList, const Base::Vector3 // Block acceptGeometry in OnChanged to avoid unnecessary checks and updates { Base::StateLocker lock(internaltransaction, true); - Geometry.setValues(newgeoVals); + Geometry.setValues(std::move(newgeoVals)); if( newconstrVals.size() > constrvals.size() ) - Constraints.setValues(newconstrVals); + Constraints.setValues(std::move(newconstrVals)); } // we inhibited update, so we trigger it now @@ -5071,6 +5165,7 @@ bool SketchObject::modifyBSplineKnotMultiplicity(int GeoId, int knotIndex, int m } } catch (const Base::Exception& e) { + delete bspline; Base::Console().Error("%s\n", e.what()); return false; } @@ -5147,11 +5242,11 @@ bool SketchObject::modifyBSplineKnotMultiplicity(int GeoId, int knotIndex, int m } } else { // it is a bspline geometry, but not a controlpoint or knot - newcVals.push_back(*it); + newcVals.push_back((*it)->clone()); } } else { - newcVals.push_back(*it); + newcVals.push_back((*it)->clone()); } } @@ -5159,14 +5254,23 @@ bool SketchObject::modifyBSplineKnotMultiplicity(int GeoId, int knotIndex, int m std::vector< Part::Geometry * > newVals(vals); - newVals[GeoId] = bspline; + // deep copy + for(size_t i=0; iclone(); + } + } // Block acceptGeometry in OnChanged to avoid unnecessary checks and updates { Base::StateLocker lock(internaltransaction, true); - Geometry.setValues(newVals); + Geometry.setValues(std::move(newVals)); - this->Constraints.setValues(newcVals); + this->Constraints.setValues(std::move(newcVals)); } // Trigger update now // Update geometry indices and rebuild vertexindex now via onChanged, so that ViewProvider::UpdateData is triggered. @@ -5214,9 +5318,17 @@ int SketchObject::carbonCopy(App::DocumentObject * pObj, bool construction) const std::vector< Sketcher::Constraint * > &cvals = Constraints.getValues(); - std::vector< Part::Geometry * > newVals(vals); + const std::vector< Part::Geometry * > &svals = psObj->getInternalGeometry(); - std::vector< Constraint * > newcVals(cvals); + const std::vector< Sketcher::Constraint * > &scvals = psObj->Constraints.getValues(); + + std::vector< Part::Geometry * > newVals; + + std::vector< Constraint * > newcVals; + + newVals.reserve(vals.size()+svals.size()); + + newcVals.reserve(cvals.size()+scvals.size()); int nextgeoid = vals.size(); @@ -5224,10 +5336,6 @@ int SketchObject::carbonCopy(App::DocumentObject * pObj, bool construction) int nextcid = cvals.size(); - const std::vector< Part::Geometry * > &svals = psObj->getInternalGeometry(); - - const std::vector< Sketcher::Constraint * > &scvals = psObj->Constraints.getValues(); - if(psObj->ExternalGeometry.getSize()>0) { std::vector Objects = ExternalGeometry.getValues(); std::vector SubElements = ExternalGeometry.getSubValues(); @@ -5277,6 +5385,12 @@ int SketchObject::carbonCopy(App::DocumentObject * pObj, bool construction) solverNeedsUpdate=true; } + for( auto &v : vals) + newVals.push_back(v->clone()); + + for( auto &v : cvals) + newcVals.push_back(v->clone()); + for (std::vector::const_iterator it=svals.begin(); it != svals.end(); ++it){ Part::Geometry *geoNew = (*it)->copy(); if(construction && geoNew->getTypeId() != Part::GeomPoint::getClassTypeId()) { @@ -5307,15 +5421,15 @@ int SketchObject::carbonCopy(App::DocumentObject * pObj, bool construction) // Block acceptGeometry in OnChanged to avoid unnecessary checks and updates { Base::StateLocker lock(internaltransaction, true); - Geometry.setValues(newVals); - this->Constraints.setValues(newcVals); + Geometry.setValues(std::move(newVals)); + this->Constraints.setValues(std::move(newcVals)); } // we trigger now the update (before dealing with expressions) // Update geometry indices and rebuild vertexindex now via onChanged, so that ViewProvider::UpdateData is triggered. Geometry.touch(); int sourceid = 0; - for (std::vector< Sketcher::Constraint * >::const_iterator it= scvals.begin(); it != scvals.end(); ++it,nextcid++,sourceid++) { + for (std::vector< Sketcher::Constraint * >::const_iterator it= scvals.begin(); it != scvals.end(); ++it, nextcid++, sourceid++) { if ((*it)->Type == Sketcher::Distance || (*it)->Type == Sketcher::Radius || @@ -5486,9 +5600,7 @@ int SketchObject::delAllExternal() } solverNeedsUpdate=true; - Constraints.setValues(newConstraints); - for (Constraint* it : newConstraints) - delete it; + Constraints.setValues(std::move(newConstraints)); acceptGeometry(); // This may need to be refactored into OnChanged for ExternalGeometry return 0; } @@ -5671,9 +5783,7 @@ void SketchObject::validateExternalLinks(void) } } - Constraints.setValues(newConstraints); - for (Constraint* it : newConstraints) - delete it; + Constraints.setValues(std::move(newConstraints)); i--; // we deleted an item, so the next one took its place } } @@ -6972,28 +7082,25 @@ int SketchObject::changeConstraintsLocking(bool bLock) std::vector< Constraint * > newVals(vals);//modifiable copy of pointers array - std::vector< Constraint * > tbd;//list of temporary Constraint copies that need to be deleted later + // deep copy + for(size_t i=0; iclone(); - for(std::size_t i = 0; iType == Tangent || newVals[i]->Type == Perpendicular ){ //create a constraint copy, affect it, replace the pointer cntToBeAffected++; - Constraint *constNew = newVals[i]->clone(); - bool ret = AutoLockTangencyAndPerpty(constNew, /*bForce=*/true, bLock); - if (ret) cntSuccess++; - tbd.push_back(constNew); - newVals[i] = constNew; + + bool ret = AutoLockTangencyAndPerpty(newVals[i], /*bForce=*/true, bLock); + + if (ret) + cntSuccess++; + Base::Console().Log("Constraint%i will be affected\n", i+1); } } - this->Constraints.setValues(newVals); - - //clean up - delete temporary copies of constraints that were made to affect the constraints - for(std::size_t i=0; iConstraints.setValues(std::move(newVals)); Base::Console().Log("ChangeConstraintsLocking: affected %i of %i tangent/perp constraints\n", cntSuccess, cntToBeAffected);