From 77bb7ab2d4ac802c922bab57f66eb8dbdfc1fe97 Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Tue, 31 Dec 2024 18:10:25 +0530 Subject: [PATCH 01/10] [Sketcher] Move internal functions out of `trim` Towards making it a function class or similar. --- src/Mod/Sketcher/App/SketchObject.cpp | 224 +++++++++++++------------- 1 file changed, 116 insertions(+), 108 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 9b3c703250..c985629230 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -3551,6 +3551,111 @@ bool getIntersectionParameter(const Part::Geometry* geo, return true; } +bool arePointsWithinPrecision(const Base::Vector3d& point1, const 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()); +} + +bool 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()); +} + +// returns true if the point defined by (GeoId1, pos1) can be considered to be coincident with +// point. +bool isPointAtPosition(const SketchObject* obj, int GeoId1, PointPos pos1, const Base::Vector3d& point) +{ + Base::Vector3d pp = obj->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. +// Returns: +// - The constraint that should be used to constraint GeoId and cuttingGeoId +std::unique_ptr transformPreexistingConstraintForTrim(const SketchObject* obj, + const Constraint* constr, + int GeoId, + int cuttingGeoId, + const 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. + */ + 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(obj, 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; +} + +std::unique_ptr getNewConstraintAtTrimCut(const SketchObject* obj, + int cuttingGeoId, + int cutGeoId, + PointPos cutPosId, + const Base::Vector3d& cutPointVec) +{ + auto newConstr = std::make_unique(); + newConstr->First = cutGeoId; + newConstr->FirstPos = cutPosId; + newConstr->Second = cuttingGeoId; + if (isPointAtPosition(obj, cuttingGeoId, PointPos::start, cutPointVec)) { + newConstr->Type = Sketcher::Coincident; + newConstr->SecondPos = PointPos::start; + } + else if (isPointAtPosition(obj, cuttingGeoId, PointPos::end, cutPointVec)) { + 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; + } + return newConstr; +}; + int SketchObject::trim(int GeoId, const Base::Vector3d& point) { // no need to check input data validity as this is an sketchobject managed operation. @@ -3571,83 +3676,6 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) //******************* Lambdas - common functions for different intersections //****************************************// - 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()); - }; - - 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()); - }; - - // 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. - // 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. - */ - 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; - }; - // makes an equality constraint between GeoId1 and GeoId2 auto constrainAsEqual = [this](int GeoId1, int GeoId2) { auto newConstr = std::make_unique(); @@ -3792,7 +3820,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) transferConstraints(GeoId, PointPos::mid, newIds.front(), PointPos::mid, true); // Make centers coincident if (newIds.size() > 1) { - Constraint* joint = new Constraint(); + auto* joint = new Constraint(); joint->Type = Coincident; joint->First = newIds.front(); joint->FirstPos = PointPos::mid; @@ -3829,14 +3857,14 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) geoIdsToBeDeleted.insert(con->First); continue; } - if (auto newConstr = transformPreexistingConstraint( - con, GeoId, cuttingGeoIds[0], cutPoints[0], newIds.front(), PointPos::end)) { + if (auto newConstr = transformPreexistingConstraintForTrim( + this, con, GeoId, cuttingGeoIds[0], cutPoints[0], newIds.front(), PointPos::end)) { newConstraints.push_back(newConstr.release()); isPoint1ConstrainedOnGeoId1 = true; continue; } - if (auto newConstr = transformPreexistingConstraint( - con, GeoId, cuttingGeoIds[1], cutPoints[1], newIds.back(), PointPos::start)) { + if (auto newConstr = transformPreexistingConstraintForTrim( + this, con, GeoId, cuttingGeoIds[1], cutPoints[1], newIds.back(), PointPos::start)) { newConstraints.push_back(newConstr.release()); isPoint2ConstrainedOnGeoId2 = true; continue; @@ -3855,36 +3883,16 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) // to coincident or end-to-end tangency/perpendicularity. // TODO: Tangent/perpendicular not yet covered - auto addNewConstraintAtCut = [&isPointAtPosition, &newConstraints](int cuttingGeoId, - int cutGeoId, - PointPos cutPosId, - Base::Vector3d cutPointVec) { - Constraint* newConstr = new Constraint(); - 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(cuttingGeoId, PointPos::end, cutPointVec)) { - 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[0] != GeoEnum::GeoUndef && !isPoint1ConstrainedOnGeoId1) { - addNewConstraintAtCut(cuttingGeoIds[0], newIds.front(), PointPos::end, cutPoints[0]); + newConstraints.emplace_back( + getNewConstraintAtTrimCut( + this, cuttingGeoIds[0], newIds.front(), PointPos::end, cutPoints[0]).release()); } if (cuttingGeoIds[1] != GeoEnum::GeoUndef && !isPoint2ConstrainedOnGeoId2) { - addNewConstraintAtCut(cuttingGeoIds[1], newIds.back(), PointPos::start, cutPoints[1]); + newConstraints.emplace_back( + getNewConstraintAtTrimCut( + this, cuttingGeoIds[1], newIds.back(), PointPos::start, cutPoints[1]).release()); } // Workaround to avoid https://github.com/FreeCAD/FreeCAD/issues/15484. From c8c8c2c20812f1ca39ddc95a09d39e8ea5b3c4bd Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Wed, 1 Jan 2025 13:01:56 +0530 Subject: [PATCH 02/10] [Sketcher] Move `trim` sub-steps to different functions --- src/Mod/Sketcher/App/SketchObject.cpp | 443 +++++++++++++++----------- src/Mod/Sketcher/App/SketchObject.h | 4 +- 2 files changed, 258 insertions(+), 189 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index c985629230..38e43c747f 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -3656,38 +3656,164 @@ std::unique_ptr getNewConstraintAtTrimCut(const SketchObject* obj, return newConstr; }; +bool isGeoIdAllowedForTrim(const SketchObject* obj, int GeoId) +{ + const auto* geo = obj->getGeometry(GeoId); + + return GeoId >= 0 && GeoId <= obj->getHighestCurveIndex() + && GeometryFacade::isInternalType(geo, InternalType::None); +} + +bool getParamLimitsOfNewGeosForTrim(const SketchObject* obj, + int GeoId, + std::array& cuttingGeoIds, + std::array& cutPoints, + std::vector>& paramsOfNewGeos) +{ + const auto* geoAsCurve = static_cast(obj->getGeometry(GeoId)); + double firstParam = geoAsCurve->getFirstParameter(); + double lastParam = geoAsCurve->getLastParameter(); + double cut0Param {firstParam}, cut1Param {lastParam}; + + bool allParamsFound = getIntersectionParameter(geoAsCurve, cutPoints[0], cut0Param) + && getIntersectionParameter(geoAsCurve, cutPoints[1], cut1Param); + if (!allParamsFound) { + return false; + } + + if (!obj->isClosedCurve(geoAsCurve) && areParamsWithinApproximation(firstParam, cut0Param)) { + cuttingGeoIds[0] = GeoEnum::GeoUndef; + } + + if (!obj->isClosedCurve(geoAsCurve) && 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. + paramsOfNewGeos.clear(); + return true; + } + + paramsOfNewGeos.assign(2 - numUndefs, {firstParam, lastParam}); + + if (paramsOfNewGeos.empty()) { + return true; + } + + if (obj->isClosedCurve(geoAsCurve)) { + paramsOfNewGeos.pop_back(); + } + + if (cuttingGeoIds[0] != GeoEnum::GeoUndef) { + paramsOfNewGeos.front().second = cut0Param; + } + if (cuttingGeoIds[1] != GeoEnum::GeoUndef) { + paramsOfNewGeos.back().first = cut1Param; + } + + return true; +} + +void createArcsFromGeoWithLimits(const Part::GeomCurve* geo, + const std::vector>& paramsOfNewGeos, + std::vector>& newGeos) +{ + for (auto& [u1, u2] : paramsOfNewGeos) { + auto newGeo = static_cast(geo)->createArc(u1, u2); + assert(newGeo); + newGeos.emplace_back(newGeo); + } +} + +void createNewConstraintsForTrim(const SketchObject* obj, + const int GeoId, + const std::array& cuttingGeoIds, + const std::array& cutPoints, + const std::vector& newIds, + std::vector& idsOfOldConstraints, + std::vector& newConstraints, + std::set>& geoIdsToBeDeleted) +{ + const auto& allConstraints = obj->Constraints.getValues(); + + bool isPoint1ConstrainedOnGeoId1 = false; + bool isPoint2ConstrainedOnGeoId2 = false; + + for (const auto& oldConstrId : idsOfOldConstraints) { + // trim-specific changes first + const Constraint* con = allConstraints[oldConstrId]; + if (con->Type == InternalAlignment) { + geoIdsToBeDeleted.insert(con->First); + continue; + } + if (auto newConstr = transformPreexistingConstraintForTrim(obj, + con, + GeoId, + cuttingGeoIds[0], + cutPoints[0], + newIds.front(), + PointPos::end)) { + newConstraints.push_back(newConstr.release()); + isPoint1ConstrainedOnGeoId1 = true; + continue; + } + if (auto newConstr = transformPreexistingConstraintForTrim(obj, + 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 + obj->deriveConstraintsForPieces(GeoId, newIds, con, newConstraints); + } + + // Add point-on-object/coincidence constraints with the newly exposed points. + // This will need to account for the constraints that were already converted + // to coincident or end-to-end tangency/perpendicularity. + // TODO: Tangent/perpendicular not yet covered + + if (cuttingGeoIds[0] != GeoEnum::GeoUndef && !isPoint1ConstrainedOnGeoId1) { + newConstraints.emplace_back(getNewConstraintAtTrimCut(obj, + cuttingGeoIds[0], + newIds.front(), + PointPos::end, + cutPoints[0]) + .release()); + } + + if (cuttingGeoIds[1] != GeoEnum::GeoUndef && !isPoint2ConstrainedOnGeoId2) { + newConstraints.emplace_back(getNewConstraintAtTrimCut(obj, + cuttingGeoIds[1], + newIds.back(), + PointPos::start, + cutPoints[1]) + .release()); + } +} + int SketchObject::trim(int GeoId, const Base::Vector3d& point) { - // no need to check input data validity as this is an sketchobject managed operation. - Base::StateLocker lock(managedoperation, true); - - //******************* Basic checks rejecting the operation - //****************************************// - if (GeoId < 0 || GeoId > getHighestCurveIndex()) { + if (!isGeoIdAllowedForTrim(this, GeoId)) { return -1; } - - auto geo = getGeometry(GeoId); - - if (!GeometryFacade::isInternalType(geo, InternalType::None)) { - return -1; // internal alignment geometry is not trimmable - } - - //******************* Lambdas - common functions for different intersections - //****************************************// - - // makes an equality constraint between GeoId1 and GeoId2 - auto constrainAsEqual = [this](int GeoId1, int GeoId2) { - auto newConstr = std::make_unique(); - - // Build Constraints associated with new pair of arcs - newConstr->Type = Sketcher::Equal; - newConstr->First = GeoId1; - newConstr->FirstPos = Sketcher::PointPos::none; - newConstr->Second = GeoId2; - newConstr->SecondPos = Sketcher::PointPos::none; - addConstraint(std::move(newConstr)); - }; + // Remove internal geometry beforehand for now + // FIXME: we should be able to transfer these to new curves smoothly + deleteUnusedInternalGeometryAndUpdateGeoId(GeoId); + // auto geo = getGeometry(GeoId); + const auto* geoAsCurve = static_cast(getGeometry(GeoId)); //******************* Step A => Detection of intersection - Common to all Geometries //****************************************// @@ -3717,105 +3843,118 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) return 0; } - const auto* geoAsCurve = static_cast(geo); - double firstParam = geoAsCurve->getFirstParameter(); - double lastParam = geoAsCurve->getLastParameter(); - double pointParam, cut0Param{firstParam}, cut1Param{lastParam}; - bool allParamsFound = getIntersectionParameter(geo, point, pointParam) - && getIntersectionParameter(geo, cutPoints[0], cut0Param) - && getIntersectionParameter(geo, cutPoints[1], cut1Param); - if (!allParamsFound) { + // TODO: find trim parameters + std::vector> paramsOfNewGeos; + paramsOfNewGeos.reserve(2); + if (!getParamLimitsOfNewGeosForTrim(this, GeoId, cuttingGeoIds, cutPoints, paramsOfNewGeos)) { return -1; } - 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}); + //******************* Step B => Creation of new geometries + //****************************************// std::vector newIds; - std::vector newGeos; + std::vector> newGeos; std::vector newGeosAsConsts; - // TODO: This should be needed, but for some reason we already get both curves as construction - // when needed. - - // bool oldGeoIsConstruction = - // GeometryFacade::getConstruction(static_cast(geo)); - - if (isClosedCurve(geo) && !paramsOfNewGeos.empty()) { - startPointRemains = false; - endPointRemains = false; - paramsOfNewGeos.pop_back(); - } switch (paramsOfNewGeos.size()) { - case 0: { - delGeometry(GeoId); - return 0; - } - case 1: { - newIds.push_back(GeoId); - break; - } - case 2: { - newIds.push_back(GeoId); - newIds.push_back(getHighestCurveIndex() + 1); - break; - } - default: { - return -1; - } + case 0: { + delGeometry(GeoId); + return 0; + } + case 1: { + newIds.push_back(GeoId); + break; + } + case 2: { + newIds.push_back(GeoId); + newIds.push_back(getHighestCurveIndex() + 1); + break; + } + default: { + 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); + createArcsFromGeoWithLimits(geoAsCurve, paramsOfNewGeos, newGeos); + for (const auto& geo : newGeos) { + newGeosAsConsts.push_back(geo.get()); } + //******************* Step C => Creation of new constraints + //****************************************// // 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`. + const auto& allConstraints = this->Constraints.getValues(); std::vector newConstraints; + std::vector idsOfOldConstraints; + std::set> geoIdsToBeDeleted; + getConstraintIndices(GeoId, idsOfOldConstraints); + // remove the constraints that we want to manually transfer + // We could transfer beforehand but in case of exception that transfer is permanent + if (!isClosedCurve(geoAsCurve)) { + if (cuttingGeoIds[0] != GeoEnum::GeoUndef) { + idsOfOldConstraints.erase( + std::remove_if( + idsOfOldConstraints.begin(), + idsOfOldConstraints.end(), + [this, &GeoId, &allConstraints](const auto& i) { + return (allConstraints[i]->involvesGeoIdAndPosId(GeoId, PointPos::start)); + }), + idsOfOldConstraints.end()); + } + if (cuttingGeoIds[1] != GeoEnum::GeoUndef) { + idsOfOldConstraints.erase( + std::remove_if(idsOfOldConstraints.begin(), + idsOfOldConstraints.end(), + [this, &GeoId, &allConstraints](const auto& i) { + return (allConstraints[i]->involvesGeoIdAndPosId(GeoId, + PointPos::end)); + }), + idsOfOldConstraints.end()); + } + } + idsOfOldConstraints.erase( + std::remove_if(idsOfOldConstraints.begin(), + idsOfOldConstraints.end(), + [this, &GeoId, &allConstraints](const auto& i) { + return (allConstraints[i]->involvesGeoIdAndPosId(GeoId, PointPos::mid)); + }), + idsOfOldConstraints.end()); - // A geoId we expect to never exist during this operation (but still not `GeoEnum::GeoUndef`). - // We use it because newIds.front() is supposed to be `GeoId`, but we will delete constraints - // on it down the line, even if we may need it later. - // Note that this will cause some malformed constraints until they are transferred back. - int nonexistentGeoId = getHighestCurveIndex() + newIds.size(); - newIds.front() = nonexistentGeoId; + createNewConstraintsForTrim(this, + GeoId, + cuttingGeoIds, + cutPoints, + newIds, + idsOfOldConstraints, + newConstraints, + geoIdsToBeDeleted); + + //******************* Step D => Replacing geometries and constraints + //****************************************// // Constraints related to start/mid/end points of original - if (startPointRemains) { + auto constrainAsEqual = [this](int GeoId1, int GeoId2) { + auto newConstr = std::make_unique(); + + // Build Constraints associated with new pair of arcs + newConstr->Type = Sketcher::Equal; + newConstr->First = GeoId1; + newConstr->FirstPos = Sketcher::PointPos::none; + newConstr->Second = GeoId2; + newConstr->SecondPos = Sketcher::PointPos::none; + addConstraint(std::move(newConstr)); + }; + + delConstraints(idsOfOldConstraints); + + if (isClosedCurve(geoAsCurve)) { transferConstraints(GeoId, PointPos::start, newIds.front(), PointPos::start, true); - } - if (endPointRemains) { transferConstraints(GeoId, PointPos::end, newIds.back(), PointPos::end, true); } - bool geomHasMid = - geo->isDerivedFrom() || geo->isDerivedFrom(); + bool geomHasMid = geoAsCurve->isDerivedFrom() + || geoAsCurve->isDerivedFrom(); if (geomHasMid) { transferConstraints(GeoId, PointPos::mid, newIds.front(), PointPos::mid, true); // Make centers coincident @@ -3834,92 +3973,19 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) } } - newIds.front() = GeoId; - - std::vector idsOfOldConstraints; - getConstraintIndices(GeoId, idsOfOldConstraints); - - const auto& allConstraints = this->Constraints.getValues(); - - bool isPoint1ConstrainedOnGeoId1 = false; - bool isPoint2ConstrainedOnGeoId2 = false; - - // 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> geoIdsToBeDeleted; - // geoIdsToBeDeleted.push_back(GeoId); - - for (const auto& oldConstrId : idsOfOldConstraints) { - // trim-specific changes first - const Constraint* con = allConstraints[oldConstrId]; - if (con->Type == InternalAlignment) { - geoIdsToBeDeleted.insert(con->First); - continue; - } - if (auto newConstr = transformPreexistingConstraintForTrim( - this, con, GeoId, cuttingGeoIds[0], cutPoints[0], newIds.front(), PointPos::end)) { - newConstraints.push_back(newConstr.release()); - isPoint1ConstrainedOnGeoId1 = true; - continue; - } - if (auto newConstr = transformPreexistingConstraintForTrim( - this, 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. - // This will need to account for the constraints that were already converted - // to coincident or end-to-end tangency/perpendicularity. - // TODO: Tangent/perpendicular not yet covered - - if (cuttingGeoIds[0] != GeoEnum::GeoUndef && !isPoint1ConstrainedOnGeoId1) { - newConstraints.emplace_back( - getNewConstraintAtTrimCut( - this, cuttingGeoIds[0], newIds.front(), PointPos::end, cutPoints[0]).release()); - } - - if (cuttingGeoIds[1] != GeoEnum::GeoUndef && !isPoint2ConstrainedOnGeoId2) { - newConstraints.emplace_back( - getNewConstraintAtTrimCut( - this, cuttingGeoIds[1], newIds.back(), PointPos::start, cutPoints[1]).release()); - } - - // Workaround to avoid https://github.com/FreeCAD/FreeCAD/issues/15484. - // Delete the geometry first to avoid it happening. - // Of course, this will change `newIds`, and that's why we offset them. - delConstraints(idsOfOldConstraints); - - // Retransfer constraints we put away to `nonexistentGeoId` - transferConstraints(nonexistentGeoId, PointPos::start, GeoId, PointPos::start, true); - transferConstraints(nonexistentGeoId, PointPos::end, GeoId, PointPos::end, true); - transferConstraints(nonexistentGeoId, PointPos::mid, GeoId, PointPos::mid, true); - + // TODO: Implement this + // replaceGeometries({GeoId}, newGeos); auto vals = getInternalGeometry(); auto newVals(vals); - GeometryFacade::copyId(geo, newGeos.front()); - newVals[GeoId] = newGeos.front(); + GeometryFacade::copyId(geoAsCurve, newGeos.front().get()); + newVals[GeoId] = newGeos.front().release(); if (newGeos.size() > 1) { - newVals.push_back(newGeos.back()); - generateId(newGeos.back()); + newVals.push_back(newGeos.back().get()); + generateId(newGeos.back().release()); newIds.back() = newVals.size() - 1; } 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. - // This somehow is still not needed (why?) - if (noRecomputes) { solve(); } @@ -3942,6 +4008,9 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) solve(); } + //******************* Cleanup + //****************************************// + // Since we used regular "non-smart" pointers, we have to handle cleanup for (auto& cons : newConstraints) { delete cons; @@ -3953,7 +4022,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) bool SketchObject::deriveConstraintsForPieces(const int oldId, const std::vector& newIds, const Constraint* con, - std::vector& newConstraints) + std::vector& newConstraints) const { std::vector newGeos; for (auto& newId: newIds) { @@ -3967,7 +4036,7 @@ bool SketchObject::deriveConstraintsForPieces(const int oldId, const std::vector& newIds, const std::vector& newGeos, const Constraint* con, - std::vector& newConstraints) + std::vector& newConstraints) const { const Part::Geometry* geo = getGeometry(oldId); int conId = con->First; diff --git a/src/Mod/Sketcher/App/SketchObject.h b/src/Mod/Sketcher/App/SketchObject.h index e586837318..05a484c7be 100644 --- a/src/Mod/Sketcher/App/SketchObject.h +++ b/src/Mod/Sketcher/App/SketchObject.h @@ -422,13 +422,13 @@ public: bool deriveConstraintsForPieces(const int oldId, const std::vector& newIds, const Constraint* con, - std::vector& newConstraints); + std::vector& newConstraints) const; // Explicitly giving `newGeos` for cases where they are not yet added bool deriveConstraintsForPieces(const int oldId, const std::vector& newIds, const std::vector& newGeo, const Constraint* con, - std::vector& newConstraints); + std::vector& newConstraints) const; /// split a curve int split(int geoId, const Base::Vector3d& point); From 909e5bfafb8ffdfe8594b8abe04ccab5a62fdacb Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Thu, 2 Jan 2025 10:28:44 +0530 Subject: [PATCH 03/10] [Sketcher] Use `replaceGeometries()` in `trim` --- src/Mod/Sketcher/App/SketchObject.cpp | 119 ++++++++++++++------------ 1 file changed, 63 insertions(+), 56 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 38e43c747f..7f2c6757ee 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -3506,24 +3506,35 @@ void SketchObject::changeConstraintAfterDeletingGeo(Constraint* constr, } } -bool SketchObject::seekTrimPoints(int GeoId, const Base::Vector3d& point, int& GeoId1, - Base::Vector3d& intersect1, int& GeoId2, +// clang-format on +bool SketchObject::seekTrimPoints(int GeoId, + const Base::Vector3d& point, + int& GeoId1, + Base::Vector3d& intersect1, + int& GeoId2, Base::Vector3d& intersect2) { - if (GeoId < 0 || GeoId > getHighestCurveIndex()) + if (GeoId < 0 || GeoId > getHighestCurveIndex()) { return false; + } - auto geos = getCompleteGeometry();// this includes the axes too + auto geos = getCompleteGeometry(); // this includes the axes too - geos.resize(geos.size() - 2);// remove the axes to avoid intersections with the axes + geos.resize(geos.size() - 2); // remove the axes to avoid intersections with the axes int localindex1, localindex2; // Not found in will be returned as -1, not as GeoUndef, Part WB is agnostic to the concept of // GeoUndef - if (!Part2DObject::seekTrimPoints( - geos, GeoId, point, localindex1, intersect1, localindex2, intersect2)) + if (!Part2DObject::seekTrimPoints(geos, + GeoId, + point, + localindex1, + intersect1, + localindex2, + intersect2)) { return false; + } // invalid complete geometry indices are mapped to GeoUndef GeoId1 = getGeoIdFromCompleteGeometryIndex(localindex1); @@ -3537,7 +3548,8 @@ bool SketchObject::seekTrimPoints(int GeoId, const Base::Vector3d& point, int& G // throwing an exception. bool getIntersectionParameter(const Part::Geometry* geo, const Base::Vector3d point, - double& pointParam) { + double& pointParam) +{ const auto* curve = static_cast(geo); try { @@ -3551,13 +3563,15 @@ bool getIntersectionParameter(const Part::Geometry* geo, return true; } -bool arePointsWithinPrecision(const Base::Vector3d& point1, const Base::Vector3d& point2) { +bool arePointsWithinPrecision(const Base::Vector3d& point1, const 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()); } -bool areParamsWithinApproximation(double param1, double param2) { +bool 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()); @@ -3565,7 +3579,10 @@ bool areParamsWithinApproximation(double param1, double param2) { // returns true if the point defined by (GeoId1, pos1) can be considered to be coincident with // point. -bool isPointAtPosition(const SketchObject* obj, int GeoId1, PointPos pos1, const Base::Vector3d& point) +bool isPointAtPosition(const SketchObject* obj, + int GeoId1, + PointPos pos1, + const Base::Vector3d& point) { Base::Vector3d pp = obj->getPoint(GeoId1, pos1); @@ -3596,36 +3613,38 @@ std::unique_ptr transformPreexistingConstraintForTrim(const SketchOb 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(obj, constr->First, constr->FirstPos, cutPointVec)) { - // This concerns the start portion of the trim + 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(obj, 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->Type = Sketcher::Coincident; - newConstr->Second = newGeoId; - newConstr->SecondPos = newPosId; + 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; } - 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; + default: + break; } return newConstr; } @@ -3719,7 +3738,7 @@ bool getParamLimitsOfNewGeosForTrim(const SketchObject* obj, void createArcsFromGeoWithLimits(const Part::GeomCurve* geo, const std::vector>& paramsOfNewGeos, - std::vector>& newGeos) + std::vector& newGeos) { for (auto& [u1, u2] : paramsOfNewGeos) { auto newGeo = static_cast(geo)->createArc(u1, u2); @@ -3853,8 +3872,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) //******************* Step B => Creation of new geometries //****************************************// std::vector newIds; - std::vector> newGeos; - std::vector newGeosAsConsts; + std::vector newGeos; switch (paramsOfNewGeos.size()) { case 0: { @@ -3876,9 +3894,6 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) } createArcsFromGeoWithLimits(geoAsCurve, paramsOfNewGeos, newGeos); - for (const auto& geo : newGeos) { - newGeosAsConsts.push_back(geo.get()); - } //******************* Step C => Creation of new constraints //****************************************// @@ -3974,17 +3989,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) } // TODO: Implement this - // replaceGeometries({GeoId}, newGeos); - auto vals = getInternalGeometry(); - auto newVals(vals); - GeometryFacade::copyId(geoAsCurve, newGeos.front().get()); - newVals[GeoId] = newGeos.front().release(); - if (newGeos.size() > 1) { - newVals.push_back(newGeos.back().get()); - generateId(newGeos.back().release()); - newIds.back() = newVals.size() - 1; - } - Geometry.setValues(std::move(newVals)); + replaceGeometries({GeoId}, newGeos); if (noRecomputes) { solve(); @@ -4019,6 +4024,8 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) return 0; } +// clang-format off + bool SketchObject::deriveConstraintsForPieces(const int oldId, const std::vector& newIds, const Constraint* con, From 502b7b9a3f2e31d025e1f5227edeb8f671fc1f52 Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Thu, 2 Jan 2025 10:31:19 +0530 Subject: [PATCH 04/10] [Sketcher] Fix some issues in `trim` 1. Don't bother deleting internal geometry first. 2. Don't assume all new constraints are well formed. --- src/Mod/Sketcher/App/SketchObject.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 7f2c6757ee..3beda7f640 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -3830,7 +3830,6 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) } // Remove internal geometry beforehand for now // FIXME: we should be able to transfer these to new curves smoothly - deleteUnusedInternalGeometryAndUpdateGeoId(GeoId); // auto geo = getGeometry(GeoId); const auto* geoAsCurve = static_cast(getGeometry(GeoId)); @@ -3964,7 +3963,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) delConstraints(idsOfOldConstraints); - if (isClosedCurve(geoAsCurve)) { + if (!isClosedCurve(geoAsCurve)) { transferConstraints(GeoId, PointPos::start, newIds.front(), PointPos::start, true); transferConstraints(GeoId, PointPos::end, newIds.back(), PointPos::end, true); } From 31457746c2bbe0ca793c2a1507cd50469f48cb8d Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Wed, 8 Jan 2025 23:13:53 +0530 Subject: [PATCH 05/10] [Sketcher][test] Move helper functions away for reusability ...in other tests --- tests/src/Mod/Sketcher/App/SketchObject.cpp | 122 +--------------- .../Mod/Sketcher/App/SketcherTestHelpers.h | 136 ++++++++++++++++++ 2 files changed, 138 insertions(+), 120 deletions(-) create mode 100644 tests/src/Mod/Sketcher/App/SketcherTestHelpers.h diff --git a/tests/src/Mod/Sketcher/App/SketchObject.cpp b/tests/src/Mod/Sketcher/App/SketchObject.cpp index 068d0d4aa8..98b1a5c837 100644 --- a/tests/src/Mod/Sketcher/App/SketchObject.cpp +++ b/tests/src/Mod/Sketcher/App/SketchObject.cpp @@ -11,127 +11,9 @@ #include #include #include +#include "SketcherTestHelpers.h" -void setupLineSegment(Part::GeomLineSegment& lineSeg) -{ - Base::Vector3d coords1(1.0, 2.0, 0.0); - Base::Vector3d coords2(3.0, 4.0, 0.0); - lineSeg.setPoints(coords1, coords2); -} - -void setupCircle(Part::GeomCircle& circle) -{ - Base::Vector3d coordsCenter(1.0, 2.0, 0.0); - Base::Vector3d splitPoint(2.0, 3.1, 0.0); - double radius = 3.0; - circle.setCenter(coordsCenter); - circle.setRadius(radius); -} - -void setupArcOfCircle(Part::GeomArcOfCircle& arcOfCircle) -{ - Base::Vector3d coordsCenter(1.0, 2.0, 0.0); - double radius = 3.0; - double startParam = M_PI / 3, endParam = M_PI * 1.5; - arcOfCircle.setCenter(coordsCenter); - arcOfCircle.setRadius(radius); - arcOfCircle.setRange(startParam, endParam, true); -} - -void setupEllipse(Part::GeomEllipse& ellipse) -{ - Base::Vector3d coordsCenter(1.0, 2.0, 0.0); - double majorRadius = 4.0; - double minorRadius = 3.0; - ellipse.setCenter(coordsCenter); - ellipse.setMajorRadius(majorRadius); - 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); - double focal = 3.0; - double startParam = -M_PI * 1.5, endParam = M_PI * 1.5; - aop.setCenter(coordsCenter); - aop.setFocal(focal); - aop.setRange(startParam, endParam, true); -} - -std::unique_ptr createTypicalNonPeriodicBSpline() -{ - int degree = 3; - std::vector poles; - poles.emplace_back(1, 0, 0); - poles.emplace_back(1, 1, 0); - poles.emplace_back(1, 0.5, 0); - poles.emplace_back(0, 1, 0); - poles.emplace_back(0, 0, 0); - std::vector weights(5, 1.0); - std::vector knotsNonPeriodic = {0.0, 1.0, 2.0}; - std::vector multiplicitiesNonPeriodic = {degree + 1, 1, degree + 1}; - return std::make_unique(poles, - weights, - knotsNonPeriodic, - multiplicitiesNonPeriodic, - degree, - false); -} - -std::unique_ptr createTypicalPeriodicBSpline() -{ - int degree = 3; - std::vector poles; - poles.emplace_back(1, 0, 0); - poles.emplace_back(1, 1, 0); - poles.emplace_back(1, 0.5, 0); - poles.emplace_back(0, 1, 0); - poles.emplace_back(0, 0, 0); - std::vector weights(5, 1.0); - std::vector knotsPeriodic = {0.0, 0.3, 1.0, 1.5, 1.8, 2.0}; - std::vector multiplicitiesPeriodic(6, 1); - return std::make_unique(poles, - weights, - knotsPeriodic, - multiplicitiesPeriodic, - degree, - true); -} - -int countConstraintsOfType(const Sketcher::SketchObject* obj, const Sketcher::ConstraintType cType) -{ - const std::vector& constraints = obj->Constraints.getValues(); - - int result = std::count_if(constraints.begin(), - constraints.end(), - [&cType](const Sketcher::Constraint* constr) { - return constr->Type == cType; - }); - - return result; -} - -// Get point at the parameter after scaling the range to [0, 1]. -Base::Vector3d getPointAtNormalizedParameter(const Part::GeomCurve& curve, double param) -{ - return curve.pointAtParameter(curve.getFirstParameter() - + (curve.getLastParameter() - curve.getFirstParameter()) * param); -} - -// TODO: How to set up B-splines here? -// It's not straightforward to change everything from a "default" one. +using namespace SketcherTestHelpers; class SketchObjectTest: public ::testing::Test { diff --git a/tests/src/Mod/Sketcher/App/SketcherTestHelpers.h b/tests/src/Mod/Sketcher/App/SketcherTestHelpers.h new file mode 100644 index 0000000000..e03655e628 --- /dev/null +++ b/tests/src/Mod/Sketcher/App/SketcherTestHelpers.h @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include +#include +#include +#include +#include +#include +#include + +namespace SketcherTestHelpers +{ + +using namespace Sketcher; + +void setupLineSegment(Part::GeomLineSegment& lineSeg) +{ + Base::Vector3d coords1(1.0, 2.0, 0.0); + Base::Vector3d coords2(3.0, 4.0, 0.0); + lineSeg.setPoints(coords1, coords2); +} + +void setupCircle(Part::GeomCircle& circle) +{ + Base::Vector3d coordsCenter(1.0, 2.0, 0.0); + Base::Vector3d splitPoint(2.0, 3.1, 0.0); + double radius = 3.0; + circle.setCenter(coordsCenter); + circle.setRadius(radius); +} + +void setupArcOfCircle(Part::GeomArcOfCircle& arcOfCircle) +{ + Base::Vector3d coordsCenter(1.0, 2.0, 0.0); + double radius = 3.0; + double startParam = M_PI / 3, endParam = M_PI * 1.5; + arcOfCircle.setCenter(coordsCenter); + arcOfCircle.setRadius(radius); + arcOfCircle.setRange(startParam, endParam, true); +} + +void setupEllipse(Part::GeomEllipse& ellipse) +{ + Base::Vector3d coordsCenter(1.0, 2.0, 0.0); + double majorRadius = 4.0; + double minorRadius = 3.0; + ellipse.setCenter(coordsCenter); + ellipse.setMajorRadius(majorRadius); + 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); + double focal = 3.0; + double startParam = -M_PI * 1.5, endParam = M_PI * 1.5; + aop.setCenter(coordsCenter); + aop.setFocal(focal); + aop.setRange(startParam, endParam, true); +} + +std::unique_ptr createTypicalNonPeriodicBSpline() +{ + int degree = 3; + std::vector poles; + poles.emplace_back(1, 0, 0); + poles.emplace_back(1, 1, 0); + poles.emplace_back(1, 0.5, 0); + poles.emplace_back(0, 1, 0); + poles.emplace_back(0, 0, 0); + std::vector weights(5, 1.0); + std::vector knotsNonPeriodic = {0.0, 1.0, 2.0}; + std::vector multiplicitiesNonPeriodic = {degree + 1, 1, degree + 1}; + return std::make_unique(poles, + weights, + knotsNonPeriodic, + multiplicitiesNonPeriodic, + degree, + false); +} + +std::unique_ptr createTypicalPeriodicBSpline() +{ + int degree = 3; + std::vector poles; + poles.emplace_back(1, 0, 0); + poles.emplace_back(1, 1, 0); + poles.emplace_back(1, 0.5, 0); + poles.emplace_back(0, 1, 0); + poles.emplace_back(0, 0, 0); + std::vector weights(5, 1.0); + std::vector knotsPeriodic = {0.0, 0.3, 1.0, 1.5, 1.8, 2.0}; + std::vector multiplicitiesPeriodic(6, 1); + return std::make_unique(poles, + weights, + knotsPeriodic, + multiplicitiesPeriodic, + degree, + true); +} + +int countConstraintsOfType(const Sketcher::SketchObject* obj, const Sketcher::ConstraintType cType) +{ + const std::vector& constraints = obj->Constraints.getValues(); + + int result = std::count_if(constraints.begin(), + constraints.end(), + [&cType](const Sketcher::Constraint* constr) { + return constr->Type == cType; + }); + + return result; +} + +// Get point at the parameter after scaling the range to [0, 1]. +Base::Vector3d getPointAtNormalizedParameter(const Part::GeomCurve& curve, double param) +{ + return curve.pointAtParameter(curve.getFirstParameter() + + (curve.getLastParameter() - curve.getFirstParameter()) * param); +} + +// TODO: How to set up B-splines here? +// It's not straightforward to change everything from a "default" one. +} // namespace SketcherTestHelpers From c48f21585d9b8813084a7e6320b7bfecd36a395e Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Sun, 12 Jan 2025 17:57:03 +0530 Subject: [PATCH 06/10] [Sketcher][test] Divide `SketchObject` tests into multiple files ...for managability. --- tests/src/Mod/Sketcher/App/CMakeLists.txt | 2 + tests/src/Mod/Sketcher/App/SketchObject.cpp | 946 ------------------ .../Mod/Sketcher/App/SketchObjectChanges.cpp | 920 +++++++++++++++++ .../Mod/Sketcher/App/SketcherTestHelpers.cpp | 155 +++ .../Mod/Sketcher/App/SketcherTestHelpers.h | 142 +-- 5 files changed, 1112 insertions(+), 1053 deletions(-) create mode 100644 tests/src/Mod/Sketcher/App/SketchObjectChanges.cpp create mode 100644 tests/src/Mod/Sketcher/App/SketcherTestHelpers.cpp diff --git a/tests/src/Mod/Sketcher/App/CMakeLists.txt b/tests/src/Mod/Sketcher/App/CMakeLists.txt index 7bcb006997..75dc0501a2 100644 --- a/tests/src/Mod/Sketcher/App/CMakeLists.txt +++ b/tests/src/Mod/Sketcher/App/CMakeLists.txt @@ -1,7 +1,9 @@ target_sources( Sketcher_tests_run PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/SketcherTestHelpers.cpp ${CMAKE_CURRENT_SOURCE_DIR}/SketchObject.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/SketchObjectChanges.cpp ) add_subdirectory(planegcs) diff --git a/tests/src/Mod/Sketcher/App/SketchObject.cpp b/tests/src/Mod/Sketcher/App/SketchObject.cpp index 98b1a5c837..30e4432387 100644 --- a/tests/src/Mod/Sketcher/App/SketchObject.cpp +++ b/tests/src/Mod/Sketcher/App/SketchObject.cpp @@ -1,7 +1,5 @@ // SPDX-License-Identifier: LGPL-2.1-or-later -#include - #include #include @@ -10,50 +8,10 @@ #include #include #include -#include #include "SketcherTestHelpers.h" using namespace SketcherTestHelpers; -class SketchObjectTest: public ::testing::Test -{ -protected: - static void SetUpTestSuite() - { - tests::initApplication(); - } - - void SetUp() override - { - _docName = App::GetApplication().getUniqueDocumentName("test"); - auto _doc = App::GetApplication().newDocument(_docName.c_str(), "testUser"); - // TODO: Do we add a body newName, or is just adding sketch sufficient for this test? - _sketchobj = - static_cast(_doc->addObject("Sketcher::SketchObject")); - } - - void TearDown() override - { - App::GetApplication().closeDocument(_docName.c_str()); - } - - Sketcher::SketchObject* getObject() - { - return _sketchobj; - } - -private: - // TODO: use shared_ptr or something else here? - Sketcher::SketchObject* _sketchobj; - std::string _docName; - std::vector allowedTypes {"Vertex", - "Edge", - "ExternalEdge", - "H_Axis", - "V_Axis", - "RootPoint"}; -}; - TEST_F(SketchObjectTest, createSketchObject) // NOLINT { // Arrange @@ -752,910 +710,6 @@ TEST_F(SketchObjectTest, testDeleteOnlyUnusedInternalGeometryOfBSpline) EXPECT_EQ(getObject()->getHighestCurveIndex(), 2); } -TEST_F(SketchObjectTest, testSplitLineSegment) -{ - // Arrange - Base::Vector3d splitPoint(2.0, 3.1, 0.0); - Part::GeomLineSegment lineSeg; - setupLineSegment(lineSeg); - int geoId = getObject()->addGeometry(&lineSeg); - - // Act - int result = getObject()->split(geoId, splitPoint); - - // Assert - EXPECT_EQ(result, 0); - // One additional curve should be added - EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId + 1); - // TODO: Expect the resultant curves are line segments and shape is conserved - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); -} - -TEST_F(SketchObjectTest, testSplitCircle) -{ - // Arrange - Base::Vector3d splitPoint(2.0, 3.1, 0.0); - Part::GeomCircle circle; - setupCircle(circle); - int geoId = getObject()->addGeometry(&circle); - - // Act - int result = getObject()->split(geoId, splitPoint); - - // Assert - EXPECT_EQ(result, 0); - // The circle should be split into an arc now - EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId); -} - -TEST_F(SketchObjectTest, testSplitEllipse) -{ - // Arrange - Base::Vector3d splitPoint(2.0, 3.1, 0.0); - Part::GeomEllipse ellipse; - setupEllipse(ellipse); - int geoId = getObject()->addGeometry(&ellipse); - - // Act - int result = getObject()->split(geoId, splitPoint); - - // Assert - EXPECT_EQ(result, 0); - // TODO: The ellipse should be split into an arc of ellipse now - // FIXME: Internal geometries may be added or removed which may cause some issues - // EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId); -} - -TEST_F(SketchObjectTest, testSplitArcOfCircle) -{ - // Arrange - Base::Vector3d splitPoint(-2.0, 3.1, 0.0); - Part::GeomArcOfCircle arcOfCircle; - setupArcOfCircle(arcOfCircle); - int geoId = getObject()->addGeometry(&arcOfCircle); - - // Act - int result = getObject()->split(geoId, splitPoint); - - // Assert - EXPECT_EQ(result, 0); - // The arcOfCircle should be split into an arc now - EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId + 1); - // Expect the end points and centers of the resultant curve are coincident. - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 2); -} - -TEST_F(SketchObjectTest, testSplitArcOfConic) -{ - // Arrange - // TODO: Define a parabola/hyperbola as reference - Base::Vector3d splitPoint(1.0, -1.1, 0.0); - Part::GeomArcOfParabola arcOfConic; - setupArcOfParabola(arcOfConic); - int geoId = getObject()->addGeometry(&arcOfConic); - - // Act - // TODO: Sample random points from both sides of the split - int result = getObject()->split(geoId, splitPoint); - for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { - getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); - } - - // Assert - EXPECT_EQ(result, 0); - // The arcOfConic should be split into two arcs of the same conic now - EXPECT_EQ(getObject()->getHighestCurveIndex(), 1); - // TODO: Expect the end points of the resultant curve are coincident. - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); -} - -TEST_F(SketchObjectTest, testSplitNonPeriodicBSpline) -{ - // Arrange - auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); - Base::Vector3d splitPoint(-0.5, 1.1, 0.0); - int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); - // TODO: Put a point on this - - // Act - // TODO: sample before point(s) at a random parameter - int result = getObject()->split(geoId, splitPoint); - for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { - getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); - } - - // Assert - EXPECT_EQ(result, 0); - EXPECT_EQ(getObject()->getHighestCurveIndex(), 1); - // TODO: confirm sampled point(s) is/are at the same place - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); -} - -TEST_F(SketchObjectTest, testSplitPeriodicBSpline) -{ - // Arrange - auto PeriodicBSpline = createTypicalPeriodicBSpline(); - Base::Vector3d splitPoint(-0.5, 1.1, 0.0); - int geoId = getObject()->addGeometry(PeriodicBSpline.get()); - // TODO: Put a point on this - - // Act - // TODO: sample before point(s) at a random parameter - int result = getObject()->split(geoId, splitPoint); - for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { - getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); - } - - // Assert - EXPECT_EQ(result, 0); - EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); - // TODO: confirm sampled point(s) is/are at the same place -} - -TEST_F(SketchObjectTest, testTrimWithoutIntersection) -{ - // Arrange - Part::GeomLineSegment lineSeg; - setupLineSegment(lineSeg); - int geoId = getObject()->addGeometry(&lineSeg); - Base::Vector3d trimPoint(2.0, 3.1, 0.0); - - // Act - int result = getObject()->trim(geoId, trimPoint); - - // Assert - EXPECT_EQ(result, 0); - // Once this line segment is trimmed, nothing should remain - EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId - 1); -} - -// TODO: There are other combinations of constraints we may want to test with trim. - -TEST_F(SketchObjectTest, testTrimLineSegmentEnd) -{ - // Arrange - Part::GeomLineSegment lineSeg; - setupLineSegment(lineSeg); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(lineSeg, 0.2)); - Base::Vector3d p1(getPointAtNormalizedParameter(lineSeg, 0.5)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - int geoId = getObject()->addGeometry(&lineSeg); - - // Act - int result = getObject()->trim(geoId, trimPoint); - - // Assert - EXPECT_EQ(result, 0); - // TODO: Once this line segment is trimmed, the curve should be "smaller" - EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId); - // TODO: There should be a "point-on-object" constraint on the intersecting curves - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); -} - -TEST_F(SketchObjectTest, testTrimLineSegmentMid) -{ - // Arrange - Part::GeomLineSegment lineSeg; - setupLineSegment(lineSeg); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(lineSeg, 0.5)); - Base::Vector3d p1(getPointAtNormalizedParameter(lineSeg, 0.3)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - Base::Vector3d p3(getPointAtNormalizedParameter(lineSeg, 0.7)); - Base::Vector3d p4(p3.x + 0.1, p3.y - 0.1, p3.z); - // to ensure that this line clearly intersects the curve, not just have a point on object - // without explicit constraint - p3.x -= 0.1; - p3.y += 0.1; - Part::GeomLineSegment lineSegCut2; - lineSegCut2.setPoints(p3, p4); - getObject()->addGeometry(&lineSegCut2); - int geoId = getObject()->addGeometry(&lineSeg); - - // Act - int result = getObject()->trim(geoId, trimPoint); - - // Assert - EXPECT_EQ(result, 0); - // TODO: Once this line segment is trimmed, there should be two "smaller" curves in its place - EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId + 1); - // TODO: There should be a "point-on-object" constraint on the intersecting curves - int numberOfPointOnObjectConstraints = - countConstraintsOfType(getObject(), Sketcher::PointOnObject); - EXPECT_EQ(numberOfPointOnObjectConstraints, 1); - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); - // TODO: Ensure shape is preserved -} - -TEST_F(SketchObjectTest, testTrimCircleEnd) -{ - // Arrange - Part::GeomCircle circle; - setupCircle(circle); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(circle, 0.2)); - Base::Vector3d p1(getPointAtNormalizedParameter(circle, 0.5)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - int geoId = getObject()->addGeometry(&circle); - - // Act - int result = getObject()->trim(geoId, trimPoint); - - // Assert - EXPECT_EQ(result, 0); - // TODO: Once this circle is trimmed, the circle should be deleted. - EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId - 1); -} - -TEST_F(SketchObjectTest, testTrimCircleMid) -{ - // Arrange - Part::GeomCircle circle; - setupCircle(circle); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(circle, 0.5)); - Base::Vector3d p1(getPointAtNormalizedParameter(circle, 0.3)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - Base::Vector3d p3(getPointAtNormalizedParameter(circle, 0.7)); - Base::Vector3d p4(p3.x + 0.1, p3.y + 0.1, p3.z); - // to ensure that this line clearly intersects the curve, not just have a point on object - // without explicit constraint - p3.x -= 0.1; - p3.y -= 0.1; - Part::GeomLineSegment lineSegCut2; - lineSegCut2.setPoints(p3, p4); - getObject()->addGeometry(&lineSegCut2); - int geoId = getObject()->addGeometry(&circle); - - // Act - int result = getObject()->trim(geoId, trimPoint); - - // Assert - EXPECT_EQ(result, 0); - // TODO: Once this circle is trimmed, there should be one arc. - EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId); - // There should be one "coincident" and one "point-on-object" constraint on the intersecting - // curves - int numberOfPointOnObjectConstraints = - countConstraintsOfType(getObject(), Sketcher::PointOnObject); - EXPECT_EQ(numberOfPointOnObjectConstraints, 1); - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); - // TODO: Ensure shape is preserved -} - -TEST_F(SketchObjectTest, testTrimArcOfCircleEnd) -{ - // This should also cover as a representative of arc of conic - - // Arrange - Part::GeomArcOfCircle arcOfCircle; - setupArcOfCircle(arcOfCircle); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(arcOfCircle, 0.2)); - Base::Vector3d p1(getPointAtNormalizedParameter(arcOfCircle, 0.5)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - int geoId = getObject()->addGeometry(&arcOfCircle); - - // Act - int result = getObject()->trim(geoId, trimPoint); - - // Assert - EXPECT_EQ(result, 0); - EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId); - // There should be a "point-on-object" constraint on the intersecting curves - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); -} - -TEST_F(SketchObjectTest, testTrimArcOfCircleMid) -{ - // Arrange - Part::GeomArcOfCircle arcOfCircle; - setupArcOfCircle(arcOfCircle); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(arcOfCircle, 0.5)); - Base::Vector3d p1(getPointAtNormalizedParameter(arcOfCircle, 0.3)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - Base::Vector3d p3(getPointAtNormalizedParameter(arcOfCircle, 0.7)); - Base::Vector3d p4(p3.x + 0.1, p3.y + 0.1, p3.z); - // to ensure that this line clearly intersects the curve, not just have a point on object - // without explicit constraint - p3.x -= 0.1; - p3.y -= 0.1; - Part::GeomLineSegment lineSegCut2; - lineSegCut2.setPoints(p3, p4); - getObject()->addGeometry(&lineSegCut2); - int geoId = getObject()->addGeometry(&arcOfCircle); - - // Act - int result = getObject()->trim(geoId, trimPoint); - - // Assert - EXPECT_EQ(result, 0); - EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId + 1); - // There should be a "point-on-object" constraint on the intersecting curves - int numberOfPointOnObjectConstraints = - countConstraintsOfType(getObject(), Sketcher::PointOnObject); - EXPECT_EQ(numberOfPointOnObjectConstraints, 1); - // There should be 2 coincident constraints: one with lineSegCut1 and one between centers of the - // new arcs - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 2); - // TODO: Ensure shape is preserved -} - -TEST_F(SketchObjectTest, testTrimEllipseEnd) -{ - // Arrange - Part::GeomEllipse ellipse; - setupEllipse(ellipse); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(ellipse, 0.2)); - Base::Vector3d p1(getPointAtNormalizedParameter(ellipse, 0.5)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - int geoId = getObject()->addGeometry(&ellipse); - - // Act - int result = getObject()->trim(geoId, trimPoint); - // remove all internal geometry - for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { - getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); - } - - // Assert - EXPECT_EQ(result, 0); - // Once this ellipse is trimmed, the ellipse should be deleted. - // Only remaining: line segment - EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); -} - -TEST_F(SketchObjectTest, testTrimEllipseMid) -{ - // Arrange - Part::GeomEllipse ellipse; - setupEllipse(ellipse); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(ellipse, 0.5)); - Base::Vector3d p1(getPointAtNormalizedParameter(ellipse, 0.3)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - Base::Vector3d p3(getPointAtNormalizedParameter(ellipse, 0.7)); - Base::Vector3d p4(p3.x + 0.1, p3.y + 0.1, p3.z); - // to ensure that this line clearly intersects the curve, not just have a point on object - // without explicit constraint - p3.x -= 0.1; - p3.y -= 0.1; - Part::GeomLineSegment lineSegCut2; - lineSegCut2.setPoints(p3, p4); - getObject()->addGeometry(&lineSegCut2); - int geoId = getObject()->addGeometry(&ellipse); - // FIXME: Doing this to avoid trimming only until minor/major axes. Should not be needed. - getObject()->deleteUnusedInternalGeometry(geoId); - - // Act - int result = getObject()->trim(geoId, trimPoint); - // remove all internal geometry - for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { - getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); - } - - // Assert - EXPECT_EQ(result, 0); - // Once this ellipse is trimmed, there should be one arc and line segments. - EXPECT_EQ(getObject()->getHighestCurveIndex(), 2); - // There should be one "coincident" and one "point-on-object" constraint on the intersecting - // curves - int numberOfPointOnObjectConstraints = - countConstraintsOfType(getObject(), Sketcher::PointOnObject); - EXPECT_EQ(numberOfPointOnObjectConstraints, 1); - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); - // TODO: Ensure shape is preserved -} - -// TODO: Tests for other arcs of conics? - -TEST_F(SketchObjectTest, testTrimPeriodicBSplineEnd) -{ - // Arrange - auto periodicBSpline = createTypicalPeriodicBSpline(); - assert(periodicBSpline); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(*periodicBSpline, 0.2)); - Base::Vector3d p1(getPointAtNormalizedParameter(*periodicBSpline, 0.5)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - int geoId = getObject()->addGeometry(periodicBSpline.get()); - - // Act - int result = getObject()->trim(geoId, trimPoint); - - // Assert - EXPECT_EQ(result, 0); - // FIXME: This will fail because of deleted internal geometry - // Once this periodicBSpline is trimmed, the periodicBSpline should be deleted, leaving only the - // line segment. - EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); - // TODO: There should be a "point-on-object" constraint on the intersecting curves -} - -TEST_F(SketchObjectTest, testTrimPeriodicBSplineMid) -{ - // Arrange - auto periodicBSpline = createTypicalPeriodicBSpline(); - assert(periodicBSpline); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(*periodicBSpline, 0.5)); - Base::Vector3d p1(getPointAtNormalizedParameter(*periodicBSpline, 0.3)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - Base::Vector3d p3(getPointAtNormalizedParameter(*periodicBSpline, 0.7)); - Base::Vector3d p4(p3.x + 0.1, p3.y + 0.1, p3.z); - // to ensure that this line clearly intersects the curve, not just have a point on object - // without explicit constraint - p3.x -= 0.1; - p3.y -= 0.1; - Part::GeomLineSegment lineSegCut2; - lineSegCut2.setPoints(p3, p4); - getObject()->addGeometry(&lineSegCut2); - int geoId = getObject()->addGeometry(periodicBSpline.get()); - - // Act - int result = getObject()->trim(geoId, trimPoint); - // remove all internal geometry - for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { - getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); - } - - // Assert - EXPECT_EQ(result, 0); - // Only remaining: Two line segments and the B-spline - EXPECT_EQ(getObject()->getHighestCurveIndex(), 2); - // There should be one "coincident" and one "point-on-object" constraint on the intersecting - // curves - int numberOfPointOnObjectConstraints = - countConstraintsOfType(getObject(), Sketcher::PointOnObject); - EXPECT_EQ(numberOfPointOnObjectConstraints, 1); - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); - // TODO: Ensure shape is preserved -} - -TEST_F(SketchObjectTest, testTrimNonPeriodicBSplineEnd) -{ - // This should also cover as a representative of arc of conic - - // Arrange - auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); - assert(nonPeriodicBSpline); - // create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(*nonPeriodicBSpline, 0.2)); - Base::Vector3d p1(getPointAtNormalizedParameter(*nonPeriodicBSpline, 0.5)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); - - // Act - int result = getObject()->trim(geoId, trimPoint); - // remove all internal geometry - for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { - getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); - } - - // Assert - EXPECT_EQ(result, 0); - // Only remaining: one line segment and the trimmed B-spline - EXPECT_EQ(getObject()->getHighestCurveIndex(), 1); - // FIXME: There should be a "point-on-object" constraint on the intersecting curves - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); -} - -TEST_F(SketchObjectTest, testTrimNonPeriodicBSplineMid) -{ - // Arrange - auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); - assert(nonPeriodicBSpline); - // TODO: create curves intersecting at the right spots - Base::Vector3d trimPoint(getPointAtNormalizedParameter(*nonPeriodicBSpline, 0.5)); - Base::Vector3d p1(getPointAtNormalizedParameter(*nonPeriodicBSpline, 0.3)); - Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); - Part::GeomLineSegment lineSegCut1; - lineSegCut1.setPoints(p1, p2); - getObject()->addGeometry(&lineSegCut1); - Base::Vector3d p3(getPointAtNormalizedParameter(*nonPeriodicBSpline, 0.7)); - Base::Vector3d p4(p3.x + 0.1, p3.y + 0.1, p3.z); - // to ensure that this line clearly intersects the curve, not just have a point on object - // without explicit constraint - p3.x -= 0.1; - p3.y -= 0.1; - Part::GeomLineSegment lineSegCut2; - lineSegCut2.setPoints(p3, p4); - getObject()->addGeometry(&lineSegCut2); - int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); - - // Act - int result = getObject()->trim(geoId, trimPoint); - // remove all internal geometry - for (int i = 0; i < getObject()->getHighestCurveIndex(); ++i) { - if (getObject()->getGeometry(i)->is()) { - getObject()->deleteUnusedInternalGeometry(i); - } - } - - // Assert - EXPECT_EQ(result, 0); - // Only remaining: one line segment and the trimmed B-spline - EXPECT_EQ(getObject()->getHighestCurveIndex(), 3); - // There should be a "point-on-object" constraint on the intersecting curves - int numberOfPointOnObjectConstraints = - countConstraintsOfType(getObject(), Sketcher::PointOnObject); - EXPECT_EQ(numberOfPointOnObjectConstraints, 1); - int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); - EXPECT_EQ(numberOfCoincidentConstraints, 1); - // 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"); diff --git a/tests/src/Mod/Sketcher/App/SketchObjectChanges.cpp b/tests/src/Mod/Sketcher/App/SketchObjectChanges.cpp new file mode 100644 index 0000000000..2b0f69aa76 --- /dev/null +++ b/tests/src/Mod/Sketcher/App/SketchObjectChanges.cpp @@ -0,0 +1,920 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include + +#include + +#include +#include +#include +#include +#include +#include +#include +#include "SketcherTestHelpers.h" + +using namespace SketcherTestHelpers; + +TEST_F(SketchObjectTest, testSplitLineSegment) +{ + // Arrange + Base::Vector3d splitPoint(2.0, 3.1, 0.0); + Part::GeomLineSegment lineSeg; + setupLineSegment(lineSeg); + int geoId = getObject()->addGeometry(&lineSeg); + + // Act + int result = getObject()->split(geoId, splitPoint); + + // Assert + EXPECT_EQ(result, 0); + // One additional curve should be added + EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId + 1); + // TODO: Expect the resultant curves are line segments and shape is conserved + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); +} + +TEST_F(SketchObjectTest, testSplitCircle) +{ + // Arrange + Base::Vector3d splitPoint(2.0, 3.1, 0.0); + Part::GeomCircle circle; + setupCircle(circle); + int geoId = getObject()->addGeometry(&circle); + + // Act + int result = getObject()->split(geoId, splitPoint); + + // Assert + EXPECT_EQ(result, 0); + // The circle should be split into an arc now + EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId); +} + +TEST_F(SketchObjectTest, testSplitEllipse) +{ + // Arrange + Base::Vector3d splitPoint(2.0, 3.1, 0.0); + Part::GeomEllipse ellipse; + setupEllipse(ellipse); + int geoId = getObject()->addGeometry(&ellipse); + + // Act + int result = getObject()->split(geoId, splitPoint); + + // Assert + EXPECT_EQ(result, 0); + // TODO: The ellipse should be split into an arc of ellipse now + // FIXME: Internal geometries may be added or removed which may cause some issues + // EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId); +} + +TEST_F(SketchObjectTest, testSplitArcOfCircle) +{ + // Arrange + Base::Vector3d splitPoint(-2.0, 3.1, 0.0); + Part::GeomArcOfCircle arcOfCircle; + setupArcOfCircle(arcOfCircle); + int geoId = getObject()->addGeometry(&arcOfCircle); + + // Act + int result = getObject()->split(geoId, splitPoint); + + // Assert + EXPECT_EQ(result, 0); + // The arcOfCircle should be split into an arc now + EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId + 1); + // Expect the end points and centers of the resultant curve are coincident. + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 2); +} + +TEST_F(SketchObjectTest, testSplitArcOfConic) +{ + // Arrange + // TODO: Define a parabola/hyperbola as reference + Base::Vector3d splitPoint(1.0, -1.1, 0.0); + Part::GeomArcOfParabola arcOfConic; + setupArcOfParabola(arcOfConic); + int geoId = getObject()->addGeometry(&arcOfConic); + + // Act + // TODO: Sample random points from both sides of the split + int result = getObject()->split(geoId, splitPoint); + for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); + } + + // Assert + EXPECT_EQ(result, 0); + // The arcOfConic should be split into two arcs of the same conic now + EXPECT_EQ(getObject()->getHighestCurveIndex(), 1); + // TODO: Expect the end points of the resultant curve are coincident. + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); +} + +TEST_F(SketchObjectTest, testSplitNonPeriodicBSpline) +{ + // Arrange + auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); + Base::Vector3d splitPoint(-0.5, 1.1, 0.0); + int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); + // TODO: Put a point on this + + // Act + // TODO: sample before point(s) at a random parameter + int result = getObject()->split(geoId, splitPoint); + for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); + } + + // Assert + EXPECT_EQ(result, 0); + EXPECT_EQ(getObject()->getHighestCurveIndex(), 1); + // TODO: confirm sampled point(s) is/are at the same place + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); +} + +TEST_F(SketchObjectTest, testSplitPeriodicBSpline) +{ + // Arrange + auto PeriodicBSpline = createTypicalPeriodicBSpline(); + Base::Vector3d splitPoint(-0.5, 1.1, 0.0); + int geoId = getObject()->addGeometry(PeriodicBSpline.get()); + // TODO: Put a point on this + + // Act + // TODO: sample before point(s) at a random parameter + int result = getObject()->split(geoId, splitPoint); + for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); + } + + // Assert + EXPECT_EQ(result, 0); + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); + // TODO: confirm sampled point(s) is/are at the same place +} + +TEST_F(SketchObjectTest, testTrimWithoutIntersection) +{ + // Arrange + Part::GeomLineSegment lineSeg; + setupLineSegment(lineSeg); + int geoId = getObject()->addGeometry(&lineSeg); + Base::Vector3d trimPoint(2.0, 3.1, 0.0); + + // Act + int result = getObject()->trim(geoId, trimPoint); + + // Assert + EXPECT_EQ(result, 0); + // Once this line segment is trimmed, nothing should remain + EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId - 1); +} + +// TODO: There are other combinations of constraints we may want to test with trim. + +TEST_F(SketchObjectTest, testTrimLineSegmentEnd) +{ + // Arrange + Part::GeomLineSegment lineSeg; + setupLineSegment(lineSeg); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(lineSeg, 0.2)); + Base::Vector3d p1(getPointAtNormalizedParameter(lineSeg, 0.5)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + int geoId = getObject()->addGeometry(&lineSeg); + + // Act + int result = getObject()->trim(geoId, trimPoint); + + // Assert + EXPECT_EQ(result, 0); + // TODO: Once this line segment is trimmed, the curve should be "smaller" + EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId); + // TODO: There should be a "point-on-object" constraint on the intersecting curves + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); +} + +TEST_F(SketchObjectTest, testTrimLineSegmentMid) +{ + // Arrange + Part::GeomLineSegment lineSeg; + setupLineSegment(lineSeg); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(lineSeg, 0.5)); + Base::Vector3d p1(getPointAtNormalizedParameter(lineSeg, 0.3)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + Base::Vector3d p3(getPointAtNormalizedParameter(lineSeg, 0.7)); + Base::Vector3d p4(p3.x + 0.1, p3.y - 0.1, p3.z); + // to ensure that this line clearly intersects the curve, not just have a point on object + // without explicit constraint + p3.x -= 0.1; + p3.y += 0.1; + Part::GeomLineSegment lineSegCut2; + lineSegCut2.setPoints(p3, p4); + getObject()->addGeometry(&lineSegCut2); + int geoId = getObject()->addGeometry(&lineSeg); + + // Act + int result = getObject()->trim(geoId, trimPoint); + + // Assert + EXPECT_EQ(result, 0); + // TODO: Once this line segment is trimmed, there should be two "smaller" curves in its place + EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId + 1); + // TODO: There should be a "point-on-object" constraint on the intersecting curves + int numberOfPointOnObjectConstraints = + countConstraintsOfType(getObject(), Sketcher::PointOnObject); + EXPECT_EQ(numberOfPointOnObjectConstraints, 1); + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); + // TODO: Ensure shape is preserved +} + +TEST_F(SketchObjectTest, testTrimCircleEnd) +{ + // Arrange + Part::GeomCircle circle; + setupCircle(circle); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(circle, 0.2)); + Base::Vector3d p1(getPointAtNormalizedParameter(circle, 0.5)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + int geoId = getObject()->addGeometry(&circle); + + // Act + int result = getObject()->trim(geoId, trimPoint); + + // Assert + EXPECT_EQ(result, 0); + // TODO: Once this circle is trimmed, the circle should be deleted. + EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId - 1); +} + +TEST_F(SketchObjectTest, testTrimCircleMid) +{ + // Arrange + Part::GeomCircle circle; + setupCircle(circle); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(circle, 0.5)); + Base::Vector3d p1(getPointAtNormalizedParameter(circle, 0.3)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + Base::Vector3d p3(getPointAtNormalizedParameter(circle, 0.7)); + Base::Vector3d p4(p3.x + 0.1, p3.y + 0.1, p3.z); + // to ensure that this line clearly intersects the curve, not just have a point on object + // without explicit constraint + p3.x -= 0.1; + p3.y -= 0.1; + Part::GeomLineSegment lineSegCut2; + lineSegCut2.setPoints(p3, p4); + getObject()->addGeometry(&lineSegCut2); + int geoId = getObject()->addGeometry(&circle); + + // Act + int result = getObject()->trim(geoId, trimPoint); + + // Assert + EXPECT_EQ(result, 0); + // TODO: Once this circle is trimmed, there should be one arc. + EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId); + // There should be one "coincident" and one "point-on-object" constraint on the intersecting + // curves + int numberOfPointOnObjectConstraints = + countConstraintsOfType(getObject(), Sketcher::PointOnObject); + EXPECT_EQ(numberOfPointOnObjectConstraints, 1); + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); + // TODO: Ensure shape is preserved +} + +TEST_F(SketchObjectTest, testTrimArcOfCircleEnd) +{ + // This should also cover as a representative of arc of conic + + // Arrange + Part::GeomArcOfCircle arcOfCircle; + setupArcOfCircle(arcOfCircle); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(arcOfCircle, 0.2)); + Base::Vector3d p1(getPointAtNormalizedParameter(arcOfCircle, 0.5)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + int geoId = getObject()->addGeometry(&arcOfCircle); + + // Act + int result = getObject()->trim(geoId, trimPoint); + + // Assert + EXPECT_EQ(result, 0); + EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId); + // There should be a "point-on-object" constraint on the intersecting curves + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); +} + +TEST_F(SketchObjectTest, testTrimArcOfCircleMid) +{ + // Arrange + Part::GeomArcOfCircle arcOfCircle; + setupArcOfCircle(arcOfCircle); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(arcOfCircle, 0.5)); + Base::Vector3d p1(getPointAtNormalizedParameter(arcOfCircle, 0.3)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + Base::Vector3d p3(getPointAtNormalizedParameter(arcOfCircle, 0.7)); + Base::Vector3d p4(p3.x + 0.1, p3.y + 0.1, p3.z); + // to ensure that this line clearly intersects the curve, not just have a point on object + // without explicit constraint + p3.x -= 0.1; + p3.y -= 0.1; + Part::GeomLineSegment lineSegCut2; + lineSegCut2.setPoints(p3, p4); + getObject()->addGeometry(&lineSegCut2); + int geoId = getObject()->addGeometry(&arcOfCircle); + + // Act + int result = getObject()->trim(geoId, trimPoint); + + // Assert + EXPECT_EQ(result, 0); + EXPECT_EQ(getObject()->getHighestCurveIndex(), geoId + 1); + // There should be a "point-on-object" constraint on the intersecting curves + int numberOfPointOnObjectConstraints = + countConstraintsOfType(getObject(), Sketcher::PointOnObject); + EXPECT_EQ(numberOfPointOnObjectConstraints, 1); + // There should be 2 coincident constraints: one with lineSegCut1 and one between centers of the + // new arcs + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 2); + // TODO: Ensure shape is preserved +} + +TEST_F(SketchObjectTest, testTrimEllipseEnd) +{ + // Arrange + Part::GeomEllipse ellipse; + setupEllipse(ellipse); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(ellipse, 0.2)); + Base::Vector3d p1(getPointAtNormalizedParameter(ellipse, 0.5)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + int geoId = getObject()->addGeometry(&ellipse); + + // Act + int result = getObject()->trim(geoId, trimPoint); + // remove all internal geometry + for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); + } + + // Assert + EXPECT_EQ(result, 0); + // Once this ellipse is trimmed, the ellipse should be deleted. + // Only remaining: line segment + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); +} + +TEST_F(SketchObjectTest, testTrimEllipseMid) +{ + // Arrange + Part::GeomEllipse ellipse; + setupEllipse(ellipse); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(ellipse, 0.5)); + Base::Vector3d p1(getPointAtNormalizedParameter(ellipse, 0.3)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + Base::Vector3d p3(getPointAtNormalizedParameter(ellipse, 0.7)); + Base::Vector3d p4(p3.x + 0.1, p3.y + 0.1, p3.z); + // to ensure that this line clearly intersects the curve, not just have a point on object + // without explicit constraint + p3.x -= 0.1; + p3.y -= 0.1; + Part::GeomLineSegment lineSegCut2; + lineSegCut2.setPoints(p3, p4); + getObject()->addGeometry(&lineSegCut2); + int geoId = getObject()->addGeometry(&ellipse); + // FIXME: Doing this to avoid trimming only until minor/major axes. Should not be needed. + getObject()->deleteUnusedInternalGeometry(geoId); + + // Act + int result = getObject()->trim(geoId, trimPoint); + // remove all internal geometry + for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); + } + + // Assert + EXPECT_EQ(result, 0); + // Once this ellipse is trimmed, there should be one arc and line segments. + EXPECT_EQ(getObject()->getHighestCurveIndex(), 2); + // There should be one "coincident" and one "point-on-object" constraint on the intersecting + // curves + int numberOfPointOnObjectConstraints = + countConstraintsOfType(getObject(), Sketcher::PointOnObject); + EXPECT_EQ(numberOfPointOnObjectConstraints, 1); + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); + // TODO: Ensure shape is preserved +} + +// TODO: Tests for other arcs of conics? + +TEST_F(SketchObjectTest, testTrimPeriodicBSplineEnd) +{ + // Arrange + auto periodicBSpline = createTypicalPeriodicBSpline(); + assert(periodicBSpline); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(*periodicBSpline, 0.2)); + Base::Vector3d p1(getPointAtNormalizedParameter(*periodicBSpline, 0.5)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + int geoId = getObject()->addGeometry(periodicBSpline.get()); + + // Act + int result = getObject()->trim(geoId, trimPoint); + + // Assert + EXPECT_EQ(result, 0); + // FIXME: This will fail because of deleted internal geometry + // Once this periodicBSpline is trimmed, the periodicBSpline should be deleted, leaving only the + // line segment. + EXPECT_EQ(getObject()->getHighestCurveIndex(), 0); + // TODO: There should be a "point-on-object" constraint on the intersecting curves +} + +TEST_F(SketchObjectTest, testTrimPeriodicBSplineMid) +{ + // Arrange + auto periodicBSpline = createTypicalPeriodicBSpline(); + assert(periodicBSpline); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(*periodicBSpline, 0.5)); + Base::Vector3d p1(getPointAtNormalizedParameter(*periodicBSpline, 0.3)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + Base::Vector3d p3(getPointAtNormalizedParameter(*periodicBSpline, 0.7)); + Base::Vector3d p4(p3.x + 0.1, p3.y + 0.1, p3.z); + // to ensure that this line clearly intersects the curve, not just have a point on object + // without explicit constraint + p3.x -= 0.1; + p3.y -= 0.1; + Part::GeomLineSegment lineSegCut2; + lineSegCut2.setPoints(p3, p4); + getObject()->addGeometry(&lineSegCut2); + int geoId = getObject()->addGeometry(periodicBSpline.get()); + + // Act + int result = getObject()->trim(geoId, trimPoint); + // remove all internal geometry + for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); + } + + // Assert + EXPECT_EQ(result, 0); + // Only remaining: Two line segments and the B-spline + EXPECT_EQ(getObject()->getHighestCurveIndex(), 2); + // There should be one "coincident" and one "point-on-object" constraint on the intersecting + // curves + int numberOfPointOnObjectConstraints = + countConstraintsOfType(getObject(), Sketcher::PointOnObject); + EXPECT_EQ(numberOfPointOnObjectConstraints, 1); + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); + // TODO: Ensure shape is preserved +} + +TEST_F(SketchObjectTest, testTrimNonPeriodicBSplineEnd) +{ + // This should also cover as a representative of arc of conic + + // Arrange + auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); + assert(nonPeriodicBSpline); + // create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(*nonPeriodicBSpline, 0.2)); + Base::Vector3d p1(getPointAtNormalizedParameter(*nonPeriodicBSpline, 0.5)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); + + // Act + int result = getObject()->trim(geoId, trimPoint); + // remove all internal geometry + for (int iterGeoId = 0; iterGeoId < getObject()->getHighestCurveIndex(); ++iterGeoId) { + getObject()->deleteUnusedInternalGeometryAndUpdateGeoId(iterGeoId); + } + + // Assert + EXPECT_EQ(result, 0); + // Only remaining: one line segment and the trimmed B-spline + EXPECT_EQ(getObject()->getHighestCurveIndex(), 1); + // FIXME: There should be a "point-on-object" constraint on the intersecting curves + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); +} + +TEST_F(SketchObjectTest, testTrimNonPeriodicBSplineMid) +{ + // Arrange + auto nonPeriodicBSpline = createTypicalNonPeriodicBSpline(); + assert(nonPeriodicBSpline); + // TODO: create curves intersecting at the right spots + Base::Vector3d trimPoint(getPointAtNormalizedParameter(*nonPeriodicBSpline, 0.5)); + Base::Vector3d p1(getPointAtNormalizedParameter(*nonPeriodicBSpline, 0.3)); + Base::Vector3d p2(p1.x + 0.1, p1.y + 0.1, p1.z); + Part::GeomLineSegment lineSegCut1; + lineSegCut1.setPoints(p1, p2); + getObject()->addGeometry(&lineSegCut1); + Base::Vector3d p3(getPointAtNormalizedParameter(*nonPeriodicBSpline, 0.7)); + Base::Vector3d p4(p3.x + 0.1, p3.y + 0.1, p3.z); + // to ensure that this line clearly intersects the curve, not just have a point on object + // without explicit constraint + p3.x -= 0.1; + p3.y -= 0.1; + Part::GeomLineSegment lineSegCut2; + lineSegCut2.setPoints(p3, p4); + getObject()->addGeometry(&lineSegCut2); + int geoId = getObject()->addGeometry(nonPeriodicBSpline.get()); + + // Act + int result = getObject()->trim(geoId, trimPoint); + // remove all internal geometry + for (int i = 0; i < getObject()->getHighestCurveIndex(); ++i) { + if (getObject()->getGeometry(i)->is()) { + getObject()->deleteUnusedInternalGeometry(i); + } + } + + // Assert + EXPECT_EQ(result, 0); + // Only remaining: one line segment and the trimmed B-spline + EXPECT_EQ(getObject()->getHighestCurveIndex(), 3); + // There should be a "point-on-object" constraint on the intersecting curves + int numberOfPointOnObjectConstraints = + countConstraintsOfType(getObject(), Sketcher::PointOnObject); + EXPECT_EQ(numberOfPointOnObjectConstraints, 1); + int numberOfCoincidentConstraints = countConstraintsOfType(getObject(), Sketcher::Coincident); + EXPECT_EQ(numberOfCoincidentConstraints, 1); + // 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; + })); +} diff --git a/tests/src/Mod/Sketcher/App/SketcherTestHelpers.cpp b/tests/src/Mod/Sketcher/App/SketcherTestHelpers.cpp new file mode 100644 index 0000000000..ae4fb2414e --- /dev/null +++ b/tests/src/Mod/Sketcher/App/SketcherTestHelpers.cpp @@ -0,0 +1,155 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include +#include +#include +#include +#include +#include +#include "SketcherTestHelpers.h" + +void SketchObjectTest::SetUpTestSuite() +{ + tests::initApplication(); +} + +void SketchObjectTest::SetUp() +{ + _docName = App::GetApplication().getUniqueDocumentName("test"); + auto _doc = App::GetApplication().newDocument(_docName.c_str(), "testUser"); + // TODO: Do we add a body newName, or is just adding sketch sufficient for this test? + _sketchobj = static_cast(_doc->addObject("Sketcher::SketchObject")); +} + +void SketchObjectTest::TearDown() +{ + App::GetApplication().closeDocument(_docName.c_str()); +} + +Sketcher::SketchObject* SketchObjectTest::getObject() +{ + return _sketchobj; +} + +namespace SketcherTestHelpers +{ + +using namespace Sketcher; + +void setupLineSegment(Part::GeomLineSegment& lineSeg) +{ + Base::Vector3d coords1(1.0, 2.0, 0.0); + Base::Vector3d coords2(3.0, 4.0, 0.0); + lineSeg.setPoints(coords1, coords2); +} + +void setupCircle(Part::GeomCircle& circle) +{ + Base::Vector3d coordsCenter(1.0, 2.0, 0.0); + Base::Vector3d splitPoint(2.0, 3.1, 0.0); + double radius = 3.0; + circle.setCenter(coordsCenter); + circle.setRadius(radius); +} + +void setupArcOfCircle(Part::GeomArcOfCircle& arcOfCircle) +{ + Base::Vector3d coordsCenter(1.0, 2.0, 0.0); + double radius = 3.0; + double startParam = M_PI / 3, endParam = M_PI * 1.5; + arcOfCircle.setCenter(coordsCenter); + arcOfCircle.setRadius(radius); + arcOfCircle.setRange(startParam, endParam, true); +} + +void setupEllipse(Part::GeomEllipse& ellipse) +{ + Base::Vector3d coordsCenter(1.0, 2.0, 0.0); + double majorRadius = 4.0; + double minorRadius = 3.0; + ellipse.setCenter(coordsCenter); + ellipse.setMajorRadius(majorRadius); + 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); + double focal = 3.0; + double startParam = -M_PI * 1.5, endParam = M_PI * 1.5; + aop.setCenter(coordsCenter); + aop.setFocal(focal); + aop.setRange(startParam, endParam, true); +} + +std::unique_ptr createTypicalNonPeriodicBSpline() +{ + int degree = 3; + std::vector poles; + poles.emplace_back(1, 0, 0); + poles.emplace_back(1, 1, 0); + poles.emplace_back(1, 0.5, 0); + poles.emplace_back(0, 1, 0); + poles.emplace_back(0, 0, 0); + std::vector weights(5, 1.0); + std::vector knotsNonPeriodic = {0.0, 1.0, 2.0}; + std::vector multiplicitiesNonPeriodic = {degree + 1, 1, degree + 1}; + return std::make_unique(poles, + weights, + knotsNonPeriodic, + multiplicitiesNonPeriodic, + degree, + false); +} + +std::unique_ptr createTypicalPeriodicBSpline() +{ + int degree = 3; + std::vector poles; + poles.emplace_back(1, 0, 0); + poles.emplace_back(1, 1, 0); + poles.emplace_back(1, 0.5, 0); + poles.emplace_back(0, 1, 0); + poles.emplace_back(0, 0, 0); + std::vector weights(5, 1.0); + std::vector knotsPeriodic = {0.0, 0.3, 1.0, 1.5, 1.8, 2.0}; + std::vector multiplicitiesPeriodic(6, 1); + return std::make_unique(poles, + weights, + knotsPeriodic, + multiplicitiesPeriodic, + degree, + true); +} + +int countConstraintsOfType(const Sketcher::SketchObject* obj, const Sketcher::ConstraintType cType) +{ + const std::vector& constraints = obj->Constraints.getValues(); + + int result = std::count_if(constraints.begin(), + constraints.end(), + [&cType](const Sketcher::Constraint* constr) { + return constr->Type == cType; + }); + + return result; +} + +Base::Vector3d getPointAtNormalizedParameter(const Part::GeomCurve& curve, double param) +{ + return curve.pointAtParameter(curve.getFirstParameter() + + (curve.getLastParameter() - curve.getFirstParameter()) * param); +} +} // namespace SketcherTestHelpers diff --git a/tests/src/Mod/Sketcher/App/SketcherTestHelpers.h b/tests/src/Mod/Sketcher/App/SketcherTestHelpers.h index e03655e628..5047a3962a 100644 --- a/tests/src/Mod/Sketcher/App/SketcherTestHelpers.h +++ b/tests/src/Mod/Sketcher/App/SketcherTestHelpers.h @@ -1,5 +1,10 @@ // SPDX-License-Identifier: LGPL-2.1-or-later +#include + +#include + + #include #include #include @@ -8,128 +13,51 @@ #include #include +class SketchObjectTest: public ::testing::Test +{ +protected: + static void SetUpTestSuite(); + void SetUp() override; + void TearDown() override; + Sketcher::SketchObject* getObject(); + +private: + // TODO: use shared_ptr or something else here? + Sketcher::SketchObject* _sketchobj; + std::string _docName; + std::vector allowedTypes {"Vertex", + "Edge", + "ExternalEdge", + "H_Axis", + "V_Axis", + "RootPoint"}; +}; + namespace SketcherTestHelpers { using namespace Sketcher; -void setupLineSegment(Part::GeomLineSegment& lineSeg) -{ - Base::Vector3d coords1(1.0, 2.0, 0.0); - Base::Vector3d coords2(3.0, 4.0, 0.0); - lineSeg.setPoints(coords1, coords2); -} +void setupLineSegment(Part::GeomLineSegment& lineSeg); -void setupCircle(Part::GeomCircle& circle) -{ - Base::Vector3d coordsCenter(1.0, 2.0, 0.0); - Base::Vector3d splitPoint(2.0, 3.1, 0.0); - double radius = 3.0; - circle.setCenter(coordsCenter); - circle.setRadius(radius); -} +void setupCircle(Part::GeomCircle& circle); -void setupArcOfCircle(Part::GeomArcOfCircle& arcOfCircle) -{ - Base::Vector3d coordsCenter(1.0, 2.0, 0.0); - double radius = 3.0; - double startParam = M_PI / 3, endParam = M_PI * 1.5; - arcOfCircle.setCenter(coordsCenter); - arcOfCircle.setRadius(radius); - arcOfCircle.setRange(startParam, endParam, true); -} +void setupArcOfCircle(Part::GeomArcOfCircle& arcOfCircle); -void setupEllipse(Part::GeomEllipse& ellipse) -{ - Base::Vector3d coordsCenter(1.0, 2.0, 0.0); - double majorRadius = 4.0; - double minorRadius = 3.0; - ellipse.setCenter(coordsCenter); - ellipse.setMajorRadius(majorRadius); - ellipse.setMinorRadius(minorRadius); -} +void setupEllipse(Part::GeomEllipse& ellipse); -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 setupArcOfHyperbola(Part::GeomArcOfHyperbola& arcOfHyperbola); -void setupArcOfParabola(Part::GeomArcOfParabola& aop) -{ - Base::Vector3d coordsCenter(1.0, 2.0, 0.0); - double focal = 3.0; - double startParam = -M_PI * 1.5, endParam = M_PI * 1.5; - aop.setCenter(coordsCenter); - aop.setFocal(focal); - aop.setRange(startParam, endParam, true); -} +void setupArcOfParabola(Part::GeomArcOfParabola& aop); -std::unique_ptr createTypicalNonPeriodicBSpline() -{ - int degree = 3; - std::vector poles; - poles.emplace_back(1, 0, 0); - poles.emplace_back(1, 1, 0); - poles.emplace_back(1, 0.5, 0); - poles.emplace_back(0, 1, 0); - poles.emplace_back(0, 0, 0); - std::vector weights(5, 1.0); - std::vector knotsNonPeriodic = {0.0, 1.0, 2.0}; - std::vector multiplicitiesNonPeriodic = {degree + 1, 1, degree + 1}; - return std::make_unique(poles, - weights, - knotsNonPeriodic, - multiplicitiesNonPeriodic, - degree, - false); -} +std::unique_ptr createTypicalNonPeriodicBSpline(); -std::unique_ptr createTypicalPeriodicBSpline() -{ - int degree = 3; - std::vector poles; - poles.emplace_back(1, 0, 0); - poles.emplace_back(1, 1, 0); - poles.emplace_back(1, 0.5, 0); - poles.emplace_back(0, 1, 0); - poles.emplace_back(0, 0, 0); - std::vector weights(5, 1.0); - std::vector knotsPeriodic = {0.0, 0.3, 1.0, 1.5, 1.8, 2.0}; - std::vector multiplicitiesPeriodic(6, 1); - return std::make_unique(poles, - weights, - knotsPeriodic, - multiplicitiesPeriodic, - degree, - true); -} +std::unique_ptr createTypicalPeriodicBSpline(); -int countConstraintsOfType(const Sketcher::SketchObject* obj, const Sketcher::ConstraintType cType) -{ - const std::vector& constraints = obj->Constraints.getValues(); - - int result = std::count_if(constraints.begin(), - constraints.end(), - [&cType](const Sketcher::Constraint* constr) { - return constr->Type == cType; - }); - - return result; -} +int countConstraintsOfType(const Sketcher::SketchObject* obj, const Sketcher::ConstraintType cType); // Get point at the parameter after scaling the range to [0, 1]. -Base::Vector3d getPointAtNormalizedParameter(const Part::GeomCurve& curve, double param) -{ - return curve.pointAtParameter(curve.getFirstParameter() - + (curve.getLastParameter() - curve.getFirstParameter()) * param); -} +Base::Vector3d getPointAtNormalizedParameter(const Part::GeomCurve& curve, double param); // TODO: How to set up B-splines here? // It's not straightforward to change everything from a "default" one. From ab2e4adb3155737f85fa287990477e893af24d63 Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Sun, 12 Jan 2025 23:17:39 +0530 Subject: [PATCH 07/10] [Sketcher] Refactor `deleteUnusedInternalGeometryWhenBSpline` --- src/Mod/Sketcher/App/SketchObject.cpp | 69 ++++++++++----------------- 1 file changed, 26 insertions(+), 43 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 3beda7f640..b477ecec28 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -6775,12 +6775,9 @@ int SketchObject::deleteUnusedInternalGeometryWhenOneFocus(int GeoId, bool delge 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}); + std::map poleGeoIdsAndConstraints; + std::map knotGeoIdsAndConstraints; const std::vector& vals = Constraints.getValues(); @@ -6792,10 +6789,10 @@ int SketchObject::deleteUnusedInternalGeometryWhenBSpline(int GeoId, bool delgeo switch (constr->AlignmentType) { case Sketcher::BSplineControlPoint: - poleGeoIdsAndConstraints[constr->InternalAlignmentIndex].first = constr->First; + poleGeoIdsAndConstraints[constr->First] = 0; break; case Sketcher::BSplineKnotPoint: - knotGeoIdsAndConstraints[constr->InternalAlignmentIndex].first = constr->First; + knotGeoIdsAndConstraints[constr->First] = 0; break; default: return -1; @@ -6804,52 +6801,38 @@ int SketchObject::deleteUnusedInternalGeometryWhenBSpline(int GeoId, bool delgeo 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) { + // Update all control point constraint counts. + // EXCLUDES internal alignment and related constraints. + for (auto const& constr : vals) { + // 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 (constr->Type == Sketcher::InternalAlignment + || constr->Type == Sketcher::Weight) { 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. + bool firstIsInCPGeoIds = poleGeoIdsAndConstraints.count(constr->First) == 1; + bool secondIsInCPGeoIds = poleGeoIdsAndConstraints.count(constr->Second) == 1; + if (constr->Type == Sketcher::Equal && firstIsInCPGeoIds == secondIsInCPGeoIds) { + continue; } + // any equality constraint constraining a pole is not interpole + if (firstIsInCPGeoIds) { + ++poleGeoIdsAndConstraints[constr->First]; + } + if (secondIsInCPGeoIds) { + ++poleGeoIdsAndConstraints[constr->Second]; + } + } + for (auto& [cpGeoId, numConstr] : poleGeoIdsAndConstraints) { 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 + // Update all control point constraint counts. + // INCLUDES internal alignment and related constraints. auto tempGeoID = kGeoId; // C++17 and earlier do not support captured structured bindings numConstr = std::count_if(vals.begin(), vals.end(), [&tempGeoID](const auto& constr) { return constr->involvesGeoId(tempGeoID); From 0358ff7ce22d642a377de73d29e53a01839a5470 Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Sun, 22 Dec 2024 06:06:08 +0530 Subject: [PATCH 08/10] [Sketcher] Refactor `SketchObject::delConstraintOnPoint()` Note that for distance constraints we remove even if the constraint is not on the point. --- src/Mod/Sketcher/App/SketchObject.cpp | 198 ++++++++++++++------------ 1 file changed, 103 insertions(+), 95 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index b477ecec28..976907a530 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -2327,125 +2327,133 @@ int SketchObject::delConstraintOnPoint(int VertexId, bool onlyCoincident) return delConstraintOnPoint(GeoId, PosId, onlyCoincident); } -int SketchObject::delConstraintOnPoint(int GeoId, PointPos PosId, bool onlyCoincident) +// clang-format on +int SketchObject::delConstraintOnPoint(int geoId, PointPos posId, bool onlyCoincident) { // no need to check input data validity as this is an sketchobject managed operation. Base::StateLocker lock(managedoperation, true); const std::vector& vals = this->Constraints.getValues(); + std::vector newVals; + newVals.reserve(vals.size()); // check if constraints can be redirected to some other point int replaceGeoId = GeoEnum::GeoUndef; PointPos replacePosId = Sketcher::PointPos::none; - if (!onlyCoincident) { - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); ++it) { - if ((*it)->Type == Sketcher::Coincident) { - if ((*it)->First == GeoId && (*it)->FirstPos == PosId) { - replaceGeoId = (*it)->Second; - replacePosId = (*it)->SecondPos; - break; - } - else if ((*it)->Second == GeoId && (*it)->SecondPos == PosId) { - replaceGeoId = (*it)->First; - replacePosId = (*it)->FirstPos; - break; - } - } + auto findReplacement = [geoId, posId, &replaceGeoId, &replacePosId, &vals]() { + auto it = std::find_if(vals.begin(), vals.end(), [geoId, posId](auto& constr) { + return constr->Type == Sketcher::Coincident + && constr->involvesGeoIdAndPosId(geoId, posId); + }); + + if (it == vals.end()) { + return; } - } + + if ((*it)->First == geoId && (*it)->FirstPos == posId) { + replaceGeoId = (*it)->Second; + replacePosId = (*it)->SecondPos; + } + else { + replaceGeoId = (*it)->First; + replacePosId = (*it)->FirstPos; + } + }; + + auto transferToReplacement = + [&geoId, &posId, &replaceGeoId, &replacePosId](int& constrGeoId, PointPos& constrPosId) { + if (replaceGeoId == GeoEnum::GeoUndef) { + return false; + } + if (geoId != constrGeoId || posId != constrPosId) { + return false; + } + constrGeoId = replaceGeoId; + constrPosId = replacePosId; + return true; + }; + + findReplacement(); + + auto performCoincidenceChecksOrChanges = [&](auto& constr) -> std::optional { + if (constr->Type != Sketcher::Coincident) { + if (onlyCoincident) { + return true; + } + return std::nullopt; + } + bool firstSucceeded = transferToReplacement(constr->First, constr->FirstPos); + bool secondSucceeded = transferToReplacement(constr->Second, constr->SecondPos); + return (firstSucceeded || secondSucceeded) + && (constr->First != constr->Second || constr->FirstPos != constr->SecondPos); + }; + + auto performOtherConstraintChecksOrChanges = [&](auto& constr) -> std::optional { + switch (constr->Type) { + case Sketcher::Distance: + case Sketcher::DistanceX: + case Sketcher::DistanceY: { + return (transferToReplacement(constr->First, constr->FirstPos) + || transferToReplacement(constr->Second, constr->SecondPos)); + } + case Sketcher::PointOnObject: { + return transferToReplacement(constr->First, constr->FirstPos); + } + case Sketcher::Tangent: + case Sketcher::Perpendicular: { + // we could keep this constraint by converting it to a simple one, but that doesn't + // always work (for example if tangent-via-point is necessary), and it is not really + // worth it + return false; + } + case Sketcher::Vertical: + case Sketcher::Horizontal: + case Sketcher::Symmetric: { + return false; + } + default: + return true; + } + }; // remove or redirect any constraints associated with the given point - std::vector newVals(0); - for (std::vector::const_iterator it = vals.begin(); it != vals.end(); ++it) { - if ((*it)->Type == Sketcher::Coincident) { - if ((*it)->First == GeoId && (*it)->FirstPos == PosId) { - // redirect this constraint - if (replaceGeoId != GeoEnum::GeoUndef - && (replaceGeoId != (*it)->Second || replacePosId != (*it)->SecondPos)) { - (*it)->First = replaceGeoId; - (*it)->FirstPos = replacePosId; - } - else - continue;// skip this constraint + for (auto& constr : vals) { + // keep the constraint if it doesn't involve the point + if (constr->involvesGeoIdAndPosId(geoId, posId)) { + auto didCoincidenceCheckWork = performCoincidenceChecksOrChanges(constr); + if (didCoincidenceCheckWork == true) { + newVals.push_back(constr); + continue; } - else if ((*it)->Second == GeoId && (*it)->SecondPos == PosId) { - // redirect this constraint - if (replaceGeoId != GeoEnum::GeoUndef - && (replaceGeoId != (*it)->First || replacePosId != (*it)->FirstPos)) { - (*it)->Second = replaceGeoId; - (*it)->SecondPos = replacePosId; - } - else - continue;// skip this constraint + else if (didCoincidenceCheckWork == false) { + continue; + } + // The check failed, which means it's not a coincidence constraint and + // `onlyCoincident` is `false` + if (performOtherConstraintChecksOrChanges(constr) == false) { + continue; } } - else if (!onlyCoincident) { - if ((*it)->Type == Sketcher::Distance || (*it)->Type == Sketcher::DistanceX - || (*it)->Type == Sketcher::DistanceY) { - if ((*it)->First == GeoId && (*it)->FirstPos == PointPos::none - && (PosId == PointPos::start || PosId == PointPos::end)) { - // remove the constraint even if it is not directly associated - // with the given point - continue;// skip this constraint - } - else if ((*it)->First == GeoId && (*it)->FirstPos == PosId) { - if (replaceGeoId != GeoEnum::GeoUndef) {// redirect this constraint - (*it)->First = replaceGeoId; - (*it)->FirstPos = replacePosId; - } - else - continue;// skip this constraint - } - else if ((*it)->Second == GeoId && (*it)->SecondPos == PosId) { - if (replaceGeoId != GeoEnum::GeoUndef) {// redirect this constraint - (*it)->Second = replaceGeoId; - (*it)->SecondPos = replacePosId; - } - else - continue;// skip this constraint - } - } - else if ((*it)->Type == Sketcher::PointOnObject) { - if ((*it)->First == GeoId && (*it)->FirstPos == PosId) { - if (replaceGeoId != GeoEnum::GeoUndef) {// redirect this constraint - (*it)->First = replaceGeoId; - (*it)->FirstPos = replacePosId; - } - else - continue;// skip this constraint - } - } - else if ((*it)->Type == Sketcher::Tangent || (*it)->Type == Sketcher::Perpendicular) { - if (((*it)->First == GeoId && (*it)->FirstPos == PosId) - || ((*it)->Second == GeoId && (*it)->SecondPos == PosId)) { - // we could keep the tangency constraint by converting it - // to a simple one but it is not really worth - continue;// skip this constraint - } - } - else if ((*it)->Type == Sketcher::Symmetric) { - if (((*it)->First == GeoId && (*it)->FirstPos == PosId) - || ((*it)->Second == GeoId && (*it)->SecondPos == PosId)) { - continue;// skip this constraint - } - } - else if ((*it)->Type == Sketcher::Vertical || (*it)->Type == Sketcher::Horizontal) { - if (((*it)->First == GeoId && (*it)->FirstPos == PosId) - || ((*it)->Second == GeoId && (*it)->SecondPos == PosId)) { - continue;// skip this constraint - } - } + // for these constraints remove the constraint even if it is not directly associated with + // the given point + if ((constr->Type == Sketcher::Distance || constr->Type == Sketcher::DistanceX + || constr->Type == Sketcher::DistanceY) + && (constr->First == geoId && constr->FirstPos == PointPos::none + && (posId == PointPos::start || posId == PointPos::end))) { + continue; } - newVals.push_back(*it); + newVals.push_back(constr); } + if (newVals.size() < vals.size()) { this->Constraints.setValues(std::move(newVals)); return 0; } - return -1;// no such constraint + return -1; // no such constraint } +// clang-format off void SketchObject::transferFilletConstraints(int geoId1, PointPos posId1, int geoId2, PointPos posId2) From 9238a7c1fd1f70019ca60203fb4ce6e2c910e2c4 Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Wed, 8 Jan 2025 10:44:03 +0530 Subject: [PATCH 09/10] [Sketcher] More refactor of `delConstraintOnPoint` --- src/Mod/Sketcher/App/SketchObject.cpp | 60 +++++++++++++-------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 976907a530..43463377b9 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -107,6 +107,7 @@ #include +#include "GeoEnum.h" #include "SketchObject.h" #include "SketchObjectPy.h" #include "SolverGeometryExtension.h" @@ -2375,21 +2376,25 @@ int SketchObject::delConstraintOnPoint(int geoId, PointPos posId, bool onlyCoinc findReplacement(); - auto performCoincidenceChecksOrChanges = [&](auto& constr) -> std::optional { - if (constr->Type != Sketcher::Coincident) { - if (onlyCoincident) { - return true; - } - return std::nullopt; + auto performCoincidenceChecksOrChanges = [&](auto& constr) -> bool { + if (replaceGeoId == GeoEnum::GeoUndef) { + return false; } - bool firstSucceeded = transferToReplacement(constr->First, constr->FirstPos); - bool secondSucceeded = transferToReplacement(constr->Second, constr->SecondPos); - return (firstSucceeded || secondSucceeded) - && (constr->First != constr->Second || constr->FirstPos != constr->SecondPos); + if (constr->involvesGeoIdAndPosId(replaceGeoId, replacePosId)) { + return false; + } + // Assuming `constr` already involves geoId and posId, all conditions are already met + constr->substituteIndexAndPos(geoId, posId, replaceGeoId, replacePosId); + return true; }; - auto performOtherConstraintChecksOrChanges = [&](auto& constr) -> std::optional { + auto performAllConstraintChecksOrChanges = [&](auto& constr) -> std::optional { + if (constr->Type != Sketcher::Coincident && onlyCoincident) { + return true; + } switch (constr->Type) { + case Sketcher::Coincident: + return performCoincidenceChecksOrChanges(constr); case Sketcher::Distance: case Sketcher::DistanceX: case Sketcher::DistanceY: { @@ -2412,37 +2417,28 @@ int SketchObject::delConstraintOnPoint(int geoId, PointPos posId, bool onlyCoinc return false; } default: - return true; + return std::nullopt; } }; // remove or redirect any constraints associated with the given point for (auto& constr : vals) { // keep the constraint if it doesn't involve the point - if (constr->involvesGeoIdAndPosId(geoId, posId)) { - auto didCoincidenceCheckWork = performCoincidenceChecksOrChanges(constr); - if (didCoincidenceCheckWork == true) { - newVals.push_back(constr); + if (!constr->involvesGeoIdAndPosId(geoId, posId)) { + // for these constraints remove the constraint even if it is not directly associated + // with the given point + if ((constr->Type == Sketcher::Distance || constr->Type == Sketcher::DistanceX + || constr->Type == Sketcher::DistanceY) + && (constr->First == geoId && constr->FirstPos == PointPos::none + && (posId == PointPos::start || posId == PointPos::end))) { continue; } - else if (didCoincidenceCheckWork == false) { - continue; - } - // The check failed, which means it's not a coincidence constraint and - // `onlyCoincident` is `false` - if (performOtherConstraintChecksOrChanges(constr) == false) { - continue; - } - } - // for these constraints remove the constraint even if it is not directly associated with - // the given point - if ((constr->Type == Sketcher::Distance || constr->Type == Sketcher::DistanceX - || constr->Type == Sketcher::DistanceY) - && (constr->First == geoId && constr->FirstPos == PointPos::none - && (posId == PointPos::start || posId == PointPos::end))) { + newVals.push_back(constr); continue; } - newVals.push_back(constr); + if (performAllConstraintChecksOrChanges(constr) != false) { + newVals.push_back(constr); + } } if (newVals.size() < vals.size()) { From 80fa694270b679a74207ff7eb7f806b79fe9d88c Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Wed, 29 Jan 2025 12:05:33 +0100 Subject: [PATCH 10/10] [Sketcher] Incorporate suggestions by hyarion from #18916 --- src/Mod/Sketcher/App/SketchObject.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 43463377b9..95e3689dda 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -2427,16 +2427,18 @@ int SketchObject::delConstraintOnPoint(int geoId, PointPos posId, bool onlyCoinc if (!constr->involvesGeoIdAndPosId(geoId, posId)) { // for these constraints remove the constraint even if it is not directly associated // with the given point - if ((constr->Type == Sketcher::Distance || constr->Type == Sketcher::DistanceX - || constr->Type == Sketcher::DistanceY) - && (constr->First == geoId && constr->FirstPos == PointPos::none - && (posId == PointPos::start || posId == PointPos::end))) { + const bool isOneOfDistanceTypes = constr->Type == Sketcher::Distance + || constr->Type == Sketcher::DistanceX || constr->Type == Sketcher::DistanceY; + const bool involvesEntireCurve = + constr->First == geoId && constr->FirstPos == PointPos::none; + const bool isPosAnEndpoint = posId == PointPos::start || posId == PointPos::end; + if (isOneOfDistanceTypes && involvesEntireCurve && isPosAnEndpoint) { continue; } newVals.push_back(constr); continue; } - if (performAllConstraintChecksOrChanges(constr) != false) { + if (performAllConstraintChecksOrChanges(constr).value_or(true)) { newVals.push_back(constr); } } @@ -3693,7 +3695,7 @@ bool getParamLimitsOfNewGeosForTrim(const SketchObject* obj, std::array& cutPoints, std::vector>& paramsOfNewGeos) { - const auto* geoAsCurve = static_cast(obj->getGeometry(GeoId)); + const auto* geoAsCurve = obj->getGeometry(GeoId); double firstParam = geoAsCurve->getFirstParameter(); double lastParam = geoAsCurve->getLastParameter(); double cut0Param {firstParam}, cut1Param {lastParam}; @@ -3835,7 +3837,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) // Remove internal geometry beforehand for now // FIXME: we should be able to transfer these to new curves smoothly // auto geo = getGeometry(GeoId); - const auto* geoAsCurve = static_cast(getGeometry(GeoId)); + const auto* geoAsCurve = getGeometry(GeoId); //******************* Step A => Detection of intersection - Common to all Geometries //****************************************// @@ -3916,7 +3918,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) std::remove_if( idsOfOldConstraints.begin(), idsOfOldConstraints.end(), - [this, &GeoId, &allConstraints](const auto& i) { + [&GeoId, &allConstraints](const auto& i) { return (allConstraints[i]->involvesGeoIdAndPosId(GeoId, PointPos::start)); }), idsOfOldConstraints.end()); @@ -3925,7 +3927,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) idsOfOldConstraints.erase( std::remove_if(idsOfOldConstraints.begin(), idsOfOldConstraints.end(), - [this, &GeoId, &allConstraints](const auto& i) { + [&GeoId, &allConstraints](const auto& i) { return (allConstraints[i]->involvesGeoIdAndPosId(GeoId, PointPos::end)); }), @@ -3935,7 +3937,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) idsOfOldConstraints.erase( std::remove_if(idsOfOldConstraints.begin(), idsOfOldConstraints.end(), - [this, &GeoId, &allConstraints](const auto& i) { + [&GeoId, &allConstraints](const auto& i) { return (allConstraints[i]->involvesGeoIdAndPosId(GeoId, PointPos::mid)); }), idsOfOldConstraints.end());