From 57f4987ce3ecc5220425269f362740826a840658 Mon Sep 17 00:00:00 2001 From: theo-vt Date: Sun, 7 Sep 2025 12:03:15 -0400 Subject: [PATCH] Sketcher: fix invalid constraint on first dimension (#23024) * Sketcher: Scale: Reorder operations and delete original modified constraints to ensure validity * Sketcher: replace boolean parameters for deletion with enum and expose solver override on some deletion functions in the python API * Use correct flag in ::delGeometry * Set default value of false to noSolve * Sketcher: autoscale: use deleteAllGeometry * Sketcher: Scale: revert to checking constraints for geoId validity and handle horizontal&vertical --- src/Mod/Sketcher/App/SketchAnalysis.cpp | 2 +- src/Mod/Sketcher/App/SketchObject.cpp | 86 ++++++------ src/Mod/Sketcher/App/SketchObject.h | 47 +++++-- src/Mod/Sketcher/App/SketchObject.pyi | 22 +++- src/Mod/Sketcher/App/SketchObjectPyImp.cpp | 79 ++++++++++-- src/Mod/Sketcher/Gui/CommandSketcherTools.cpp | 7 +- src/Mod/Sketcher/Gui/CommandSketcherTools.h | 4 +- src/Mod/Sketcher/Gui/DrawSketchHandlerScale.h | 122 +++++++++++++----- src/Mod/Sketcher/Gui/EditDatumDialog.cpp | 8 +- 9 files changed, 277 insertions(+), 100 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchAnalysis.cpp b/src/Mod/Sketcher/App/SketchAnalysis.cpp index a0275bdd5d..a64bf6a560 100644 --- a/src/Mod/Sketcher/App/SketchAnalysis.cpp +++ b/src/Mod/Sketcher/App/SketchAnalysis.cpp @@ -622,7 +622,7 @@ void SketchAnalysis::solveSketch(const char* errorText) solvesketch(status, dofs, true); if (status == int(Solver::RedundantConstraints)) { - sketch->autoRemoveRedundants(false); + sketch->autoRemoveRedundants(DeleteOption::NoFlag); solvesketch(status, dofs, false); } diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 4c01a16039..8587ea5cc6 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -1771,7 +1771,7 @@ bool SketchObject::hasInternalGeometry(const Part::Geometry* geo) || geo->is()); } -int SketchObject::delGeometry(int GeoId, bool deleteinternalgeo) +int SketchObject::delGeometry(int GeoId, DeleteOptions options) { if (GeoId < 0) { if(GeoId > GeoEnum::RefExt) @@ -1787,7 +1787,7 @@ int SketchObject::delGeometry(int GeoId, bool deleteinternalgeo) return -1; } - if (deleteinternalgeo && hasInternalGeometry(getGeometry(GeoId))) { + if (options.testFlag(DeleteOption::IncludeInternalGeometry) && hasInternalGeometry(getGeometry(GeoId))) { // Only for supported types this->deleteUnusedInternalGeometry(GeoId, true); return 0; @@ -1828,19 +1828,20 @@ int SketchObject::delGeometry(int GeoId, bool deleteinternalgeo) Geometry.touch(); // if we do not have a recompute, the sketch must be solved to update the DoF of the solver - if (noRecomputes) - solve(); + if (noRecomputes && !options.testFlag(DeleteOption::NoSolve)) { + solve(options.testFlag(DeleteOption::UpdateGeometry)); + } return 0; } -int SketchObject::delGeometries(const std::vector& GeoIds) +int SketchObject::delGeometries(const std::vector& GeoIds, DeleteOptions options) { - return delGeometries(GeoIds.begin(), GeoIds.end()); + return delGeometries(GeoIds.begin(), GeoIds.end(), options); } template -int SketchObject::delGeometries(InputIt first, InputIt last) +int SketchObject::delGeometries(InputIt first, InputIt last, DeleteOptions options) { std::vector sGeoIds; std::vector negativeGeoIds; @@ -1885,23 +1886,25 @@ int SketchObject::delGeometries(InputIt first, InputIt last) auto newend = std::unique(sGeoIds.begin(), sGeoIds.end()); sGeoIds.resize(std::distance(sGeoIds.begin(), newend)); - return delGeometriesExclusiveList(sGeoIds); + return delGeometriesExclusiveList(sGeoIds, options); } -int SketchObject::delGeometriesExclusiveList(const std::vector& GeoIds) +int SketchObject::delGeometriesExclusiveList(const std::vector& GeoIds, DeleteOptions options) { std::vector sGeoIds(GeoIds); std::ranges::sort(sGeoIds); - if (sGeoIds.empty()) + if (sGeoIds.empty()) { return 0; + } // no need to check input data validity as this is an sketchobject managed operation. Base::StateLocker lock(managedoperation, true); const std::vector& vals = getInternalGeometry(); - if (sGeoIds.front() < 0 || sGeoIds.back() >= int(vals.size())) + if (sGeoIds.front() < 0 || sGeoIds.back() >= int(vals.size())) { return -1; + } std::vector newVals(vals); for (auto it = sGeoIds.rbegin(); it != sGeoIds.rend(); ++it) { @@ -1950,8 +1953,9 @@ int SketchObject::delGeometriesExclusiveList(const std::vector& GeoIds) Geometry.touch(); // if we do not have a recompute, the sketch must be solved to update the DoF of the solver - if (noRecomputes) - solve(); + if (noRecomputes && !options.testFlag(DeleteOption::NoSolve)) { + solve(options.testFlag(DeleteOption::UpdateGeometry)); + } return 0; } @@ -1990,7 +1994,7 @@ void SketchObject::replaceGeometries(std::vector oldGeoIds, Geometry.setValues(std::move(newVals)); } -int SketchObject::deleteAllGeometry() +int SketchObject::deleteAllGeometry(DeleteOptions options) { // no need to check input data validity as this is an sketchobject managed operation. Base::StateLocker lock(managedoperation, true); @@ -2009,13 +2013,14 @@ int SketchObject::deleteAllGeometry() Geometry.touch(); // if we do not have a recompute, the sketch must be solved to update the DoF of the solver - if (noRecomputes) - solve(); + if (noRecomputes && !options.testFlag(DeleteOption::NoSolve)) { + solve(options.testFlag(DeleteOption::UpdateGeometry)); + } return 0; } -int SketchObject::deleteAllConstraints() +int SketchObject::deleteAllConstraints(DeleteOptions options) { // no need to check input data validity as this is an sketchobject managed operation. Base::StateLocker lock(managedoperation, true); @@ -2025,8 +2030,9 @@ int SketchObject::deleteAllConstraints() this->Constraints.setValues(newConstraints); // if we do not have a recompute, the sketch must be solved to update the DoF of the solver - if (noRecomputes) - solve(); + if (noRecomputes && !options.testFlag(DeleteOption::NoSolve)) { + solve(options.testFlag(DeleteOption::UpdateGeometry)); + } return 0; } @@ -2307,14 +2313,15 @@ int SketchObject::addConstraint(std::unique_ptr constraint) return this->Constraints.getSize() - 1; } -int SketchObject::delConstraint(int ConstrId) +int SketchObject::delConstraint(int ConstrId, DeleteOptions options) { // 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(); - if (ConstrId < 0 || ConstrId >= int(vals.size())) + if (ConstrId < 0 || ConstrId >= int(vals.size())) { return -1; + } std::vector newVals(vals); auto ctriter = newVals.begin() + ConstrId; @@ -2323,18 +2330,20 @@ int SketchObject::delConstraint(int ConstrId) this->Constraints.setValues(std::move(newVals)); // if we do not have a recompute, the sketch must be solved to update the DoF of the solver - if (noRecomputes) - solve(); + if (noRecomputes && !options.testFlag(DeleteOption::NoSolve)) { + solve(options.testFlag(DeleteOption::UpdateGeometry)); + } return 0; } -int SketchObject::delConstraints(std::vector ConstrIds, bool updategeometry) +int SketchObject::delConstraints(std::vector ConstrIds, DeleteOptions options) { // no need to check input data validity as this is an sketchobject managed operation. Base::StateLocker lock(managedoperation, true); - if (ConstrIds.empty()) + if (ConstrIds.empty()) { return 0; + } const std::vector& vals = this->Constraints.getValues(); @@ -2354,8 +2363,9 @@ int SketchObject::delConstraints(std::vector ConstrIds, bool updategeometry this->Constraints.setValues(std::move(newVals)); // if we do not have a recompute, the sketch must be solved to update the DoF of the solver - if (noRecomputes) - solve(updategeometry); + if (noRecomputes && !options.testFlag(DeleteOption::NoSolve)) { + solve(options.testFlag(DeleteOption::UpdateGeometry)); + } return 0; } @@ -2524,7 +2534,7 @@ void SketchObject::transferFilletConstraints(int geoId1, PointPos posId1, int ge } } } - delConstraints(std::move(deleteme), false); + delConstraints(std::move(deleteme), DeleteOption::UpdateGeometry); return; } @@ -3486,7 +3496,7 @@ int SketchObject::trim(int GeoId, const Base::Vector3d& point) addConstraint(std::move(newConstr)); }; - delConstraints(std::move(idsOfOldConstraints), false); + delConstraints(std::move(idsOfOldConstraints), DeleteOption::UpdateGeometry); if (!isOriginalCurvePeriodic) { transferConstraints(GeoId, PointPos::start, newIds.front(), PointPos::start, true); @@ -6163,7 +6173,7 @@ int SketchObject::deleteUnusedInternalGeometryWhenTwoFoci(int GeoId, bool delgeo std::sort(delgeometries.begin(), delgeometries.end(), std::greater<>()); for (auto& dGeoId : delgeometries) { - delGeometry(dGeoId, false); + delGeometry(dGeoId, DeleteOption::UpdateGeometry); } int ndeleted = delgeometries.size(); @@ -6230,7 +6240,7 @@ int SketchObject::deleteUnusedInternalGeometryWhenOneFocus(int GeoId, bool delge std::sort(delgeometries.begin(), delgeometries.end(), std::greater<>()); for (auto& dGeoId : delgeometries) { - delGeometry(dGeoId, false); + delGeometry(dGeoId, DeleteOption::UpdateGeometry); } int ndeleted = delgeometries.size(); @@ -7391,7 +7401,7 @@ int SketchObject::delAllExternal() } // clang-format off -int SketchObject::delConstraintsToExternal() +int SketchObject::delConstraintsToExternal(DeleteOptions options) { // no need to check input data validity as this is an sketchobject managed operation. Base::StateLocker lock(managedoperation, true); @@ -7411,8 +7421,9 @@ int SketchObject::delConstraintsToExternal() Constraints.acceptGeometry(getCompleteGeometry()); // if we do not have a recompute, the sketch must be solved to update the DoF of the solver - if (noRecomputes) - solve(); + if (noRecomputes && !options.testFlag(DeleteOption::NoFlag)) { + solve(options.testFlag(DeleteOption::UpdateGeometry)); + } return 0; } @@ -11555,18 +11566,19 @@ int SketchObject::removeDegeneratedGeometries(double tolerance) return analyser->removeDegeneratedGeometries(tolerance); } -int SketchObject::autoRemoveRedundants(bool updategeo) +int SketchObject::autoRemoveRedundants(DeleteOptions options) { auto redundants = getLastRedundant(); - if (redundants.empty()) + if (redundants.empty()) { return 0; + } // getLastRedundant is base 1, while delConstraints is base 0 for (size_t i = 0; i < redundants.size(); i++) redundants[i]--; - delConstraints(redundants, updategeo); + delConstraints(redundants, options); return redundants.size(); } diff --git a/src/Mod/Sketcher/App/SketchObject.h b/src/Mod/Sketcher/App/SketchObject.h index 81e4252f77..6d1ca2a0a2 100644 --- a/src/Mod/Sketcher/App/SketchObject.h +++ b/src/Mod/Sketcher/App/SketchObject.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,23 @@ #include "SketchGeometryExtension.h" #include "ExternalGeometryExtension.h" +namespace Sketcher +{ +// Options for deleting geometries/constraints +enum class DeleteOption +{ + NoFlag = 0, + IncludeInternalGeometry = + 1, // Only makes sense when deleting a geometry - (default for deleting a single geometry) + UpdateGeometry = 2, // Should the solver update the geomtries ? (default) - has no effect if + // noRecompute is false + NoSolve = 4, // Can be useful if the call will do many operations and a single solve +}; +using DeleteOptions = Base::Flags; +} // namespace Sketcher + +ENABLE_BITMASK_OPERATORS(Sketcher::DeleteOption) + namespace Sketcher { @@ -152,17 +170,23 @@ public: \param deleteinternalgeo - if true deletes the associated and unconstraint internal geometry, otherwise deletes only the GeoId \retval int - 0 if successful */ - int delGeometry(int GeoId, bool deleteinternalgeo = true); + int delGeometry(int GeoId, + DeleteOptions options = DeleteOption::UpdateGeometry + | DeleteOption::IncludeInternalGeometry); /// Deletes just the GeoIds indicated, it does not look for internal geometry - int delGeometriesExclusiveList(const std::vector& GeoIds); + int delGeometriesExclusiveList(const std::vector& GeoIds, + DeleteOptions options = DeleteOption::UpdateGeometry); /// Does the same as \a delGeometry but allows one to delete several geometries in one step - int delGeometries(const std::vector& GeoIds); + int delGeometries(const std::vector& GeoIds, + DeleteOptions options = DeleteOption::UpdateGeometry); template - int delGeometries(InputIt first, InputIt last); + int delGeometries(InputIt first, + InputIt last, + DeleteOptions options = DeleteOption::UpdateGeometry); /// deletes all the elements/constraints of the sketch except for external geometry - int deleteAllGeometry(); + int deleteAllGeometry(DeleteOptions options = DeleteOption::UpdateGeometry); /// deletes all the constraints of the sketch - int deleteAllConstraints(); + int deleteAllConstraints(DeleteOptions options = DeleteOption::UpdateGeometry); /// add all constraints in the list int addConstraints(const std::vector& ConstraintList); /// Copy the constraints instead of cloning them and copying the expressions if any @@ -172,7 +196,7 @@ public: /// add constraint int addConstraint(std::unique_ptr constraint); /// delete constraint - int delConstraint(int ConstrId); + int delConstraint(int ConstrId, DeleteOptions options = DeleteOption::UpdateGeometry); /** deletes a group of constraints at once, if norecomputes is active, the default behaviour is * that it will solve the sketch. * @@ -180,12 +204,13 @@ public: * updategeometry=false, prevents the update. This allows one to update the solve status (e.g. * dof), without updating the geometry (i.e. make it move to fulfil the constraints). */ - int delConstraints(std::vector ConstrIds, bool updategeometry = true); + int delConstraints(std::vector ConstrIds, + DeleteOptions options = DeleteOption::UpdateGeometry); int delConstraintOnPoint(int GeoId, PointPos PosId, bool onlyCoincident = true); int delConstraintOnPoint(int VertexId, bool onlyCoincident = true); /// Deletes all constraints referencing an external geometry - int delConstraintsToExternal(); - /// transfers all constraints of a point to a new point + int delConstraintsToExternal(DeleteOptions options = DeleteOption::UpdateGeometry); + /// transfers all constraints of a point to a new int transferConstraints(int fromGeoId, PointPos fromPosId, int toGeoId, @@ -877,7 +902,7 @@ public: // helper /// returns the number of redundant constraints detected - int autoRemoveRedundants(bool updategeo = true); + int autoRemoveRedundants(DeleteOptions options = DeleteOption::UpdateGeometry); int renameConstraint(int GeoId, std::string name); diff --git a/src/Mod/Sketcher/App/SketchObject.pyi b/src/Mod/Sketcher/App/SketchObject.pyi index 86d2983c3e..93db13d14e 100644 --- a/src/Mod/Sketcher/App/SketchObject.pyi +++ b/src/Mod/Sketcher/App/SketchObject.pyi @@ -111,7 +111,7 @@ class SketchObject(Part2DObject): """ ... - def delGeometry(self, geoId: int) -> None: + def delGeometry(self, geoId: int, noSolve: bool) -> None: """ Delete a geometric object from the sketch. @@ -123,7 +123,7 @@ class SketchObject(Part2DObject): """ ... - def delGeometries(self, geoIds: List[int]) -> None: + def delGeometries(self, geoIds: List[int], noSolve: bool) -> None: """ Delete a list of geometric objects from the sketch. @@ -135,7 +135,7 @@ class SketchObject(Part2DObject): """ ... - def deleteAllGeometry(self) -> None: + def deleteAllGeometry(self, noSolve: bool) -> None: """ Delete all the geometry objects from the sketch, except external geometry. @@ -244,7 +244,7 @@ class SketchObject(Part2DObject): """ ... - def delConstraint(self, constraintIndex: int) -> None: + def delConstraint(self, constraintIndex: int, noSolve: bool) -> None: """ Delete a constraint from the sketch. @@ -255,6 +255,20 @@ class SketchObject(Part2DObject): """ ... + def delConstraints( + self, constraintIndices: List[int], updateGeometry: bool, noSolve: bool + ) -> None: + """ + Delete multiple constraints from a sketch + + delConstraints(constraintIndices: List[int], updateGeometry: bool) + + Args: + constraintIndices: The zero-based indices of the constraints to delete + updateGeometry: Wheter to update the geometry after solve + """ + ... + def renameConstraint(self, constraintIndex: int, name: str) -> None: """ Rename a constraint in the sketch. diff --git a/src/Mod/Sketcher/App/SketchObjectPyImp.cpp b/src/Mod/Sketcher/App/SketchObjectPyImp.cpp index a63e545775..451751ea33 100644 --- a/src/Mod/Sketcher/App/SketchObjectPyImp.cpp +++ b/src/Mod/Sketcher/App/SketchObjectPyImp.cpp @@ -198,11 +198,14 @@ PyObject* SketchObjectPy::addGeometry(PyObject* args) PyObject* SketchObjectPy::delGeometry(PyObject* args) { int Index; - if (!PyArg_ParseTuple(args, "i", &Index)) { + PyObject* noSolve = Py_False; + if (!PyArg_ParseTuple(args, "i|O!", &Index, &PyBool_Type, &noSolve)) { return nullptr; } - if (this->getSketchObjectPtr()->delGeometry(Index)) { + if (this->getSketchObjectPtr()->delGeometry( + Index, + Base::asBoolean(noSolve) ? DeleteOption::NoSolve : DeleteOption::UpdateGeometry)) { std::stringstream str; str << "Not able to delete a geometry with the given index: " << Index; PyErr_SetString(PyExc_ValueError, str.str().c_str()); @@ -215,8 +218,8 @@ PyObject* SketchObjectPy::delGeometry(PyObject* args) PyObject* SketchObjectPy::delGeometries(PyObject* args) { PyObject* pcObj; - - if (!PyArg_ParseTuple(args, "O", &pcObj)) { + PyObject* noSolve = Py_False; + if (!PyArg_ParseTuple(args, "O|O!", &pcObj, &PyBool_Type, &noSolve)) { return nullptr; } @@ -230,7 +233,9 @@ PyObject* SketchObjectPy::delGeometries(PyObject* args) } } - if (this->getSketchObjectPtr()->delGeometries(geoIdList)) { + if (this->getSketchObjectPtr()->delGeometries( + geoIdList, + Base::asBoolean(noSolve) ? DeleteOption::NoSolve : DeleteOption::UpdateGeometry)) { std::stringstream str; str << "Not able to delete geometries"; PyErr_SetString(PyExc_ValueError, str.str().c_str()); @@ -247,11 +252,13 @@ PyObject* SketchObjectPy::delGeometries(PyObject* args) PyObject* SketchObjectPy::deleteAllGeometry(PyObject* args) { - if (!PyArg_ParseTuple(args, "")) { + PyObject* noSolve = Py_False; + if (!PyArg_ParseTuple(args, "|O!", &PyBool_Type, &noSolve)) { return nullptr; } - if (this->getSketchObjectPtr()->deleteAllGeometry()) { + if (this->getSketchObjectPtr()->deleteAllGeometry( + Base::asBoolean(noSolve) ? DeleteOption::NoSolve : DeleteOption::UpdateGeometry)) { std::stringstream str; str << "Unable to delete Geometry"; PyErr_SetString(PyExc_ValueError, str.str().c_str()); @@ -435,11 +442,15 @@ PyObject* SketchObjectPy::addConstraint(PyObject* args) PyObject* SketchObjectPy::delConstraint(PyObject* args) { int Index; - if (!PyArg_ParseTuple(args, "i", &Index)) { + PyObject* noSolve = Py_False; + + if (!PyArg_ParseTuple(args, "i|O!", &Index, &PyBool_Type, &noSolve)) { return nullptr; } - if (this->getSketchObjectPtr()->delConstraint(Index)) { + if (this->getSketchObjectPtr()->delConstraint( + Index, + Base::asBoolean(noSolve) ? DeleteOption::NoSolve : DeleteOption::UpdateGeometry)) { std::stringstream str; str << "Not able to delete a constraint with the given index: " << Index; PyErr_SetString(PyExc_ValueError, str.str().c_str()); @@ -448,6 +459,53 @@ PyObject* SketchObjectPy::delConstraint(PyObject* args) Py_Return; } +PyObject* SketchObjectPy::delConstraints(PyObject* args) +{ + PyObject* pcObj; + PyObject* updateGeometry = Py_True; + PyObject* noSolve = Py_False; + + if (!PyArg_ParseTuple(args, + "O|O!O!", + &pcObj, + &PyBool_Type, + &updateGeometry, + &PyBool_Type, + &noSolve)) { + return nullptr; + } + + if (PyObject_TypeCheck(pcObj, &(PyList_Type)) || PyObject_TypeCheck(pcObj, &(PyTuple_Type))) { + + std::vector constraintIdList; + Py::Sequence list(pcObj); + for (Py::Sequence::iterator it = list.begin(); it != list.end(); ++it) { + if (PyLong_Check((*it).ptr())) { + constraintIdList.push_back(PyLong_AsLong((*it).ptr())); + } + } + + if (this->getSketchObjectPtr()->delConstraints( + constraintIdList, + (Base::asBoolean(updateGeometry) ? DeleteOption::UpdateGeometry + : DeleteOption::NoFlag) + | (Base::asBoolean(noSolve) ? DeleteOption::NoSolve : DeleteOption::NoFlag)) + == -1) { + std::stringstream str; + str << "Not able to delete constraints, invalid indices"; + PyErr_SetString(PyExc_ValueError, str.str().c_str()); + return nullptr; + } + + Py_Return; + } + + std::string error = std::string("type must be list of constraint indices (int), not "); + error += pcObj->ob_type->tp_name; + throw Py::TypeError(error); + + Py_Return; +} PyObject* SketchObjectPy::renameConstraint(PyObject* args) { @@ -2055,7 +2113,8 @@ PyObject* SketchObjectPy::autoRemoveRedundants(PyObject* args) return nullptr; } - this->getSketchObjectPtr()->autoRemoveRedundants(Base::asBoolean(updategeo)); + this->getSketchObjectPtr()->autoRemoveRedundants( + Base::asBoolean(updategeo) ? DeleteOption::UpdateGeometry : DeleteOption::NoFlag); Py_Return; } diff --git a/src/Mod/Sketcher/Gui/CommandSketcherTools.cpp b/src/Mod/Sketcher/Gui/CommandSketcherTools.cpp index 72535b7191..3a46309bd2 100644 --- a/src/Mod/Sketcher/Gui/CommandSketcherTools.cpp +++ b/src/Mod/Sketcher/Gui/CommandSketcherTools.cpp @@ -2533,14 +2533,11 @@ void CreateSketcherCommandsConstraintAccel() } // clang-format on -void SketcherGui::centerScale(Sketcher::SketchObject* Obj, double scaleFactor) +void SketcherGui::centerScale(double scaleFactor) { - std::vector allGeoIds(Obj->Geometry.getValues().size()); - std::iota(allGeoIds.begin(), allGeoIds.end(), 0); - Gui::Document* doc = Gui::Application::Instance->activeDocument(); auto* vp = static_cast(doc->getInEdit()); - auto scaler = DrawSketchHandlerScale::make_centerScale(allGeoIds, scaleFactor, false); + auto scaler = DrawSketchHandlerScale::make_centerScaleAll(vp, scaleFactor, false); scaler->setSketchGui(vp); scaler->executeCommands(); diff --git a/src/Mod/Sketcher/Gui/CommandSketcherTools.h b/src/Mod/Sketcher/Gui/CommandSketcherTools.h index c0d6204da4..e0824a8b4a 100644 --- a/src/Mod/Sketcher/Gui/CommandSketcherTools.h +++ b/src/Mod/Sketcher/Gui/CommandSketcherTools.h @@ -9,9 +9,9 @@ namespace SketcherGui // These functions are declared here to promote code reuse from other modules -/// Scale the sketch around it's origin by a factor +/// Scale the sketch from the current document in edit around it's origin by a factor /// and will not abort the current transaction if it fails -void centerScale(Sketcher::SketchObject* Obj, double scale_factor); +void centerScale(double scale_factor); } // namespace SketcherGui #endif // SKETCHERGUI_CommandSketcherTools_H diff --git a/src/Mod/Sketcher/Gui/DrawSketchHandlerScale.h b/src/Mod/Sketcher/Gui/DrawSketchHandlerScale.h index 96e0a43e9f..603437c565 100644 --- a/src/Mod/Sketcher/Gui/DrawSketchHandlerScale.h +++ b/src/Mod/Sketcher/Gui/DrawSketchHandlerScale.h @@ -76,6 +76,7 @@ public: , deleteOriginal(true) , abortOnFail(true) , allowOriginConstraint(false) + , isAllGeoIds(false) , refLength(0.0) , length(0.0) , scaleFactor(0.0) @@ -88,14 +89,20 @@ public: ~DrawSketchHandlerScale() override = default; + static std::unique_ptr - make_centerScale(std::vector listOfGeoIds, double scaleFactor, bool abortOnFail) + make_centerScaleAll(SketcherGui::ViewProviderSketch* vp, double scaleFactor, bool abortOnFail) { - auto out = std::make_unique(listOfGeoIds); + std::vector allGeoIds(vp->getSketchObject()->Geometry.getValues().size()); + std::iota(allGeoIds.begin(), allGeoIds.end(), 0); + auto out = std::make_unique(std::move(allGeoIds)); + + out->setSketchGui(vp); out->referencePoint = Base::Vector2d(0.0, 0.0); out->scaleFactor = scaleFactor; out->abortOnFail = abortOnFail; out->allowOriginConstraint = true; + out->isAllGeoIds = true; return out; } @@ -115,10 +122,13 @@ public: createShape(false); + if (deleteOriginal) { + deleteOriginalGeos(); + } + commandAddShapeGeometryAndConstraints(); if (deleteOriginal) { - deleteOriginalGeos(); reassignFacadeIds(); } @@ -257,26 +267,46 @@ private: bool abortOnFail; // When the scale operation is part of a larger transaction, one might want // to continue even if the scaling failed bool allowOriginConstraint; // Conserve constraints with origin + bool isAllGeoIds; // if true (default for centerScaleAll), and deleteOriginal is true + // (default), use deleteAllGeometries to avoid many searches in a vector double refLength, length, scaleFactor; void deleteOriginalGeos() { - std::stringstream stream; - for (size_t j = 0; j < listOfGeoIds.size() - 1; j++) { - stream << listOfGeoIds[j] << ","; + if (listOfGeoIds.empty()) { + return; } - stream << listOfGeoIds[listOfGeoIds.size() - 1]; - try { - Gui::cmdAppObjectArgs(sketchgui->getObject(), - "delGeometries([%s])", - stream.str().c_str()); + + if (isAllGeoIds) { + try { + Gui::cmdAppObjectArgs(sketchgui->getObject(), "deleteAllGeometry(True)"); + } + catch (const Base::Exception& e) { + Base::Console().error("%s\n", e.what()); + } } - catch (const Base::Exception& e) { - Base::Console().error("%s\n", e.what()); + else { + std::stringstream stream; + for (size_t j = 0; j < listOfGeoIds.size() - 1; j++) { + stream << listOfGeoIds[j] << ","; + } + stream << listOfGeoIds[listOfGeoIds.size() - 1]; + try { + Gui::cmdAppObjectArgs(sketchgui->getObject(), + "delGeometries([%s], True)", + stream.str().c_str()); + } + catch (const Base::Exception& e) { + Base::Console().error("%s\n", e.what()); + } } } void reassignFacadeIds() { + if (listOfFacadeIds.empty()) { + return; + } + std::stringstream stream; int geoId = getHighestCurveIndex() - listOfFacadeIds.size() + 1; for (size_t j = 0; j < listOfFacadeIds.size() - 1; j++) { @@ -407,44 +437,70 @@ private: addLineToShapeGeometry(toVector3d(referencePoint), toVector3d(endPoint), true); } else { - int firstCurveCreated = getHighestCurveIndex() + 1; + int firstCurveCreated = 0; + if (deleteOriginal) { + // Initial geometries will be removed before adding new geometries + firstCurveCreated = getHighestCurveIndex() + 1 - listOfGeoIds.size(); + } + else { + firstCurveCreated = getHighestCurveIndex() + 1; + } const std::vector& vals = Obj->Constraints.getValues(); - // avoid applying equal several times if cloning distanceX and distanceY of the - // same part. - std::vector geoIdsWhoAlreadyHasEqual = {}; for (auto& cstr : vals) { if (skipConstraint(cstr)) { continue; } + int firstIndex = offsetGeoID(cstr->First, firstCurveCreated); + int secondIndex = offsetGeoID(cstr->Second, firstCurveCreated); + int thirdIndex = offsetGeoID(cstr->Third, firstCurveCreated); + auto newConstr = std::unique_ptr(cstr->copy()); - newConstr->First = offsetGeoID(newConstr->First, firstCurveCreated); if ((cstr->Type == Symmetric || cstr->Type == Tangent || cstr->Type == Perpendicular || cstr->Type == Angle) - && cstr->Second != GeoEnum::GeoUndef && cstr->Third != GeoEnum::GeoUndef) { - newConstr->Second = offsetGeoID(cstr->Second, firstCurveCreated); - newConstr->Third = offsetGeoID(cstr->Third, firstCurveCreated); + && firstIndex != GeoEnum::GeoUndef && secondIndex != GeoEnum::GeoUndef + && thirdIndex != GeoEnum::GeoUndef) { + newConstr->First = firstIndex; + newConstr->Second = secondIndex; + newConstr->Third = thirdIndex; } else if ((cstr->Type == Coincident || cstr->Type == Tangent || cstr->Type == Symmetric || cstr->Type == Perpendicular || cstr->Type == Parallel || cstr->Type == Equal || cstr->Type == Angle || cstr->Type == PointOnObject || cstr->Type == InternalAlignment) - && cstr->Second != GeoEnum::GeoUndef && cstr->Third == GeoEnum::GeoUndef) { - newConstr->Second = offsetGeoID(cstr->Second, firstCurveCreated); + && firstIndex != GeoEnum::GeoUndef && secondIndex != GeoEnum::GeoUndef + && thirdIndex == GeoEnum::GeoUndef) { + newConstr->First = firstIndex; + newConstr->Second = secondIndex; } - else if (cstr->Type == Radius || cstr->Type == Diameter) { + else if ((cstr->Type == Radius || cstr->Type == Diameter) + && firstIndex != GeoEnum::GeoUndef) { + newConstr->First = firstIndex; newConstr->setValue(newConstr->getValue() * scaleFactor); } else if ((cstr->Type == Distance || cstr->Type == DistanceX || cstr->Type == DistanceY) - && cstr->Second != GeoEnum::GeoUndef) { - newConstr->Second = offsetGeoID(cstr->Second, firstCurveCreated); + && firstIndex != GeoEnum::GeoUndef && secondIndex != GeoEnum::GeoUndef) { + newConstr->First = firstIndex; + newConstr->Second = secondIndex; newConstr->setValue(newConstr->getValue() * scaleFactor); } - // (cstr->Type == Block || cstr->Type == Weight) + else if ((cstr->Type == Block || cstr->Type == Weight) + && firstIndex != GeoEnum::GeoUndef) { + newConstr->First = firstIndex; + } + else if ((cstr->Type == Vertical || cstr->Type == Horizontal) + && (firstIndex != GeoEnum::GeoUndef + && (cstr->Second == GeoEnum::GeoUndef || secondIndex != GeoUndef))) { + newConstr->First = firstIndex; + newConstr->Second = secondIndex; + } + else { + continue; + } ShapeConstraints.push_back(std::move(newConstr)); } @@ -479,9 +535,17 @@ private: int offsetGeoID(int id, int firstCurveCreated) { if (id < 0) { // Covers external geometry, origin and undef - return id; + if (allowOriginConstraint && (id == GeoEnum::HAxis || id == GeoEnum::VAxis)) { + return id; + } + return GeoEnum::GeoUndef; } - return indexOfGeoId(listOfGeoIds, id) + firstCurveCreated; + int index = indexOfGeoId(listOfGeoIds, id); + + if (index < 0) { + return GeoEnum::GeoUndef; + } + return index + firstCurveCreated; } Base::Vector3d getScaledPoint(Base::Vector3d&& pointToScale, const Base::Vector2d& referencePoint, diff --git a/src/Mod/Sketcher/Gui/EditDatumDialog.cpp b/src/Mod/Sketcher/Gui/EditDatumDialog.cpp index a5418a276d..fe8d6de477 100644 --- a/src/Mod/Sketcher/Gui/EditDatumDialog.cpp +++ b/src/Mod/Sketcher/Gui/EditDatumDialog.cpp @@ -422,7 +422,13 @@ void EditDatumDialog::performAutoScale(double newDatum) double scaleFactor = newDatum / oldDatum; float initLabelDistance = sketch->Constraints[ConstrNbr]->LabelDistance; float initLabelPosition = sketch->Constraints[ConstrNbr]->LabelPosition; - centerScale(sketch, scaleFactor); + centerScale(scaleFactor); + + // Some constraints cannot be scaled so the actual datum constraint + // might change index + ConstrNbr = sketch->getSingleScaleDefiningConstraint(); + + sketch->setLabelDistance(ConstrNbr, initLabelDistance * scaleFactor); // Label position or radii and diameters represent an angle, so