From 5a4072587bb3685018c65c0b6c40b646f200551c Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Sun, 11 Aug 2024 22:20:56 +0530 Subject: [PATCH] [Sketcher] Some trivial `for` loop changes in `SketchObject` No tests added since this should be no more different than existing code. --- src/Mod/Sketcher/App/SketchObject.cpp | 212 +++++++++++++------------- 1 file changed, 107 insertions(+), 105 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index ba9bedba3dc8..ae18fc8904fd 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -1865,7 +1865,6 @@ int SketchObject::delGeometriesExclusiveList(const std::vector& GeoIds) if (sGeoIds.front() < 0 || sGeoIds.back() >= int(vals.size())) return -1; - std::vector newVals(vals); for (auto it = sGeoIds.rbegin(); it != sGeoIds.rend(); ++it) { int GeoId = *it; @@ -1880,8 +1879,6 @@ int SketchObject::delGeometriesExclusiveList(const std::vector& GeoIds) delConstraintOnPoint(GeoId, PosId, true /* only coincidence */); transferConstraints(GeoIdList[0], PosIdList[0], GeoIdList[1], PosIdList[1]); } - // loop through [start, end, mid] - PosId = (PosId == PointPos::start) ? PointPos::end : PointPos::mid; } } @@ -1892,23 +1889,22 @@ int SketchObject::delGeometriesExclusiveList(const std::vector& GeoIds) std::vector filteredConstraints(0); for (auto itGeo = sGeoIds.rbegin(); itGeo != sGeoIds.rend(); ++itGeo) { int GeoId = *itGeo; - for (std::vector::const_iterator it = constraints.begin(); - it != constraints.end(); - ++it) { - - Constraint* copiedConstr(*it); - if ((*it)->First != GeoId && (*it)->Second != GeoId && (*it)->Third != GeoId) { - if (copiedConstr->First > GeoId) - copiedConstr->First -= 1; - if (copiedConstr->Second > GeoId) - copiedConstr->Second -= 1; - if (copiedConstr->Third > GeoId) - copiedConstr->Third -= 1; - filteredConstraints.push_back(copiedConstr); - } - else { + for (const auto& constr : constraints) { + Constraint* copiedConstr(constr); + if (constr->First == GeoId || + constr->Second == GeoId || + constr->Third == GeoId) { delete copiedConstr; + continue; } + + if (copiedConstr->First > GeoId) + copiedConstr->First -= 1; + if (copiedConstr->Second > GeoId) + copiedConstr->Second -= 1; + if (copiedConstr->Third > GeoId) + copiedConstr->Third -= 1; + filteredConstraints.push_back(copiedConstr); } constraints = filteredConstraints; @@ -9500,57 +9496,58 @@ const std::vector> SketchObject::getCoincidenc std::vector> coincidenttree; // push the constraints - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); - ++it) { - if ((*it)->Type == Sketcher::Coincident) { - int firstpresentin = -1; - int secondpresentin = -1; + for (const auto& constr : vals) { + if (constr->Type != Sketcher::Coincident) { + continue; + } - int i = 0; + int firstpresentin = -1; + int secondpresentin = -1; - for (std::vector>::const_iterator iti = - coincidenttree.begin(); - iti != coincidenttree.end(); - ++iti, i++) { - // First - std::map::const_iterator filiterator; - filiterator = (*iti).find((*it)->First); - if (filiterator != (*iti).end()) { - if ((*it)->FirstPos == (*filiterator).second) - firstpresentin = i; - } - // Second - filiterator = (*iti).find((*it)->Second); - if (filiterator != (*iti).end()) { - if ((*it)->SecondPos == (*filiterator).second) - secondpresentin = i; - } - } + int i = 0; - if (firstpresentin != -1 && secondpresentin != -1) { - // we have to merge those sets into one - coincidenttree[firstpresentin].insert(coincidenttree[secondpresentin].begin(), - coincidenttree[secondpresentin].end()); - coincidenttree.erase(coincidenttree.begin() + secondpresentin); - } - else if (firstpresentin == -1 && secondpresentin == -1) { - // we do not have any of the values, so create a setCursor - std::map tmp; - tmp.insert(std::pair((*it)->First, (*it)->FirstPos)); - tmp.insert(std::pair((*it)->Second, (*it)->SecondPos)); - coincidenttree.push_back(tmp); + for (std::vector>::const_iterator iti = + coincidenttree.begin(); + iti != coincidenttree.end(); + ++iti, ++i) { + // First + std::map::const_iterator filiterator; + filiterator = (*iti).find(constr->First); + if (filiterator != (*iti).end()) { + if (constr->FirstPos == (*filiterator).second) + firstpresentin = i; } - else if (firstpresentin != -1) { - // add to existing group - coincidenttree[firstpresentin].insert( - std::pair((*it)->Second, (*it)->SecondPos)); - } - else {// secondpresentin != -1 - // add to existing group - coincidenttree[secondpresentin].insert( - std::pair((*it)->First, (*it)->FirstPos)); + // Second + filiterator = (*iti).find(constr->Second); + if (filiterator != (*iti).end()) { + if (constr->SecondPos == (*filiterator).second) + secondpresentin = i; } } + + if (firstpresentin != -1 && secondpresentin != -1) { + // we have to merge those sets into one + coincidenttree[firstpresentin].insert(coincidenttree[secondpresentin].begin(), + coincidenttree[secondpresentin].end()); + coincidenttree.erase(coincidenttree.begin() + secondpresentin); + } + else if (firstpresentin == -1 && secondpresentin == -1) { + // we do not have any of the values, so create a setCursor + std::map tmp; + tmp.insert(std::pair(constr->First, constr->FirstPos)); + tmp.insert(std::pair(constr->Second, constr->SecondPos)); + coincidenttree.push_back(tmp); + } + else if (firstpresentin != -1) { + // add to existing group + coincidenttree[firstpresentin].insert( + std::pair(constr->Second, constr->SecondPos)); + } + else {// secondpresentin != -1 + // add to existing group + coincidenttree[secondpresentin].insert( + std::pair(constr->First, constr->FirstPos)); + } } return coincidenttree; @@ -9751,50 +9748,55 @@ void SketchObject::getGeometryWithDependentParameters( { auto geos = getInternalGeometry(); - int geoid = 0; + int geoid = -1; for (auto geo : geos) { - if (geo) { - if (geo->hasExtension(Sketcher::SolverGeometryExtension::getClassTypeId())) { - - auto solvext = std::static_pointer_cast( - geo->getExtension(Sketcher::SolverGeometryExtension::getClassTypeId()).lock()); - - if (solvext->getGeometry() - == Sketcher::SolverGeometryExtension::NotFullyConstraint) { - // The solver differentiates whether the parameters that are dependent are not - // those of start, end, mid, and assigns them to the edge (edge params = curve - // params - parms of start, end, mid). The user looking at the UI expects that - // the edge of a NotFullyConstraint geometry will always move, even if the edge - // parameters are independent, for example if mid is the only dependent - // parameter. In other words, the user could reasonably restrict the edge to - // reach a fully constrained element. Under this understanding, the edge - // parameter would always be dependent, unless the element is fully constrained. - // - // While this is ok from a user visual expectation point of view, it leads to a - // loss of information of whether restricting the point start, end, mid that is - // dependent may suffice, or even if such points are restricted, the edge would - // still need to be restricted. - // - // Because Python gets the information in this function, it would lead to Python - // users having access to a lower amount of detail. - // - // For this reason, this function returns edge as dependent parameter if and - // only if constraining the parameters of the points would not suffice to - // constraint the element. - if (solvext->getEdge() == SolverGeometryExtension::Dependent) - geometrymap.emplace_back(geoid, Sketcher::PointPos::none); - if (solvext->getStart() == SolverGeometryExtension::Dependent) - geometrymap.emplace_back(geoid, Sketcher::PointPos::start); - if (solvext->getEnd() == SolverGeometryExtension::Dependent) - geometrymap.emplace_back(geoid, Sketcher::PointPos::start); - if (solvext->getMid() == SolverGeometryExtension::Dependent) - geometrymap.emplace_back(geoid, Sketcher::PointPos::start); - } - } - } - - geoid++; + ++geoid; + + if (!geo) { + continue; + } + + if (!geo->hasExtension(Sketcher::SolverGeometryExtension::getClassTypeId())) { + continue; + } + + auto solvext = std::static_pointer_cast( + geo->getExtension(Sketcher::SolverGeometryExtension::getClassTypeId()).lock()); + + if (solvext->getGeometry() + != Sketcher::SolverGeometryExtension::NotFullyConstraint) { + continue; + } + + // The solver differentiates whether the parameters that are dependent are not + // those of start, end, mid, and assigns them to the edge (edge params = curve + // params - parms of start, end, mid). The user looking at the UI expects that + // the edge of a NotFullyConstraint geometry will always move, even if the edge + // parameters are independent, for example if mid is the only dependent + // parameter. In other words, the user could reasonably restrict the edge to + // reach a fully constrained element. Under this understanding, the edge + // parameter would always be dependent, unless the element is fully constrained. + // + // While this is ok from a user visual expectation point of view, it leads to a + // loss of information of whether restricting the point start, end, mid that is + // dependent may suffice, or even if such points are restricted, the edge would + // still need to be restricted. + // + // Because Python gets the information in this function, it would lead to Python + // users having access to a lower amount of detail. + // + // For this reason, this function returns edge as dependent parameter if and + // only if constraining the parameters of the points would not suffice to + // constraint the element. + if (solvext->getEdge() == SolverGeometryExtension::Dependent) + geometrymap.emplace_back(geoid, Sketcher::PointPos::none); + if (solvext->getStart() == SolverGeometryExtension::Dependent) + geometrymap.emplace_back(geoid, Sketcher::PointPos::start); + if (solvext->getEnd() == SolverGeometryExtension::Dependent) + geometrymap.emplace_back(geoid, Sketcher::PointPos::start); + if (solvext->getMid() == SolverGeometryExtension::Dependent) + geometrymap.emplace_back(geoid, Sketcher::PointPos::start); } }