From 930a5d74240a50426b5d6532b402d33ab8c4ddc6 Mon Sep 17 00:00:00 2001
From: Matthias Koefferlein <matthias.koefferlein@infineon.com>
Date: Mon, 8 Jul 2024 12:06:39 +0200
Subject: [PATCH 1/2] Bugfix issue #1780

---
 src/edt/edt/edtInstPropertiesPage.cc  |  8 ++-
 src/edt/edt/edtMainService.cc         | 97 ++++++++++++++++++---------
 src/edt/edt/edtPropertiesPageUtils.cc |  2 +-
 src/edt/edt/edtPropertiesPages.cc     |  8 ++-
 src/edt/edt/edtService.cc             | 32 ++++++---
 5 files changed, 97 insertions(+), 50 deletions(-)

diff --git a/src/edt/edt/edtInstPropertiesPage.cc b/src/edt/edt/edtInstPropertiesPage.cc
index bd388235ae..d84474411b 100644
--- a/src/edt/edt/edtInstPropertiesPage.cc
+++ b/src/edt/edt/edtInstPropertiesPage.cc
@@ -104,8 +104,9 @@ static bool is_orthogonal (const db::DVector &rv, const db::DVector &cv)
 InstPropertiesPage::InstPropertiesPage (edt::Service *service, db::Manager *manager, QWidget *parent)
   : lay::PropertiesPage (parent, manager, service), mp_service (service), m_enable_cb_callback (true), mp_pcell_parameters (0)
 {
-  m_selection_ptrs.reserve (service->selection ().size ());
-  for (edt::Service::obj_iterator s = service->selection ().begin (); s != service->selection ().end (); ++s) {
+  const edt::Service::objects &selection = service->selection ();
+  m_selection_ptrs.reserve (selection.size ());
+  for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
     m_selection_ptrs.push_back (s);
   }
 
@@ -787,7 +788,8 @@ InstPropertiesPage::recompute_selection_ptrs (const std::vector<lay::ObjectInstP
 {
   std::map<lay::ObjectInstPath, edt::Service::obj_iterator> ptrs;
 
-  for (edt::Service::obj_iterator pos = mp_service->selection ().begin (); pos != mp_service->selection ().end (); ++pos) {
+  const edt::Service::objects &selection = mp_service->selection ();
+  for (edt::Service::obj_iterator pos = selection.begin (); pos != selection.end (); ++pos) {
     ptrs.insert (std::make_pair (*pos, pos));
   }
 
diff --git a/src/edt/edt/edtMainService.cc b/src/edt/edt/edtMainService.cc
index e85f213d2c..bee8c5b43c 100644
--- a/src/edt/edt/edtMainService.cc
+++ b/src/edt/edt/edtMainService.cc
@@ -310,7 +310,8 @@ MainService::cm_descend ()
 
   std::vector<edt::Service *> edt_services = view ()->get_plugins <edt::Service> ();
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end () && common_inst.valid (); ++es) {
-    for (edt::Service::objects::const_iterator sel = (*es)->selection ().begin (); sel != (*es)->selection ().end () && common_inst.valid (); ++sel) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::objects::const_iterator sel = selection.begin (); sel != selection.end () && common_inst.valid (); ++sel) {
       common_inst.add (*sel, 1);
     }
   }
@@ -334,10 +335,12 @@ MainService::cm_descend ()
 
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
 
+    const edt::Service::objects &selection = (*es)->selection ();
+
     new_selections.push_back (std::vector<lay::ObjectInstPath> ());
-    new_selections.back ().reserve ((*es)->selection ().size ());
+    new_selections.back ().reserve (selection.size ());
 
-    for (edt::Service::objects::const_iterator sel = (*es)->selection ().begin (); sel != (*es)->selection ().end (); ++sel) {
+    for (edt::Service::objects::const_iterator sel = selection.begin (); sel != selection.end (); ++sel) {
 
       new_selections.back ().push_back (*sel);
       lay::ObjectInstPath &new_sel = new_selections.back ().back ();
@@ -436,7 +439,8 @@ MainService::cm_flatten_insts ()
   std::vector<edt::Service *> edt_services = view ()->get_plugins <edt::Service> ();
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
 
-    for (edt::Service::objects::const_iterator r = (*es)->selection ().begin (); r != (*es)->selection ().end (); ++r) {
+    const edt::Service::objects &sel = (*es)->selection ();
+    for (edt::Service::objects::const_iterator r = sel.begin (); r != sel.end (); ++r) {
 
       const lay::CellView &cv = view ()->cellview (r->cv_index ());
       if (cv.is_valid ()) {
@@ -490,10 +494,12 @@ MainService::cm_move_hier_up ()
   std::vector<edt::Service *> edt_services = view ()->get_plugins <edt::Service> ();
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
 
+    const edt::Service::objects &selection = (*es)->selection ();
+
     std::vector<lay::ObjectInstPath> new_selection;
-    new_selection.reserve ((*es)->selection ().size ());
+    new_selection.reserve (selection.size ());
 
-    for (edt::Service::objects::const_iterator r = (*es)->selection ().begin (); r != (*es)->selection ().end (); ++r) {
+    for (edt::Service::objects::const_iterator r = selection.begin (); r != selection.end (); ++r) {
 
       const lay::CellView &cv = view ()->cellview (r->cv_index ());
       if (cv.is_valid ()) {
@@ -725,7 +731,8 @@ MainService::cm_make_cell_variants ()
   //  TODO: this limitation is not really necessary, but makes the code somewhat simpler
   int cv_index = -1;
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::objects::const_iterator r = (*es)->selection ().begin (); r != (*es)->selection ().end (); ++r) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::objects::const_iterator r = selection.begin (); r != selection.end (); ++r) {
       if (cv_index < 0) {
         cv_index = r->cv_index ();
       } else if (cv_index != int (r->cv_index ())) {
@@ -937,7 +944,8 @@ MainService::cm_resolve_arefs ()
 
   int cv_index = -1;
 
-  for (edt::Service::objects::const_iterator r = inst_service->selection ().begin (); r != inst_service->selection ().end (); ++r) {
+  const edt::Service::objects &selection = inst_service->selection ();
+  for (edt::Service::objects::const_iterator r = selection.begin (); r != selection.end (); ++r) {
     if (r->is_cell_inst () && r->back ().inst_ptr.size () > 1) {
       if (cv_index < 0) {
         cv_index = r->cv_index ();
@@ -1018,7 +1026,8 @@ MainService::cm_make_cell ()
   int cv_index = -1;
   std::vector<edt::Service *> edt_services = view ()->get_plugins <edt::Service> ();
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::objects::const_iterator r = (*es)->selection ().begin (); r != (*es)->selection ().end (); ++r) {
+    const edt::Service::objects &selection = (*es)->selection () ;
+    for (edt::Service::objects::const_iterator r = selection.begin (); r != selection.end (); ++r) {
       if (cv_index < 0) {
         cv_index = r->cv_index ();
       } else if (cv_index != int (r->cv_index ())) {
@@ -1042,7 +1051,8 @@ MainService::cm_make_cell ()
     db::Box selection_bbox;
     db::box_convert<db::CellInst> bc (cv->layout ());
     for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-      for (edt::Service::objects::const_iterator r = (*es)->selection ().begin (); r != (*es)->selection ().end (); ++r) {
+      const edt::Service::objects &selection = (*es)->selection () ;
+      for (edt::Service::objects::const_iterator r = selection.begin (); r != selection.end (); ++r) {
         if (r->is_cell_inst ()) {
           selection_bbox += db::ICplxTrans (r->trans ()) * r->back ().bbox (bc);
         } else {
@@ -1076,7 +1086,8 @@ MainService::cm_make_cell ()
 
     for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
 
-      for (edt::Service::objects::const_iterator r = (*es)->selection ().begin (); r != (*es)->selection ().end (); ++r) {
+      const edt::Service::objects &selection = (*es)->selection () ;
+      for (edt::Service::objects::const_iterator r = selection.begin (); r != selection.end (); ++r) {
 
         if (r->is_cell_inst ()) {
 
@@ -1137,7 +1148,8 @@ MainService::cm_convert_to_cell ()
     //  Do the conversion
     for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
 
-      for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+      const edt::Service::objects &selection = (*es)->selection () ;
+      for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
         const lay::CellView &cv = view ()->cellview (s->cv_index ());
         db::cell_index_type ci = s->cell_index_tot ();
@@ -1207,8 +1219,9 @@ MainService::cm_convert_to_pcell ()
   //  check whether the selection contains instances and reject it in that case
   size_t num_selected = 0;
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    num_selected += (*es)->selection ().size ();
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    num_selected += selection.size ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
       if (s->is_cell_inst ()) {
         throw tl::Exception (tl::to_string (tr ("Selection contains instances - they cannot be converted to PCells.")));
       }
@@ -1230,7 +1243,8 @@ MainService::cm_convert_to_pcell ()
         const db::PCellDeclaration *pc_decl = lib->layout ().pcell_declaration (pc->second);
         size_t n = 1000; // 1000 tries max.
         for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); n > 0 && pc_decl && es != edt_services.end (); ++es) {
-          for (edt::Service::obj_iterator s = (*es)->selection ().begin (); n > 0 && pc_decl && s != (*es)->selection ().end (); ++s) {
+          const edt::Service::objects &selection = (*es)->selection ();
+          for (edt::Service::obj_iterator s = selection.begin (); n > 0 && pc_decl && s != selection.end (); ++s) {
             const lay::CellView &cv = view ()->cellview (s->cv_index ());
             if (pc_decl->can_create_from_shape (cv->layout (), s->shape (), s->layer ())) {
               --n;
@@ -1307,7 +1321,8 @@ MainService::cm_convert_to_pcell ()
       //  convert the shapes which can be converted
       for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
 
-        for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+        const edt::Service::objects &selection = (*es)->selection ();
+        for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
           const lay::CellView &cv = view ()->cellview (s->cv_index ());
 
@@ -1406,7 +1421,8 @@ void MainService::cm_area_perimeter ()
   //  get (common) cellview index of the primary selection
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
 
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
       if (s->is_cell_inst ()) {
         continue;
@@ -1511,7 +1527,9 @@ MainService::cm_round_corners ()
 
   //  get (common) cellview index of the primary selection
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
       if (! s->is_cell_inst () && (s->shape ().is_polygon () || s->shape ().is_path () || s->shape ().is_box ())) {
 
@@ -1532,6 +1550,7 @@ MainService::cm_round_corners ()
       }
 
     }
+
   }
 
   if (cv_index < 0 || layer_index < 0) {
@@ -1584,7 +1603,8 @@ MainService::cm_round_corners ()
 
   //  Delete the current selection
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
       if (! s->is_cell_inst () && (s->shape ().is_polygon () || s->shape ().is_path () || s->shape ().is_box ())) {
         db::Cell &cell = view ()->cellview (s->cv_index ())->layout ().cell (s->cell_index ());
         if (cell.shapes (s->layer ()).is_valid (s->shape ())) {
@@ -1644,7 +1664,8 @@ MainService::cm_size ()
 
   //  get (common) cellview index of the primary selection
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
       if (! s->is_cell_inst () && (s->shape ().is_polygon () || s->shape ().is_path () || s->shape ().is_box ())) {
 
@@ -1714,7 +1735,8 @@ MainService::cm_size ()
 
   //  Delete the current selection
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
       if (! s->is_cell_inst () && (s->shape ().is_polygon () || s->shape ().is_path () || s->shape ().is_box ())) {
         db::Cell &cell = view ()->cellview (s->cv_index ())->layout ().cell (s->cell_index ());
         if (cell.shapes (s->layer ()).is_valid (s->shape ())) {
@@ -1770,7 +1792,8 @@ MainService::boolean_op (int mode)
 
   //  get (common) cellview index of the primary selection
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
       if (s->seq () == 0 && ! s->is_cell_inst () && (s->shape ().is_polygon () || s->shape ().is_path () || s->shape ().is_box ())) {
 
@@ -1804,7 +1827,8 @@ MainService::boolean_op (int mode)
 
   //  get the secondary selection
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
       if (s->seq () > 0 && ! s->is_cell_inst () && (s->shape ().is_polygon () || s->shape ().is_path () || s->shape ().is_box ())) {
 
@@ -1841,7 +1865,8 @@ MainService::boolean_op (int mode)
   //  Let's see whether this heuristics is more accepted.
 
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
       if (! s->is_cell_inst () && int (s->layer ()) == layer_index && (s->shape ().is_polygon () || s->shape ().is_path () || s->shape ().is_box ())) {
         db::Cell &cell = view ()->cellview (s->cv_index ())->layout ().cell (s->cell_index ());
         if (cell.shapes (s->layer ()).is_valid (s->shape ())) {
@@ -1976,7 +2001,8 @@ MainService::cm_align ()
 
   //  get (common) bbox index of the primary selection
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
       if (s->seq () == 0) {
 
@@ -2006,11 +2032,13 @@ MainService::cm_align ()
     //  do the alignment
     for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
 
+      const edt::Service::objects &selection = (*es)->selection ();
+
       //  create a transformation vector that describes each shape's transformation
       std::vector <db::DCplxTrans> tv;
-      tv.reserve ((*es)->selection ().size ());
+      tv.reserve (selection.size ());
 
-      for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+      for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
         db::DVector v;
 
@@ -2077,7 +2105,8 @@ MainService::cm_distribute ()
   //  count the items
   size_t n = 0;
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
       ++n;
     }
   }
@@ -2099,7 +2128,8 @@ MainService::cm_distribute ()
 
       objects_for_service.push_back (std::make_pair (i, i));
 
-      for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+      const edt::Service::objects &selection = (*es)->selection ();
+      for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
         const db::Layout &layout = view ()->cellview (s->cv_index ())->layout ();
         db::CplxTrans tr = db::CplxTrans (layout.dbu ()) * s->trans ();
@@ -2181,7 +2211,8 @@ MainService::cm_make_array ()
   std::vector<edt::Service *> edt_services = view ()->get_plugins <edt::Service> ();
 
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
       ++n;
     }
   }
@@ -2215,7 +2246,8 @@ MainService::cm_make_array ()
 
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
 
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
 
       const lay::CellView &cv = view ()->cellview (s->cv_index ());
       if (! cv.is_valid ()) {
@@ -2512,7 +2544,8 @@ MainService::check_no_guiding_shapes ()
 {
   std::vector<edt::Service *> edt_services = view ()->get_plugins <edt::Service> ();
   for (std::vector<edt::Service *>::const_iterator es = edt_services.begin (); es != edt_services.end (); ++es) {
-    for (edt::Service::obj_iterator s = (*es)->selection ().begin (); s != (*es)->selection ().end (); ++s) {
+    const edt::Service::objects &selection = (*es)->selection ();
+    for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
       if (! s->is_cell_inst ()) {
         if (s->layer () == view ()->cellview (s->cv_index ())->layout ().guiding_shape_layer ()) {
           throw tl::Exception (tl::to_string (tr ("This function cannot be applied to PCell guiding shapes")));
diff --git a/src/edt/edt/edtPropertiesPageUtils.cc b/src/edt/edt/edtPropertiesPageUtils.cc
index 38ceb65b9f..62c732b2d5 100644
--- a/src/edt/edt/edtPropertiesPageUtils.cc
+++ b/src/edt/edt/edtPropertiesPageUtils.cc
@@ -68,7 +68,7 @@ CombinedChangeApplicator::~CombinedChangeApplicator ()
 bool CombinedChangeApplicator::supports_relative_mode () const 
 { 
   for (std::vector<ChangeApplicator *>::const_iterator a = m_appl.begin (); a != m_appl.end (); ++a) {
-    if ((*a)->supports_relative_mode ()) {
+    if ((*a) && (*a)->supports_relative_mode ()) {
       return true;
     }
   }
diff --git a/src/edt/edt/edtPropertiesPages.cc b/src/edt/edt/edtPropertiesPages.cc
index cb62dd30b7..ac31333c7a 100644
--- a/src/edt/edt/edtPropertiesPages.cc
+++ b/src/edt/edt/edtPropertiesPages.cc
@@ -47,8 +47,9 @@ ShapePropertiesPage::ShapePropertiesPage (const std::string &description, edt::S
   : lay::PropertiesPage (parent, manager, service),
     m_description (description), mp_service (service), m_enable_cb_callback (true)
 {
-  m_selection_ptrs.reserve (service->selection ().size ());
-  for (edt::Service::obj_iterator s = service->selection ().begin (); s != service->selection ().end (); ++s) {
+  const edt::Service::objects &selection = service->selection ();
+  m_selection_ptrs.reserve (selection.size ());
+  for (edt::Service::obj_iterator s = selection.begin (); s != selection.end (); ++s) {
     m_selection_ptrs.push_back (s);
   }
   m_prop_id = 0;
@@ -202,7 +203,8 @@ ShapePropertiesPage::recompute_selection_ptrs (const std::vector<lay::ObjectInst
 {
   std::map<lay::ObjectInstPath, edt::Service::obj_iterator> ptrs;
 
-  for (edt::Service::obj_iterator pos = mp_service->selection ().begin (); pos != mp_service->selection ().end (); ++pos) {
+  const edt::Service::objects &selection = mp_service->selection ();
+  for (edt::Service::obj_iterator pos = selection.begin (); pos != selection.end (); ++pos) {
     ptrs.insert (std::make_pair (*pos, pos));
   }
 
diff --git a/src/edt/edt/edtService.cc b/src/edt/edt/edtService.cc
index 7260b7cb6c..f001af81c0 100644
--- a/src/edt/edt/edtService.cc
+++ b/src/edt/edt/edtService.cc
@@ -459,7 +459,8 @@ Service::copy_selected ()
   unsigned int inst_mode = 0;
 
   if (m_hier_copy_mode < 0) {
-    for (objects::const_iterator r = selection ().begin (); r != selection ().end () && ! need_to_ask_for_copy_mode; ++r) {
+    const objects &sel = selection ();
+    for (objects::const_iterator r = sel.begin (); r != sel.end () && ! need_to_ask_for_copy_mode; ++r) {
       if (r->is_cell_inst ()) {
         const db::Cell &cell = view ()->cellview (r->cv_index ())->layout ().cell (r->back ().inst_ptr.cell_index ());
         if (! cell.is_proxy ()) {
@@ -499,10 +500,12 @@ Service::copy_selected ()
 void
 Service::copy_selected (unsigned int inst_mode)
 {
+  const objects &sel = selection ();
+
   //  create one ClipboardData object per cv_index because, this one assumes that there is
   //  only one source layout object.
   std::set <unsigned int> cv_indices;
-  for (objects::const_iterator r = selection ().begin (); r != selection ().end (); ++r) {
+  for (objects::const_iterator r = sel.begin (); r != sel.end (); ++r) {
     cv_indices.insert (r->cv_index ());
   }
 
@@ -512,7 +515,7 @@ Service::copy_selected (unsigned int inst_mode)
 
     //  add the selected objects to the clipboard data objects.
     const lay::CellView &cv = view ()->cellview (*cvi);
-    for (objects::const_iterator r = selection ().begin (); r != selection ().end (); ++r) {
+    for (objects::const_iterator r = sel.begin (); r != sel.end (); ++r) {
       if (r->cv_index () == *cvi) {
         if (! r->is_cell_inst ()) {
           cd->get ().add (cv->layout (), r->layer (), r->shape (), cv.context_trans () * r->trans ());
@@ -614,7 +617,9 @@ Service::selection_bbox ()
   lay::TextInfo text_info (view ());
 
   db::DBox box;
-  for (objects::const_iterator r = selection ().begin (); r != selection ().end (); ++r) {
+
+  const objects &sel = selection ();
+  for (objects::const_iterator r = sel.begin (); r != sel.end (); ++r) {
 
     const lay::CellView &cv = view ()->cellview (r->cv_index ());
     const db::Layout &layout = cv->layout ();
@@ -694,10 +699,12 @@ Service::transform (const db::DCplxTrans &trans, const std::vector<db::DCplxTran
   
   size_t n;
 
+  const objects &sel = selection ();
+
   //  build a list of object references corresponding to the p_trv vector 
   std::vector <objects::iterator> obj_ptrs;
-  obj_ptrs.reserve (selection ().size ());
-  for (objects::iterator r = selection ().begin (); r != selection ().end (); ++r) {
+  obj_ptrs.reserve (sel.size ());
+  for (objects::iterator r = sel.begin (); r != sel.end (); ++r) {
     obj_ptrs.push_back (r);
   }
 
@@ -710,7 +717,7 @@ Service::transform (const db::DCplxTrans &trans, const std::vector<db::DCplxTran
   //  The key is a triple: cell_index, cv_index, layer
   std::map <std::pair <db::cell_index_type, std::pair <unsigned int, unsigned int> >, std::vector <size_t> > shapes_by_cell;
   n = 0;
-  for (objects::iterator r = selection ().begin (); r != selection ().end (); ++r, ++n) {
+  for (objects::iterator r = sel.begin (); r != sel.end (); ++r, ++n) {
     if (! r->is_cell_inst ()) {
       shapes_by_cell.insert (std::make_pair (std::make_pair (r->cell_index (), std::make_pair (r->cv_index (), r->layer ())), std::vector <size_t> ())).first->second.push_back (n);
     }
@@ -780,7 +787,7 @@ Service::transform (const db::DCplxTrans &trans, const std::vector<db::DCplxTran
   //  The key is a pair: cell_index, cv_index
   std::map <std::pair <db::cell_index_type, unsigned int>, std::vector <size_t> > insts_by_cell;
   n = 0;
-  for (objects::iterator r = selection ().begin (); r != selection ().end (); ++r, ++n) {
+  for (objects::iterator r = sel.begin (); r != sel.end (); ++r, ++n) {
     if (r->is_cell_inst ()) {
       insts_by_cell.insert (std::make_pair (std::make_pair (r->cell_index (), r->cv_index ()), std::vector <size_t> ())).first->second.push_back (n);
     }
@@ -1032,7 +1039,8 @@ Service::del_selected ()
   std::set<db::Layout *> needs_cleanup;
 
   //  delete all shapes and instances.
-  for (objects::const_iterator r = selection ().begin (); r != selection ().end (); ++r) {
+  const objects &sel = selection ();
+  for (objects::const_iterator r = sel.begin (); r != sel.end (); ++r) {
     const lay::CellView &cv = view ()->cellview (r->cv_index ());
     if (cv.is_valid ()) {
       db::Cell &cell = cv->layout ().cell (r->cell_index ());
@@ -1722,15 +1730,17 @@ Service::selection_to_view ()
 void 
 Service::do_selection_to_view ()
 {
+  const objects &sel = selection ();
+
   //  Hint: this is a lower bound:
-  m_markers.reserve (selection ().size ());
+  m_markers.reserve (sel.size ());
 
   //  build the transformation variants cache
   TransformationVariants tv (view ());
 
   //  Build markers
 
-  for (std::set<lay::ObjectInstPath>::iterator r = selection ().begin (); r != selection ().end (); ++r) {
+  for (objects::const_iterator r = sel.begin (); r != sel.end (); ++r) {
 
     const lay::CellView &cv = view ()->cellview (r->cv_index ());
 

From 6bca4a07b6b7fb48711f4dfa25f6574ffac7f10c Mon Sep 17 00:00:00 2001
From: Matthias Koefferlein <matthias@klayout.de>
Date: Mon, 8 Jul 2024 22:43:56 +0200
Subject: [PATCH 2/2] Code cleanup + fixing another bug (crash when doing
 'change layer' on multiple selected shapes)

---
 src/edt/edt/edtMainService.cc |  4 +--
 src/edt/edt/edtService.cc     | 10 ++++--
 src/edt/edt/edtService.h      |  5 +--
 src/edt/edt/edtUtils.cc       | 67 -----------------------------------
 src/edt/edt/edtUtils.h        | 67 -----------------------------------
 5 files changed, 13 insertions(+), 140 deletions(-)

diff --git a/src/edt/edt/edtMainService.cc b/src/edt/edt/edtMainService.cc
index bee8c5b43c..2477a77881 100644
--- a/src/edt/edt/edtMainService.cc
+++ b/src/edt/edt/edtMainService.cc
@@ -2409,7 +2409,7 @@ MainService::cm_change_layer ()
   int cv_index = -1;
 
   //  get (common) cellview index of the selected shapes
-  for (SelectionIterator s (view ()); ! s.at_end (); ++s) {
+  for (EditableSelectionIterator s = begin_objects_selected (view ()); ! s.at_end (); ++s) {
     if (cv_index >= 0 && cv_index != int (s->cv_index ())) {
       throw tl::Exception (tl::to_string (tr ("Selections originate from different layouts - cannot switch layer in this case.")));
     }
@@ -2477,7 +2477,7 @@ MainService::cm_change_layer ()
 
     //  Insert and delete the shape. This exploits the fact, that a shape can be erased multiple times -
     //  this is important since the selection potentially contains the same shape multiple times.
-    for (SelectionIterator s (view ()); ! s.at_end (); ++s) {
+    for (EditableSelectionIterator s = begin_objects_selected (view ()); ! s.at_end (); ++s) {
 
       if (!s->is_cell_inst () && int (s->layer ()) != layer) {
 
diff --git a/src/edt/edt/edtService.cc b/src/edt/edt/edtService.cc
index f001af81c0..d0f766888f 100644
--- a/src/edt/edt/edtService.cc
+++ b/src/edt/edt/edtService.cc
@@ -1994,10 +1994,16 @@ EditableSelectionIterator::operator++ ()
   return *this;
 }
 
-const EditableSelectionIterator::value_type &
+EditableSelectionIterator::pointer
+EditableSelectionIterator::operator-> () const
+{
+  return m_iter.operator-> ();
+}
+
+EditableSelectionIterator::reference
 EditableSelectionIterator::operator* () const
 {
-  return *m_iter;
+  return m_iter.operator* ();
 }
 
 void
diff --git a/src/edt/edt/edtService.h b/src/edt/edt/edtService.h
index ac2e5ad4c4..dc0250f81b 100644
--- a/src/edt/edt/edtService.h
+++ b/src/edt/edt/edtService.h
@@ -700,7 +700,7 @@ class EditableSelectionIterator
 public:
   typedef edt::Service::objects::value_type value_type;
   typedef edt::Service::objects::const_iterator iterator_type;
-  typedef void pointer;
+  typedef const value_type *pointer;
   typedef const value_type &reference;
   typedef std::forward_iterator_tag iterator_category;
   typedef void difference_type;
@@ -710,7 +710,8 @@ class EditableSelectionIterator
   bool at_end () const;
 
   EditableSelectionIterator &operator++ ();
-  const value_type &operator* () const;
+  reference operator* () const;
+  pointer operator-> () const;
 
 private:
   std::vector<edt::Service *> m_services;
diff --git a/src/edt/edt/edtUtils.cc b/src/edt/edt/edtUtils.cc
index bcae4ea72e..1aac2097fe 100644
--- a/src/edt/edt/edtUtils.cc
+++ b/src/edt/edt/edtUtils.cc
@@ -74,73 +74,6 @@ std::map<std::string, tl::Variant> pcell_parameters_from_string (const std::stri
   return pm;
 }
 
-// -------------------------------------------------------------
-//  SelectionIterator implementation
-
-SelectionIterator::SelectionIterator (lay::LayoutViewBase *view, bool including_transient)
-  : m_transient_mode (false)
-{
-  mp_edt_services = view->get_plugins <edt::Service> ();
-
-  m_current_service = mp_edt_services.begin ();
-  if (m_current_service != mp_edt_services.end ()) {
-    m_current_object = (*m_current_service)->selection ().begin ();
-  }
-
-  next ();
-
-  if (at_end () && including_transient) {
-
-    m_transient_mode = true;
-
-    m_current_service = mp_edt_services.begin ();
-    if (m_current_service != mp_edt_services.end ()) {
-      m_current_object = (*m_current_service)->transient_selection ().begin ();
-    }
-
-    next ();
-
-  }
-}
-
-bool
-SelectionIterator::at_end () const
-{
-  return m_current_service == mp_edt_services.end ();
-}
-
-void
-SelectionIterator::inc ()
-{
-  tl_assert (! at_end ());
-  ++m_current_object;
-}
-
-void
-SelectionIterator::next ()
-{
-  if (at_end ()) {
-    return;
-  }
-
-  const edt::Service::objects *sel = m_transient_mode ? &(*m_current_service)->transient_selection () : &(*m_current_service)->selection ();
-
-  while (m_current_object == sel->end ()) {
-
-    ++m_current_service;
-
-    if (m_current_service != mp_edt_services.end ()) {
-
-      sel = m_transient_mode ? &(*m_current_service)->transient_selection () : &(*m_current_service)->selection ();
-      m_current_object = sel->begin ();
-
-    } else {
-      break;
-    }
-
-  }
-}
-
 // -------------------------------------------------------------
 //  TransformationsVariants implementation
 //  for a lay::LayoutView
diff --git a/src/edt/edt/edtUtils.h b/src/edt/edt/edtUtils.h
index 17470054d6..1f9afec798 100644
--- a/src/edt/edt/edtUtils.h
+++ b/src/edt/edt/edtUtils.h
@@ -92,73 +92,6 @@ class TransformationVariants
   std::map < std::pair<unsigned int, unsigned int>, std::vector<db::DCplxTrans> > m_per_cv_and_layer_tv;
 };
 
-/**
- *  @brief An iterator for the selected objects of all edt services in a layout view
- */
-class SelectionIterator
-{
-public:
-  typedef lay::ObjectInstPath value_type;
-  typedef const lay::ObjectInstPath &reference;
-  typedef const lay::ObjectInstPath *pointer;
-
-  /**
-   *  @brief Creates a new iterator iterating over all selected edt objects from the given view
-   *
-   *  If "including_transient" is true, the transient selection will be used as fallback.
-   */
-  SelectionIterator (lay::LayoutViewBase *view, bool including_transient = true);
-
-  /**
-   *  @brief Returns a value indicating whether the transient selection is taken
-   */
-  bool is_transient () const
-  {
-    return m_transient_mode;
-  }
-
-  /**
-   *  @brief Increments the iterator
-   */
-  void operator++ ()
-  {
-    inc ();
-    next ();
-  }
-
-  /**
-   *  @brief Dereferencing
-   */
-  const lay::ObjectInstPath &operator* () const
-  {
-    tl_assert (! at_end ());
-    return *m_current_object;
-  }
-
-  /**
-   *  @brief Arrow operator
-   */
-  const lay::ObjectInstPath *operator-> () const
-  {
-    return & operator* ();
-  }
-
-  /**
-   *  @brief Returns a value indicating whether the iterator has finished
-   */
-  bool at_end () const;
-
-private:
-  void inc ();
-  void next ();
-
-private:
-  std::vector<edt::Service *> mp_edt_services;
-  std::vector<edt::Service *>::const_iterator m_current_service;
-  std::set<lay::ObjectInstPath>::const_iterator m_current_object;
-  bool m_transient_mode;
-};
-
 } // namespace edt
 
 #endif