From 841c328eb3c120cfe27a965030d04aae576f8e22 Mon Sep 17 00:00:00 2001 From: Abdullah Tahiri Date: Sat, 24 Apr 2021 14:31:18 +0200 Subject: [PATCH] Sketcher: Changes to split edge functionality ============================================= This commit is directed to external functionality of the split() function. 1. getAppliedConstraints renamed to getConstraintIndices This is just for clarity being a general function 2. SwapInvolvedGeometry functionality moved to Constraint class Why? i. Because it is a specific operation on a constraint, it must not be a public function, as it does not define interface of the Sketch. ii. It could be a lambda or a private utility function, but them it would not be reusable. iii. It could be part of a helper class, but then, it is would be less reusable. 3. renaming of the flag passed to transferConstraints function --- src/Mod/Sketcher/App/Constraint.cpp | 13 +++++++++++ src/Mod/Sketcher/App/Constraint.h | 8 ++++++- src/Mod/Sketcher/App/SketchObject.cpp | 27 ++++++---------------- src/Mod/Sketcher/App/SketchObject.h | 9 +++----- src/Mod/Sketcher/App/SketchObjectPyImp.cpp | 2 +- 5 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/Mod/Sketcher/App/Constraint.cpp b/src/Mod/Sketcher/App/Constraint.cpp index 1b00d25061..82eecc011c 100644 --- a/src/Mod/Sketcher/App/Constraint.cpp +++ b/src/Mod/Sketcher/App/Constraint.cpp @@ -217,3 +217,16 @@ void Constraint::Restore(XMLReader &reader) if (reader.hasAttribute("IsActive")) isActive = reader.getAttributeAsInteger("IsActive") ? true : false; } + +void Constraint::substituteIndex(int fromGeoId, int toGeoId) +{ + if (this->First == fromGeoId) { + this->First = toGeoId; + } + if (this->Second == fromGeoId) { + this->Second = toGeoId; + } + if (this->Third == fromGeoId) { + this->Third = toGeoId; + } +} diff --git a/src/Mod/Sketcher/App/Constraint.h b/src/Mod/Sketcher/App/Constraint.h index 6e526030cc..03a8bcdc47 100644 --- a/src/Mod/Sketcher/App/Constraint.h +++ b/src/Mod/Sketcher/App/Constraint.h @@ -82,7 +82,10 @@ enum InternalAlignmentType { * complex geometries like parabola focus or b-spline knots use InternalAlignment constraints * in addition to PointPos. */ -enum PointPos { none, start, end, mid }; +enum PointPos { none = 0, + start = 1, + end = 2, + mid = 3 }; class SketcherExport Constraint : public Base::Persistence { @@ -127,6 +130,9 @@ public: Type == Radius || Type == Diameter || Type == Angle || Type == SnellsLaw || Type == Weight; } + /// utility function to swap the index in First/Second/Third of the provided constraint from the fromGeoId GeoId to toGeoId + void substituteIndex(int fromGeoId, int toGeoId); + friend class PropertyConstraintList; private: diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index e671585ecf..4d36e22d1e 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -1647,7 +1647,7 @@ void SketchObject::transferFilletConstraints(int geoId1, PointPos posId1, int ge this->Constraints.setValues(std::move(newConstraints)); } -int SketchObject::transferConstraints(int fromGeoId, PointPos fromPosId, int toGeoId, PointPos toPosId, bool tangencyHolds) +int SketchObject::transferConstraints(int fromGeoId, PointPos fromPosId, int toGeoId, PointPos toPosId, bool doNotTransformTangencies) { Base::StateLocker lock(managedoperation, true); // no need to check input data validity as this is an sketchobject managed operation. @@ -1667,7 +1667,7 @@ int SketchObject::transferConstraints(int fromGeoId, PointPos fromPosId, int toG // point, as the transfer destination edge most likely won't be intended to be tangent. However, if it is // an end to end point tangency, the user expects it to be substituted by a coincidence constraint. if (vals[i]->Type == Sketcher::Tangent || vals[i]->Type == Sketcher::Perpendicular) { - if (!tangencyHolds) { + if (!doNotTransformTangencies) { constNew->Type = Sketcher::Coincident; } } @@ -1696,7 +1696,7 @@ int SketchObject::transferConstraints(int fromGeoId, PointPos fromPosId, int toG // point, as the transfer destination edge most likely won't be intended to be tangent. However, if it is // an end to end point tangency, the user expects it to be substituted by a coincidence constraint. if (vals[i]->Type == Sketcher::Tangent || vals[i]->Type == Sketcher::Perpendicular) { - if (!tangencyHolds) { + if (!doNotTransformTangencies) { constNew->Type = Sketcher::Coincident; } } @@ -1717,19 +1717,6 @@ int SketchObject::transferConstraints(int fromGeoId, PointPos fromPosId, int toG return 0; } -void SketchObject::swapInvolvedGeometry(Constraint *constraint, int fromGeoId, int toGeoId) -{ - if (constraint->First == fromGeoId) { - constraint->First = toGeoId; - } - if (constraint->Second == fromGeoId) { - constraint->Second = toGeoId; - } - if (constraint->Third == fromGeoId) { - constraint->Third = toGeoId; - } -} - int SketchObject::fillet(int GeoId, PointPos PosId, double radius, bool trim, bool createCorner) { if (GeoId < 0 || GeoId > getHighestCurveIndex()) @@ -3148,7 +3135,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) if (ok) { std::vector oldConstraints; - getAppliedConstraints(GeoId, oldConstraints); + getConstraintIndices(GeoId, oldConstraints); for (unsigned int i = 0; i < oldConstraints.size(); ++i) { @@ -3194,7 +3181,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) for (unsigned int i = initial; i < limit; ++i) { Constraint *trans = con->copy(); - swapInvolvedGeometry(trans, GeoId, newIds[i]); + trans->substituteIndex(GeoId, newIds[i]); newConstraints.push_back(trans); } break; @@ -3258,7 +3245,7 @@ int SketchObject::split(int GeoId, const Base::Vector3d &point) if (transferToAll) { for (unsigned int i = 0; i < newIds.size(); ++i) { Constraint *trans = con->copy(); - swapInvolvedGeometry(trans, GeoId, newIds[i]); + trans->substituteIndex(GeoId, newIds[i]); newConstraints.push_back(trans); } } @@ -7149,7 +7136,7 @@ bool SketchObject::arePointsCoincident(int GeoId1, PointPos PosId1, return false; } -void SketchObject::getAppliedConstraints(int GeoId, std::vector &constraintList) +void SketchObject::getConstraintIndices (int GeoId, std::vector &constraintList) { const std::vector &constraints = this->Constraints.getValues(); int i = 0; diff --git a/src/Mod/Sketcher/App/SketchObject.h b/src/Mod/Sketcher/App/SketchObject.h index 2d1def3f9f..5254669778 100644 --- a/src/Mod/Sketcher/App/SketchObject.h +++ b/src/Mod/Sketcher/App/SketchObject.h @@ -158,10 +158,7 @@ public: int delConstraintsToExternal(); /// transfers all constraints of a point to a new point int transferConstraints(int fromGeoId, PointPos fromPosId, int toGeoId, PointPos toPosId, - bool tangencyHolds = false); - - /// swaps original GeoId for a new one - void swapInvolvedGeometry(Constraint *constraint, int fromGeoId, int toGeoId); + bool doNotTransformTangencies = false); /// Carbon copy another sketch geometry and constraints int carbonCopy(App::DocumentObject * pObj, bool construction = true); @@ -365,8 +362,8 @@ public: void getDirectlyCoincidentPoints(int VertexId, std::vector &GeoIdList, std::vector &PosIdList); bool arePointsCoincident(int GeoId1, PointPos PosId1, int GeoId2, PointPos PosId2); - /// fetches all constraints involving given GeoId - void getAppliedConstraints(int GeoId, std::vector &constraintList); + /// returns a list of indices of all constraints involving given GeoId + void getConstraintIndices(int GeoId, std::vector &constraintList); /// generates a warning message about constraint conflicts and appends it to the given message static void appendConflictMsg(const std::vector &conflicting, std::string &msg); diff --git a/src/Mod/Sketcher/App/SketchObjectPyImp.cpp b/src/Mod/Sketcher/App/SketchObjectPyImp.cpp index 1162db047e..d466d72cf1 100644 --- a/src/Mod/Sketcher/App/SketchObjectPyImp.cpp +++ b/src/Mod/Sketcher/App/SketchObjectPyImp.cpp @@ -570,7 +570,7 @@ PyObject* SketchObjectPy::delConstraintOnPoint(PyObject *args) if (!PyArg_ParseTuple(args, "i|i", &Index, &pos)) return 0; - if (pos>=0 && pos<=3) { // Sketcher::none Sketcher::mid + if (pos>=Sketcher::none && pos<=Sketcher::mid) { // This is the whole range of valid positions if (this->getSketchObjectPtr()->delConstraintOnPoint(Index,(Sketcher::PointPos)pos)) { std::stringstream str; str << "Not able to delete a constraint on point with the given index: " << Index