From 0b812405a0e35e561ca29cded6710e44bc0fad66 Mon Sep 17 00:00:00 2001
From: Ajinkya Dahale <AjinkyaDahale@users.noreply.github.com>
Date: Mon, 7 Apr 2025 02:56:05 +0530
Subject: [PATCH] [Sketcher] Fix issue in `replaceGeometries` when more old
 than new

---
 src/Mod/Sketcher/App/SketchObject.cpp | 16 ++++++++--------
 src/Mod/Sketcher/App/SketchObject.h   | 12 ++++++------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/Mod/Sketcher/App/SketchObject.cpp b/src/Mod/Sketcher/App/SketchObject.cpp
index 7d57db6bbf..3c8f4359b3 100644
--- a/src/Mod/Sketcher/App/SketchObject.cpp
+++ b/src/Mod/Sketcher/App/SketchObject.cpp
@@ -2021,6 +2021,7 @@ int SketchObject::delGeometriesExclusiveList(const std::vector<int>& GeoIds, Del
     return 0;
 }
 
+// clang-format on
 void SketchObject::replaceGeometries(std::vector<int> oldGeoIds,
                                      std::vector<Part::Geometry*>& newGeos)
 {
@@ -2042,18 +2043,17 @@ void SketchObject::replaceGeometries(std::vector<int> oldGeoIds,
         newVals[*oldGeoIdIter] = *newGeoIter;
     }
 
-    if (newGeoIter != newGeos.end()) {
-        for (; newGeoIter != newGeos.end(); ++newGeoIter) {
-            generateId(*newGeoIter);
-            newVals.push_back(*newGeoIter);
-        }
-    }
-    else {
-        delGeometries(oldGeoIdIter, oldGeoIds.end());
+    for (; newGeoIter != newGeos.end(); ++newGeoIter) {
+        generateId(*newGeoIter);
+        newVals.push_back(*newGeoIter);
     }
 
+    // Set geometries first, then delete the old ones. This allows to use `delGeometries`.
     Geometry.setValues(std::move(newVals));
+
+    delGeometries(oldGeoIdIter, oldGeoIds.end());
 }
+// clang-format off
 
 int SketchObject::deleteAllGeometry(DeleteOptions options)
 {
diff --git a/src/Mod/Sketcher/App/SketchObject.h b/src/Mod/Sketcher/App/SketchObject.h
index c318643c91..f42e398a38 100644
--- a/src/Mod/Sketcher/App/SketchObject.h
+++ b/src/Mod/Sketcher/App/SketchObject.h
@@ -984,6 +984,12 @@ public:  // geometry extension functionalities for single element sketch object
     int setGeometryIds(std::vector<std::pair<int, long>> GeoIdsToIds);
     int getGeometryId(int GeoId, long& id) const;
 
+    /// Replaces geometries at `oldGeoIds` with `newGeos`, lower Ids first.
+    /// If `oldGeoIds` is bigger, deletes the remaining.
+    /// If `newGeos` is bigger, adds the remaining geometries at the end.
+    /// NOTE: Does NOT move any constraints
+    void replaceGeometries(std::vector<int> oldGeoIds, std::vector<Part::Geometry*>& newGeos);
+
 protected:
     // Only the first flag is toggled, the rest of the flags is set or cleared following the first
     // flag.
@@ -996,12 +1002,6 @@ protected:
     /// get called by the container when a property has changed
     void onChanged(const App::Property* /*prop*/) override;
 
-    /// Replaces geometries at `oldGeoIds` with `newGeos`, lower Ids first.
-    /// If `oldGeoIds` is bigger, deletes the remaining.
-    /// If `newGeos` is bigger, adds the remaining geometries at the end.
-    /// NOTE: Does NOT move any constraints
-    void replaceGeometries(std::vector<int> oldGeoIds, std::vector<Part::Geometry*>& newGeos);
-
     /// Helper functions for `deleteUnusedInternalGeometry` by cases
     /// two foci for ellipses and arcs of ellipses and hyperbolas
     int deleteUnusedInternalGeometryWhenTwoFoci(int GeoId, bool delgeoid = false);