diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index ba9bedba3d..746767bb0e 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -1596,7 +1596,7 @@ Base::Axis SketchObject::getAxis(int axId) const if (geo && GeometryFacade::getConstruction(geo) && geo->is()) { if (count == axId) { - Part::GeomLineSegment* lineSeg = static_cast(geo); + auto* lineSeg = static_cast(geo); Base::Vector3d start = lineSeg->getStartPoint(); Base::Vector3d end = lineSeg->getEndPoint(); return Base::Axis(start, end - start); @@ -1770,19 +1770,10 @@ int SketchObject::delGeometry(int GeoId, bool deleteinternalgeo) const std::vector& constraints = this->Constraints.getValues(); std::vector newConstraints; newConstraints.reserve(constraints.size()); - for (auto cstr : constraints) { - if (cstr->First == GeoId || cstr->Second == GeoId || cstr->Third == GeoId) - continue; - if (cstr->First > GeoId || cstr->Second > GeoId || cstr->Third > GeoId) { - cstr = cstr->clone(); - if (cstr->First > GeoId) - cstr->First -= 1; - if (cstr->Second > GeoId) - cstr->Second -= 1; - if (cstr->Third > GeoId) - cstr->Third -= 1; + for (const auto& constr : constraints) { + if (auto newConstr = getConstraintAfterDeletingGeo(constr, GeoId)) { + newConstraints.push_back(newConstr.release()); } - newConstraints.push_back(cstr); } // Block acceptGeometry in OnChanged to avoid unnecessary checks and updates @@ -1804,12 +1795,19 @@ int SketchObject::delGeometry(int GeoId, bool deleteinternalgeo) } int SketchObject::delGeometries(const std::vector& GeoIds) +{ + return delGeometries(GeoIds.begin(), GeoIds.end()); +} + +template +int SketchObject::delGeometries(InputIt first, InputIt last) { std::vector sGeoIds; std::vector negativeGeoIds; // Separate GeoIds into negative (external) and non-negative GeoIds - for (int geoId : GeoIds) { + for (auto it = first; it != last; ++it) { + int geoId = *it; if (geoId < 0 && geoId <= GeoEnum::RefExt) { negativeGeoIds.push_back(geoId); } @@ -1865,7 +1863,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,40 +1877,27 @@ 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; } } // Copy the original constraints std::vector constraints; - for (const auto ptr : this->Constraints.getValues()) + for (const auto& ptr : this->Constraints.getValues()) { constraints.push_back(ptr->clone()); - 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 { - delete copiedConstr; - } - } - - constraints = filteredConstraints; - filteredConstraints.clear(); } + for (auto itGeo = sGeoIds.rbegin(); itGeo != sGeoIds.rend(); ++itGeo) { + const int GeoId = *itGeo; + for (auto& constr : constraints) { + changeConstraintAfterDeletingGeo(constr, GeoId); + } + } + + constraints.erase(std::remove_if(constraints.begin(), + constraints.end(), + [](const auto& constr) { + return constr->Type == ConstraintType::None; + }), + constraints.end()); // Block acceptGeometry in OnChanged to avoid unnecessary checks and updates { @@ -1932,6 +1916,40 @@ int SketchObject::delGeometriesExclusiveList(const std::vector& GeoIds) return 0; } +void SketchObject::replaceGeometries(std::vector oldGeoIds, + std::vector& newGeos) +{ + auto vals = getInternalGeometry(); + auto newVals(vals); + + if (std::any_of(oldGeoIds.begin(), oldGeoIds.end(), [](auto geoId) { + return geoId < 0; + })) { + THROWM(ValueError, "Cannot replace external geometries and axes."); + } + + auto oldGeoIdIter = oldGeoIds.begin(); + auto newGeoIter = newGeos.begin(); + + for (; oldGeoIdIter != oldGeoIds.end() && newGeoIter != newGeos.end(); + ++oldGeoIdIter, ++newGeoIter) { + GeometryFacade::copyId(getGeometry(*oldGeoIdIter), *newGeoIter); + newVals[*oldGeoIdIter] = *newGeoIter; + } + + if (newGeoIter != newGeos.end()) { + for (; newGeoIter != newGeos.end(); ++newGeoIter) { + generateId(*newGeoIter); + newVals.push_back(*newGeoIter); + } + } + else { + delGeometries(oldGeoIdIter, oldGeoIds.end()); + } + + Geometry.setValues(std::move(newVals)); +} + int SketchObject::deleteAllGeometry() { // no need to check input data validity as this is an sketchobject managed operation. @@ -2609,8 +2627,12 @@ void SketchObject::transferFilletConstraints(int geoId1, PointPos posId1, int ge this->Constraints.setValues(std::move(newConstraints)); } -int SketchObject::transferConstraints(int fromGeoId, PointPos fromPosId, int toGeoId, - PointPos toPosId, bool doNotTransformTangencies) +// clang-format on +int SketchObject::transferConstraints(int fromGeoId, + PointPos fromPosId, + int toGeoId, + PointPos toPosId, + bool doNotTransformTangencies) { // no need to check input data validity as this is an sketchobject managed operation. Base::StateLocker lock(managedoperation, true); @@ -2619,58 +2641,46 @@ int SketchObject::transferConstraints(int fromGeoId, PointPos fromPosId, int toG std::vector newVals(vals); bool changed = false; for (int i = 0; i < int(newVals.size()); i++) { - if (vals[i]->First == fromGeoId && vals[i]->FirstPos == fromPosId - && !(vals[i]->Second == toGeoId && vals[i]->SecondPos == toPosId) - && !(toGeoId < 0 && vals[i]->Second < 0)) { - - std::unique_ptr constNew(newVals[i]->clone()); - constNew->First = toGeoId; - constNew->FirstPos = toPosId; - - // If not explicitly confirmed, nothing guarantees that a tangent can be freely - // transferred to another coincident point, as the transfer destination edge most likely - // won't be intended to be tangent. However, if it is an end to end point tangency, the - // user expects it to be substituted by a coincidence constraint. - if (vals[i]->Type == Sketcher::Tangent || vals[i]->Type == Sketcher::Perpendicular) { - if (!doNotTransformTangencies) { - constNew->Type = Sketcher::Coincident; - } - } - // With respect to angle constraints, if it is a DeepSOIC style angle constraint - // (segment+segment+point), then no problem arises as the segments are PosId=none. In - // this case there is no call to this function. - // - // However, other angle constraints are problematic because they are created on - // segments, but internally operate on vertices, PosId=start Such constraint may not be - // successfully transferred on deletion of the segments. - else if (vals[i]->Type == Sketcher::Angle) { - continue; - } - - Constraint* constPtr = constNew.release(); - newVals[i] = constPtr; - changed = true; + if (vals[i]->Type == Sketcher::InternalAlignment) { + // Transferring internal alignment constraint can cause malformed constraints. + // For example a B-spline pole being a point instead of a circle. + continue; } - else if (vals[i]->Second == fromGeoId && vals[i]->SecondPos == fromPosId - && !(vals[i]->First == toGeoId && vals[i]->FirstPos == toPosId) - && !(toGeoId < 0 && vals[i]->First < 0)) { - + else if (vals[i]->involvesGeoIdAndPosId(fromGeoId, fromPosId) + && !vals[i]->involvesGeoIdAndPosId(toGeoId, toPosId)) { std::unique_ptr constNew(newVals[i]->clone()); - constNew->Second = toGeoId; - constNew->SecondPos = toPosId; - // If not explicitly confirmed, nothing guarantees that a tangent can be freely - // transferred to another coincident point, as the transfer destination edge most likely - // won't be intended to be tangent. However, if it is an end to end point tangency, the - // user expects it to be substituted by a coincidence constraint. - if (vals[i]->Type == Sketcher::Tangent || vals[i]->Type == Sketcher::Perpendicular) { - if (!doNotTransformTangencies) { - constNew->Type = Sketcher::Coincident; - } - } - else if (vals[i]->Type == Sketcher::Angle) { + constNew->substituteIndexAndPos(fromGeoId, fromPosId, toGeoId, toPosId); + if (vals[i]->First < 0 && vals[i]->Second < 0) { + // TODO: Can `vals[i]->Third` be involved as well? + // If it is, we need to be sure at most ONE of these is external continue; } + switch (vals[i]->Type) { + case Sketcher::Tangent: + case Sketcher::Perpendicular: { + // If not explicitly confirmed, nothing guarantees that a tangent can be freely + // transferred to another coincident point, as the transfer destination edge + // most likely won't be intended to be tangent. However, if it is an end to end + // point tangency, the user expects it to be substituted by a coincidence + // constraint. + if (!doNotTransformTangencies) { + constNew->Type = Sketcher::Coincident; + } + } + case Sketcher::Angle: + // With respect to angle constraints, if it is a DeepSOIC style angle constraint + // (segment+segment+point), then no problem arises as the segments are + // PosId=none. In this case there is no call to this function. + // + // However, other angle constraints are problematic because they are created on + // segments, but internally operate on vertices, PosId=start Such constraint may + // not be successfully transferred on deletion of the segments. + continue; + default: + break; + } + Constraint* constPtr = constNew.release(); newVals[i] = constPtr; changed = true; @@ -2683,6 +2693,7 @@ int SketchObject::transferConstraints(int fromGeoId, PointPos fromPosId, int toG } return 0; } +// clang-format off int SketchObject::fillet(int GeoId, PointPos PosId, double radius, bool trim, bool createCorner, bool chamfer) { @@ -3439,6 +3450,62 @@ void SketchObject::addConstraint(Sketcher::ConstraintType constrType, int firstG this->addConstraint(std::move(newConstr)); } +std::unique_ptr +SketchObject::getConstraintAfterDeletingGeo(const Constraint* constr, + const int deletedGeoId) const +{ + if (!constr) { + return nullptr; + } + + // TODO: While this is not incorrect, it recreates all constraints regardless of whether or not we need to. + auto newConstr = std::unique_ptr(constr->clone()); + + changeConstraintAfterDeletingGeo(newConstr.get(), deletedGeoId); + + if (newConstr->Type == ConstraintType::None) { + return nullptr; + } + + return newConstr; +} + +void SketchObject::changeConstraintAfterDeletingGeo(Constraint* constr, + const int deletedGeoId) const +{ + if (!constr) { + return; + } + + if (constr->First == deletedGeoId || + constr->Second == deletedGeoId || + constr->Third == deletedGeoId) { + constr->Type = ConstraintType::None; + return; + } + + int step = 1; + std::function needsUpdate = [&deletedGeoId](const int& givenId) -> bool { + return givenId > deletedGeoId; + }; + if (deletedGeoId < 0) { + step = -1; + needsUpdate = [&deletedGeoId](const int& givenId) -> bool { + return givenId < deletedGeoId && givenId != GeoEnum::GeoUndef; + }; + } + + if (needsUpdate(constr->First)) { + constr->First -= step; + } + if (needsUpdate(constr->Second)) { + constr->Second -= step; + } + if (needsUpdate(constr->Third)) { + constr->Third -= step; + } +} + bool SketchObject::seekTrimPoints(int GeoId, const Base::Vector3d& point, int& GeoId1, Base::Vector3d& intersect1, int& GeoId2, Base::Vector3d& intersect2) @@ -3465,6 +3532,25 @@ bool SketchObject::seekTrimPoints(int GeoId, const Base::Vector3d& point, int& G return true; } +// given a geometry and a point, returns the corresponding parameter of the geometry point +// closest to the point. Wrapped around a try-catch so the calling operation can fail without +// throwing an exception. +bool getIntersectionParameter(const Part::Geometry* geo, + const Base::Vector3d point, + double& pointParam) { + const auto* curve = static_cast(geo); + + try { + curve->closestParameter(point, pointParam); + } + catch (Base::CADKernelError& e) { + e.ReportException(); + return false; + } + + return true; +} + int SketchObject::trim(int GeoId, const Base::Vector3d& point) { // no need to check input data validity as this is an sketchobject managed operation. @@ -3472,50 +3558,96 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) //******************* Basic checks rejecting the operation //****************************************// - if (GeoId < 0 || GeoId > getHighestCurveIndex()) + if (GeoId < 0 || GeoId > getHighestCurveIndex()) { return -1; + } auto geo = getGeometry(GeoId); - if (!GeometryFacade::isInternalType(geo, InternalType::None)) - return -1;// internal alignment geometry is not trimmable + if (!GeometryFacade::isInternalType(geo, InternalType::None)) { + return -1; // internal alignment geometry is not trimmable + } //******************* Lambdas - common functions for different intersections //****************************************// - // returns true if the point defined by (GeoId1, pos1) can be considered to be coincident with - // point. - auto arePointsWithinPrecision = [](Base::Vector3d point1, Base::Vector3d& point2) { + auto arePointsWithinPrecision = [](Base::Vector3d& point1, Base::Vector3d& point2) { // From testing: 500x (or 0.000050) is needed in order to not falsely distinguish points // calculated with seekTrimPoints - return ((point1 - point2).Length() < 500 * Precision::Confusion()) ; + return ((point1 - point2).Length() < 500 * Precision::Confusion()); }; - auto isPointAtPosition = [this, &arePointsWithinPrecision] - (int GeoId1, PointPos pos1, Base::Vector3d point) { - Base::Vector3d pp = getPoint(GeoId1, pos1); - - return arePointsWithinPrecision(point, pp); + auto areParamsWithinApproximation = [](double param1, double param2) { + // From testing: 500x (or 0.000050) is needed in order to not falsely distinguish points + // calculated with seekTrimPoints + return (std::abs(param1 - param2) < Precision::PApproximation()); }; -#if 0 + + // returns true if the point defined by (GeoId1, pos1) can be considered to be coincident with + // point. + auto isPointAtPosition = + [this, &arePointsWithinPrecision](int GeoId1, PointPos pos1, Base::Vector3d& point) { + Base::Vector3d pp = getPoint(GeoId1, pos1); + + return arePointsWithinPrecision(point, pp); + }; + // Checks whether preexisting constraints must be converted to new constraints. - // Preexisting point on object constraints get converted to coincidents, unless an end-to-end - // tangency is more relevant. returns by reference: - // - The type of constraint that should be used to constraint GeoId1 and GeoId - // - The element of GeoId1 to which the constraint should be applied. - auto transformPreexistingConstraint = [this, isPointAtPosition](int GeoId, - int GeoId1, - Base::Vector3d point1, - Constraint* constr) { - // TODO: Move code currently later in this method (that does as per the following description) here. - /* It is possible that the trimming entity has both a PointOnObject constraint to the + // Preexisting point on object constraints get converted to coincidents. + // Returns: + // - The constraint that should be used to constraint GeoId and cuttingGeoId + auto transformPreexistingConstraint = [&isPointAtPosition](const Constraint* constr, + int GeoId, + int cuttingGeoId, + Base::Vector3d& cutPointVec, + int newGeoId, + PointPos newPosId) { + /* TODO: It is possible that the trimming entity has both a PointOnObject constraint to the * trimmed entity, and a simple Tangent constraint to the trimmed entity. In this case we - * want to change to a single end-to-end tangency, i.e we want to ensure that constrType1 is - * set to Sketcher::Tangent, that the secondPos1 is captured from the PointOnObject, and - * also make sure that the PointOnObject constraint is deleted. The below loop ensures this, - * also in case the ordering of the constraints is first Tangent and then PointOnObject. */ + * want to change to a single end-to-end tangency, i.e we want to ensure that constrType1 + * is set to Sketcher::Tangent, that the secondPos1 is captured from the PointOnObject, + * and also make sure that the PointOnObject constraint is deleted. + */ + std::unique_ptr newConstr; + if (!constr->involvesGeoId(cuttingGeoId) + || !constr->involvesGeoIdAndPosId(GeoId, PointPos::none)) { + return newConstr; + } + switch (constr->Type) { + case PointOnObject: { + // we might want to transform this (and the new point-on-object constraints) into a coincidence + // At this stage of the check the point has to be an end of `cuttingGeoId` on the edge of `GeoId`. + if (isPointAtPosition(constr->First, constr->FirstPos, cutPointVec)) { + // This concerns the start portion of the trim + newConstr.reset(constr->copy()); + newConstr->Type = Sketcher::Coincident; + newConstr->Second = newGeoId; + newConstr->SecondPos = newPosId; + } + break; + } + case Tangent: + case Perpendicular: { + // These may have to be turned into endpoint-to-endpoint or endpoint-to-edge + // TODO: could there be tangent/perpendicular constraints not involving the trim that are modified below? + newConstr.reset(constr->copy()); + newConstr->substituteIndexAndPos(GeoId, PointPos::none, newGeoId, newPosId); + // make sure the first position is a point + if (newConstr->FirstPos == PointPos::none) { + std::swap(newConstr->First, newConstr->Second); + std::swap(newConstr->FirstPos, newConstr->SecondPos); + } + // there is no need for the third point if it exists + newConstr->Third = GeoEnum::GeoUndef; + newConstr->ThirdPos = PointPos::none; + break; + } + default: + break; + } + return newConstr; }; -#endif + // makes an equality constraint between GeoId1 and GeoId2 auto constrainAsEqual = [this](int GeoId1, int GeoId2) { auto newConstr = std::make_unique(); @@ -3529,104 +3661,80 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) addConstraint(std::move(newConstr)); }; - // given a geometry and tree points, returns the corresponding parameters of the geometry points - // closest to them - auto getIntersectionParameters = [](const Part::Geometry* geo, - const Base::Vector3d point, - double& pointParam, - const Base::Vector3d point1, - double& point1Param, - const Base::Vector3d point2, - double& point2Param) { - auto curve = static_cast(geo); - - try { - curve->closestParameter(point, pointParam); - curve->closestParameter(point1, point1Param); - curve->closestParameter(point2, point2Param); - } - catch (Base::CADKernelError& e) { - e.ReportException(); - return false; - } - - return true; - }; - //******************* Step A => Detection of intersection - Common to all Geometries //****************************************// - int GeoId1 = GeoEnum::GeoUndef, GeoId2 = GeoEnum::GeoUndef; - Base::Vector3d point1, point2; - - // Remove internal geometry beforehand so GeoId1 and GeoId2 do not change - deleteUnusedInternalGeometryAndUpdateGeoId(GeoId); - geo = getGeometry(GeoId); + // GeoIds intersecting the curve around `point` + std::array cuttingGeoIds {GeoEnum::GeoUndef, GeoEnum::GeoUndef}; + // Points at the intersection + std::array cutPoints; // Using SketchObject wrapper, as Part2DObject version returns GeoId = -1 when intersection not // found, which is wrong for a GeoId (axis). seekTrimPoints returns: // - For a parameter associated with "point" between an intersection and the end point - // (non-periodic case) GeoId1 != GeoUndef and GeoId2 == GeoUndef + // (non-periodic case) cuttingGeoIds[0] != GeoUndef and cuttingGeoIds[1] == GeoUndef // - For a parameter associated with "point" between the start point and an intersection - // (non-periodic case) GeoId2 != GeoUndef and GeoId1 == GeoUndef - // - For a parameter associated with "point" between two intersection points, GeoId1 != GeoUndef - // and GeoId2 != GeoUndef + // (non-periodic case) cuttingGeoIds[1] != GeoUndef and cuttingGeoIds[0] == GeoUndef + // - For a parameter associated with "point" between two intersection points, cuttingGeoIds[0] + // != GeoUndef and cuttingGeoIds[1] != GeoUndef // // FirstParam < point1param < point2param < LastParam - if (!SketchObject::seekTrimPoints(GeoId, point, GeoId1, point1, GeoId2, point2)) { + if (!SketchObject::seekTrimPoints(GeoId, + point, + cuttingGeoIds[0], + cutPoints[0], + cuttingGeoIds[1], + cutPoints[1])) { // If no suitable trim points are found, then trim defaults to deleting the geometry delGeometry(GeoId); return 0; } - if (GeoId1 != GeoEnum::GeoUndef && GeoId2 != GeoEnum::GeoUndef - && arePointsWithinPrecision(point1, point2)) { - // If both points are detected and are coincident, deletion is the only option. - delGeometry(GeoId); - return 0; - } - const auto* geoAsCurve = static_cast(geo); double firstParam = geoAsCurve->getFirstParameter(); double lastParam = geoAsCurve->getLastParameter(); - double pointParam, point1Param, point2Param; - if (!getIntersectionParameters( - geo, point, pointParam, point1, point1Param, point2, point2Param)) { + double pointParam, cut0Param{firstParam}, cut1Param{lastParam}; + bool allParamsFound = getIntersectionParameter(geo, point, pointParam) + && getIntersectionParameter(geo, cutPoints[0], cut0Param) + && getIntersectionParameter(geo, cutPoints[1], cut1Param); + if (!allParamsFound) { return -1; } - bool startPointRemains = false; - bool endPointRemains = false; - std::vector > paramsOfNewGeos; + if (!isClosedCurve(geo) && areParamsWithinApproximation(firstParam, cut0Param)) { + cuttingGeoIds[0] = GeoEnum::GeoUndef; + } + + if (!isClosedCurve(geo) && areParamsWithinApproximation(lastParam, cut1Param)) { + cuttingGeoIds[1] = GeoEnum::GeoUndef; + } + + size_t numUndefs = std::count(cuttingGeoIds.begin(), cuttingGeoIds.end(), GeoEnum::GeoUndef); + + if (numUndefs == 0 && arePointsWithinPrecision(cutPoints[0], cutPoints[1])) { + // If both points are detected and are coincident, deletion is the only option. + delGeometry(GeoId); + return 0; + } + + bool startPointRemains = (cuttingGeoIds[0] != GeoEnum::GeoUndef); + bool endPointRemains = (cuttingGeoIds[1] != GeoEnum::GeoUndef); + std::vector> paramsOfNewGeos(2 - numUndefs, {firstParam, lastParam}); std::vector newIds; std::vector newGeos; std::vector newGeosAsConsts; + // TODO: This should be needed, but for some reason we already get both curves as construction + // when needed. - if (isClosedCurve(geo)) { + // bool oldGeoIsConstruction = + // GeometryFacade::getConstruction(static_cast(geo)); + + if (isClosedCurve(geo) && !paramsOfNewGeos.empty()) { startPointRemains = false; endPointRemains = false; - if (GeoId1 != GeoEnum::GeoUndef && GeoId2 != GeoEnum::GeoUndef) { - paramsOfNewGeos.emplace_back(point2Param, point1Param); - } - } - else { - if (GeoId1 != GeoEnum::GeoUndef) { - startPointRemains = true; - paramsOfNewGeos.emplace_back(firstParam, point1Param); - } - if (GeoId2 != GeoEnum::GeoUndef) { - endPointRemains = true; - paramsOfNewGeos.emplace_back(point2Param, lastParam); - } + paramsOfNewGeos.pop_back(); } - for (auto& [u1, u2] : paramsOfNewGeos) { - auto newGeo = geoAsCurve->createArc(u1, u2); - assert(newGeo); - newGeos.push_back(newGeo); - newGeosAsConsts.push_back(newGeo); - } - - switch (newGeos.size()) { + switch (paramsOfNewGeos.size()) { case 0: { delGeometry(GeoId); return 0; @@ -3641,13 +3749,24 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) break; } default: { - for (auto& newGeo : newGeos) { - delete newGeo; - } return -1; } } + if (cuttingGeoIds[0] != GeoEnum::GeoUndef) { + paramsOfNewGeos.front().second = cut0Param; + } + if (cuttingGeoIds[1] != GeoEnum::GeoUndef) { + paramsOfNewGeos.back().first = cut1Param; + } + + for (auto& [u1, u2] : paramsOfNewGeos) { + auto newGeo = static_cast(geo)->createArc(u1, u2); + assert(newGeo); + newGeos.push_back(newGeo); + newGeosAsConsts.push_back(newGeo); + } + // Now that we have the new curves, change constraints as needed // Some are covered with `deriveConstraintsForPieces`, others are specific to trim // FIXME: We are using non-smart pointers since that's what's needed in `addConstraints`. @@ -3667,13 +3786,13 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) if (endPointRemains) { transferConstraints(GeoId, PointPos::end, newIds.back(), PointPos::end, true); } - bool geomHasMid = geo->isDerivedFrom() || geo->isDerivedFrom(); + bool geomHasMid = + geo->isDerivedFrom() || geo->isDerivedFrom(); if (geomHasMid) { transferConstraints(GeoId, PointPos::mid, newIds.front(), PointPos::mid, true); // Make centers coincident if (newIds.size() > 1) { Constraint* joint = new Constraint(); - joint = new Constraint(); joint->Type = Coincident; joint->First = newIds.front(); joint->FirstPos = PointPos::mid; @@ -3700,80 +3819,35 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) // Workaround to avoid https://github.com/FreeCAD/FreeCAD/issues/15484. // We will delete its internal geometry first and then replace GeoId with one of the curves. // Of course, this will change `newIds`, and that's why we offset them. - std::set< int, std::greater<> > geoIdsToBeDeleted; + std::set> geoIdsToBeDeleted; // geoIdsToBeDeleted.push_back(GeoId); - for (const auto& oldConstrId: idsOfOldConstraints) { + for (const auto& oldConstrId : idsOfOldConstraints) { // trim-specific changes first const Constraint* con = allConstraints[oldConstrId]; - bool constraintWasModified = false; - switch (con->Type) { - case PointOnObject: { - // we might want to transform this (and the new point-on-object constraints) into a coincidence - if (con->First == GeoId1 && con->Second == GeoId - && isPointAtPosition(con->First, con->FirstPos, point1)) { - // This concerns the start portion of the trim - Constraint* newConstr = con->copy(); - newConstr->Type = Sketcher::Coincident; - newConstr->Second = newIds.front(); - newConstr->SecondPos = PointPos::end; - newConstraints.push_back(newConstr); - isPoint1ConstrainedOnGeoId1 = true; - constraintWasModified = true; - } - else if (con->First == GeoId2 && con->Second == GeoId - && isPointAtPosition(con->First, con->FirstPos, point2)) { - // This concerns the end portion of the trim - Constraint* newConstr = con->copy(); - newConstr->Type = Sketcher::Coincident; - newConstr->Second = newIds.back(); - newConstr->SecondPos = PointPos::start; - newConstraints.push_back(newConstr); - isPoint2ConstrainedOnGeoId2 = true; - constraintWasModified = true; - } - break; - } - case Tangent: - case Perpendicular: { - // These may have to be turned into endpoint-to-endpoint or endpoint-to-edge - // TODO: could there be tangent/perpendicular constraints not involving the trim that are modified below? - if (con->involvesGeoId(GeoId1) && con->involvesGeoIdAndPosId(GeoId, PointPos::none)) { - Constraint* newConstr = con->copy(); - newConstr->substituteIndexAndPos(GeoId, PointPos::none, newIds.front(), PointPos::end); - newConstraints.push_back(newConstr); - isPoint1ConstrainedOnGeoId1 = true; - constraintWasModified = true; - } - if (con->involvesGeoId(GeoId2) && con->involvesGeoIdAndPosId(GeoId, PointPos::none)) { - Constraint* newConstr = con->copy(); - newConstr->substituteIndexAndPos(GeoId, PointPos::none, newIds.back(), PointPos::start); - newConstraints.push_back(newConstr); - isPoint2ConstrainedOnGeoId2 = true; - constraintWasModified = true; - } - if (constraintWasModified) { - // make sure the first position is a point - if (newConstraints.back()->FirstPos == PointPos::none) { - std::swap(newConstraints.back()->First, newConstraints.back()->Second); - std::swap(newConstraints.back()->FirstPos, newConstraints.back()->SecondPos); - } - // there is no need for the third point if it exists - newConstraints.back()->Third = GeoEnum::GeoUndef; - newConstraints.back()->ThirdPos = PointPos::none; - } - break; - } - case InternalAlignment: { + if (con->Type == InternalAlignment) { geoIdsToBeDeleted.insert(con->First); - break; + continue; } - default: - break; + if (auto newConstr = transformPreexistingConstraint( + con, GeoId, cuttingGeoIds[0], cutPoints[0], newIds.front(), PointPos::end)) { + newConstraints.push_back(newConstr.release()); + isPoint1ConstrainedOnGeoId1 = true; + continue; } - if (!constraintWasModified) { - deriveConstraintsForPieces(GeoId, newIds, newGeosAsConsts, con, newConstraints); + if (auto newConstr = transformPreexistingConstraint( + con, GeoId, cuttingGeoIds[1], cutPoints[1], newIds.back(), PointPos::start)) { + newConstraints.push_back(newConstr.release()); + isPoint2ConstrainedOnGeoId2 = true; + continue; } + // We have already transferred all constraints on endpoints to the new pieces. + // If there is still any left, this means one of the remaining pieces was degenerate. + if (!con->involvesGeoIdAndPosId(GeoId, PointPos::none)) { + continue; + } + // constraint has not yet been changed + deriveConstraintsForPieces(GeoId, newIds, newGeosAsConsts, con, newConstraints); } // Add point-on-object/coincidence constraints with the newly exposed points. @@ -3781,16 +3855,19 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) // to coincident or end-to-end tangency/perpendicularity. // TODO: Tangent/perpendicular not yet covered - if (GeoId1 != GeoEnum::GeoUndef && !isPoint1ConstrainedOnGeoId1) { + auto addNewConstraintAtCut = [&isPointAtPosition, &newConstraints](int cuttingGeoId, + int cutGeoId, + PointPos cutPosId, + Base::Vector3d cutPointVec) { Constraint* newConstr = new Constraint(); - newConstr->First = newIds.front(); - newConstr->FirstPos = PointPos::end; - newConstr->Second = GeoId1; - if (isPointAtPosition(GeoId1, Sketcher::PointPos::start, point1)) { + newConstr->First = cutGeoId; + newConstr->FirstPos = cutPosId; + newConstr->Second = cuttingGeoId; + if (isPointAtPosition(cuttingGeoId, PointPos::start, cutPointVec)) { newConstr->Type = Sketcher::Coincident; newConstr->SecondPos = PointPos::start; } - else if (isPointAtPosition(GeoId1, Sketcher::PointPos::end, point1)) { + else if (isPointAtPosition(cuttingGeoId, PointPos::end, cutPointVec)) { newConstr->Type = Sketcher::Coincident; newConstr->SecondPos = PointPos::end; } @@ -3800,27 +3877,14 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) newConstr->SecondPos = PointPos::none; } newConstraints.push_back(newConstr); + }; + + if (cuttingGeoIds[0] != GeoEnum::GeoUndef && !isPoint1ConstrainedOnGeoId1) { + addNewConstraintAtCut(cuttingGeoIds[0], newIds.front(), PointPos::end, cutPoints[0]); } - if (GeoId2 != GeoEnum::GeoUndef && !isPoint2ConstrainedOnGeoId2) { - Constraint* newConstr = new Constraint(); - newConstr->First = newIds.back(); - newConstr->FirstPos = PointPos::start; - newConstr->Second = GeoId2; - if (isPointAtPosition(GeoId2, Sketcher::PointPos::start, point2)) { - newConstr->Type = Sketcher::Coincident; - newConstr->SecondPos = PointPos::start; - } - else if (isPointAtPosition(GeoId2, Sketcher::PointPos::end, point2)) { - newConstr->Type = Sketcher::Coincident; - newConstr->SecondPos = PointPos::end; - } - else { - // Points are sufficiently far apart: use point-on-object - newConstr->Type = Sketcher::PointOnObject; - newConstr->SecondPos = PointPos::none; - } - newConstraints.push_back(newConstr); + if (cuttingGeoIds[1] != GeoEnum::GeoUndef && !isPoint2ConstrainedOnGeoId2) { + addNewConstraintAtCut(cuttingGeoIds[1], newIds.back(), PointPos::start, cutPoints[1]); } // Workaround to avoid https://github.com/FreeCAD/FreeCAD/issues/15484. @@ -3835,6 +3899,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) auto vals = getInternalGeometry(); auto newVals(vals); + GeometryFacade::copyId(geo, newGeos.front()); newVals[GeoId] = newGeos.front(); if (newGeos.size() > 1) { newVals.push_back(newGeos.back()); @@ -3844,7 +3909,8 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) Geometry.setValues(std::move(newVals)); // FIXME: Somewhere here we need to delete the internal geometry (even if in use) - // FIXME: Set the second geoid as construction or not depending on what the original was + // FIXME: Set the second geoid as construction or not depending on what the original was. + // This somehow is still not needed (why?) if (noRecomputes) { solve(); @@ -3855,10 +3921,18 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) changeConstraintAfterDeletingGeo(cons, deletedGeoId); } } + newConstraints.erase(std::remove_if(newConstraints.begin(), + newConstraints.end(), + [](const auto& constr) { + return constr->Type == ConstraintType::None; + }), + newConstraints.end()); + delGeometries(geoIdsToBeDeleted.begin(), geoIdsToBeDeleted.end()); addConstraints(newConstraints); - if (noRecomputes) + if (noRecomputes) { solve(); + } // Since we used regular "non-smart" pointers, we have to handle cleanup for (auto& cons : newConstraints) { @@ -3868,49 +3942,6 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) return 0; } -std::unique_ptr -SketchObject::getConstraintAfterDeletingGeo(const Constraint* constr, - const int deletedGeoId) const -{ - if (constr->First == deletedGeoId || - constr->Second == deletedGeoId || - constr->Third == deletedGeoId) { - return nullptr; - } - - // TODO: While this is not incorrect, it recreates all constraints regardless of whether or not we need to. - auto newConstr = std::unique_ptr(constr->clone()); - - changeConstraintAfterDeletingGeo(newConstr.get(), deletedGeoId); - - return newConstr; -} - -void SketchObject::changeConstraintAfterDeletingGeo(Constraint* constr, - const int deletedGeoId) const -{ - int step = 1; - std::function needsUpdate = [&deletedGeoId](const int& givenId) -> bool { - return givenId > deletedGeoId; - }; - if (deletedGeoId < 0) { - step = -1; - needsUpdate = [&deletedGeoId](const int& givenId) -> bool { - return givenId < deletedGeoId && givenId != GeoEnum::GeoUndef; - }; - } - - if (needsUpdate(constr->First)) { - constr->First -= step; - } - if (needsUpdate(constr->Second)) { - constr->Second -= step; - } - if (needsUpdate(constr->Third)) { - constr->Third -= step; - } -} - bool SketchObject::deriveConstraintsForPieces(const int oldId, const std::vector& newIds, const Constraint* con, @@ -3992,7 +4023,7 @@ bool SketchObject::deriveConstraintsForPieces(const int oldId, Base::Vector3d conPoint(getPoint(conId, conPos)); double conParam; - auto geoAsCurve = static_cast(geo); + auto* geoAsCurve = static_cast(geo); geoAsCurve->closestParameter(conPoint, conParam); // Choose based on where the closest point lies // If it's not there, just leave this constraint out @@ -4022,8 +4053,7 @@ bool SketchObject::deriveConstraintsForPieces(const int oldId, case Diameter: case Equal: { // FIXME: This sounds good by itself, but results in redundant constraints when equality is applied between newIds - transferToAll = geo->is() - || geo->is(); + transferToAll = geo->is() || geo->is(); break; } default: @@ -4054,17 +4084,15 @@ int SketchObject::split(int GeoId, const Base::Vector3d& point) return -1; } - bool originalIsPeriodic = false; const Part::Geometry* geo = getGeometry(GeoId); + bool originalIsPeriodic = isClosedCurve(geo); std::vector newIds; std::vector newConstraints; Base::Vector3d startPoint, endPoint, splitPoint; double startParam, endParam, splitParam = 0.0; - auto createGeosFromPeriodic = [&](const Part::GeomCurve* curve, - auto getCurveWithLimitParams, - auto createAndTransferConstraints) { + auto createGeosFromPeriodic = [&](const Part::GeomCurve* curve) { // find split point curve->closestParameter(point, splitParam); double period = curve->getLastParameter() - curve->getFirstParameter(); @@ -4072,7 +4100,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d& point) endParam = splitParam + period; // create new arc and restrict it - auto newCurve = getCurveWithLimitParams(curve, startParam, endParam); + auto newCurve = curve->createArc(startParam, endParam); int newId(GeoEnum::GeoUndef); newId = addGeometry(std::move(newCurve));// after here newCurve is a shell if (newId < 0) { @@ -4083,14 +4111,10 @@ int SketchObject::split(int GeoId, const Base::Vector3d& point) setConstruction(newId, GeometryFacade::getConstruction(curve)); exposeInternalGeometry(newId); - // transfer any constraints - createAndTransferConstraints(GeoId, newId); return true; }; - auto createGeosFromNonPeriodic = [&](const Part::GeomBoundedCurve* curve, - auto getCurveWithLimitParams, - auto createAndTransferConstraints) { + auto createGeosFromNonPeriodic = [&](const Part::GeomBoundedCurve* curve) { startPoint = curve->getStartPoint(); endPoint = curve->getEndPoint(); @@ -4106,7 +4130,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d& point) } // create new curves - auto newCurve = getCurveWithLimitParams(curve, startParam, splitParam); + auto newCurve = curve->createArc(startParam, splitParam); int newId(GeoEnum::GeoUndef); newId = addGeometry(std::move(newCurve)); if (newId < 0) { @@ -4117,7 +4141,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d& point) exposeInternalGeometry(newId); // the "second" half - newCurve = getCurveWithLimitParams(curve, splitParam, endParam); + newCurve = curve->createArc(splitParam, endParam); newId = addGeometry(std::move(newCurve)); if (newId < 0) { return false; @@ -4126,169 +4150,15 @@ int SketchObject::split(int GeoId, const Base::Vector3d& point) setConstruction(newId, GeometryFacade::getConstruction(curve)); exposeInternalGeometry(newId); - // TODO: Certain transfers and new constraint can be directly made here. - // But this may reduce readability. - // apply appropriate constraints on the new points at split point and - // transfer constraints from start and end of original spline - createAndTransferConstraints(GeoId, newIds[0], newIds[1]); return true; }; bool ok = false; - if (geo->is()) { - originalIsPeriodic = false; - ok = createGeosFromNonPeriodic( - static_cast(geo), - [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newArc = std::unique_ptr( - static_cast(curve->copy())); - newArc->setRange(startParam, endParam); - return newArc; - }, - [this, &newConstraints](int GeoId, int newId0, int newId1) { - Constraint* joint = new Constraint(); - joint->Type = Coincident; - joint->First = newId0; - joint->FirstPos = PointPos::end; - joint->Second = newId1; - joint->SecondPos = PointPos::start; - newConstraints.push_back(joint); - - transferConstraints(GeoId, PointPos::start, newId0, PointPos::start); - transferConstraints(GeoId, PointPos::end, newId1, PointPos::end); - }); + if (originalIsPeriodic) { + ok = createGeosFromPeriodic(static_cast(geo)); } - else if (geo->is()) { - originalIsPeriodic = true; - ok = createGeosFromPeriodic( - static_cast(geo), - [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newArc = std::make_unique( - Handle(Geom_Circle)::DownCast(curve->handle()->Copy())); - newArc->setRange(startParam, endParam, false); - return newArc; - }, - [this](int GeoId, int newId) { - transferConstraints(GeoId, PointPos::mid, newId, PointPos::mid); - }); - } - else if (geo->is()) { - originalIsPeriodic = true; - ok = createGeosFromPeriodic( - static_cast(geo), - [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newArc = std::make_unique( - Handle(Geom_Ellipse)::DownCast(curve->handle()->Copy())); - newArc->setRange(startParam, endParam, false); - return newArc; - }, - [this](int GeoId, int newId) { - transferConstraints(GeoId, PointPos::mid, newId, PointPos::mid); - }); - } - else if (geo->is()) { - originalIsPeriodic = false; - ok = createGeosFromNonPeriodic( - static_cast(geo), - [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newArc = std::unique_ptr( - static_cast(curve->copy())); - newArc->setRange(startParam, endParam, false); - return newArc; - }, - [this, &newConstraints](int GeoId, int newId0, int newId1) { - Constraint* joint = new Constraint(); - joint->Type = Coincident; - joint->First = newId0; - joint->FirstPos = PointPos::end; - joint->Second = newId1; - joint->SecondPos = PointPos::start; - newConstraints.push_back(joint); - - joint = new Constraint(); - joint->Type = Coincident; - joint->First = newId0; - joint->FirstPos = PointPos::mid; - joint->Second = newId1; - joint->SecondPos = PointPos::mid; - newConstraints.push_back(joint); - - transferConstraints(GeoId, PointPos::start, newId0, PointPos::start, true); - transferConstraints(GeoId, PointPos::mid, newId0, PointPos::mid); - transferConstraints(GeoId, PointPos::end, newId1, PointPos::end, true); - }); - } - else if (geo->isDerivedFrom(Part::GeomArcOfConic::getClassTypeId())) { - originalIsPeriodic = false; - ok = createGeosFromNonPeriodic( - static_cast(geo), - [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newArc = std::unique_ptr( - static_cast(curve->copy())); - newArc->setRange(startParam, endParam); - return newArc; - }, - [this, &newConstraints](int GeoId, int newId0, int newId1) { - // apply appropriate constraints on the new points at split point - Constraint* joint = new Constraint(); - joint->Type = Coincident; - joint->First = newId0; - joint->FirstPos = PointPos::end; - joint->Second = newId1; - joint->SecondPos = PointPos::start; - newConstraints.push_back(joint); - - // TODO: Do we apply constraints on center etc of the conics? - - // transfer constraints from start, mid and end of original - transferConstraints(GeoId, PointPos::start, newId0, PointPos::start, true); - transferConstraints(GeoId, PointPos::mid, newId0, PointPos::mid); - transferConstraints(GeoId, PointPos::end, newId1, PointPos::end, true); - }); - } - else if (geo->is()) { - const Part::GeomBSplineCurve* bsp = static_cast(geo); - - // what to do for periodic b-splines? - originalIsPeriodic = bsp->isPeriodic(); - if (originalIsPeriodic) { - ok = createGeosFromPeriodic( - static_cast(geo), - [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newBsp = std::unique_ptr( - static_cast(curve->copy())); - newBsp->Trim(startParam, endParam); - return newBsp; - }, - [](int, int) { - // no constraints to transfer here, and we assume the split is to "break" the - // b-spline - }); - } - else { - ok = createGeosFromNonPeriodic( - static_cast(geo), - [](const Part::GeomCurve* curve, double startParam, double endParam) { - auto newBsp = std::unique_ptr( - static_cast(curve->copy())); - newBsp->Trim(startParam, endParam); - return newBsp; - }, - [this, &newConstraints](int GeoId, int newId0, int newId1) { - // apply appropriate constraints on the new points at split point - Constraint* joint = new Constraint(); - joint->Type = Coincident; - joint->First = newId0; - joint->FirstPos = PointPos::end; - joint->Second = newId1; - joint->SecondPos = PointPos::start; - newConstraints.push_back(joint); - - // transfer constraints from start and end of original spline - transferConstraints(GeoId, PointPos::start, newId0, PointPos::start, true); - transferConstraints(GeoId, PointPos::end, newId1, PointPos::end, true); - }); - } + else { + ok = createGeosFromNonPeriodic(static_cast(geo)); } if (!ok) { @@ -4299,6 +4169,35 @@ int SketchObject::split(int GeoId, const Base::Vector3d& point) return -1; } + if (!originalIsPeriodic) { + Constraint* joint = new Constraint(); + joint->Type = Coincident; + joint->First = newIds.front(); + joint->FirstPos = PointPos::end; + joint->Second = newIds.back(); + joint->SecondPos = PointPos::start; + newConstraints.push_back(joint); + + transferConstraints(GeoId, PointPos::start, newIds.front(), PointPos::start); + transferConstraints(GeoId, PointPos::end, newIds.back(), PointPos::end); + } + + // This additional constraint is there to maintain existing behavior. + // TODO: Decide whether to remove it altogether or also apply to other curves with centers. + if (geo->is()) { + Constraint* joint = new Constraint(); + joint->Type = Coincident; + joint->First = newIds.front(); + joint->FirstPos = PointPos::mid; + joint->Second = newIds.back(); + joint->SecondPos = PointPos::mid; + newConstraints.push_back(joint); + } + + if (geo->isDerivedFrom() || geo->isDerivedFrom()) { + transferConstraints(GeoId, PointPos::mid, newIds.front(), PointPos::mid); + } + std::vector idsOfOldConstraints; getConstraintIndices(GeoId, idsOfOldConstraints); @@ -4306,20 +4205,22 @@ int SketchObject::split(int GeoId, const Base::Vector3d& point) // keep constraints on internal geometries so they are deleted // when the old curve is deleted - idsOfOldConstraints.erase(std::remove_if(idsOfOldConstraints.begin(), - idsOfOldConstraints.end(), - [=](const auto& i) { - return allConstraints[i]->Type == InternalAlignment; - }), - idsOfOldConstraints.end()); + idsOfOldConstraints.erase( + std::remove_if(idsOfOldConstraints.begin(), + idsOfOldConstraints.end(), + [&allConstraints](const auto& i) { + return allConstraints[i]->Type == InternalAlignment; + }), + idsOfOldConstraints.end()); for (const auto& oldConstrId: idsOfOldConstraints) { Constraint* con = allConstraints[oldConstrId]; deriveConstraintsForPieces(GeoId, newIds, con, newConstraints); } - if (noRecomputes) + if (noRecomputes) { solve(); + } delConstraints(idsOfOldConstraints); addConstraints(newConstraints); @@ -4453,11 +4354,9 @@ int SketchObject::join(int geoId1, Sketcher::PointPos posId1, int geoId2, Sketch // end knots can have a multiplicity of (degree + 1) if (bsp1->getDegree() < newMults.back()) { + newMults.back() = bsp1->getDegree(); if (makeC1Continuous) { - newMults.back() = bsp1->getDegree()-1; - } - else { - newMults.back() = bsp1->getDegree(); + newMults.back() -= 1; } } @@ -4475,24 +4374,22 @@ int SketchObject::join(int geoId1, Sketcher::PointPos posId1, int geoId2, Sketch THROWM(ValueError, "Failed to create joined curve."); return -1; } - else { - exposeInternalGeometry(newGeoId); - setConstruction(newGeoId, GeometryFacade::getConstruction(geo1)); - // TODO: transfer constraints on the non-connected ends - auto otherPosId1 = (Sketcher::PointPos::start == posId1) ? Sketcher::PointPos::end - : Sketcher::PointPos::start; - auto otherPosId2 = (Sketcher::PointPos::start == posId2) ? Sketcher::PointPos::end - : Sketcher::PointPos::start; + exposeInternalGeometry(newGeoId); + setConstruction(newGeoId, GeometryFacade::getConstruction(geo1)); - transferConstraints(geoId1, otherPosId1, newGeoId, PointPos::start, true); - transferConstraints(geoId2, otherPosId2, newGeoId, PointPos::end, true); + // TODO: transfer constraints on the non-connected ends + auto otherPosId1 = (Sketcher::PointPos::start == posId1) ? Sketcher::PointPos::end + : Sketcher::PointPos::start; + auto otherPosId2 = (Sketcher::PointPos::start == posId2) ? Sketcher::PointPos::end + : Sketcher::PointPos::start; - delGeometries({geoId1, geoId2}); - return 0; - } + transferConstraints(geoId1, otherPosId1, newGeoId, PointPos::start, true); + transferConstraints(geoId2, otherPosId2, newGeoId, PointPos::end, true); - return -1; + delGeometries({geoId1, geoId2}); + + return 0; } bool SketchObject::isExternalAllowed(App::Document* pDoc, App::DocumentObject* pObj, @@ -4570,7 +4467,7 @@ bool SketchObject::isCarbonCopyAllowed(App::Document* pDoc, App::DocumentObject* } - SketchObject* psObj = static_cast(pObj); + auto* psObj = static_cast(pObj); // Sketches outside of the Document are NOT allowed if (this->getDocument() != pDoc) { @@ -4907,7 +4804,6 @@ int SketchObject::addSymmetric(const std::vector& geoIdList, int refGeoId, return Geometry.getSize() - 1; } - std::vector SketchObject::getSymmetric(const std::vector& geoIdList, std::map& geoIdMap, std::map& isStartEndInverted, @@ -5450,7 +5346,7 @@ int SketchObject::addCopy(const std::vector& geoIdList, const Base::Vector3 iterfirstpoint = ssp; } else if (geocopy->is()) { - Part::GeomCircle* geosymcircle = static_cast(geocopy); + auto* geosymcircle = static_cast(geocopy); Base::Vector3d cp = geosymcircle->getCenter(); Base::Vector3d scp = cp + double(x) * displacement + double(y) * perpendicularDisplacement; @@ -5461,7 +5357,7 @@ int SketchObject::addCopy(const std::vector& geoIdList, const Base::Vector3 iterfirstpoint = scp; } else if (geocopy->is()) { - Part::GeomArcOfCircle* geoaoc = static_cast(geocopy); + auto* geoaoc = static_cast(geocopy); Base::Vector3d cp = geoaoc->getCenter(); Base::Vector3d scp = cp + double(x) * displacement + double(y) * perpendicularDisplacement; @@ -5472,7 +5368,7 @@ int SketchObject::addCopy(const std::vector& geoIdList, const Base::Vector3 iterfirstpoint = geoaoc->getStartPoint(true); } else if (geocopy->is()) { - Part::GeomEllipse* geosymellipse = static_cast(geocopy); + auto* geosymellipse = static_cast(geocopy); Base::Vector3d cp = geosymellipse->getCenter(); Base::Vector3d scp = cp + double(x) * displacement + double(y) * perpendicularDisplacement; @@ -5483,7 +5379,7 @@ int SketchObject::addCopy(const std::vector& geoIdList, const Base::Vector3 iterfirstpoint = scp; } else if (geocopy->is()) { - Part::GeomArcOfEllipse* geoaoe = static_cast(geocopy); + auto* geoaoe = static_cast(geocopy); Base::Vector3d cp = geoaoe->getCenter(); Base::Vector3d scp = cp + double(x) * displacement + double(y) * perpendicularDisplacement; @@ -5518,7 +5414,7 @@ int SketchObject::addCopy(const std::vector& geoIdList, const Base::Vector3 iterfirstpoint = geoaoe->getStartPoint(true); } else if (geocopy->is()) { - Part::GeomBSplineCurve* geobsp = static_cast(geocopy); + auto* geobsp = static_cast(geocopy); std::vector poles = geobsp->getPoles(); @@ -5536,7 +5432,7 @@ int SketchObject::addCopy(const std::vector& geoIdList, const Base::Vector3 iterfirstpoint = geobsp->getStartPoint(); } else if (geocopy->is()) { - Part::GeomPoint* geopoint = static_cast(geocopy); + auto* geopoint = static_cast(geocopy); Base::Vector3d cp = geopoint->getPoint(); Base::Vector3d scp = cp + double(x) * displacement + double(y) * perpendicularDisplacement; @@ -5834,7 +5730,6 @@ int SketchObject::addCopy(const std::vector& geoIdList, const Base::Vector3 return Geometry.getSize() - 1; } - int SketchObject::removeAxesAlignment(const std::vector& geoIdList) { // no need to check input data validity as this is an sketchobject managed operation. @@ -5969,6 +5864,630 @@ int SketchObject::removeAxesAlignment(const std::vector& geoIdList) return 0; } +template <> +int SketchObject::exposeInternalGeometryForType(const int GeoId) +{ + const Part::Geometry* geo = getGeometry(GeoId); + // First we search what has to be restored + bool major = false; + bool minor = false; + bool focus1 = false; + bool focus2 = false; + + const std::vector& vals = Constraints.getValues(); + + for (const auto& constr : vals) { + if (constr->Type != Sketcher::InternalAlignment || constr->Second != GeoId) { + continue; + } + + switch (constr->AlignmentType) { + case Sketcher::EllipseMajorDiameter: + major = true; + break; + case Sketcher::EllipseMinorDiameter: + minor = true; + break; + case Sketcher::EllipseFocus1: + focus1 = true; + break; + case Sketcher::EllipseFocus2: + focus2 = true; + break; + default: + return -1; + } + } + + int currentgeoid = getHighestCurveIndex(); + int incrgeo = 0; + + std::vector igeo; + std::vector icon; + + const auto* ellipse = static_cast(geo); + + Base::Vector3d center {ellipse->getCenter()}; + double majord {ellipse->getMajorRadius()}; + double minord {ellipse->getMinorRadius()}; + Base::Vector3d majdir {ellipse->getMajorAxisDir()}; + + Base::Vector3d mindir = Vector3d(-majdir.y, majdir.x); + + Base::Vector3d majorpositiveend = center + majord * majdir; + Base::Vector3d majornegativeend = center - majord * majdir; + Base::Vector3d minorpositiveend = center + minord * mindir; + Base::Vector3d minornegativeend = center - minord * mindir; + + double df = sqrt(majord * majord - minord * minord); + + Base::Vector3d focus1P = center + df * majdir; + Base::Vector3d focus2P = center - df * majdir; + + if (!major) { + Part::GeomLineSegment* lmajor = new Part::GeomLineSegment(); + lmajor->setPoints(majorpositiveend, majornegativeend); + + igeo.push_back(lmajor); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = EllipseMajorDiameter; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + incrgeo++; + } + if (!minor) { + Part::GeomLineSegment* lminor = new Part::GeomLineSegment(); + lminor->setPoints(minorpositiveend, minornegativeend); + + igeo.push_back(lminor); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = EllipseMinorDiameter; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + incrgeo++; + } + if (!focus1) { + Part::GeomPoint* pf1 = new Part::GeomPoint(); + pf1->setPoint(focus1P); + + igeo.push_back(pf1); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = EllipseFocus1; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->FirstPos = Sketcher::PointPos::start; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + incrgeo++; + } + if (!focus2) { + Part::GeomPoint* pf2 = new Part::GeomPoint(); + pf2->setPoint(focus2P); + igeo.push_back(pf2); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = EllipseFocus2; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->FirstPos = Sketcher::PointPos::start; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + } + + this->addGeometry(igeo, true); + this->addConstraints(icon); + + for (auto& geo : igeo) { + delete geo; + } + + for (auto& con : icon) { + delete con; + } + + return incrgeo;// number of added elements +} + +// TODO: This is a repeat of ellipse. Can we do some code reuse? +template <> +int SketchObject::exposeInternalGeometryForType(const int GeoId) +{ + const Part::Geometry* geo = getGeometry(GeoId); + // First we search what has to be restored + bool major = false; + bool minor = false; + bool focus1 = false; + bool focus2 = false; + + const std::vector& vals = Constraints.getValues(); + + for (const auto& constr : vals) { + if (constr->Type != Sketcher::InternalAlignment || constr->Second != GeoId) { + continue; + } + + switch (constr->AlignmentType) { + case Sketcher::EllipseMajorDiameter: + major = true; + break; + case Sketcher::EllipseMinorDiameter: + minor = true; + break; + case Sketcher::EllipseFocus1: + focus1 = true; + break; + case Sketcher::EllipseFocus2: + focus2 = true; + break; + default: + return -1; + } + } + + int currentgeoid = getHighestCurveIndex(); + int incrgeo = 0; + + std::vector igeo; + std::vector icon; + + const auto* aoe = static_cast(geo); + + Base::Vector3d center {aoe->getCenter()}; + double majord {aoe->getMajorRadius()}; + double minord {aoe->getMinorRadius()}; + Base::Vector3d majdir {aoe->getMajorAxisDir()}; + + Base::Vector3d mindir {-majdir.y, majdir.x}; + + Base::Vector3d majorpositiveend {center + majord * majdir}; + Base::Vector3d majornegativeend {center - majord * majdir}; + Base::Vector3d minorpositiveend {center + minord * mindir}; + Base::Vector3d minornegativeend {center - minord * mindir}; + + double df = sqrt(majord * majord - minord * minord); + + Base::Vector3d focus1P {center + df * majdir}; + Base::Vector3d focus2P {center - df * majdir}; + + if (!major) { + Part::GeomLineSegment* lmajor = new Part::GeomLineSegment(); + lmajor->setPoints(majorpositiveend, majornegativeend); + + igeo.push_back(lmajor); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = EllipseMajorDiameter; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + incrgeo++; + } + if (!minor) { + Part::GeomLineSegment* lminor = new Part::GeomLineSegment(); + lminor->setPoints(minorpositiveend, minornegativeend); + + igeo.push_back(lminor); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = EllipseMinorDiameter; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + incrgeo++; + } + if (!focus1) { + Part::GeomPoint* pf1 = new Part::GeomPoint(); + pf1->setPoint(focus1P); + + igeo.push_back(pf1); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = EllipseFocus1; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->FirstPos = Sketcher::PointPos::start; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + incrgeo++; + } + if (!focus2) { + Part::GeomPoint* pf2 = new Part::GeomPoint(); + pf2->setPoint(focus2P); + igeo.push_back(pf2); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = EllipseFocus2; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->FirstPos = Sketcher::PointPos::start; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + } + + this->addGeometry(igeo, true); + this->addConstraints(icon); + + for (auto& geo : igeo) { + delete geo; + } + + for (auto& con : icon) { + delete con; + } + + return incrgeo;// number of added elements + +} + +template <> +int SketchObject::exposeInternalGeometryForType(const int GeoId) +{ + const Part::Geometry* geo = getGeometry(GeoId); + // First we search what has to be restored + bool major = false; + bool minor = false; + bool focus = false; + + const std::vector& vals = Constraints.getValues(); + + for (auto const& constr : vals) { + if (constr->Type != Sketcher::InternalAlignment || constr->Second != GeoId) { + continue; + } + + switch (constr->AlignmentType) { + case Sketcher::HyperbolaMajor: + major = true; + break; + case Sketcher::HyperbolaMinor: + minor = true; + break; + case Sketcher::HyperbolaFocus: + focus = true; + break; + default: + return -1; + } + } + + int currentgeoid = getHighestCurveIndex(); + int incrgeo = 0; + + const auto* aoh = static_cast(geo); + + Base::Vector3d center {aoh->getCenter()}; + double majord {aoh->getMajorRadius()}; + double minord {aoh->getMinorRadius()}; + Base::Vector3d majdir {aoh->getMajorAxisDir()}; + + std::vector igeo; + std::vector icon; + + Base::Vector3d mindir = Vector3d(-majdir.y, majdir.x); + + Base::Vector3d majorpositiveend = center + majord * majdir; + Base::Vector3d majornegativeend = center - majord * majdir; + Base::Vector3d minorpositiveend = majorpositiveend + minord * mindir; + Base::Vector3d minornegativeend = majorpositiveend - minord * mindir; + + double df = sqrt(majord * majord + minord * minord); + + Base::Vector3d focus1P = center + df * majdir; + + if (!major) { + Part::GeomLineSegment* lmajor = new Part::GeomLineSegment(); + lmajor->setPoints(majorpositiveend, majornegativeend); + + igeo.push_back(lmajor); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = Sketcher::HyperbolaMajor; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + incrgeo++; + } + if (!minor) { + Part::GeomLineSegment* lminor = new Part::GeomLineSegment(); + lminor->setPoints(minorpositiveend, minornegativeend); + + igeo.push_back(lminor); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = Sketcher::HyperbolaMinor; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + + incrgeo++; + } + if (!focus) { + Part::GeomPoint* pf1 = new Part::GeomPoint(); + pf1->setPoint(focus1P); + + igeo.push_back(pf1); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = Sketcher::HyperbolaFocus; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->FirstPos = Sketcher::PointPos::start; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + incrgeo++; + } + + this->addGeometry(igeo, true); + this->addConstraints(icon); + + for (auto& geo : igeo) { + delete geo; + } + + for (auto& con : icon) { + delete con; + } + + return incrgeo;// number of added elements +} + +template <> +int SketchObject::exposeInternalGeometryForType(const int GeoId) +{ + const Part::Geometry* geo = getGeometry(GeoId); + // First we search what has to be restored + bool focus = false; + bool focus_to_vertex = false; + + const std::vector& vals = Constraints.getValues(); + + for (auto const& constr : vals) { + if (constr->Type != Sketcher::InternalAlignment || constr->Second != GeoId) { + continue; + } + + switch (constr->AlignmentType) { + case Sketcher::ParabolaFocus: + focus = true; + break; + case Sketcher::ParabolaFocalAxis: + focus_to_vertex = true; + break; + default: + return -1; + } + } + + int currentgeoid = getHighestCurveIndex(); + int incrgeo = 0; + + const auto* aop = static_cast(geo); + + Base::Vector3d center {aop->getCenter()}; + Base::Vector3d focusp {aop->getFocus()}; + + std::vector igeo; + std::vector icon; + + if (!focus) { + Part::GeomPoint* pf1 = new Part::GeomPoint(); + pf1->setPoint(focusp); + + igeo.push_back(pf1); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = Sketcher::ParabolaFocus; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->FirstPos = Sketcher::PointPos::start; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + incrgeo++; + } + + if (!focus_to_vertex) { + Part::GeomLineSegment* paxis = new Part::GeomLineSegment(); + paxis->setPoints(center, focusp); + + igeo.push_back(paxis); + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = Sketcher::ParabolaFocalAxis; + newConstr->First = currentgeoid + incrgeo + 1; + newConstr->FirstPos = Sketcher::PointPos::none; + newConstr->Second = GeoId; + + icon.push_back(newConstr); + + incrgeo++; + } + + this->addGeometry(igeo, true); + this->addConstraints(icon); + + for (auto& geo : igeo) { + delete geo; + } + + for (auto& con : icon) { + delete con; + } + + return incrgeo;// number of added elements +} + +template <> +int SketchObject::exposeInternalGeometryForType(const int GeoId) +{ + const Part::Geometry* geo = getGeometry(GeoId); + + const auto* bsp = static_cast(geo); + // First we search what has to be restored + std::vector controlpointgeoids(bsp->countPoles(), GeoEnum::GeoUndef); + + std::vector knotgeoids(bsp->countKnots(), GeoEnum::GeoUndef); + + bool isfirstweightconstrained = false; + + const std::vector& vals = Constraints.getValues(); + + // search for existing poles + for (auto const& constr : vals) { + if (constr->Type != Sketcher::InternalAlignment || constr->Second != GeoId) { + continue; + } + + switch (constr->AlignmentType) { + case Sketcher::BSplineControlPoint: + controlpointgeoids[constr->InternalAlignmentIndex] = constr->First; + break; + case Sketcher::BSplineKnotPoint: + knotgeoids[constr->InternalAlignmentIndex] = constr->First; + break; + default: + return -1; + } + } + + if (controlpointgeoids[0] != GeoEnum::GeoUndef) { + isfirstweightconstrained = + std::any_of(vals.begin(), vals.end(), [&controlpointgeoids](const auto& constr) { + return (constr->Type == Sketcher::Weight && constr->First == controlpointgeoids[0]); + }); + } + + int currentgeoid = getHighestCurveIndex(); + int incrgeo = 0; + + std::vector igeo; + std::vector icon; + + std::vector poles = bsp->getPoles(); + std::vector weights = bsp->getWeights(); + std::vector knots = bsp->getKnots(); + + double distance_p0_p1 = (poles[1] - poles[0]).Length();// for visual purposes only + + for (size_t index = 0; index < controlpointgeoids.size(); ++index) { + auto& cpGeoId = controlpointgeoids.at(index); + if (cpGeoId != GeoEnum::GeoUndef) { + continue; + } + + // if controlpoint not existing + Part::GeomCircle* pc = new Part::GeomCircle(); + pc->setCenter(poles[index]); + pc->setRadius(distance_p0_p1 / 6); + + igeo.push_back(pc); + incrgeo++; + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = Sketcher::BSplineControlPoint; + newConstr->First = currentgeoid + incrgeo; + newConstr->FirstPos = Sketcher::PointPos::mid; + newConstr->Second = GeoId; + newConstr->InternalAlignmentIndex = index; + + icon.push_back(newConstr); + + if (index == 0) { + controlpointgeoids[0] = currentgeoid + incrgeo; + if (weights[0] == 1.0) { + // if the first weight is 1.0 it's probably going to be non-rational + Sketcher::Constraint* newConstr3 = new Sketcher::Constraint(); + newConstr3->Type = Sketcher::Weight; + newConstr3->First = controlpointgeoids[0]; + newConstr3->setValue(weights[0]); + + icon.push_back(newConstr3); + + isfirstweightconstrained = true; + } + + continue; + } + + if (isfirstweightconstrained && weights[0] == weights[index]) { + // if pole-weight newly created AND first weight is radius-constrained, + // AND these weights are equal, constrain them to be equal + Sketcher::Constraint* newConstr2 = new Sketcher::Constraint(); + newConstr2->Type = Sketcher::Equal; + newConstr2->First = currentgeoid + incrgeo; + newConstr2->FirstPos = Sketcher::PointPos::none; + newConstr2->Second = controlpointgeoids[0]; + newConstr2->SecondPos = Sketcher::PointPos::none; + + icon.push_back(newConstr2); + } + } + + for (size_t index = 0; index < knotgeoids.size(); ++index) { + auto& kGeoId = knotgeoids.at(index); + if (kGeoId != GeoEnum::GeoUndef) { + continue; + } + + // if knot point not existing + Part::GeomPoint* kp = new Part::GeomPoint(); + + kp->setPoint(bsp->pointAtParameter(knots[index])); + + igeo.push_back(kp); + incrgeo++; + + Sketcher::Constraint* newConstr = new Sketcher::Constraint(); + newConstr->Type = Sketcher::InternalAlignment; + newConstr->AlignmentType = Sketcher::BSplineKnotPoint; + newConstr->First = currentgeoid + incrgeo; + newConstr->FirstPos = Sketcher::PointPos::start; + newConstr->Second = GeoId; + newConstr->InternalAlignmentIndex = index; + + icon.push_back(newConstr); + } + + Q_UNUSED(isfirstweightconstrained); + + this->addGeometry(igeo, true); + this->addConstraints(icon); + + for (auto& geo : igeo) { + delete geo; + } + + for (auto& con : icon) { + delete con; + } + + return incrgeo;// number of added elements +} + int SketchObject::exposeInternalGeometry(int GeoId) { if (GeoId < 0 || GeoId > getHighestCurveIndex()) @@ -5976,533 +6495,20 @@ int SketchObject::exposeInternalGeometry(int GeoId) const Part::Geometry* geo = getGeometry(GeoId); // Only for supported types - if (geo->is() - || geo->is()) { - // First we search what has to be restored - bool major = false; - bool minor = false; - bool focus1 = false; - bool focus2 = false; - - const std::vector& vals = Constraints.getValues(); - - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); - ++it) { - if ((*it)->Type == Sketcher::InternalAlignment && (*it)->Second == GeoId) { - switch ((*it)->AlignmentType) { - case Sketcher::EllipseMajorDiameter: - major = true; - break; - case Sketcher::EllipseMinorDiameter: - minor = true; - break; - case Sketcher::EllipseFocus1: - focus1 = true; - break; - case Sketcher::EllipseFocus2: - focus2 = true; - break; - default: - return -1; - } - } - } - - int currentgeoid = getHighestCurveIndex(); - int incrgeo = 0; - - Base::Vector3d center; - double majord; - double minord; - Base::Vector3d majdir; - - std::vector igeo; - std::vector icon; - - if (geo->is()) { - const Part::GeomEllipse* ellipse = static_cast(geo); - - center = ellipse->getCenter(); - majord = ellipse->getMajorRadius(); - minord = ellipse->getMinorRadius(); - majdir = ellipse->getMajorAxisDir(); - } - else { - const Part::GeomArcOfEllipse* aoe = static_cast(geo); - - center = aoe->getCenter(); - majord = aoe->getMajorRadius(); - minord = aoe->getMinorRadius(); - majdir = aoe->getMajorAxisDir(); - } - - Base::Vector3d mindir = Vector3d(-majdir.y, majdir.x); - - Base::Vector3d majorpositiveend = center + majord * majdir; - Base::Vector3d majornegativeend = center - majord * majdir; - Base::Vector3d minorpositiveend = center + minord * mindir; - Base::Vector3d minornegativeend = center - minord * mindir; - - double df = sqrt(majord * majord - minord * minord); - - Base::Vector3d focus1P = center + df * majdir; - Base::Vector3d focus2P = center - df * majdir; - - if (!major) { - Part::GeomLineSegment* lmajor = new Part::GeomLineSegment(); - lmajor->setPoints(majorpositiveend, majornegativeend); - - igeo.push_back(lmajor); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = EllipseMajorDiameter; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->Second = GeoId; - - icon.push_back(newConstr); - incrgeo++; - } - if (!minor) { - Part::GeomLineSegment* lminor = new Part::GeomLineSegment(); - lminor->setPoints(minorpositiveend, minornegativeend); - - igeo.push_back(lminor); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = EllipseMinorDiameter; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->Second = GeoId; - - icon.push_back(newConstr); - incrgeo++; - } - if (!focus1) { - Part::GeomPoint* pf1 = new Part::GeomPoint(); - pf1->setPoint(focus1P); - - igeo.push_back(pf1); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = EllipseFocus1; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->FirstPos = Sketcher::PointPos::start; - newConstr->Second = GeoId; - - icon.push_back(newConstr); - incrgeo++; - } - if (!focus2) { - Part::GeomPoint* pf2 = new Part::GeomPoint(); - pf2->setPoint(focus2P); - igeo.push_back(pf2); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = EllipseFocus2; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->FirstPos = Sketcher::PointPos::start; - newConstr->Second = GeoId; - - icon.push_back(newConstr); - } - - this->addGeometry(igeo, true); - this->addConstraints(icon); - - for (std::vector::iterator it = igeo.begin(); it != igeo.end(); ++it) { - if (*it) - delete *it; - } - - for (std::vector::iterator it = icon.begin(); it != icon.end(); ++it) { - if (*it) - delete *it; - } - - icon.clear(); - igeo.clear(); - - return incrgeo;// number of added elements + if (geo->is()) { + return exposeInternalGeometryForType(GeoId); + } + else if (geo->is()) { + return exposeInternalGeometryForType(GeoId); } else if (geo->is()) { - // First we search what has to be restored - bool major = false; - bool minor = false; - bool focus = false; - - const std::vector& vals = Constraints.getValues(); - - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); - ++it) { - if ((*it)->Type == Sketcher::InternalAlignment && (*it)->Second == GeoId) { - switch ((*it)->AlignmentType) { - case Sketcher::HyperbolaMajor: - major = true; - break; - case Sketcher::HyperbolaMinor: - minor = true; - break; - case Sketcher::HyperbolaFocus: - focus = true; - break; - default: - return -1; - } - } - } - - int currentgeoid = getHighestCurveIndex(); - int incrgeo = 0; - - const Part::GeomArcOfHyperbola* aoh = static_cast(geo); - - Base::Vector3d center = aoh->getCenter(); - double majord = aoh->getMajorRadius(); - double minord = aoh->getMinorRadius(); - Base::Vector3d majdir = aoh->getMajorAxisDir(); - - std::vector igeo; - std::vector icon; - - Base::Vector3d mindir = Vector3d(-majdir.y, majdir.x); - - Base::Vector3d majorpositiveend = center + majord * majdir; - Base::Vector3d majornegativeend = center - majord * majdir; - Base::Vector3d minorpositiveend = majorpositiveend + minord * mindir; - Base::Vector3d minornegativeend = majorpositiveend - minord * mindir; - - double df = sqrt(majord * majord + minord * minord); - - Base::Vector3d focus1P = center + df * majdir; - - if (!major) { - Part::GeomLineSegment* lmajor = new Part::GeomLineSegment(); - lmajor->setPoints(majorpositiveend, majornegativeend); - - igeo.push_back(lmajor); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = Sketcher::HyperbolaMajor; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->Second = GeoId; - - icon.push_back(newConstr); - incrgeo++; - } - if (!minor) { - Part::GeomLineSegment* lminor = new Part::GeomLineSegment(); - lminor->setPoints(minorpositiveend, minornegativeend); - - igeo.push_back(lminor); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = Sketcher::HyperbolaMinor; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->Second = GeoId; - - icon.push_back(newConstr); - - incrgeo++; - } - if (!focus) { - Part::GeomPoint* pf1 = new Part::GeomPoint(); - pf1->setPoint(focus1P); - - igeo.push_back(pf1); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = Sketcher::HyperbolaFocus; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->FirstPos = Sketcher::PointPos::start; - newConstr->Second = GeoId; - - icon.push_back(newConstr); - incrgeo++; - } - - this->addGeometry(igeo, true); - this->addConstraints(icon); - - for (std::vector::iterator it = igeo.begin(); it != igeo.end(); ++it) - if (*it) - delete *it; - - for (std::vector::iterator it = icon.begin(); it != icon.end(); ++it) - if (*it) - delete *it; - - icon.clear(); - igeo.clear(); - - return incrgeo;// number of added elements + return exposeInternalGeometryForType(GeoId); } else if (geo->is()) { - // First we search what has to be restored - bool focus = false; - bool focus_to_vertex = false; - - const std::vector& vals = Constraints.getValues(); - - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); - ++it) { - if ((*it)->Type == Sketcher::InternalAlignment && (*it)->Second == GeoId) { - switch ((*it)->AlignmentType) { - case Sketcher::ParabolaFocus: - focus = true; - break; - case Sketcher::ParabolaFocalAxis: - focus_to_vertex = true; - break; - default: - return -1; - } - } - } - - int currentgeoid = getHighestCurveIndex(); - int incrgeo = 0; - - const Part::GeomArcOfParabola* aop = static_cast(geo); - - Base::Vector3d center = aop->getCenter(); - Base::Vector3d focusp = aop->getFocus(); - - std::vector igeo; - std::vector icon; - - if (!focus) { - Part::GeomPoint* pf1 = new Part::GeomPoint(); - pf1->setPoint(focusp); - - igeo.push_back(pf1); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = Sketcher::ParabolaFocus; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->FirstPos = Sketcher::PointPos::start; - newConstr->Second = GeoId; - - icon.push_back(newConstr); - incrgeo++; - } - - if (!focus_to_vertex) { - Part::GeomLineSegment* paxis = new Part::GeomLineSegment(); - paxis->setPoints(center, focusp); - - igeo.push_back(paxis); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = Sketcher::ParabolaFocalAxis; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->FirstPos = Sketcher::PointPos::none; - newConstr->Second = GeoId; - - icon.push_back(newConstr); - - incrgeo++; - } - - this->addGeometry(igeo, true); - this->addConstraints(icon); - - for (std::vector::iterator it = igeo.begin(); it != igeo.end(); ++it) { - if (*it) - delete *it; - } - - for (std::vector::iterator it = icon.begin(); it != icon.end(); ++it) { - if (*it) - delete *it; - } - - icon.clear(); - igeo.clear(); - - return incrgeo;// number of added elements + return exposeInternalGeometryForType(GeoId); } else if (geo->is()) { - - const Part::GeomBSplineCurve* bsp = static_cast(geo); - // First we search what has to be restored - std::vector controlpoints(bsp->countPoles()); - std::vector controlpointgeoids(bsp->countPoles()); - - std::vector knotpoints(bsp->countKnots()); - std::vector knotgeoids(bsp->countKnots()); - - bool isfirstweightconstrained = false; - - std::vector::iterator itb; - std::vector::iterator it; - - for (it = controlpointgeoids.begin(), itb = controlpoints.begin(); - it != controlpointgeoids.end() && itb != controlpoints.end(); - ++it, ++itb) { - (*it) = -1; - (*itb) = false; - } - - for (it = knotgeoids.begin(), itb = knotpoints.begin(); - it != knotgeoids.end() && itb != knotpoints.end(); - ++it, ++itb) { - (*it) = -1; - (*itb) = false; - } - - const std::vector& vals = Constraints.getValues(); - - // search for existing poles - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); - ++it) { - if ((*it)->Type == Sketcher::InternalAlignment && (*it)->Second == GeoId) { - switch ((*it)->AlignmentType) { - case Sketcher::BSplineControlPoint: - controlpoints[(*it)->InternalAlignmentIndex] = true; - controlpointgeoids[(*it)->InternalAlignmentIndex] = (*it)->First; - break; - case Sketcher::BSplineKnotPoint: - knotpoints[(*it)->InternalAlignmentIndex] = true; - knotgeoids[(*it)->InternalAlignmentIndex] = (*it)->First; - break; - default: - return -1; - } - } - } - - if (controlpoints[0]) { - // search for first pole weight constraint - for (std::vector::const_iterator it = vals.begin(); - it != vals.end(); - ++it) { - if ((*it)->Type == Sketcher::Weight && (*it)->First == controlpointgeoids[0]) { - isfirstweightconstrained = true; - } - } - } - - int currentgeoid = getHighestCurveIndex(); - int incrgeo = 0; - - std::vector igeo; - std::vector icon; - - std::vector poles = bsp->getPoles(); - std::vector weights = bsp->getWeights(); - std::vector knots = bsp->getKnots(); - - double distance_p0_p1 = (poles[1] - poles[0]).Length();// for visual purposes only - - int index = 0; - - for (it = controlpointgeoids.begin(), itb = controlpoints.begin(); - it != controlpointgeoids.end() && itb != controlpoints.end(); - ++it, ++itb, index++) { - - if (!(*itb))// if controlpoint not existing - { - Part::GeomCircle* pc = new Part::GeomCircle(); - pc->setCenter(poles[index]); - pc->setRadius(distance_p0_p1 / 6); - - igeo.push_back(pc); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = Sketcher::BSplineControlPoint; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->FirstPos = Sketcher::PointPos::mid; - newConstr->Second = GeoId; - newConstr->InternalAlignmentIndex = index; - - icon.push_back(newConstr); - - if (it != controlpointgeoids.begin()) { - if (isfirstweightconstrained && weights[0] == weights[index]) { - // if pole-weight newly created AND first weight is radius-constrained, - // AND these weights are equal, constrain them to be equal - Sketcher::Constraint* newConstr2 = new Sketcher::Constraint(); - newConstr2->Type = Sketcher::Equal; - newConstr2->First = currentgeoid + incrgeo + 1; - newConstr2->FirstPos = Sketcher::PointPos::none; - newConstr2->Second = controlpointgeoids[0]; - newConstr2->SecondPos = Sketcher::PointPos::none; - - icon.push_back(newConstr2); - } - } - else { - controlpointgeoids[0] = currentgeoid + incrgeo + 1; - if (weights[0] == 1.0) { - // if the first weight is 1.0 it's probably going to be non-rational - Sketcher::Constraint* newConstr3 = new Sketcher::Constraint(); - newConstr3->Type = Sketcher::Weight; - newConstr3->First = controlpointgeoids[0]; - newConstr3->setValue(weights[0]); - - icon.push_back(newConstr3); - - isfirstweightconstrained = true; - } - } - incrgeo++; - } - } - - index = 0; - - for (it = knotgeoids.begin(), itb = knotpoints.begin(); - it != knotgeoids.end() && itb != knotpoints.end(); - ++it, ++itb, index++) { - - if (!(*itb))// if knot point not existing - { - Part::GeomPoint* kp = new Part::GeomPoint(); - - kp->setPoint(bsp->pointAtParameter(knots[index])); - - igeo.push_back(kp); - - Sketcher::Constraint* newConstr = new Sketcher::Constraint(); - newConstr->Type = Sketcher::InternalAlignment; - newConstr->AlignmentType = Sketcher::BSplineKnotPoint; - newConstr->First = currentgeoid + incrgeo + 1; - newConstr->FirstPos = Sketcher::PointPos::start; - newConstr->Second = GeoId; - newConstr->InternalAlignmentIndex = index; - - icon.push_back(newConstr); - - incrgeo++; - } - } - - Q_UNUSED(isfirstweightconstrained); - - this->addGeometry(igeo, true); - this->addConstraints(icon); - - for (std::vector::iterator it = igeo.begin(); it != igeo.end(); ++it) - if (*it) - delete *it; - - for (std::vector::iterator it = icon.begin(); it != icon.end(); ++it) - if (*it) - delete *it; - - icon.clear(); - igeo.clear(); - - return incrgeo;// number of added elements + return exposeInternalGeometryForType(GeoId); } else return -1;// not supported type @@ -6518,289 +6524,265 @@ int SketchObject::deleteUnusedInternalGeometry(int GeoId, bool delgeoid) if (geo->is() || geo->is() || geo->is()) { - - int majorelementindex = -1; - int minorelementindex = -1; - int focus1elementindex = -1; - int focus2elementindex = -1; - - const std::vector& vals = Constraints.getValues(); - - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); - ++it) { - if ((*it)->Type == Sketcher::InternalAlignment && (*it)->Second == GeoId) { - switch ((*it)->AlignmentType) { - case Sketcher::EllipseMajorDiameter: - case Sketcher::HyperbolaMajor: - majorelementindex = (*it)->First; - break; - case Sketcher::EllipseMinorDiameter: - case Sketcher::HyperbolaMinor: - minorelementindex = (*it)->First; - break; - case Sketcher::EllipseFocus1: - case Sketcher::HyperbolaFocus: - focus1elementindex = (*it)->First; - break; - case Sketcher::EllipseFocus2: - focus2elementindex = (*it)->First; - break; - default: - return -1; - } - } - } - - // Hide unused geometry here - int majorconstraints = 0;// number of constraints associated to the geoid of the major axis - int minorconstraints = 0; - int focus1constraints = 0; - int focus2constraints = 0; - - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); - ++it) { - - if ((*it)->Second == majorelementindex || (*it)->First == majorelementindex - || (*it)->Third == majorelementindex) - majorconstraints++; - else if ((*it)->Second == minorelementindex || (*it)->First == minorelementindex - || (*it)->Third == minorelementindex) - minorconstraints++; - else if ((*it)->Second == focus1elementindex || (*it)->First == focus1elementindex - || (*it)->Third == focus1elementindex) - focus1constraints++; - else if ((*it)->Second == focus2elementindex || (*it)->First == focus2elementindex - || (*it)->Third == focus2elementindex) - focus2constraints++; - } - - std::vector delgeometries; - - // those with less than 2 constraints must be removed - if (focus2constraints < 2) - delgeometries.push_back(focus2elementindex); - - if (focus1constraints < 2) - delgeometries.push_back(focus1elementindex); - - if (minorconstraints < 2) - delgeometries.push_back(minorelementindex); - - if (majorconstraints < 2) - delgeometries.push_back(majorelementindex); - - if (delgeoid) - delgeometries.push_back(GeoId); - - // indices over an erased element get automatically updated!! - std::sort(delgeometries.begin(), delgeometries.end()); - - if (!delgeometries.empty()) { - for (std::vector::reverse_iterator it = delgeometries.rbegin(); - it != delgeometries.rend(); - ++it) { - delGeometry(*it, false); - } - } - - int ndeleted = delgeometries.size(); - - delgeometries.clear(); - - return ndeleted;// number of deleted elements + return deleteUnusedInternalGeometryWhenTwoFoci(GeoId, delgeoid); } - else if (geo->is()) { - // if the focus-to-vertex line is constrained, then never delete the focus - // if the line is unconstrained, then the line may be deleted, - // in this case the focus may be deleted if unconstrained. - int majorelementindex = -1; - int focus1elementindex = -1; - const std::vector& vals = Constraints.getValues(); - - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); - ++it) { - if ((*it)->Type == Sketcher::InternalAlignment && (*it)->Second == GeoId) { - switch ((*it)->AlignmentType) { - case Sketcher::ParabolaFocus: - focus1elementindex = (*it)->First; - break; - case Sketcher::ParabolaFocalAxis: - majorelementindex = (*it)->First; - break; - default: - return -1; - } - } - } - - // Hide unused geometry here - // number of constraints associated to the geoid of the major axis other than the coincident - // ones - int majorconstraints = 0; - int focus1constraints = 0; - - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); - ++it) { - if ((*it)->Second == majorelementindex || (*it)->First == majorelementindex - || (*it)->Third == majorelementindex) - majorconstraints++; - else if ((*it)->Second == focus1elementindex || (*it)->First == focus1elementindex - || (*it)->Third == focus1elementindex) - focus1constraints++; - } - - std::vector delgeometries; - - // major has minimum one constraint, the specific internal alignment constraint - if (majorelementindex != -1 && majorconstraints < 2) - delgeometries.push_back(majorelementindex); - - // focus has minimum one constraint now, the specific internal alignment constraint - if (focus1elementindex != -1 && focus1constraints < 2) - delgeometries.push_back(focus1elementindex); - - if (delgeoid) - delgeometries.push_back(GeoId); - - // indices over an erased element get automatically updated!! - std::sort(delgeometries.begin(), delgeometries.end()); - - if (!delgeometries.empty()) { - for (std::vector::reverse_iterator it = delgeometries.rbegin(); - it != delgeometries.rend(); - ++it) { - delGeometry(*it, false); - } - } - - int ndeleted = delgeometries.size(); - - delgeometries.clear(); - - return ndeleted;// number of deleted elements + if (geo->is()) { + return deleteUnusedInternalGeometryWhenOneFocus(GeoId, delgeoid); } - else if (geo->is()) { - const Part::GeomBSplineCurve* bsp = static_cast(geo); - - // First we search existing IA - std::vector controlpointgeoids(bsp->countPoles()); - std::vector cpassociatedconstraints(bsp->countPoles()); - - std::vector knotgeoids(bsp->countKnots()); - std::vector kassociatedconstraints(bsp->countKnots()); - - std::vector::iterator it; - std::vector::iterator ita; - - for (it = controlpointgeoids.begin(), ita = cpassociatedconstraints.begin(); - it != controlpointgeoids.end() && ita != cpassociatedconstraints.end(); - ++it, ++ita) { - (*it) = -1; - (*ita) = 0; - } - - for (it = knotgeoids.begin(), ita = kassociatedconstraints.begin(); - it != knotgeoids.end() && ita != kassociatedconstraints.end(); - ++it, ++ita) { - (*it) = -1; - (*ita) = 0; - } - - const std::vector& vals = Constraints.getValues(); - - // search for existing poles - for (std::vector::const_iterator jt = vals.begin(); jt != vals.end(); - ++jt) { - if ((*jt)->Type == Sketcher::InternalAlignment && (*jt)->Second == GeoId) { - switch ((*jt)->AlignmentType) { - case Sketcher::BSplineControlPoint: - controlpointgeoids[(*jt)->InternalAlignmentIndex] = (*jt)->First; - break; - case Sketcher::BSplineKnotPoint: - knotgeoids[(*jt)->InternalAlignmentIndex] = (*jt)->First; - break; - default: - return -1; - } - } - } - - std::vector delgeometries; - - for (it = controlpointgeoids.begin(), ita = cpassociatedconstraints.begin(); - it != controlpointgeoids.end() && ita != cpassociatedconstraints.end(); - ++it, ++ita) { - if ((*it) != -1) { - // look for a circle at geoid index - for (std::vector::const_iterator itc = vals.begin(); - itc != vals.end(); - ++itc) { - - if ((*itc)->Type == Sketcher::Equal) { - bool f = false, s = false; - for (std::vector::iterator its = controlpointgeoids.begin(); - its != controlpointgeoids.end(); - ++its) { - if ((*itc)->First == *its) { - f = true; - } - else if ((*itc)->Second == *its) { - s = true; - } - - if (f && s) {// the equality constraint is not interpole - break; - } - } - - // the equality constraint constraints a pole but it is not interpole - if (f != s) { - (*ita)++; - } - } - // We do not ignore weight constraints as we did with radius constraints, - // because the radius magnitude no longer makes sense without the B-Spline. - } - - if ((*ita) < 2) {// IA - delgeometries.push_back((*it)); - } - } - } - - for (it = knotgeoids.begin(), ita = kassociatedconstraints.begin(); - it != knotgeoids.end() && ita != kassociatedconstraints.end(); - ++it, ++ita) { - if ((*it) != -1) { - // look for a point at geoid index - for (std::vector::const_iterator itc = vals.begin(); - itc != vals.end(); - ++itc) { - if ((*itc)->Second == (*it) || (*itc)->First == (*it) - || (*itc)->Third == (*it)) { - (*ita)++; - } - } - - if ((*ita) < 2) {// IA - delgeometries.push_back((*it)); - } - } - } - - - if (delgeoid) - delgeometries.push_back(GeoId); - - int ndeleted = delGeometriesExclusiveList(delgeometries); - - return ndeleted;// number of deleted elements + if (geo->is()) { + return deleteUnusedInternalGeometryWhenBSpline(GeoId, delgeoid); } - else { - return -1;// not supported type + + // Default case: type not supported + return -1; +} + +int SketchObject::deleteUnusedInternalGeometryWhenTwoFoci(int GeoId, bool delgeoid) +{ + int majorelementindex = -1; + int minorelementindex = -1; + int focus1elementindex = -1; + int focus2elementindex = -1; + + const std::vector& vals = Constraints.getValues(); + + for (auto const& constr : vals) { + if (constr->Type != Sketcher::InternalAlignment || constr->Second != GeoId) { + continue; + } + + switch (constr->AlignmentType) { + case Sketcher::EllipseMajorDiameter: + case Sketcher::HyperbolaMajor: + majorelementindex = constr->First; + break; + case Sketcher::EllipseMinorDiameter: + case Sketcher::HyperbolaMinor: + minorelementindex = constr->First; + break; + case Sketcher::EllipseFocus1: + case Sketcher::HyperbolaFocus: + focus1elementindex = constr->First; + break; + case Sketcher::EllipseFocus2: + focus2elementindex = constr->First; + break; + default: + return -1; + } } + + // Hide unused geometry here + int majorconstraints = 0;// number of constraints associated to the geoid of the major axis + int minorconstraints = 0; + int focus1constraints = 0; + int focus2constraints = 0; + + for (const auto& constr : vals) { + if (constr->involvesGeoId(majorelementindex)) + majorconstraints++; + else if (constr->involvesGeoId(minorelementindex)) + minorconstraints++; + else if (constr->involvesGeoId(focus1elementindex)) + focus1constraints++; + else if (constr->involvesGeoId(focus2elementindex)) + focus2constraints++; + } + + std::vector delgeometries; + + // those with less than 2 constraints must be removed + if (focus2constraints < 2) + delgeometries.push_back(focus2elementindex); + + if (focus1constraints < 2) + delgeometries.push_back(focus1elementindex); + + if (minorconstraints < 2) + delgeometries.push_back(minorelementindex); + + if (majorconstraints < 2) + delgeometries.push_back(majorelementindex); + + if (delgeoid) + delgeometries.push_back(GeoId); + + // indices over an erased element get automatically updated!! + std::sort(delgeometries.begin(), delgeometries.end(), std::greater<>()); + + for (auto& dGeoId : delgeometries) { + delGeometry(dGeoId, false); + } + + int ndeleted = delgeometries.size(); + + return ndeleted;// number of deleted elements +} + +int SketchObject::deleteUnusedInternalGeometryWhenOneFocus(int GeoId, bool delgeoid) +{ + // if the focus-to-vertex line is constrained, then never delete the focus + // if the line is unconstrained, then the line may be deleted, + // in this case the focus may be deleted if unconstrained. + int majorelementindex = -1; + int focus1elementindex = -1; + + const std::vector& vals = Constraints.getValues(); + + for (auto const& constr : vals) { + if (constr->Type != Sketcher::InternalAlignment || constr->Second != GeoId) { + continue; + } + + switch (constr->AlignmentType) { + case Sketcher::ParabolaFocus: + focus1elementindex = constr->First; + break; + case Sketcher::ParabolaFocalAxis: + majorelementindex = constr->First; + break; + default: + return -1; + } + } + + // Hide unused geometry here + // number of constraints associated to the geoid of the major axis other than the coincident + // ones + int majorconstraints = 0; + int focus1constraints = 0; + + for (const auto& constr : vals) { + if (constr->involvesGeoId(majorelementindex)) { + majorconstraints++; + } + else if (constr->involvesGeoId(focus1elementindex)) { + focus1constraints++; + } + } + + std::vector delgeometries; + + // major has minimum one constraint, the specific internal alignment constraint + if (majorelementindex != -1 && majorconstraints < 2) + delgeometries.push_back(majorelementindex); + + // focus has minimum one constraint now, the specific internal alignment constraint + if (focus1elementindex != -1 && focus1constraints < 2) + delgeometries.push_back(focus1elementindex); + + if (delgeoid) + delgeometries.push_back(GeoId); + + // indices over an erased element get automatically updated!! + std::sort(delgeometries.begin(), delgeometries.end(), std::greater<>()); + + for (auto& dGeoId : delgeometries) { + delGeometry(dGeoId, false); + } + + int ndeleted = delgeometries.size(); + + delgeometries.clear(); + + return ndeleted;// number of deleted elements +} + +int SketchObject::deleteUnusedInternalGeometryWhenBSpline(int GeoId, bool delgeoid) +{ + const auto* bsp = static_cast(getGeometry(GeoId)); + + // First we search existing IA + std::vector > poleGeoIdsAndConstraints(bsp->countPoles(), {GeoEnum::GeoUndef, 0}); + + std::vector > knotGeoIdsAndConstraints(bsp->countKnots(), {GeoEnum::GeoUndef, 0}); + + const std::vector& vals = Constraints.getValues(); + + // search for existing poles + for (auto const& constr : vals) { + if (constr->Type != Sketcher::InternalAlignment || constr->Second != GeoId) { + continue; + } + + switch (constr->AlignmentType) { + case Sketcher::BSplineControlPoint: + poleGeoIdsAndConstraints[constr->InternalAlignmentIndex].first = constr->First; + break; + case Sketcher::BSplineKnotPoint: + knotGeoIdsAndConstraints[constr->InternalAlignmentIndex].first = constr->First; + break; + default: + return -1; + } + } + + std::vector delgeometries; + + // TODO: This can become significantly costly if there are lots of constraints and poles + for (auto& [cpGeoId, numConstr] : poleGeoIdsAndConstraints) { + if (cpGeoId == GeoEnum::GeoUndef) { + continue; + } + + // look for a circle at geoid index + for (auto const& constr : vals) { + if (constr->Type == Sketcher::InternalAlignment + || constr->Type == Sketcher::Weight + || !constr->involvesGeoId(cpGeoId)) { + continue; + } + if (constr->Type != Sketcher::Equal) { + ++numConstr; + continue; + } + bool firstIsInCPGeoIds = std::any_of(poleGeoIdsAndConstraints.begin(), + poleGeoIdsAndConstraints.end(), + [&constr](const auto& _pair) { + return _pair.first == constr->First; + }); + bool secondIsInCPGeoIds = std::any_of(poleGeoIdsAndConstraints.begin(), + poleGeoIdsAndConstraints.end(), + [&constr](const auto& _pair) { + return _pair.first == constr->Second; + }); + // the equality constraint constrains a pole but it is not interpole + if (firstIsInCPGeoIds != secondIsInCPGeoIds) { + ++numConstr; + } + // We do not ignore weight constraints as we did with radius constraints, + // because the radius magnitude no longer makes sense without the B-Spline. + } + + if (numConstr < 1) { // IA + delgeometries.push_back(cpGeoId); + } + } + + for (auto& [kGeoId, numConstr] : knotGeoIdsAndConstraints) { + if (kGeoId == GeoEnum::GeoUndef) { + continue; + } + + // look for a point at geoid index + numConstr = std::count_if(vals.begin(), vals.end(), [&kGeoId](const auto& constr) { + return constr->involvesGeoId(kGeoId); + }); + + if (numConstr < 2) { // IA + delgeometries.push_back(kGeoId); + } + } + + if (delgeoid) { + delgeometries.push_back(GeoId); + } + + int ndeleted = delGeometriesExclusiveList(delgeometries); + + return ndeleted;// number of deleted elements } int SketchObject::deleteUnusedInternalGeometryAndUpdateGeoId(int& GeoId, bool delgeoid) @@ -6851,7 +6833,7 @@ bool SketchObject::convertToNURBS(int GeoId) if (geo->is()) return false; - const Part::GeomCurve* geo1 = static_cast(geo); + const auto* geo1 = static_cast(geo); Part::GeomBSplineCurve* bspline; @@ -6859,7 +6841,7 @@ bool SketchObject::convertToNURBS(int GeoId) bspline = geo1->toNurbs(geo1->getFirstParameter(), geo1->getLastParameter()); if (geo1->isDerivedFrom(Part::GeomArcOfConic::getClassTypeId())) { - const Part::GeomArcOfConic* geoaoc = static_cast(geo1); + const auto* geoaoc = static_cast(geo1); if (geoaoc->isReversed()) bspline->reverse(); @@ -6938,7 +6920,7 @@ bool SketchObject::increaseBSplineDegree(int GeoId, int degreeincrement /*= 1*/) if (geo->getTypeId() != Part::GeomBSplineCurve::getClassTypeId()) return false; - const Part::GeomBSplineCurve* bsp = static_cast(geo); + const auto* bsp = static_cast(geo); const Handle(Geom_BSplineCurve) curve = Handle(Geom_BSplineCurve)::DownCast(bsp->handle()); @@ -6980,7 +6962,7 @@ bool SketchObject::decreaseBSplineDegree(int GeoId, int degreedecrement /*= 1*/) if (geo->getTypeId() != Part::GeomBSplineCurve::getClassTypeId()) return false; - const Part::GeomBSplineCurve* bsp = static_cast(geo); + const auto* bsp = static_cast(geo); const Handle(Geom_BSplineCurve) curve = Handle(Geom_BSplineCurve)::DownCast(bsp->handle()); @@ -7028,63 +7010,70 @@ bool SketchObject::modifyBSplineKnotMultiplicity(int GeoId, int knotIndex, int m if (GeoId < 0 || GeoId > getHighestCurveIndex()) { THROWMT( Base::ValueError, - QT_TRANSLATE_NOOP("Exceptions", "B-spline Geometry Index (GeoID) is out of bounds.")) + QT_TRANSLATE_NOOP("Exceptions", "B-spline Geometry Index (GeoID) is out of bounds.")); } - if (multiplicityincr == 0)// no change in multiplicity + if (multiplicityincr == 0) { + // no change in multiplicity THROWMT( Base::ValueError, - QT_TRANSLATE_NOOP("Exceptions", "You are requesting no change in knot multiplicity.")) + QT_TRANSLATE_NOOP("Exceptions", "You are requesting no change in knot multiplicity.")); + } const Part::Geometry* geo = getGeometry(GeoId); - if (geo->getTypeId() != Part::GeomBSplineCurve::getClassTypeId()) + if (!geo->is()) { THROWMT(Base::TypeError, QT_TRANSLATE_NOOP("Exceptions", - "The Geometry Index (GeoId) provided is not a B-spline.")) + "The Geometry Index (GeoId) provided is not a B-spline.")); + } - const Part::GeomBSplineCurve* bsp = static_cast(geo); + const auto* bsp = static_cast(geo); int degree = bsp->getDegree(); - if (knotIndex > bsp->countKnots() || knotIndex < 1)// knotindex in OCC 1 -> countKnots + if (knotIndex > bsp->countKnots() || knotIndex < 1) { + // knotindex in OCC 1 -> countKnots THROWMT(Base::ValueError, QT_TRANSLATE_NOOP("Exceptions", "The knot index is out of bounds. Note that in accordance with " - "OCC notation, the first knot has index 1 and not zero.")) + "OCC notation, the first knot has index 1 and not zero.")); + } std::unique_ptr bspline; int curmult = bsp->getMultiplicity(knotIndex); // zero is removing the knot, degree is just positional continuity - if ((curmult + multiplicityincr) > degree) + if ((curmult + multiplicityincr) > degree) { THROWMT(Base::ValueError, QT_TRANSLATE_NOOP( "Exceptions", - "The multiplicity cannot be increased beyond the degree of the B-spline.")) + "The multiplicity cannot be increased beyond the degree of the B-spline.")); + } // zero is removing the knot, degree is just positional continuity - if ((curmult + multiplicityincr) < 0) + if ((curmult + multiplicityincr) < 0) { THROWMT( Base::ValueError, - QT_TRANSLATE_NOOP("Exceptions", "The multiplicity cannot be decreased beyond zero.")) + QT_TRANSLATE_NOOP("Exceptions", "The multiplicity cannot be decreased beyond zero.")); + } try { bspline.reset(static_cast(bsp->clone())); - if (multiplicityincr > 0) {// increase multiplicity + if (multiplicityincr > 0) { // increase multiplicity bspline->increaseMultiplicity(knotIndex, curmult + multiplicityincr); } - else {// decrease multiplicity + else { // decrease multiplicity bool result = bspline->removeKnot(knotIndex, curmult + multiplicityincr, 1E6); - if (!result) - THROWMT( - Base::CADKernelError, - QT_TRANSLATE_NOOP( - "Exceptions", - "OCC is unable to decrease the multiplicity within the maximum tolerance.")) + if (!result) { + THROWMT(Base::CADKernelError, + QT_TRANSLATE_NOOP("Exceptions", + "OCC is unable to decrease the multiplicity within the " + "maximum tolerance.")); + } } } catch (const Base::Exception& e) { @@ -7094,87 +7083,62 @@ bool SketchObject::modifyBSplineKnotMultiplicity(int GeoId, int knotIndex, int m // we succeeded with the multiplicity modification, so alignment geometry may be // invalid/inconsistent for the new bspline - std::vector delGeoId; std::vector poles = bsp->getPoles(); - std::vector newpoles = bspline->getPoles(); - std::vector prevpole(bsp->countPoles()); - - for (int i = 0; i < int(poles.size()); i++) - prevpole[i] = -1; - - int taken = 0; - for (int j = 0; j < int(poles.size()); j++) { - for (int i = taken; i < int(newpoles.size()); i++) { - if (newpoles[i] == poles[j]) { - prevpole[j] = i; - taken++; - break; - } - } - } + std::vector newPoles = bspline->getPoles(); // on fully removing a knot the knot geometry changes std::vector knots = bsp->getKnots(); - std::vector newknots = bspline->getKnots(); - std::vector prevknot(bsp->countKnots()); + std::vector newKnots = bspline->getKnots(); - for (int i = 0; i < int(knots.size()); i++) - prevknot[i] = -1; + std::map> + indexInNew {{Sketcher::BSplineControlPoint, {}}, + {Sketcher::BSplineKnotPoint, {}}}; + indexInNew[Sketcher::BSplineControlPoint].reserve(poles.size()); + indexInNew[Sketcher::BSplineKnotPoint].reserve(knots.size()); - taken = 0; - for (int j = 0; j < int(knots.size()); j++) { - for (int i = taken; i < int(newknots.size()); i++) { - if (newknots[i] == knots[j]) { - prevknot[j] = i; - taken++; - break; - } - } + for (const auto& pole : poles) { + const auto it = std::find(newPoles.begin(), newPoles.end(), pole); + indexInNew[Sketcher::BSplineControlPoint].emplace_back(it - newPoles.begin()); } + std::replace(indexInNew[Sketcher::BSplineControlPoint].begin(), + indexInNew[Sketcher::BSplineControlPoint].end(), + int(newPoles.size()), + -1); + + for (const auto& knot : knots) { + const auto it = std::find(newKnots.begin(), newKnots.end(), knot); + indexInNew[Sketcher::BSplineKnotPoint].emplace_back(it - newKnots.begin()); + } + std::replace(indexInNew[Sketcher::BSplineKnotPoint].begin(), + indexInNew[Sketcher::BSplineKnotPoint].end(), + int(newKnots.size()), + -1); const std::vector& cvals = Constraints.getValues(); std::vector newcVals(0); - // modify pole constraints - for (std::vector::const_iterator it = cvals.begin(); it != cvals.end(); - ++it) { - if ((*it)->Type == Sketcher::InternalAlignment && (*it)->Second == GeoId) { - if ((*it)->AlignmentType == Sketcher::BSplineControlPoint) { - if (prevpole[(*it)->InternalAlignmentIndex] != -1) { - assert(prevpole[(*it)->InternalAlignmentIndex] < bspline->countPoles()); - Constraint* newConstr = (*it)->clone(); - newConstr->InternalAlignmentIndex = prevpole[(*it)->InternalAlignmentIndex]; - newcVals.push_back(newConstr); - } - else { - // it is an internal alignment geometry that is no longer valid => delete it and - // the pole circle - delGeoId.push_back((*it)->First); - } - } - else if ((*it)->AlignmentType == Sketcher::BSplineKnotPoint) { - if (prevknot[(*it)->InternalAlignmentIndex] != -1) { - assert(prevknot[(*it)->InternalAlignmentIndex] < bspline->countKnots()); - Constraint* newConstr = (*it)->clone(); - newConstr->InternalAlignmentIndex = prevknot[(*it)->InternalAlignmentIndex]; - newcVals.push_back(newConstr); - } - else { - // it is an internal alignment geometry that is no longer valid => delete it and - // the knot point - delGeoId.push_back((*it)->First); - } - } - else {// it is a bspline geometry, but not a controlpoint or knot - newcVals.push_back(*it); - } + // modify pole and knot constraints + for (const auto& constr : cvals) { + if (!(constr->Type == Sketcher::InternalAlignment && constr->Second == GeoId)) { + newcVals.push_back(constr); + continue; } - else { - newcVals.push_back(*it); + + int index = indexInNew.at(constr->AlignmentType).at(constr->InternalAlignmentIndex); + + if (index == -1) { + // it is an internal alignment geometry that is no longer valid + // => delete it and the geometry + delGeoId.push_back(constr->First); + continue; } + + Constraint* newConstr = constr->clone(); + newConstr->InternalAlignmentIndex = index; + newcVals.push_back(newConstr); } const std::vector& vals = getInternalGeometry(); @@ -7211,38 +7175,43 @@ bool SketchObject::insertBSplineKnot(int GeoId, double param, int multiplicity) Base::StateLocker lock(managedoperation, true); // handling unacceptable cases - if (GeoId < 0 || GeoId > getHighestCurveIndex()) + if (GeoId < 0 || GeoId > getHighestCurveIndex()) { THROWMT( Base::ValueError, QT_TRANSLATE_NOOP("Exceptions", "B-spline Geometry Index (GeoID) is out of bounds.")); + } - if (multiplicity == 0) + if (multiplicity == 0) { THROWMT(Base::ValueError, QT_TRANSLATE_NOOP("Exceptions", "Knot cannot have zero multiplicity.")); + } const Part::Geometry* geo = getGeometry(GeoId); - if (geo->getTypeId() != Part::GeomBSplineCurve::getClassTypeId()) + if (!geo->is()) { THROWMT(Base::TypeError, QT_TRANSLATE_NOOP("Exceptions", "The Geometry Index (GeoId) provided is not a B-spline.")); + } - const Part::GeomBSplineCurve* bsp = static_cast(geo); + const auto* bsp = static_cast(geo); int degree = bsp->getDegree(); double firstParam = bsp->getFirstParameter(); double lastParam = bsp->getLastParameter(); - if (multiplicity > degree) + if (multiplicity > degree) { THROWMT(Base::ValueError, QT_TRANSLATE_NOOP( "Exceptions", "Knot multiplicity cannot be higher than the degree of the B-spline.")); + } - if (param > lastParam || param < firstParam) + if (param > lastParam || param < firstParam) { THROWMT(Base::ValueError, QT_TRANSLATE_NOOP("Exceptions", "Knot cannot be inserted outside the B-spline parameter range.")); + } std::unique_ptr bspline; @@ -7261,90 +7230,67 @@ bool SketchObject::insertBSplineKnot(int GeoId, double param, int multiplicity) std::vector delGeoId; std::vector poles = bsp->getPoles(); - std::vector newpoles = bspline->getPoles(); - std::vector prevpole(bsp->countPoles()); + std::vector newPoles = bspline->getPoles(); + std::vector poleIndexInNew(poles.size(), -1); - for (int i = 0; i < int(poles.size()); i++) - prevpole[i] = -1; - - int taken = 0; - for (int j = 0; j < int(poles.size()); j++) { - for (int i = taken; i < int(newpoles.size()); i++) { - if (newpoles[i] == poles[j]) { - prevpole[j] = i; - taken++; - break; - } - } + for (size_t j = 0; j < poles.size(); j++) { + const auto it = std::find(newPoles.begin(), newPoles.end(), poles[j]); + poleIndexInNew[j] = it - newPoles.begin(); } + std::replace(poleIndexInNew.begin(), poleIndexInNew.end(), int(newPoles.size()), -1); - // on fully removing a knot the knot geometry changes std::vector knots = bsp->getKnots(); - std::vector newknots = bspline->getKnots(); - std::vector prevknot(bsp->countKnots()); + std::vector newKnots = bspline->getKnots(); + std::vector knotIndexInNew(knots.size(), -1); - for (int i = 0; i < int(knots.size()); i++) - prevknot[i] = -1; - - taken = 0; - for (int j = 0; j < int(knots.size()); j++) { - for (int i = taken; i < int(newknots.size()); i++) { - if (newknots[i] == knots[j]) { - prevknot[j] = i; - taken++; - break; - } - } + for (size_t j = 0; j < knots.size(); j++) { + const auto it = std::find(newKnots.begin(), newKnots.end(), knots[j]); + knotIndexInNew[j] = it - newKnots.begin(); } + std::replace(knotIndexInNew.begin(), knotIndexInNew.end(), int(newKnots.size()), -1); const std::vector& cvals = Constraints.getValues(); std::vector newcVals(0); - // modify pole constraints - for (std::vector::const_iterator it = cvals.begin(); it != cvals.end(); - ++it) { - if ((*it)->Type == Sketcher::InternalAlignment && (*it)->Second == GeoId) { - if ((*it)->AlignmentType == Sketcher::BSplineControlPoint) { - if (prevpole[(*it)->InternalAlignmentIndex] != -1) { - assert(prevpole[(*it)->InternalAlignmentIndex] < bspline->countPoles()); - Constraint* newConstr = (*it)->clone(); - newConstr->InternalAlignmentIndex = prevpole[(*it)->InternalAlignmentIndex]; - newcVals.push_back(newConstr); - } - else { - // it is an internal alignment geometry that is no longer valid => delete it and - // the pole circle - delGeoId.push_back((*it)->First); - } - } - else if ((*it)->AlignmentType == Sketcher::BSplineKnotPoint) { - if (prevknot[(*it)->InternalAlignmentIndex] != -1) { - assert(prevknot[(*it)->InternalAlignmentIndex] < bspline->countKnots()); - Constraint* newConstr = (*it)->clone(); - newConstr->InternalAlignmentIndex = prevknot[(*it)->InternalAlignmentIndex]; - newcVals.push_back(newConstr); - } - else { - // it is an internal alignment geometry that is no longer valid => delete it and - // the knot point - delGeoId.push_back((*it)->First); - } - } - else { - // it is a bspline geometry, but not a controlpoint or knot - newcVals.push_back(*it); - } + // modify pole and knot constraints + for (const auto& constr : cvals) { + if (!(constr->Type == Sketcher::InternalAlignment && constr->Second == GeoId)) { + newcVals.push_back(constr); + continue; + } + + std::vector* indexInNew = nullptr; + + if (constr->AlignmentType == Sketcher::BSplineControlPoint) { + indexInNew = &poleIndexInNew; + } + else if (constr->AlignmentType == Sketcher::BSplineKnotPoint) { + indexInNew = &knotIndexInNew; } else { - newcVals.push_back(*it); + // it is a bspline geometry, but not a controlpoint or knot + newcVals.push_back(constr); + continue; } + + if (indexInNew && indexInNew->at(constr->InternalAlignmentIndex) == -1) { + // it is an internal alignment geometry that is no longer valid + // => delete it and the pole circle + delGeoId.push_back(constr->First); + continue; + } + + Constraint* newConstr = constr->clone(); + newConstr->InternalAlignmentIndex = indexInNew->at(constr->InternalAlignmentIndex); + newcVals.push_back(newConstr); } const std::vector& vals = getInternalGeometry(); std::vector newVals(vals); + GeometryFacade::copyId(geo, bspline.get()); newVals[GeoId] = bspline.release(); // Block acceptGeometry in OnChanged to avoid unnecessary checks and updates @@ -7368,12 +7314,11 @@ bool SketchObject::insertBSplineKnot(int GeoId, double param, int multiplicity) // See 247a9f0876a00e08c25b07d1f8802479d8623e87 for suggestions. // Geometry.touch(); delGeometriesExclusiveList(delGeoId); - } - else { - Geometry.touch(); + return true; } - // handle this last return + Geometry.touch(); + return true; } @@ -8275,12 +8220,12 @@ void SketchObject::validateExternalLinks() refSubShape = refShape.getSubShape(SubElement.c_str()); } } - catch ( Base::IndexError& indexError) { + catch (Base::IndexError& indexError) { removeBadLink = true; Base::Console().Warning( this->getFullLabel(), (indexError.getMessage() + "\n").c_str()); } - catch ( Base::ValueError& valueError ) { + catch (Base::ValueError& valueError) { removeBadLink = true; Base::Console().Warning( this->getFullLabel(), (valueError.getMessage() + "\n").c_str()); @@ -8288,7 +8233,7 @@ void SketchObject::validateExternalLinks() catch (Standard_Failure&) { removeBadLink = true; } - if ( removeBadLink ) { + if (removeBadLink) { rebuild = true; Objects.erase(Objects.begin() + i); SubElements.erase(SubElements.begin() + i); @@ -8296,19 +8241,10 @@ void SketchObject::validateExternalLinks() const std::vector& constraints = Constraints.getValues(); std::vector newConstraints(0); int GeoId = GeoEnum::RefExt - i; - for (std::vector::const_iterator it = constraints.begin(); - it != constraints.end(); - ++it) { - if ((*it)->First != GeoId && (*it)->Second != GeoId && (*it)->Third != GeoId) { - Constraint* copiedConstr = (*it)->clone(); - if (copiedConstr->First < GeoId && copiedConstr->First != GeoEnum::GeoUndef) - copiedConstr->First += 1; - if (copiedConstr->Second < GeoId && copiedConstr->Second != GeoEnum::GeoUndef) - copiedConstr->Second += 1; - if (copiedConstr->Third < GeoId && copiedConstr->Third != GeoEnum::GeoUndef) - copiedConstr->Third += 1; - - newConstraints.push_back(copiedConstr); + for (const auto& constr : constraints) { + auto newConstr = getConstraintAfterDeletingGeo(constr, GeoId); + if (newConstr) { + newConstraints.push_back(newConstr.release()); } } @@ -9500,57 +9436,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); + 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 && 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); - } - 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 +9688,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())) { + ++geoid; - 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); - } - } + if (!geo) { + continue; } - geoid++; + 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); } } @@ -9972,8 +9914,8 @@ double SketchObject::calculateAngleViaPoint(int GeoId1, int GeoId2, double px, d // Temporary sketch based calculation. Slow, but guaranteed consistency with constraints. Sketcher::Sketch sk; - const Part::GeomCurve* p1 = dynamic_cast(this->getGeometry(GeoId1)); - const Part::GeomCurve* p2 = dynamic_cast(this->getGeometry(GeoId2)); + const auto* p1 = dynamic_cast(this->getGeometry(GeoId1)); + const auto* p2 = dynamic_cast(this->getGeometry(GeoId2)); if (p1 && p2) { // TODO: Check if any of these are B-splines @@ -10771,20 +10713,19 @@ void SketchObject::migrateSketch() auto constraints = Constraints.getValues(); auto geometries = getInternalGeometry(); - auto parabolafound = std::find_if(geometries.begin(), geometries.end(), [](Part::Geometry* g) { + bool parabolaFound = std::any_of(geometries.begin(), geometries.end(), [](Part::Geometry* g) { return g->is(); }); - if (parabolafound != geometries.end()) { + if (parabolaFound) { - auto focalaxisfound = std::find_if(constraints.begin(), constraints.end(), [](auto c) { + bool focalaxisfound = std::any_of(constraints.begin(), constraints.end(), [](auto c) { return c->Type == InternalAlignment && c->AlignmentType == ParabolaFocalAxis; }); // There are parabolas and there isn't an IA axis. (1) there are no axis or (2) there is a // legacy construction line - if (focalaxisfound == constraints.end()) { - + if (!focalaxisfound) { // maps parabola geoid to focusgeoid std::map parabolageoid2focusgeoid; @@ -10835,18 +10776,18 @@ void SketchObject::migrateSketch() } else { auto axismajorcoincidentfound = - std::find_if(axisgeoid2parabolageoid.begin(), - axisgeoid2parabolageoid.end(), - [&](const auto& pair) { - auto parabolageoid = pair.second; - auto axisgeoid = pair.first; - return (c->First == axisgeoid && c->Second == parabolageoid - && c->SecondPos == PointPos::mid) - || (c->Second == axisgeoid && c->First == parabolageoid - && c->FirstPos == PointPos::mid); - }); + std::any_of(axisgeoid2parabolageoid.begin(), + axisgeoid2parabolageoid.end(), + [&](const auto& pair) { + auto parabolageoid = pair.second; + auto axisgeoid = pair.first; + return (c->First == axisgeoid && c->Second == parabolageoid + && c->SecondPos == PointPos::mid) + || (c->Second == axisgeoid && c->First == parabolageoid + && c->FirstPos == PointPos::mid); + }); - if (axismajorcoincidentfound != axisgeoid2parabolageoid.end()) { + if (axismajorcoincidentfound) { // we skip this coincident, the other coincident on axis will be substituted // by internal geometry constraint continue; diff --git a/src/Mod/Sketcher/App/SketchObject.h b/src/Mod/Sketcher/App/SketchObject.h index f9e95e0615..e586837318 100644 --- a/src/Mod/Sketcher/App/SketchObject.h +++ b/src/Mod/Sketcher/App/SketchObject.h @@ -157,6 +157,8 @@ public: int delGeometriesExclusiveList(const std::vector& GeoIds); /// Does the same as \a delGeometry but allows one to delete several geometries in one step int delGeometries(const std::vector& GeoIds); + template + int delGeometries(InputIt first, InputIt last); /// deletes all the elements/constraints of the sketch except for external geometry int deleteAllGeometry(); /// deletes all the constraints of the sketch @@ -364,8 +366,8 @@ public: /// retrieves the coordinates of a point static Base::Vector3d getPoint(const Part::Geometry* geo, PointPos PosId); Base::Vector3d getPoint(int GeoId, PointPos PosId) const; - template - static Base::Vector3d getPointForGeometry(const geomType* geo, PointPos PosId) + template + static Base::Vector3d getPointForGeometry(const GeomType* geo, PointPos PosId) { (void)geo; (void)PosId; @@ -476,6 +478,11 @@ public: * \return -1 on error */ int exposeInternalGeometry(int GeoId); + template + int exposeInternalGeometryForType([[maybe_unused]] const int GeoId) + { + return -1; // By default internal geometry is not supported + } /*! \brief Deletes all unused (not further constrained) internal geometry \param GeoId - the geometry having the internal geometry to delete @@ -893,6 +900,21 @@ protected: void buildShape(); /// get called by the container when a property has changed void onChanged(const App::Property* /*prop*/) override; + + /// Replaces geometries at `oldGeoIds` with `newGeos`, lower Ids first. + /// If `oldGeoIds` is bigger, deletes the remaining. + /// If `newGeos` is bigger, adds the remaining geometries at the end. + /// NOTE: Does NOT move any constraints + void replaceGeometries(std::vector oldGeoIds, std::vector& newGeos); + + /// Helper functions for `deleteUnusedInternalGeometry` by cases + /// two foci for ellipses and arcs of ellipses and hyperbolas + int deleteUnusedInternalGeometryWhenTwoFoci(int GeoId, bool delgeoid = false); + /// one focus for parabolas + int deleteUnusedInternalGeometryWhenOneFocus(int GeoId, bool delgeoid = false); + /// b-splines need their own treatment + int deleteUnusedInternalGeometryWhenBSpline(int GeoId, bool delgeoid = false); + void onDocumentRestored() override; void restoreFinished() override; @@ -982,6 +1004,8 @@ protected: int thirdGeoId = GeoEnum::GeoUndef, Sketcher::PointPos thirdPos = Sketcher::PointPos::none); +public: + // FIXME: These may not need to be public. Decide before merging. std::unique_ptr getConstraintAfterDeletingGeo(const Constraint* constr, const int deletedGeoId) const; diff --git a/src/Mod/Sketcher/App/planegcs/GCS.cpp b/src/Mod/Sketcher/App/planegcs/GCS.cpp index 68f4e2b54a..6bf522a210 100644 --- a/src/Mod/Sketcher/App/planegcs/GCS.cpp +++ b/src/Mod/Sketcher/App/planegcs/GCS.cpp @@ -525,7 +525,7 @@ void System::clear() reference.clear(); clearSubSystems(); - free(clist); + deleteAllContent(clist); c2p.clear(); p2c.clear(); } @@ -1827,18 +1827,18 @@ void System::initSolution(Algorithm alg) // diagnose conflicting or redundant constraints if (!hasDiagnosis) { diagnose(alg); - if (!hasDiagnosis) { - return; - } } + + // if still no diagnosis after explicitly calling `diagnose`, nothing to do here + if (!hasDiagnosis) { + return; + } + std::vector clistR; if (!redundant.empty()) { - for (std::vector::const_iterator constr = clist.begin(); constr != clist.end(); - ++constr) { - if (redundant.count(*constr) == 0) { - clistR.push_back(*constr); - } - } + std::copy_if(clist.begin(), clist.end(), std::back_inserter(clistR), [this](auto constr) { + return this->redundant.count(constr) == 0; + }); } else { clistR = clist; @@ -1851,15 +1851,15 @@ void System::initSolution(Algorithm alg) } int cvtid = int(plist.size()); - for (std::vector::const_iterator constr = clistR.begin(); constr != clistR.end(); - ++constr, cvtid++) { - VEC_pD& cparams = c2p[*constr]; - for (VEC_pD::const_iterator param = cparams.begin(); param != cparams.end(); ++param) { - MAP_pD_I::const_iterator it = pIndex.find(*param); + for (const auto constr : clistR) { + VEC_pD& cparams = c2p[constr]; + for (const auto param : cparams) { + MAP_pD_I::const_iterator it = pIndex.find(param); if (it != pIndex.end()) { boost::add_edge(cvtid, it->second, g); } } + ++cvtid; } VEC_I components(boost::num_vertices(g)); @@ -1875,26 +1875,21 @@ void System::initSolution(Algorithm alg) { VEC_pD reducedParams = plist; - for (std::vector::const_iterator constr = clistR.begin(); - constr != clistR.end(); - ++constr) { - if ((*constr)->getTag() >= 0 && (*constr)->getTypeId() == Equal) { - MAP_pD_I::const_iterator it1, it2; - it1 = pIndex.find((*constr)->params()[0]); - it2 = pIndex.find((*constr)->params()[1]); - if (it1 != pIndex.end() && it2 != pIndex.end()) { - reducedConstrs.insert(*constr); - double* p_kept = reducedParams[it1->second]; - double* p_replaced = reducedParams[it2->second]; - for (int i = 0; i < int(plist.size()); ++i) { - if (reducedParams[i] == p_replaced) { - reducedParams[i] = p_kept; - } - } - } + for (const auto& constr : clistR) { + if (!(constr->getTag() >= 0 && constr->getTypeId() == Equal)) { + continue; } + const auto it1 = pIndex.find(constr->params()[0]); + const auto it2 = pIndex.find(constr->params()[1]); + if (it1 == pIndex.end() || it2 == pIndex.end()) { + continue; + } + reducedConstrs.insert(constr); + double* p_kept = reducedParams[it1->second]; + double* p_replaced = reducedParams[it2->second]; + std::replace(reducedParams.begin(), reducedParams.end(), p_replaced, p_kept); } - for (int i = 0; i < int(plist.size()); ++i) { + for (size_t i = 0; i < plist.size(); ++i) { if (plist[i] != reducedParams[i]) { int cid = components[i]; reductionmaps[cid][plist[i]] = reducedParams[i]; @@ -1902,41 +1897,41 @@ void System::initSolution(Algorithm alg) } } + // TODO: Why are the later (constraint-related) items added first? + // Adding plist-related items first would simplify assignment of `i`, but is not a big expense + // overall. Leaving as is to avoid any unintended consequences. clists.clear(); // destroy any lists clists.resize(componentsSize); // create empty lists to be filled in - int i = int(plist.size()); - for (std::vector::const_iterator constr = clistR.begin(); constr != clistR.end(); - ++constr, i++) { - if (reducedConstrs.count(*constr) == 0) { + size_t i = plist.size(); + for (const auto& constr : clistR) { + if (reducedConstrs.count(constr) == 0) { int cid = components[i]; - clists[cid].push_back(*constr); + clists[cid].push_back(constr); } + ++i; } plists.clear(); // destroy any lists plists.resize(componentsSize); // create empty lists to be filled in - for (int i = 0; i < int(plist.size()); ++i) { + for (size_t i = 0; i < plist.size(); ++i) { int cid = components[i]; plists[cid].push_back(plist[i]); } // calculates subSystems and subSystemsAux from clists, plists and reductionmaps clearSubSystems(); - for (std::size_t cid = 0; cid < clists.size(); cid++) { + subSystems.resize(clists.size(), nullptr); + subSystemsAux.resize(clists.size(), nullptr); + for (std::size_t cid = 0; cid < clists.size(); ++cid) { std::vector clist0, clist1; - for (std::vector::const_iterator constr = clists[cid].begin(); - constr != clists[cid].end(); - ++constr) { - if ((*constr)->getTag() >= 0) { - clist0.push_back(*constr); - } - else { // move or distance from reference constraints - clist1.push_back(*constr); - } - } + std::partition_copy(clists[cid].begin(), + clists[cid].end(), + std::back_inserter(clist0), + std::back_inserter(clist1), + [](auto constr) { + return constr->getTag() >= 0; + }); - subSystems.push_back(nullptr); - subSystemsAux.push_back(nullptr); if (!clist0.empty()) { subSystems[cid] = new SubSystem(clist0, plists[cid], reductionmaps[cid]); } @@ -2069,15 +2064,18 @@ int System::solve_BFGS(SubSystem* subsys, bool /*isFine*/, bool isRedundantsolvi h = x - h; // = x - xold // double convergence = isFine ? convergence : XconvergenceRough; - int maxIterNumber = - (isRedundantsolving - ? (sketchSizeMultiplierRedundant ? maxIterRedundant * xsize : maxIterRedundant) - : (sketchSizeMultiplier ? maxIter * xsize : maxIter)); + int maxIterNumber = (sketchSizeMultiplier ? maxIter * xsize : maxIter); + double convCriterion = convergence; + if (isRedundantsolving) { + maxIterNumber = + (sketchSizeMultiplierRedundant ? maxIterRedundant * xsize : maxIterRedundant); + convCriterion = convergenceRedundant; + } if (debugMode == IterationLevel) { std::stringstream stream; - stream << "BFGS: convergence: " << (isRedundantsolving ? convergenceRedundant : convergence) - << ", xsize: " << xsize << ", maxIter: " << maxIterNumber << "\n"; + stream << "BFGS: convergence: " << convCriterion << ", xsize: " << xsize + << ", maxIter: " << maxIterNumber << "\n"; const std::string tmp = stream.str(); Base::Console().Log(tmp.c_str()); @@ -2086,9 +2084,9 @@ int System::solve_BFGS(SubSystem* subsys, bool /*isFine*/, bool isRedundantsolvi double divergingLim = 1e6 * err + 1e12; double h_norm {}; - for (int iter = 1; iter < maxIterNumber; iter++) { + for (int iter = 1; iter < maxIterNumber; ++iter) { h_norm = h.norm(); - if (h_norm <= (isRedundantsolving ? convergenceRedundant : convergence) || err <= smallF) { + if (h_norm <= convCriterion || err <= smallF) { if (debugMode == IterationLevel) { std::stringstream stream; stream << "BFGS Converged!!: " @@ -2099,7 +2097,8 @@ int System::solve_BFGS(SubSystem* subsys, bool /*isFine*/, bool isRedundantsolvi } break; } - if (err > divergingLim || err != err) { // check for diverging and NaN + if (err > divergingLim || err != err) { + // check for diverging and NaN if (debugMode == IterationLevel) { std::stringstream stream; stream << "BFGS Failed: Diverging!!: " @@ -2152,7 +2151,7 @@ int System::solve_BFGS(SubSystem* subsys, bool /*isFine*/, bool isRedundantsolvi if (err <= smallF) { return Success; } - if (h.norm() <= (isRedundantsolving ? convergenceRedundant : convergence)) { + if (h.norm() <= convCriterion) { return Converged; } return Failed; @@ -2183,16 +2182,21 @@ int System::solve_LM(SubSystem* subsys, bool isRedundantsolving) subsys->calcResidual(e); e *= -1; - int maxIterNumber = - (isRedundantsolving - ? (sketchSizeMultiplierRedundant ? maxIterRedundant * xsize : maxIterRedundant) - : (sketchSizeMultiplier ? maxIter * xsize : maxIter)); + int maxIterNumber = (sketchSizeMultiplier ? maxIter * xsize : maxIter); double divergingLim = 1e6 * e.squaredNorm() + 1e12; - double eps = (isRedundantsolving ? LM_epsRedundant : LM_eps); - double eps1 = (isRedundantsolving ? LM_eps1Redundant : LM_eps1); - double tau = (isRedundantsolving ? LM_tauRedundant : LM_tau); + double eps = LM_eps; + double eps1 = LM_eps1; + double tau = LM_tau; + + if (isRedundantsolving) { + maxIterNumber = + (sketchSizeMultiplierRedundant ? maxIterRedundant * xsize : maxIterRedundant); + eps = LM_epsRedundant; + eps1 = LM_eps1Redundant; + tau = LM_tauRedundant; + } if (debugMode == IterationLevel) { std::stringstream stream; @@ -2207,21 +2211,21 @@ int System::solve_LM(SubSystem* subsys, bool isRedundantsolving) double nu = 2, mu = 0; int iter = 0, stop = 0; for (iter = 0; iter < maxIterNumber && !stop; ++iter) { - // check error double err = e.squaredNorm(); - if (err <= eps * eps) { // error is small, Success + if (err <= eps * eps) { + // error is small, Success stop = 1; break; } - else if (err > divergingLim || err != err) { // check for diverging and NaN + else if (err > divergingLim || err != err) { + // check for diverging and NaN stop = 6; break; } // J^T J, J^T e subsys->calcJacobi(J); - ; A = J.transpose() * J; g = J.transpose() * e; @@ -2256,7 +2260,6 @@ int System::solve_LM(SubSystem* subsys, bool isRedundantsolving) // check if solving works if (rel_error < 1e-5) { - // restrict h according to maxStep double scale = subsys->maxStep(h); if (scale < 1.) { @@ -2267,12 +2270,13 @@ int System::solve_LM(SubSystem* subsys, bool isRedundantsolving) x_new = x + h; h_norm = h.squaredNorm(); - if (h_norm <= eps1 * eps1 * x.norm()) { // relative change in p is small, stop + if (h_norm <= eps1 * eps1 * x.norm()) { + // relative change in p is small, stop stop = 3; break; } - else if (h_norm - >= (x.norm() + eps1) / (DBL_EPSILON * DBL_EPSILON)) { // almost singular + else if (h_norm >= (x.norm() + eps1) / (DBL_EPSILON * DBL_EPSILON)) { + // almost singular stop = 4; break; } @@ -2332,17 +2336,12 @@ int System::solve_LM(SubSystem* subsys, bool isRedundantsolving) return (stop == 1) ? Success : Failed; } - int System::solve_DL(SubSystem* subsys, bool isRedundantsolving) { #ifdef _GCS_EXTRACT_SOLVER_SUBSYSTEM_ extractSubsystem(subsys, isRedundantsolving); #endif - double tolg = (isRedundantsolving ? DL_tolgRedundant : DL_tolg); - double tolx = (isRedundantsolving ? DL_tolxRedundant : DL_tolx); - double tolf = (isRedundantsolving ? DL_tolfRedundant : DL_tolf); - int xsize = subsys->pSize(); int csize = subsys->cSize(); @@ -2350,10 +2349,19 @@ int System::solve_DL(SubSystem* subsys, bool isRedundantsolving) return Success; } - int maxIterNumber = - (isRedundantsolving - ? (sketchSizeMultiplierRedundant ? maxIterRedundant * xsize : maxIterRedundant) - : (sketchSizeMultiplier ? maxIter * xsize : maxIter)); + double tolg = DL_tolg; + double tolx = DL_tolx; + double tolf = DL_tolf; + + int maxIterNumber = (sketchSizeMultiplier ? maxIter * xsize : maxIter); + if (isRedundantsolving) { + tolg = DL_tolgRedundant; + tolx = DL_tolxRedundant; + tolf = DL_tolfRedundant; + + maxIterNumber = + (sketchSizeMultiplierRedundant ? maxIterRedundant * xsize : maxIterRedundant); + } if (debugMode == IterationLevel) { std::stringstream stream; @@ -2396,84 +2404,83 @@ int System::solve_DL(SubSystem* subsys, bool isRedundantsolving) double nu = 2.; int iter = 0, stop = 0, reduce = 0; while (!stop) { - // check if finished - if (fx_inf <= tolf) { // Success + if (fx_inf <= tolf) { + // Success stop = 1; + break; } else if (g_inf <= tolg) { stop = 2; + break; } else if (delta <= tolx * (tolx + x.norm())) { stop = 2; + break; } else if (iter >= maxIterNumber) { stop = 4; + break; } - else if (err > divergingLim || err != err) { // check for diverging and NaN + else if (err > divergingLim || err != err) { + // check for diverging and NaN stop = 6; - } - else { - // get the steepest descent direction - alpha = g.squaredNorm() / (Jx * g).squaredNorm(); - h_sd = alpha * g; - - // get the gauss-newton step - // https://forum.freecad.org/viewtopic.php?f=10&t=12769&start=50#p106220 - // https://forum.kde.org/viewtopic.php?f=74&t=129439#p346104 - switch (dogLegGaussStep) { - case FullPivLU: - h_gn = Jx.fullPivLu().solve(-fx); - break; - case LeastNormFullPivLU: - h_gn = Jx.adjoint() * (Jx * Jx.adjoint()).fullPivLu().solve(-fx); - break; - case LeastNormLdlt: - h_gn = Jx.adjoint() * (Jx * Jx.adjoint()).ldlt().solve(-fx); - break; - } - - double rel_error = (Jx * h_gn + fx).norm() / fx.norm(); - if (rel_error > 1e15) { - break; - } - - // compute the dogleg step - if (h_gn.norm() < delta) { - h_dl = h_gn; - if (h_dl.norm() <= tolx * (tolx + x.norm())) { - stop = 5; - break; - } - } - else if (alpha * g.norm() >= delta) { - h_dl = (delta / (alpha * g.norm())) * h_sd; - } - else { - // compute beta - double beta = 0; - Eigen::VectorXd b = h_gn - h_sd; - double bb = (b.transpose() * b).norm(); - double gb = (h_sd.transpose() * b).norm(); - double c = (delta + h_sd.norm()) * (delta - h_sd.norm()); - - if (gb > 0) { - beta = c / (gb + sqrt(gb * gb + c * bb)); - } - else { - beta = (sqrt(gb * gb + c * bb) - gb) / bb; - } - - // and update h_dl and dL with beta - h_dl = h_sd + beta * b; - } - } - - // see if we are already finished - if (stop) { break; } + // get the steepest descent direction + alpha = g.squaredNorm() / (Jx * g).squaredNorm(); + h_sd = alpha * g; + + // get the gauss-newton step + // https://forum.freecad.org/viewtopic.php?f=10&t=12769&start=50#p106220 + // https://forum.kde.org/viewtopic.php?f=74&t=129439#p346104 + switch (dogLegGaussStep) { + case FullPivLU: + h_gn = Jx.fullPivLu().solve(-fx); + break; + case LeastNormFullPivLU: + h_gn = Jx.adjoint() * (Jx * Jx.adjoint()).fullPivLu().solve(-fx); + break; + case LeastNormLdlt: + h_gn = Jx.adjoint() * (Jx * Jx.adjoint()).ldlt().solve(-fx); + break; + } + + double rel_error = (Jx * h_gn + fx).norm() / fx.norm(); + if (rel_error > 1e15) { + break; + } + + // compute the dogleg step + if (h_gn.norm() < delta) { + h_dl = h_gn; + if (h_dl.norm() <= tolx * (tolx + x.norm())) { + stop = 5; + break; + } + } + else if (alpha * g.norm() >= delta) { + h_dl = (delta / (alpha * g.norm())) * h_sd; + } + else { + // compute beta + double beta = 0; + Eigen::VectorXd b = h_gn - h_sd; + double bb = (b.transpose() * b).norm(); + double gb = (h_sd.transpose() * b).norm(); + double c = (delta + h_sd.norm()) * (delta - h_sd.norm()); + + if (gb > 0) { + beta = c / (gb + sqrt(gb * gb + c * bb)); + } + else { + beta = (sqrt(gb * gb + c * bb) - gb) / bb; + } + + // and update h_dl and dL with beta + h_dl = h_sd + beta * b; + } // get the new values double err_new; @@ -4984,7 +4991,6 @@ int System::diagnose(Algorithm alg) // like 0 and -1. std::map tagmultiplicity; - makeReducedJacobian(J, jacobianconstraintmap, pdiagnoselist, tagmultiplicity); // this function will exit with a diagnosis and, unless overridden by functions below, with full @@ -4992,10 +4998,6 @@ int System::diagnose(Algorithm alg) hasDiagnosis = true; dofs = pdiagnoselist.size(); - if (J.rows() > 0) { - emptyDiagnoseMatrix = false; - } - // There is a legacy decision to use QR decomposition. I (abdullah) do not know all the // consideration taken in that decisions. I see that: // - QR decomposition is able to provide information about the rank and @@ -5050,65 +5052,73 @@ int System::diagnose(Algorithm alg) } #endif + if (J.rows() == 0) { + return dofs; + } + + // From here on, presuming `J.rows() > 0`. + emptyDiagnoseMatrix = false; + if (qrAlgorithm == EigenDenseQR) { #ifdef PROFILE_DIAGNOSE Base::TimeElapsed DenseQR_start_time; #endif - if (J.rows() > 0) { - int rank = 0; // rank is not cheap to retrieve from qrJT in DenseQR - Eigen::MatrixXd R; - Eigen::FullPivHouseholderQR qrJT; - // Here we give the system the possibility to run the two QR decompositions in parallel, - // depending on the load of the system so we are using the default std::launch::async | - // std::launch::deferred policy, as nobody better than the system nows if it can run the - // task in parallel or is oversubscribed and should deferred it. Care to wait() for the - // future before any prospective detection of conflicting/redundant, because the - // redundant solve modifies pdiagnoselist and it would NOT be thread-safe. Care to call - // the thread with silent=true, unless the present thread does not use Base::Console, or - // the launch policy is set to std::launch::deferred policy, as it is not thread-safe to - // use them in both at the same time. - // - // identifyDependentParametersDenseQR(J, jacobianconstraintmap, pdiagnoselist, true) - // - auto fut = std::async(&System::identifyDependentParametersDenseQR, - this, - J, - jacobianconstraintmap, - pdiagnoselist, - true); - makeDenseQRDecomposition(J, jacobianconstraintmap, qrJT, rank, R); + int rank = 0; // rank is not cheap to retrieve from qrJT in DenseQR + Eigen::MatrixXd R; + Eigen::FullPivHouseholderQR qrJT; + // Here we give the system the possibility to run the two QR decompositions in parallel, + // depending on the load of the system so we are using the default std::launch::async | + // std::launch::deferred policy, as nobody better than the system nows if it can run the + // task in parallel or is oversubscribed and should deferred it. Care to wait() for the + // future before any prospective detection of conflicting/redundant, because the + // redundant solve modifies pdiagnoselist and it would NOT be thread-safe. Care to call + // the thread with silent=true, unless the present thread does not use Base::Console, or + // the launch policy is set to std::launch::deferred policy, as it is not thread-safe to + // use them in both at the same time. + // + // identifyDependentParametersDenseQR(J, jacobianconstraintmap, pdiagnoselist, true) + // + auto fut = std::async(&System::identifyDependentParametersDenseQR, + this, + J, + jacobianconstraintmap, + pdiagnoselist, + true); - int paramsNum = qrJT.rows(); - int constrNum = qrJT.cols(); + makeDenseQRDecomposition(J, jacobianconstraintmap, qrJT, rank, R); - // This function is legacy code that was used to obtain partial geometry dependency - // information from a SINGLE Dense QR decomposition. I am reluctant to remove it from - // here until everything new is well tested. - // identifyDependentGeometryParametersInTransposedJacobianDenseQRDecomposition( qrJT, - // pdiagnoselist, paramsNum, rank); + int paramsNum = qrJT.rows(); + int constrNum = qrJT.cols(); - fut.wait(); // wait for the execution of identifyDependentParametersSparseQR to finish + // This function is legacy code that was used to obtain partial geometry dependency + // information from a SINGLE Dense QR decomposition. I am reluctant to remove it from + // here until everything new is well tested. + // identifyDependentGeometryParametersInTransposedJacobianDenseQRDecomposition( qrJT, + // pdiagnoselist, paramsNum, rank); - dofs = paramsNum - rank; // unless overconstraint, which will be overridden below + fut.wait(); // wait for the execution of identifyDependentParametersSparseQR to finish - // Detecting conflicting or redundant constraints - if (constrNum > rank) { // conflicting or redundant constraints - int nonredundantconstrNum; - identifyConflictingRedundantConstraints(alg, - qrJT, - jacobianconstraintmap, - tagmultiplicity, - pdiagnoselist, - R, - constrNum, - rank, - nonredundantconstrNum); - if (paramsNum == rank && nonredundantconstrNum > rank) { // over-constrained - dofs = paramsNum - nonredundantconstrNum; - } + dofs = paramsNum - rank; // unless overconstraint, which will be overridden below + + // Detecting conflicting or redundant constraints + if (constrNum > rank) { + // conflicting or redundant constraints + int nonredundantconstrNum; + identifyConflictingRedundantConstraints(alg, + qrJT, + jacobianconstraintmap, + tagmultiplicity, + pdiagnoselist, + R, + constrNum, + rank, + nonredundantconstrNum); + if (paramsNum == rank && nonredundantconstrNum > rank) { // over-constrained + dofs = paramsNum - nonredundantconstrNum; } } + #ifdef PROFILE_DIAGNOSE Base::TimeElapsed DenseQR_end_time; @@ -5123,66 +5133,64 @@ int System::diagnose(Algorithm alg) #ifdef PROFILE_DIAGNOSE Base::TimeElapsed SparseQR_start_time; #endif - if (J.rows() > 0) { - int rank = 0; - Eigen::MatrixXd R; - Eigen::SparseQR, Eigen::COLAMDOrdering> SqrJT; - // Here we give the system the possibility to run the two QR decompositions in parallel, - // depending on the load of the system so we are using the default std::launch::async | - // std::launch::deferred policy, as nobody better than the system nows if it can run the - // task in parallel or is oversubscribed and should deferred it. Care to wait() for the - // future before any prospective detection of conflicting/redundant, because the - // redundant solve modifies pdiagnoselist and it would NOT be thread-safe. Care to call - // the thread with silent=true, unless the present thread does not use Base::Console, or - // the launch policy is set to std::launch::deferred policy, as it is not thread-safe to - // use them in both at the same time. - // - // identifyDependentParametersSparseQR(J, jacobianconstraintmap, pdiagnoselist, true) - // - // Debug: - // auto fut = - // std::async(std::launch::deferred,&System::identifyDependentParametersSparseQR, this, - // J, jacobianconstraintmap, pdiagnoselist, false); - auto fut = std::async(&System::identifyDependentParametersSparseQR, - this, - J, + int rank = 0; + Eigen::MatrixXd R; + Eigen::SparseQR, Eigen::COLAMDOrdering> SqrJT; + // Here we give the system the possibility to run the two QR decompositions in parallel, + // depending on the load of the system so we are using the default std::launch::async | + // std::launch::deferred policy, as nobody better than the system nows if it can run the + // task in parallel or is oversubscribed and should deferred it. Care to wait() for the + // future before any prospective detection of conflicting/redundant, because the + // redundant solve modifies pdiagnoselist and it would NOT be thread-safe. Care to call + // the thread with silent=true, unless the present thread does not use Base::Console, or + // the launch policy is set to std::launch::deferred policy, as it is not thread-safe to + // use them in both at the same time. + // + // identifyDependentParametersSparseQR(J, jacobianconstraintmap, pdiagnoselist, true) + // + // Debug: + // auto fut = + // std::async(std::launch::deferred,&System::identifyDependentParametersSparseQR, this, + // J, jacobianconstraintmap, pdiagnoselist, false); + auto fut = std::async(&System::identifyDependentParametersSparseQR, + this, + J, + jacobianconstraintmap, + pdiagnoselist, + /*silent=*/true); + + makeSparseQRDecomposition(J, jacobianconstraintmap, - pdiagnoselist, - /*silent=*/true); + SqrJT, + rank, + R, + /*transposed=*/true, + /*silent=*/false); - makeSparseQRDecomposition(J, - jacobianconstraintmap, - SqrJT, - rank, - R, - /*transposed=*/true, - /*silent=*/false); + int paramsNum = SqrJT.rows(); + int constrNum = SqrJT.cols(); - int paramsNum = SqrJT.rows(); - int constrNum = SqrJT.cols(); + fut.wait(); // wait for the execution of identifyDependentParametersSparseQR to finish - fut.wait(); // wait for the execution of identifyDependentParametersSparseQR to finish + dofs = paramsNum - rank; // unless overconstraint, which will be overridden below - dofs = paramsNum - rank; // unless overconstraint, which will be overridden below + // Detecting conflicting or redundant constraints + if (constrNum > rank) { + int nonredundantconstrNum; - // Detecting conflicting or redundant constraints - if (constrNum > rank) { + identifyConflictingRedundantConstraints(alg, + SqrJT, + jacobianconstraintmap, + tagmultiplicity, + pdiagnoselist, + R, + constrNum, + rank, + nonredundantconstrNum); - int nonredundantconstrNum; - - identifyConflictingRedundantConstraints(alg, - SqrJT, - jacobianconstraintmap, - tagmultiplicity, - pdiagnoselist, - R, - constrNum, - rank, - nonredundantconstrNum); - - if (paramsNum == rank && nonredundantconstrNum > rank) { // over-constrained - dofs = paramsNum - nonredundantconstrNum; - } + if (paramsNum == rank && nonredundantconstrNum > rank) { + // over-constrained + dofs = paramsNum - nonredundantconstrNum; } } @@ -5607,23 +5615,23 @@ void System::identifyConflictingRedundantConstraints( SET_I satisfiedGroups; while (1) { // conflictingMap contains all the eligible constraints of conflict groups not yet - // satisfied. as groups get satisfied, the map created on every iteration is smaller, until + // satisfied. As groups get satisfied, the map created on every iteration is smaller, until // such time it is empty and the infinite loop is exited. The guarantee that the loop will // be exited originates from the fact that in each iteration the algorithm will select one // constraint from the conflict groups, which will satisfy at least one group. std::map conflictingMap; for (std::size_t i = 0; i < conflictGroups.size(); i++) { - if (satisfiedGroups.count(i) == 0) { - for (std::size_t j = 0; j < conflictGroups[i].size(); j++) { - Constraint* constr = conflictGroups[i][j]; - bool isinternalalignment = - (constr->isInternalAlignment() == Constraint::Alignment::InternalAlignment); - bool priorityconstraint = (constr->getTag() == 0); - if (!priorityconstraint && !isinternalalignment) { // exclude constraints - // tagged with zero and - // internal alignment - conflictingMap[constr].insert(i); - } + if (satisfiedGroups.count(i) != 0) { + continue; + } + + for (const auto& constr : conflictGroups[i]) { + bool isinternalalignment = + (constr->isInternalAlignment() == Constraint::Alignment::InternalAlignment); + bool priorityconstraint = (constr->getTag() == 0); + if (!priorityconstraint && !isinternalalignment) { + // exclude constraints tagged with zero and internal alignment + conflictingMap[constr].insert(i); } } } @@ -5632,65 +5640,61 @@ void System::identifyConflictingRedundantConstraints( break; } - int maxPopularity = 0; - Constraint* mostPopular = nullptr; - for (std::map::const_iterator it = conflictingMap.begin(); - it != conflictingMap.end(); - ++it) { + /* This is a heuristic algorithm to propose the user which constraints from a + * redundant/conflicting set should be removed. It is based on the following principles: + * 1. if the current constraint is more popular than previous ones (appears in more + * sets), take it. This prioritises removal of constraints that cause several + * independent groups of constraints to be conflicting/redundant. It is based on the + * observation that the redundancy/conflict is caused by the lesser amount of + * constraints. + * 2. if there is already a constraint ranking in the contest, and the current one is as + * popular, prefer the constraint that removes a lesser amount of DoFs. This prioritises + * removal of sketcher constraints (not solver constraints) that generates a higher + * amount of solver constraints. It is based on the observation that constraints taking + * a higher amount of DoFs (such as symmetry) are preferred by the user, who may not see + * the redundancy of simpler ones. + * 3. if there is already a constraint ranking in the context, the current one is as + * popular, and they remove the same amount of DoFs, prefer removal of the latest + * introduced. + */ + auto iterMostPopular = std::max_element( + conflictingMap.begin(), + conflictingMap.end(), + [&tagmultiplicity](const auto& pair1, const auto& pair2) { + size_t sizeOfSet1 = pair1.second.size(); + size_t sizeOfSet2 = pair2.second.size(); + auto tag1 = pair1.first->getTag(); + auto tag2 = pair2.first->getTag(); - int numberofsets = static_cast( - it->second.size()); // number of sets in which the constraint appears + return (sizeOfSet2 > sizeOfSet1 // (1) + || (sizeOfSet2 == sizeOfSet1 + && tagmultiplicity.at(tag2) < tagmultiplicity.at(tag1)) // (2) + || (sizeOfSet2 == sizeOfSet1 + && tagmultiplicity.at(tag2) == tagmultiplicity.at(tag1) + && tag2 > tag1)); // (3) + }); - /* This is a heuristic algorithm to propose the user which constraints from a - * redundant/conflicting set should be removed. It is based on the following principles: - * 1. if the current constraint is more popular than previous ones (appears in more - * sets), take it. This prioritises removal of constraints that cause several - * independent groups of constraints to be conflicting/redundant. It is based on the - * observation that the redundancy/conflict is caused by the lesser amount of - * constraints. - * 2. if there is already a constraint ranking in the contest, and the current one is as - * popular, prefer the constraint that removes a lesser amount of DoFs. This prioritises - * removal of sketcher constraints (not solver constraints) that generates a higher - * amount of solver constraints. It is based on the observation that constraints taking - * a higher amount of DoFs (such as symmetry) are preferred by the user, who may not see - * the redundancy of simpler ones. - * 3. if there is already a constraint ranking in the context, the current one is as - * popular, and they remove the same amount of DoFs, prefer removal of the latest - * introduced. - */ + Constraint* mostPopular = iterMostPopular->first; + int maxPopularity = iterMostPopular->second.size(); - if ((numberofsets > maxPopularity || // (1) - (numberofsets == maxPopularity && mostPopular - && tagmultiplicity.at(it->first->getTag()) - < tagmultiplicity.at(mostPopular->getTag())) - || // (2) - - (numberofsets == maxPopularity && mostPopular - && tagmultiplicity.at(it->first->getTag()) - == tagmultiplicity.at(mostPopular->getTag()) - && it->first->getTag() > mostPopular->getTag())) // (3) - - ) { - mostPopular = it->first; - maxPopularity = numberofsets; - } + if (!(maxPopularity > 0)) { + continue; } - if (maxPopularity > 0) { - // adding for skipping not only the mostPopular, but also any other constraint in the - // conflicting map associated with the same tag (namely any other solver - // constraint associated with the same sketcher constraint that is also conflicting) - auto maxPopularityTag = mostPopular->getTag(); - for (const auto& c : conflictingMap) { - if (c.first->getTag() == maxPopularityTag) { - skipped.insert(c.first); - for (SET_I::const_iterator it = conflictingMap[c.first].begin(); - it != conflictingMap[c.first].end(); - ++it) { - satisfiedGroups.insert(*it); - } - } + // adding for skipping not only the mostPopular, but also any other constraint in the + // conflicting map associated with the same tag (namely any other solver + // constraint associated with the same sketcher constraint that is also conflicting) + auto maxPopularityTag = mostPopular->getTag(); + + for (const auto& [constr, conflSet] : conflictingMap) { + if (!(constr->getTag() == maxPopularityTag)) { + continue; } + + skipped.insert(constr); + std::copy(conflSet.begin(), + conflSet.end(), + std::inserter(satisfiedGroups, satisfiedGroups.begin())); } } @@ -5701,12 +5705,12 @@ void System::identifyConflictingRedundantConstraints( std::vector clistTmp; clistTmp.reserve(clist.size()); - for (std::vector::iterator constr = clist.begin(); constr != clist.end(); - ++constr) { - if ((*constr)->isDriving() && skipped.count(*constr) == 0) { - clistTmp.push_back(*constr); - } - } + std::copy_if(clist.begin(), + clist.end(), + std::back_inserter(clistTmp), + [&skipped](const auto& constr) { + return (constr->isDriving() && skipped.count(constr) == 0); + }); SubSystem* subSysTmp = new SubSystem(clistTmp, pdiagnoselist); int res = solve(subSysTmp, true, alg, true); @@ -5730,78 +5734,77 @@ void System::identifyConflictingRedundantConstraints( if (res == Success) { subSysTmp->applySolution(); - for (std::set::const_iterator constr = skipped.begin(); - constr != skipped.end(); - ++constr) { - double err = (*constr)->error(); - if (err * err < convergenceRedundant) { - redundant.insert(*constr); - } - } + std::copy_if(skipped.begin(), + skipped.end(), + std::inserter(redundant, redundant.begin()), + [this](const auto& constr) { + double err = constr->error(); + return (err * err < this->convergenceRedundant); + }); resetToReference(); if (debugMode == Minimal || debugMode == IterationLevel) { Base::Console().Log("Sketcher Redundant solving: %d redundants\n", redundant.size()); } + // TODO: Figure out why we need to iterate in reverse order and add explanation here. std::vector> conflictGroupsOrig = conflictGroups; conflictGroups.clear(); for (int i = conflictGroupsOrig.size() - 1; i >= 0; i--) { - bool isRedundant = false; - for (std::size_t j = 0; j < conflictGroupsOrig[i].size(); j++) { - if (redundant.count(conflictGroupsOrig[i][j]) > 0) { - isRedundant = true; - - if (debugMode == IterationLevel) { - Base::Console().Log("(Partially) Redundant, Group %d, index %d, Tag: %d\n", - i, - j, - (conflictGroupsOrig[i][j])->getTag()); - } - - break; - } - } - if (!isRedundant) { + auto iterRedundantEntry = std::find_if(conflictGroups[i].begin(), + conflictGroups[i].end(), + [this](const auto item) { + return (this->redundant.count(item) > 0); + }); + bool hasRedundant = (iterRedundantEntry != conflictGroups[i].end()); + if (!hasRedundant) { conflictGroups.push_back(conflictGroupsOrig[i]); + continue; } - else { - constrNum--; + + if (debugMode == IterationLevel) { + Base::Console().Log("(Partially) Redundant, Group %d, index %d, Tag: %d\n", + i, + iterRedundantEntry - conflictGroupsOrig[i].begin(), + (*iterRedundantEntry)->getTag()); } + + constrNum--; } } delete subSysTmp; // simplified output of conflicting tags SET_I conflictingTagsSet; - for (std::size_t i = 0; i < conflictGroups.size(); i++) { - for (std::size_t j = 0; j < conflictGroups[i].size(); j++) { - bool isinternalalignment = (conflictGroups[i][j]->isInternalAlignment() - == Constraint::Alignment::InternalAlignment); - if (conflictGroups[i][j]->getTag() != 0 - && !isinternalalignment) { // exclude constraints tagged with zero and internal - // alignment - conflictingTagsSet.insert(conflictGroups[i][j]->getTag()); - } - } + for (const auto& cGroup : conflictGroups) { + // exclude internal alignment + std::transform(cGroup.begin(), + cGroup.end(), + std::inserter(conflictingTagsSet, conflictingTagsSet.begin()), + [](const auto& constr) { + bool isinternalalignment = (constr->isInternalAlignment() + == Constraint::Alignment::InternalAlignment); + return (isinternalalignment ? 0 : constr->getTag()); + }); } + // exclude constraints tagged with zero + conflictingTagsSet.erase(0); + conflictingTags.resize(conflictingTagsSet.size()); std::copy(conflictingTagsSet.begin(), conflictingTagsSet.end(), conflictingTags.begin()); // output of redundant tags SET_I redundantTagsSet, partiallyRedundantTagsSet; - for (std::set::iterator constr = redundant.begin(); constr != redundant.end(); - ++constr) { - redundantTagsSet.insert((*constr)->getTag()); - partiallyRedundantTagsSet.insert((*constr)->getTag()); + for (const auto& constr : redundant) { + redundantTagsSet.insert(constr->getTag()); + partiallyRedundantTagsSet.insert(constr->getTag()); } // remove tags represented at least in one non-redundant constraint - for (std::vector::iterator constr = clist.begin(); constr != clist.end(); - ++constr) { - if (redundant.count(*constr) == 0) { - redundantTagsSet.erase((*constr)->getTag()); + for (const auto& constr : clist) { + if (redundant.count(constr) == 0) { + redundantTagsSet.erase(constr->getTag()); } } @@ -5820,12 +5823,11 @@ void System::identifyConflictingRedundantConstraints( nonredundantconstrNum = constrNum; } - void System::clearSubSystems() { isInit = false; - free(subSystems); - free(subSystemsAux); + deleteAllContent(subSystems); + deleteAllContent(subSystemsAux); subSystems.clear(); subSystemsAux.clear(); } @@ -5907,70 +5909,27 @@ double lineSearch(SubSystem* subsys, Eigen::VectorXd& xdir) return alphaStar; } - -void free(VEC_pD& doublevec) +void deleteAllContent(VEC_pD& doublevec) { - for (VEC_pD::iterator it = doublevec.begin(); it != doublevec.end(); ++it) { - if (*it) { - delete *it; - } + for (auto& doubleptr : doublevec) { + delete doubleptr; } doublevec.clear(); } -void free(std::vector& constrvec) +void deleteAllContent(std::vector& constrvec) { - for (std::vector::iterator constr = constrvec.begin(); constr != constrvec.end(); - ++constr) { - if (*constr) { - switch ((*constr)->getTypeId()) { - case Equal: - delete static_cast(*constr); - break; - case Difference: - delete static_cast(*constr); - break; - case P2PDistance: - delete static_cast(*constr); - break; - case P2PAngle: - delete static_cast(*constr); - break; - case P2LDistance: - delete static_cast(*constr); - break; - case PointOnLine: - delete static_cast(*constr); - break; - case Parallel: - delete static_cast(*constr); - break; - case Perpendicular: - delete static_cast(*constr); - break; - case L2LAngle: - delete static_cast(*constr); - break; - case MidpointOnLine: - delete static_cast(*constr); - break; - case None: - default: - delete *constr; - } - } + for (auto& constr : constrvec) { + delete constr; } constrvec.clear(); } -void free(std::vector& subsysvec) +void deleteAllContent(std::vector& subsysvec) { - for (std::vector::iterator it = subsysvec.begin(); it != subsysvec.end(); ++it) { - if (*it) { - delete *it; - } + for (auto& subsys : subsysvec) { + delete subsys; } } - } // namespace GCS diff --git a/src/Mod/Sketcher/App/planegcs/GCS.h b/src/Mod/Sketcher/App/planegcs/GCS.h index 6b0b12da05..440c8d278e 100644 --- a/src/Mod/Sketcher/App/planegcs/GCS.h +++ b/src/Mod/Sketcher/App/planegcs/GCS.h @@ -633,9 +633,9 @@ protected: // Helper elements /////////////////////////////////////// -void free(VEC_pD& doublevec); -void free(std::vector& constrvec); -void free(std::vector& subsysvec); +void deleteAllContent(VEC_pD& doublevec); +void deleteAllContent(std::vector& constrvec); +void deleteAllContent(std::vector& subsysvec); } // namespace GCS diff --git a/tests/src/Mod/Sketcher/App/SketchObject.cpp b/tests/src/Mod/Sketcher/App/SketchObject.cpp index 69c4fdcd54..068d0d4aa8 100644 --- a/tests/src/Mod/Sketcher/App/SketchObject.cpp +++ b/tests/src/Mod/Sketcher/App/SketchObject.cpp @@ -48,6 +48,18 @@ void setupEllipse(Part::GeomEllipse& ellipse) ellipse.setMinorRadius(minorRadius); } +void setupArcOfHyperbola(Part::GeomArcOfHyperbola& arcOfHyperbola) +{ + Base::Vector3d coordsCenter(1.0, 2.0, 0.0); + double majorRadius = 4.0; + double minorRadius = 3.0; + double startParam = M_PI / 3, endParam = M_PI * 1.5; + arcOfHyperbola.setCenter(coordsCenter); + arcOfHyperbola.setMajorRadius(majorRadius); + arcOfHyperbola.setMinorRadius(minorRadius); + arcOfHyperbola.setRange(startParam, endParam, true); +} + void setupArcOfParabola(Part::GeomArcOfParabola& aop) { Base::Vector3d coordsCenter(1.0, 2.0, 0.0); @@ -562,6 +574,302 @@ TEST_F(SketchObjectTest, testGetPointFromGeomBSplineCurvePeriodic) EXPECT_DOUBLE_EQ(ptStart[1], ptEnd[1]); } +TEST_F(SketchObjectTest, testConstraintAfterDeletingGeo) +{ + // Arrange + int geoId1 = 42, geoId2 = 10, geoId3 = 0, geoId4 = -8; + + Sketcher::Constraint* nullConstr = nullptr; + + Sketcher::Constraint constr1; + constr1.Type = Sketcher::ConstraintType::Coincident; + constr1.First = geoId1; + constr1.FirstPos = Sketcher::PointPos::start; + constr1.Second = geoId2; + constr1.SecondPos = Sketcher::PointPos::end; + + Sketcher::Constraint constr2; + constr2.Type = Sketcher::ConstraintType::Tangent; + constr2.First = geoId4; + constr2.FirstPos = Sketcher::PointPos::none; + constr2.Second = geoId3; + constr2.SecondPos = Sketcher::PointPos::none; + constr2.Third = geoId1; + constr2.ThirdPos = Sketcher::PointPos::start; + + // Act + auto nullConstrAfter = getObject()->getConstraintAfterDeletingGeo(nullConstr, 5); + + // Assert + EXPECT_EQ(nullConstrAfter, nullptr); + + // Act + getObject()->changeConstraintAfterDeletingGeo(nullConstr, 5); + + // Assert + EXPECT_EQ(nullConstr, nullptr); + + // Act + // delete typical in-sketch geo + auto constr1PtrAfter1 = getObject()->getConstraintAfterDeletingGeo(&constr1, 5); + // delete external geo (negative id) + auto constr1PtrAfter2 = getObject()->getConstraintAfterDeletingGeo(&constr1, -5); + // Delete a geo involved in the constraint + auto constr1PtrAfter3 = getObject()->getConstraintAfterDeletingGeo(&constr1, 10); + + // Assert + EXPECT_EQ(constr1.Type, Sketcher::ConstraintType::Coincident); + EXPECT_EQ(constr1.First, geoId1); + EXPECT_EQ(constr1.Second, geoId2); + EXPECT_EQ(constr1PtrAfter1->First, geoId1 - 1); + EXPECT_EQ(constr1PtrAfter1->Second, geoId2 - 1); + EXPECT_EQ(constr1PtrAfter2->Third, Sketcher::GeoEnum::GeoUndef); + EXPECT_EQ(constr1PtrAfter3.get(), nullptr); + + // Act + getObject()->changeConstraintAfterDeletingGeo(&constr2, -3); + + // Assert + EXPECT_EQ(constr2.Type, Sketcher::ConstraintType::Tangent); + EXPECT_EQ(constr2.First, geoId4 + 1); + EXPECT_EQ(constr2.Second, geoId3); + EXPECT_EQ(constr2.Third, geoId1); + + // Act + // Delete a geo involved in the constraint + getObject()->changeConstraintAfterDeletingGeo(&constr2, 0); + + // Assert + EXPECT_EQ(constr2.Type, Sketcher::ConstraintType::None); +} + +TEST_F(SketchObjectTest, testDeleteExposeInternalGeometryOfEllipse) +{ + // Arrange + Part::GeomEllipse ellipse; + setupEllipse(ellipse); + int geoId = getObject()->addGeometry(&ellipse); + + // Act + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(geoId); + + // Assert + // Ensure there's only one curve + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); + + // Act + // "Expose" internal geometry + getObject()->exposeInternalGeometry(geoId); + + // Assert + // Ensure all internal geometry is satisfied + // TODO: Also try to ensure types of geometries that have this type + const auto constraints = getObject()->Constraints.getValues(); + for (auto alignmentType : {Sketcher::InternalAlignmentType::EllipseMajorDiameter, + Sketcher::InternalAlignmentType::EllipseMinorDiameter, + Sketcher::InternalAlignmentType::EllipseFocus1, + Sketcher::InternalAlignmentType::EllipseFocus2}) { + // TODO: Ensure there exists one and only one curve with this type + int numConstraintsOfThisType = + std::count_if(constraints.begin(), + constraints.end(), + [&geoId, &alignmentType](const auto* constr) { + return constr->Type == Sketcher::ConstraintType::InternalAlignment + && constr->AlignmentType == alignmentType + && constr->Second == geoId; + }); + EXPECT_EQ(numConstraintsOfThisType, 1); + } + + // Act + // Delete internal geometry (again) + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(geoId); + + // Assert + // Ensure there's only one curve + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); +} + +TEST_F(SketchObjectTest, testDeleteExposeInternalGeometryOfHyperbola) +{ + // Arrange + Part::GeomArcOfHyperbola aoh; + setupArcOfHyperbola(aoh); + int geoId = getObject()->addGeometry(&aoh); + + // Act + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(geoId); + + // Assert + // Ensure there's only one curve + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); + + // Act + // "Expose" internal geometry + getObject()->exposeInternalGeometry(geoId); + + // Assert + // Ensure all internal geometry is satisfied + // TODO: Also try to ensure types of geometries that have this type + const auto constraints = getObject()->Constraints.getValues(); + for (auto alignmentType : {Sketcher::InternalAlignmentType::HyperbolaMajor, + Sketcher::InternalAlignmentType::HyperbolaMinor, + Sketcher::InternalAlignmentType::HyperbolaFocus}) { + // TODO: Ensure there exists one and only one curve with this type + int numConstraintsOfThisType = + std::count_if(constraints.begin(), + constraints.end(), + [&geoId, &alignmentType](const auto* constr) { + return constr->Type == Sketcher::ConstraintType::InternalAlignment + && constr->AlignmentType == alignmentType + && constr->Second == geoId; + }); + EXPECT_EQ(numConstraintsOfThisType, 1); + } + + // Act + // Delete internal geometry (again) + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(geoId); + + // Assert + // Ensure there's only one curve + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); +} + +TEST_F(SketchObjectTest, testDeleteExposeInternalGeometryOfParabola) +{ + // Arrange + Part::GeomArcOfParabola aoh; + setupArcOfParabola(aoh); + int geoId = getObject()->addGeometry(&aoh); + + // Act + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(geoId); + + // Assert + // Ensure there's only one curve + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); + + // Act + // "Expose" internal geometry + getObject()->exposeInternalGeometry(geoId); + + // Assert + // Ensure all internal geometry is satisfied + // TODO: Also try to ensure types of geometries that have this type + const auto constraints = getObject()->Constraints.getValues(); + for (auto alignmentType : {Sketcher::InternalAlignmentType::ParabolaFocalAxis, + Sketcher::InternalAlignmentType::ParabolaFocus}) { + // TODO: Ensure there exists one and only one curve with this type + int numConstraintsOfThisType = + std::count_if(constraints.begin(), + constraints.end(), + [&geoId, &alignmentType](const auto* constr) { + return constr->Type == Sketcher::ConstraintType::InternalAlignment + && constr->AlignmentType == alignmentType + && constr->Second == geoId; + }); + EXPECT_EQ(numConstraintsOfThisType, 1); + } + + // Act + // Delete internal geometry (again) + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(geoId); + + // Assert + // Ensure there's only one curve + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); +} + +TEST_F(SketchObjectTest, testDeleteExposeInternalGeometryOfBSpline) +{ + // NOTE: We test only non-periodic B-spline here. Periodic B-spline should behave exactly the + // same. + + // Arrange + auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); + int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); + + // Act + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(geoId); + + // Assert + // Ensure there's only one curve + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); + + // Act + // "Expose" internal geometry + getObject()->exposeInternalGeometry(geoId); + + // Assert + // Ensure all internal geometry is satisfied + // TODO: Also try to ensure types of geometries that have this type + const auto constraints = getObject()->Constraints.getValues(); + std::map numConstraintsOfThisType; + for (auto alignmentType : {Sketcher::InternalAlignmentType::BSplineControlPoint, + Sketcher::InternalAlignmentType::BSplineKnotPoint}) { + // TODO: Ensure there exists one and only one curve with this type + numConstraintsOfThisType[alignmentType] = + std::count_if(constraints.begin(), + constraints.end(), + [&geoId, &alignmentType](const auto* constr) { + return constr->Type == Sketcher::ConstraintType::InternalAlignment + && constr->AlignmentType == alignmentType + && constr->Second == geoId; + }); + } + EXPECT_EQ(numConstraintsOfThisType[Sketcher::InternalAlignmentType::BSplineControlPoint], + nonPeriodicBSpline->countPoles()); + EXPECT_EQ(numConstraintsOfThisType[Sketcher::InternalAlignmentType::BSplineKnotPoint], + nonPeriodicBSpline->countKnots()); + + // Act + // Delete internal geometry (again) + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(geoId); + + // Assert + // Ensure there's only one curve + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); +} + +// TODO: Needs to be done for other curves too but currently they are working as intended +TEST_F(SketchObjectTest, testDeleteOnlyUnusedInternalGeometryOfBSpline) +{ + // NOTE: We test only non-periodic B-spline here. Periodic B-spline should behave exactly the + // same. + + // Arrange + auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); + int geoIdBsp = getObject()->addGeometry(nonPeriodicBSpline.get()); + // Ensure "exposed" internal geometry + getObject()->exposeInternalGeometry(geoIdBsp); + Base::Vector3d coords(1.0, 1.0, 0.0); + Part::GeomPoint point(coords); + int geoIdPnt = getObject()->addGeometry(&point); + const auto constraints = getObject()->Constraints.getValues(); + auto it = std::find_if(constraints.begin(), constraints.end(), [&geoIdBsp](const auto* constr) { + return constr->Type == Sketcher::ConstraintType::InternalAlignment + && constr->AlignmentType == Sketcher::InternalAlignmentType::BSplineControlPoint + && constr->Second == geoIdBsp && constr->InternalAlignmentIndex == 1; + }); + // One Assert to avoid + EXPECT_NE(it, constraints.end()); + auto constraint = new Sketcher::Constraint(); // Ownership will be transferred to the sketch + constraint->Type = Sketcher::ConstraintType::Coincident; + constraint->First = geoIdPnt; + constraint->FirstPos = Sketcher::PointPos::start; + constraint->Second = (*it)->First; + constraint->SecondPos = Sketcher::PointPos::mid; + getObject()->addConstraint(constraint); + + // Act + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(geoIdBsp); + + // Assert + // Ensure there are 3 curves: the B-spline, its pole, and the point coincident on the pole + EXPECT_EQ(getObject()->getHighestCurveIndex(), 2); +} + TEST_F(SketchObjectTest, testSplitLineSegment) { // Arrange @@ -1143,6 +1451,329 @@ TEST_F(SketchObjectTest, testTrimNonPeriodicBSplineMid) // TODO: Ensure shape is preserved } +TEST_F(SketchObjectTest, testModifyKnotMultInNonPeriodicBSplineToZero) +{ + // Arrange + auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); + assert(nonPeriodicBSpline); + int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); + auto bsp1 = static_cast(getObject()->getGeometry(geoId)); + int oldKnotCount = bsp1->countKnots(); + + // Act + // Try decreasing mult to zero. + // NOTE: we still use OCCT notation of knot index starting with 1 (not 0). + getObject()->modifyBSplineKnotMultiplicity(geoId, 2, -1); + // Assert + // Knot should disappear. We start with 3 (unique) knots, so expect 2. + auto bsp2 = static_cast(getObject()->getGeometry(geoId)); + EXPECT_EQ(bsp2->countKnots(), oldKnotCount - 1); +} + +TEST_F(SketchObjectTest, testModifyKnotMultInNonPeriodicBSplineToDisallowed) +{ + // Arrange + auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); + assert(nonPeriodicBSpline); + int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); + + // Act and Assert + // TODO: Try modifying such that resultant multiplicity > degree + // TODO: This should immediately throw exception + EXPECT_THROW(getObject()->modifyBSplineKnotMultiplicity(geoId, 2, 3), Base::ValueError); + // TODO: Try modifying such that resultant multiplicity < 0 + // TODO: This should immediately throw exception + EXPECT_THROW(getObject()->modifyBSplineKnotMultiplicity(geoId, 2, -2), Base::ValueError); +} + +TEST_F(SketchObjectTest, testModifyKnotMultInNonPeriodicBSpline) +{ + // Arrange + auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); + assert(nonPeriodicBSpline); + int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); + + auto bsp = static_cast(getObject()->getGeometry(geoId)); + int oldKnotsNum = bsp->countKnots(); + int oldMultiplicityOfTargetKnot = bsp->getMultiplicities()[1]; + + // Act + // TODO: Increase/decrease knot multiplicity normally + getObject()->modifyBSplineKnotMultiplicity(geoId, 2, 1); + // Assert + // This should not alter the sizes of knot and multiplicity vectors. + bsp = static_cast(getObject()->getGeometry(geoId)); + EXPECT_EQ(bsp->countKnots(), oldKnotsNum); + // This should increment the multiplicity. + EXPECT_EQ(bsp->getMultiplicities()[1], oldMultiplicityOfTargetKnot + 1); + // This should still be a non-periodic spline + EXPECT_FALSE(bsp->isPeriodic()); + // TODO: Expect shape is preserved + + // Act + // TODO: Increase/decrease knot multiplicity normally + getObject()->modifyBSplineKnotMultiplicity(geoId, 2, -1); + // Assert + // This should not alter the sizes of knot and multiplicity vectors. + bsp = static_cast(getObject()->getGeometry(geoId)); + EXPECT_EQ(bsp->countKnots(), oldKnotsNum); + // This should increment the multiplicity. + EXPECT_EQ(bsp->getMultiplicities()[1], oldMultiplicityOfTargetKnot); + // This should still be a non-periodic spline + EXPECT_FALSE(bsp->isPeriodic()); +} + +TEST_F(SketchObjectTest, testModifyKnotMultInPeriodicBSplineToZero) +{ + // Arrange + auto PeriodicBSpline = createTypicalPeriodicBSpline(); + assert(PeriodicBSpline); + int geoId = getObject()->addGeometry(PeriodicBSpline.get()); + auto bsp1 = static_cast(getObject()->getGeometry(geoId)); + int oldKnotCount = bsp1->countKnots(); + + // Act + // Try decreasing mult to zero. + // NOTE: we still use OCCT notation of knot index starting with 1 (not 0). + getObject()->modifyBSplineKnotMultiplicity(geoId, 2, -1); + // Assert + // Knot should disappear. + auto bsp2 = static_cast(getObject()->getGeometry(geoId)); + EXPECT_EQ(bsp2->countKnots(), oldKnotCount - 1); +} + +TEST_F(SketchObjectTest, testModifyKnotMultInPeriodicBSplineToDisallowed) +{ + // Arrange + auto PeriodicBSpline = createTypicalPeriodicBSpline(); + assert(PeriodicBSpline); + int geoId = getObject()->addGeometry(PeriodicBSpline.get()); + + // Act and Assert + // TODO: Try modifying such that resultant multiplicity > degree + // TODO: This should immediately throw exception + EXPECT_THROW(getObject()->modifyBSplineKnotMultiplicity(geoId, 2, 3), Base::ValueError); + // TODO: Try modifying such that resultant multiplicity < 0 + // TODO: This should immediately throw exception + EXPECT_THROW(getObject()->modifyBSplineKnotMultiplicity(geoId, 2, -2), Base::ValueError); +} + +TEST_F(SketchObjectTest, testModifyKnotMultInPeriodicBSpline) +{ + // Arrange + auto PeriodicBSpline = createTypicalPeriodicBSpline(); + assert(PeriodicBSpline); + int geoId = getObject()->addGeometry(PeriodicBSpline.get()); + + auto bsp = static_cast(getObject()->getGeometry(geoId)); + int oldKnotsNum = bsp->countKnots(); + int oldMultiplicityOfTargetKnot = bsp->getMultiplicities()[1]; + + // Act + // TODO: Increase/decrease knot multiplicity normally + getObject()->modifyBSplineKnotMultiplicity(geoId, 2, 1); + // Assert + // This should not alter the sizes of knot and multiplicity vectors. + bsp = static_cast(getObject()->getGeometry(geoId)); + EXPECT_EQ(bsp->countKnots(), oldKnotsNum); + // This should increment the multiplicity. + EXPECT_EQ(bsp->getMultiplicities()[1], oldMultiplicityOfTargetKnot + 1); + // This should still be a periodic spline + EXPECT_TRUE(bsp->isPeriodic()); + // TODO: Expect shape is preserved + + // Act + // TODO: Increase/decrease knot multiplicity normally + getObject()->modifyBSplineKnotMultiplicity(geoId, 2, -1); + // Assert + // This should not alter the sizes of knot and multiplicity vectors. + bsp = static_cast(getObject()->getGeometry(geoId)); + EXPECT_EQ(bsp->countKnots(), oldKnotsNum); + // This should decrement the multiplicity. + EXPECT_EQ(bsp->getMultiplicities()[1], oldMultiplicityOfTargetKnot); + // This should still be a non-periodic spline + EXPECT_TRUE(bsp->isPeriodic()); +} + +TEST_F(SketchObjectTest, testInsertKnotInNonPeriodicBSpline) +{ + // Arrange + auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); + assert(nonPeriodicBSpline); + int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); + + // Act and Assert + // Try inserting knot with zero multiplicity + // zero multiplicity knot should immediately throw exception + EXPECT_THROW(getObject()->insertBSplineKnot(geoId, 0.5, 0), Base::ValueError); + + // Act and Assert + // Try inserting knot with multiplicity > degree + // This should immediately throw exception + EXPECT_THROW(getObject()->insertBSplineKnot(geoId, 0.5, 4), Base::ValueError); + + // Act and Assert + // TODO: Try inserting at an existing knot with resultant multiplicity > degree + // TODO: This should immediately throw exception + // FIXME: Not happening. May be ignoring existing values. + // EXPECT_THROW(getObject()->insertBSplineKnot(geoId, 1.0, 3), Base::ValueError); + + auto bsp = static_cast(getObject()->getGeometry(geoId)); + int oldKnotsNum = bsp->countKnots(); + int oldMultiplicityOfTargetKnot = bsp->getMultiplicities()[1]; + + // Act + // Add at a general position (where no knot exists) + getObject()->insertBSplineKnot(geoId, 0.5, 1); + // Assert + // This should add to both the knot and multiplicity "vectors" + bsp = static_cast(getObject()->getGeometry(geoId)); + EXPECT_EQ(bsp->countKnots(), oldKnotsNum + 1); + // This should still be a non-periodic spline + EXPECT_FALSE(bsp->isPeriodic()); + + // Act + // Add a knot at an existing knot + getObject()->insertBSplineKnot(geoId, 1.0, 1); + // Assert + // This should not alter the sizes of knot and multiplicity vectors. + // (Since we previously added a knot, this means the total is still one more than original) + bsp = static_cast(getObject()->getGeometry(geoId)); + EXPECT_EQ(bsp->countKnots(), oldKnotsNum + 1); + // This should increment the multiplicity. + EXPECT_EQ(bsp->getMultiplicities()[2], oldMultiplicityOfTargetKnot + 1); + // This should still be a non-periodic spline + EXPECT_FALSE(bsp->isPeriodic()); +} + +TEST_F(SketchObjectTest, testInsertKnotInPeriodicBSpline) +{ + // This should also cover as a representative of arc of conic + + // Arrange + auto PeriodicBSpline = createTypicalPeriodicBSpline(); + assert(PeriodicBSpline); + int geoId = getObject()->addGeometry(PeriodicBSpline.get()); + + // Act and Assert + // Try inserting knot with zero multiplicity + // zero multiplicity knot should immediately throw exception + EXPECT_THROW(getObject()->insertBSplineKnot(geoId, 0.5, 0), Base::ValueError); + + // Act and Assert + // Try inserting knot with multiplicity > degree + // This should immediately throw exception + EXPECT_THROW(getObject()->insertBSplineKnot(geoId, 0.5, 4), Base::ValueError); + + // Act and Assert + // TODO: Try inserting at an existing knot with resultant multiplicity > degree + // TODO: This should immediately throw exception + + auto bsp = static_cast(getObject()->getGeometry(geoId)); + int oldKnotsNum = bsp->countKnots(); + int oldMultiplicityOfTargetKnot = bsp->getMultiplicities()[2]; + + // Act + // Add at a general position (where no knot exists) + getObject()->insertBSplineKnot(geoId, 0.5, 1); + // Assert + // This should add to both the knot and multiplicity "vectors" + bsp = static_cast(getObject()->getGeometry(geoId)); + EXPECT_EQ(bsp->countKnots(), oldKnotsNum + 1); + // This should still be a periodic spline + EXPECT_TRUE(bsp->isPeriodic()); + + // Act + // Add a knot at an existing knot + getObject()->insertBSplineKnot(geoId, 1.0, 1); + // Assert + // This should not alter the sizes of knot and multiplicity vectors. + bsp = static_cast(getObject()->getGeometry(geoId)); + EXPECT_EQ(bsp->countKnots(), oldKnotsNum + 1); + // This should increment the multiplicity. + EXPECT_EQ(bsp->getMultiplicities()[3], oldMultiplicityOfTargetKnot + 1); + // This should still be a periodic spline + EXPECT_TRUE(bsp->isPeriodic()); +} + +TEST_F(SketchObjectTest, testJoinCurves) +{ + // Arrange + // Make two curves + Base::Vector3d coordsCenter(0.0, 0.0, 0.0); + double radius = 3.0, startParam = M_PI / 2, endParam = M_PI; + Part::GeomArcOfCircle arcOfCircle; + arcOfCircle.setCenter(coordsCenter); + arcOfCircle.setRadius(radius); + arcOfCircle.setRange(startParam, endParam, true); + int geoId1 = getObject()->addGeometry(&arcOfCircle); + + Base::Vector3d coords1(0.1, 0.0, 0.0); + Base::Vector3d coords2(3.0, 4.0, 0.0); + Part::GeomLineSegment lineSeg; + lineSeg.setPoints(coords1, coords2); + int geoId2 = getObject()->addGeometry(&lineSeg); + + // Act + // Join these curves + getObject()->join(geoId1, Sketcher::PointPos::start, geoId2, Sketcher::PointPos::start); + + // Assert + // Check they are replaced (here it means there is only one curve left after internal + // geometries are removed) + for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); + } + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); +} + +TEST_F(SketchObjectTest, testJoinCurvesWhenTangent) +{ + // Arrange + // Make two curves + Base::Vector3d coordsCenter(0.0, 0.0, 0.0); + double radius = 3.0, startParam = M_PI / 2, endParam = M_PI; + Part::GeomArcOfCircle arcOfCircle; + arcOfCircle.setCenter(coordsCenter); + arcOfCircle.setRadius(radius); + arcOfCircle.setRange(startParam, endParam, true); + int geoId1 = getObject()->addGeometry(&arcOfCircle); + + Base::Vector3d coords1(0.0, 0.0, 0.0); + Base::Vector3d coords2(3.0, 0.0, 0.0); + Part::GeomLineSegment lineSeg; + lineSeg.setPoints(coords1, coords2); + int geoId2 = getObject()->addGeometry(&lineSeg); + + // Add end-to-end tangent between these + auto constraint = new Sketcher::Constraint(); // Ownership will be transferred to the sketch + constraint->Type = Sketcher::ConstraintType::Tangent; + constraint->First = geoId1; + constraint->FirstPos = Sketcher::PointPos::start; + constraint->Second = geoId2; + constraint->SecondPos = Sketcher::PointPos::start; + getObject()->addConstraint(constraint); + + // Act + // Join these curves + getObject()->join(geoId1, Sketcher::PointPos::start, geoId2, Sketcher::PointPos::start, 1); + + // Assert + // Check they are replaced (here it means there is only one curve left after internal + // geometries are removed) + for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); + } + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); + // TODO: Check the shape is conserved (how?) + // Check there is no C-0 knot (should be possible for the chosen example) + auto mults = static_cast(getObject()->getGeometry(0)) + ->getMultiplicities(); + EXPECT_TRUE(std::all_of(mults.begin(), mults.end(), [](auto mult) { + return mult >= 1; + })); +} + TEST_F(SketchObjectTest, testReverseAngleConstraintToSupplementaryExpressionNoUnits1) { std::string expr = Sketcher::SketchObject::reverseAngleConstraintExpression("180 - 60");