From c8bddd2f2bc1eae1945b511dac70475a5a673e61 Mon Sep 17 00:00:00 2001 From: theo-vt Date: Mon, 14 Jul 2025 11:39:03 -0400 Subject: [PATCH] Sketcher: assign the old geometries' GeometryId[s] to new geometries after scaling (#22263) * Reassign facade ids after scale operation if deleting geometries * Fix failing CI --- src/Mod/Sketcher/App/SketchObject.cpp | 52 +++++++++++++++---- src/Mod/Sketcher/App/SketchObject.h | 1 + src/Mod/Sketcher/App/SketchObject.pyi | 6 +++ src/Mod/Sketcher/App/SketchObjectPyImp.cpp | 43 +++++++++++++++ src/Mod/Sketcher/Gui/DrawSketchHandlerScale.h | 26 +++++++++- 5 files changed, 118 insertions(+), 10 deletions(-) diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp index 4b5d53e1cd..421eff14f0 100644 --- a/src/Mod/Sketcher/App/SketchObject.cpp +++ b/src/Mod/Sketcher/App/SketchObject.cpp @@ -11631,6 +11631,24 @@ std::vector SketchObject::getOpenVertices() const // SketchGeometryExtension interface +size_t setGeometryIdHelper(int GeoId, long id, std::vector& newVals, size_t searchOffset = 0, bool returnOnMatch = false) +{ + // deep copy + for (size_t i = searchOffset; i < newVals.size(); i++) { + newVals[i] = newVals[i]->clone(); + + if ((int)i == GeoId) { + auto gf = GeometryFacade::getFacade(newVals[i]); + + gf->setId(id); + + if (returnOnMatch) { + return i; + } + } + } + return 0; +} int SketchObject::setGeometryId(int GeoId, long id) { // no need to check input data validity as this is an sketchobject managed operation. @@ -11641,18 +11659,34 @@ int SketchObject::setGeometryId(int GeoId, long id) const std::vector& vals = getInternalGeometry(); + std::vector newVals(vals); + setGeometryIdHelper(GeoId, id, newVals); + + // There is not actual internal transaction going on here, however neither the geometry indices + // nor the vertices need to be updated so this is a convenient way of preventing it. + { + Base::StateLocker lock(internaltransaction, true); + this->Geometry.setValues(std::move(newVals)); + } + + return 0; +} +int SketchObject::setGeometryIds(std::vector> GeoIdsToIds) +{ + Base::StateLocker lock(managedoperation, true); + + std::sort(GeoIdsToIds.begin(), GeoIdsToIds.end()); + + size_t searchOffset = 0; + + const std::vector& vals = getInternalGeometry(); std::vector newVals(vals); - // deep copy - for (size_t i = 0; i < newVals.size(); i++) { - newVals[i] = newVals[i]->clone(); - - if ((int)i == GeoId) { - auto gf = GeometryFacade::getFacade(newVals[i]); - - gf->setId(id); - } + for (size_t i = 0; i < GeoIdsToIds.size(); ++i) { + int GeoId = GeoIdsToIds[i].first; + long id = GeoIdsToIds[i].second; + searchOffset = setGeometryIdHelper(GeoId, id, newVals, searchOffset, i != GeoIdsToIds.size()-1); } // There is not actual internal transaction going on here, however neither the geometry indices diff --git a/src/Mod/Sketcher/App/SketchObject.h b/src/Mod/Sketcher/App/SketchObject.h index eb8a5433b5..9958ba94ef 100644 --- a/src/Mod/Sketcher/App/SketchObject.h +++ b/src/Mod/Sketcher/App/SketchObject.h @@ -893,6 +893,7 @@ public: public: // geometry extension functionalities for single element sketch object user convenience int setGeometryId(int GeoId, long id); + int setGeometryIds(std::vector> GeoIdsToIds); int getGeometryId(int GeoId, long& id) const; protected: diff --git a/src/Mod/Sketcher/App/SketchObject.pyi b/src/Mod/Sketcher/App/SketchObject.pyi index ebb8615186..86d2983c3e 100644 --- a/src/Mod/Sketcher/App/SketchObject.pyi +++ b/src/Mod/Sketcher/App/SketchObject.pyi @@ -868,6 +868,12 @@ class SketchObject(Part2DObject): """ ... + def setGeometryIds(GeoIdsToIds: List[Tuple[int, int]]): + """ + Sets the GeometryId of the SketchGeometryExtension of the geometries with the provided GeoIds + Expects a list of pairs (GeoId, id) + """ + def getGeometryId(): """ Gets the GeometryId of the SketchGeometryExtension of the geometry with the provided GeoId diff --git a/src/Mod/Sketcher/App/SketchObjectPyImp.cpp b/src/Mod/Sketcher/App/SketchObjectPyImp.cpp index 24f7d684fa..a63e545775 100644 --- a/src/Mod/Sketcher/App/SketchObjectPyImp.cpp +++ b/src/Mod/Sketcher/App/SketchObjectPyImp.cpp @@ -2447,6 +2447,49 @@ PyObject* SketchObjectPy::setGeometryId(PyObject* args) Py_Return; } +PyObject* SketchObjectPy::setGeometryIds(PyObject* args) +{ + PyObject* pyList; + + // Parse arguments: list of pairs, Base::VectorPy, optional relative flag + if (!PyArg_ParseTuple(args, "O!", &PyList_Type, &pyList)) { + return nullptr; + } + + // Convert Python list to std::vector> + std::vector> geoIdsToIds; + Py_ssize_t listSize = PyList_Size(pyList); + + for (Py_ssize_t i = 0; i < listSize; ++i) { + PyObject* pyPair = PyList_GetItem(pyList, i); // Borrowed reference + + if (!PyTuple_Check(pyPair) || PyTuple_Size(pyPair) != 2) { + PyErr_SetString(PyExc_ValueError, "List must contain pairs (geoId, id)."); + return nullptr; + } + + int geoId = PyLong_AsLong(PyTuple_GetItem(pyPair, 0)); + long id = PyLong_AsLong(PyTuple_GetItem(pyPair, 1)); + + if (PyErr_Occurred()) { + PyErr_SetString(PyExc_ValueError, "Invalid geoId or id in the list."); + return nullptr; + } + + geoIdsToIds.emplace_back(geoId, id); + } + + // Call the C++ method + if (this->getSketchObjectPtr()->setGeometryIds(geoIdsToIds)) { + std::stringstream str; + str << "Not able to set geometry Ids of geometries with the given indices: "; + PyErr_SetString(PyExc_ValueError, str.str().c_str()); + return nullptr; + } + + Py_Return; +} + Py::Long SketchObjectPy::getDoF() const { diff --git a/src/Mod/Sketcher/Gui/DrawSketchHandlerScale.h b/src/Mod/Sketcher/Gui/DrawSketchHandlerScale.h index 28e155ea88..3166bd7ecb 100644 --- a/src/Mod/Sketcher/Gui/DrawSketchHandlerScale.h +++ b/src/Mod/Sketcher/Gui/DrawSketchHandlerScale.h @@ -110,6 +110,7 @@ public: if (deleteOriginal) { deleteOriginalGeos(); + reassignFacadeIds(); } Gui::Command::commitCommand(); @@ -226,6 +227,7 @@ private: private: std::vector listOfGeoIds; + std::vector listOfFacadeIds; Base::Vector2d referencePoint, startPoint, endPoint; bool deleteOriginal; bool abortOnFail; // When the scale operation is part of a larger transaction, one might want @@ -261,6 +263,24 @@ private: Base::Console().error("%s\n", e.what()); } } + void reassignFacadeIds() + { + std::stringstream stream; + int geoId = getHighestCurveIndex() - listOfFacadeIds.size() + 1; + for (size_t j = 0; j < listOfFacadeIds.size() - 1; j++) { + stream << "(" << geoId << "," << listOfFacadeIds[j] << "),"; + geoId++; + } + stream << "(" << geoId << "," << listOfFacadeIds.back() << ")"; + try { + Gui::cmdAppObjectArgs(sketchgui->getObject(), + "setGeometryIds([%s])", + stream.str().c_str()); + } + catch (const Base::Exception& e) { + Base::Console().error("%s\n", e.what()); + } + } void createShape(bool onlyeditoutline) override { @@ -281,7 +301,11 @@ private: for (auto& geoId : listOfGeoIds) { const Part::Geometry* pGeo = Obj->getGeometry(geoId); - auto geoUniquePtr = std::unique_ptr(pGeo->copy()); + long facadeId; + + Obj->getGeometryId(geoId, facadeId); + listOfFacadeIds.push_back(facadeId); + auto geoUniquePtr = std::unique_ptr(pGeo->clone()); Part::Geometry* geo = geoUniquePtr.get(); if (isCircle(*geo)) {