From d31ddccd029133c2eb33fe0e737762035d8ddc7c Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Tue, 13 Aug 2024 20:35:54 +0530 Subject: [PATCH] [Sketcher] Refactor `delExternalPrivate()` --- src/Mod/Sketcher/App/SketchObject.cpp | 135 ++++++++++++-------------- 1 file changed, 60 insertions(+), 75 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 9e7a437efc..ec7376df61 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -7580,112 +7580,97 @@ int SketchObject::delExternal(const std::vector& ExtGeoIds) return 0; } -void SketchObject::delExternalPrivate(const std::set &ids, bool removeRef) { - - Base::StateLocker lock(managedoperation, true); // no need to check input data validity as this is an sketchobject managed operation. +// clang-format on +void SketchObject::delExternalPrivate(const std::set& ids, bool removeRef) +{ + Base::StateLocker lock(managedoperation, true); // no need to check input data validity as this + // is an sketchobject managed operation. std::set refs; // Must sort in reverse order so as to delete geo from back to front to // avoid index change std::set> geoIds; - for(auto id : ids) { + for (auto id : ids) { auto it = externalGeoMap.find(id); - if(it == externalGeoMap.end()) + if (it == externalGeoMap.end()) { continue; + } auto egf = ExternalGeometryFacade::getFacade(ExternalGeo[it->second]); - if(removeRef && egf->getRef().size()) + if (removeRef && egf->getRef().size()) { refs.insert(egf->getRef()); - geoIds.insert(-it->second-1); + } + geoIds.insert(-it->second - 1); } - if(geoIds.empty()) + if (geoIds.empty()) { return; + } - std::vector< Constraint * > newConstraints; - for(auto cstr : Constraints.getValues()) { - if(!geoIds.count(cstr->First) && - (cstr->Second==GeoEnum::GeoUndef || !geoIds.count(cstr->Second)) && - (cstr->Third==GeoEnum::GeoUndef || !geoIds.count(cstr->Third))) - { - bool cloned = false; - int offset = 0; - for(auto GeoId : geoIds) { - GeoId += offset++; - bool done = true; - if (cstr->First < GeoId && cstr->First != GeoEnum::GeoUndef) { - if (!cloned) { - cloned = true; - cstr = cstr->clone(); - } - cstr->First += 1; - done = false; - } - if (cstr->Second < GeoId && cstr->Second != GeoEnum::GeoUndef) { - if (!cloned) { - cloned = true; - cstr = cstr->clone(); - } - cstr->Second += 1; - done = false; - } - if (cstr->Third < GeoId && cstr->Third != GeoEnum::GeoUndef) { - if (!cloned) { - cloned = true; - cstr = cstr->clone(); - } - cstr->Third += 1; - done = false; - } - if(done) break; - } - newConstraints.push_back(cstr); + std::vector newConstraints; + for (const auto& cstr : Constraints.getValues()) { + if (geoIds.count(cstr->First) + || (cstr->Second != GeoEnum::GeoUndef && geoIds.count(cstr->Second)) + || (cstr->Third != GeoEnum::GeoUndef && geoIds.count(cstr->Third))) { + continue; } + int offset = 0; + std::unique_ptr newCstr(cstr->clone()); + for (auto GeoId : geoIds) { + GeoId += offset++; + if (newCstr->First >= GeoId && newCstr->Second >= GeoId && newCstr->Third >= GeoId) { + break; + } + changeConstraintAfterDeletingGeo(newCstr.get(), GeoId); + } + // need to provide raw pointer because that's the only one supported by `setValues` + newConstraints.push_back(newCstr.release()); } auto geos = ExternalGeo.getValues(); int offset = 0; - for(auto geoId : geoIds) { - int idx = -geoId-1; - geos.erase(geos.begin()+idx-offset); + for (auto geoId : geoIds) { + int idx = -geoId - 1; + geos.erase(geos.begin() + idx - offset); ++offset; } - if(refs.size()) { - std::vector newSubs; - std::vector newObjs; - const auto &subs = ExternalGeometry.getSubValues(); - auto itSub = subs.begin(); - const auto &objs = ExternalGeometry.getValues(); - auto itObj = objs.begin(); - bool touched = false; - assert(externalGeoRef.size() == objs.size()); - assert(externalGeoRef.size() == subs.size()); - for(auto it=externalGeoRef.begin();it!=externalGeoRef.end();++it,++itObj,++itSub) { - if(refs.count(*it)) { - if(!touched) { - touched = true; - if(newObjs.empty()) { - newObjs.insert(newObjs.end(),objs.begin(),itObj); - newSubs.insert(newSubs.end(),subs.begin(),itSub); - } - } - }else if(touched) { - newObjs.push_back(*itObj); - newSubs.push_back(*itSub); - } + if (refs.empty()) { + ExternalGeo.setValues(std::move(geos)); + + solverNeedsUpdate = true; + Constraints.setValues(std::move(newConstraints)); + acceptGeometry(); // This may need to be refactored into OnChanged for ExternalGeometry. + } + + std::vector newSubs; + std::vector newObjs; + const auto& subs = ExternalGeometry.getSubValues(); + auto itSub = subs.begin(); + const auto& objs = ExternalGeometry.getValues(); + auto itObj = objs.begin(); + bool touched = false; + assert(externalGeoRef.size() == objs.size()); + assert(externalGeoRef.size() == subs.size()); + for (auto it = externalGeoRef.begin(); it != externalGeoRef.end(); ++it, ++itObj, ++itSub) { + if (refs.count(*it) == 0) { + touched = true; + newObjs.push_back(*itObj); + newSubs.push_back(*itSub); } - if(touched) - ExternalGeometry.setValues(newObjs,newSubs); + } + if (touched) { + ExternalGeometry.setValues(newObjs, newSubs); } ExternalGeo.setValues(std::move(geos)); solverNeedsUpdate = true; Constraints.setValues(std::move(newConstraints)); - acceptGeometry(); // This may need to be refactored into OnChanged for ExternalGeometry. + acceptGeometry(); // This may need to be refactored into OnChanged for ExternalGeometry. } +// clang-format off // clang-format on int SketchObject::delAllExternal()