From 46936b5d56674baa679cee4c12052b77d1c4d1db Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 9 Jun 2024 18:58:39 +0200 Subject: [PATCH 1/2] Fixing issue #1733 (Instance selecion in object properties does not match with view port object highlight) Plus: better implementation of fix for issue #1145 (Crash when clearing a Shapes container by script while a shape is selected) The previous solution was based on deferred execution and between execution of that cleanup and the use of the selection, invalid states could be present. --- src/db/db/dbNetlistDeviceExtractorClasses.cc | 2 +- src/edt/edt/edtInstPropertiesPage.cc | 6 +- src/edt/edt/edtPropertiesPages.cc | 6 +- src/edt/edt/edtService.cc | 144 ++++++++++++------- src/edt/edt/edtService.h | 39 ++--- 5 files changed, 123 insertions(+), 74 deletions(-) diff --git a/src/db/db/dbNetlistDeviceExtractorClasses.cc b/src/db/db/dbNetlistDeviceExtractorClasses.cc index 0ef9a074b..d7e30d6f0 100644 --- a/src/db/db/dbNetlistDeviceExtractorClasses.cc +++ b/src/db/db/dbNetlistDeviceExtractorClasses.cc @@ -144,7 +144,7 @@ void NetlistDeviceExtractorMOS3Transistor::extract_devices (const std::vector diffpoly; diffpoly.reserve (2); for (db::Region::const_iterator d2g = rdiff2gate.begin (); ! d2g.at_end (); ++d2g) { diff --git a/src/edt/edt/edtInstPropertiesPage.cc b/src/edt/edt/edtInstPropertiesPage.cc index a0e34a875..bd388235a 100644 --- a/src/edt/edt/edtInstPropertiesPage.cc +++ b/src/edt/edt/edtInstPropertiesPage.cc @@ -345,7 +345,11 @@ InstPropertiesPage::update () edt::Service::obj_iterator pos = m_selection_ptrs [m_indexes.front ()]; tl_assert (pos->is_cell_inst ()); - mp_service->highlight (m_indexes); + std::set highlights; + for (auto i = m_indexes.begin (); i != m_indexes.end (); ++i) { + highlights.insert (m_selection_ptrs [*i].operator-> ()); + } + mp_service->highlight (highlights); m_enable_cb_callback = false; dbu_cb->setChecked (mp_service->view ()->dbu_coordinates ()); diff --git a/src/edt/edt/edtPropertiesPages.cc b/src/edt/edt/edtPropertiesPages.cc index a47c941af..cb62dd30b 100644 --- a/src/edt/edt/edtPropertiesPages.cc +++ b/src/edt/edt/edtPropertiesPages.cc @@ -188,7 +188,11 @@ END_PROTECTED void ShapePropertiesPage::update () { - mp_service->highlight (m_indexes); + std::set highlights; + for (auto i = m_indexes.begin (); i != m_indexes.end (); ++i) { + highlights.insert (m_selection_ptrs [*i].operator-> ()); + } + mp_service->highlight (highlights); update_shape (); } diff --git a/src/edt/edt/edtService.cc b/src/edt/edt/edtService.cc index 84a7fb536..53991397e 100644 --- a/src/edt/edt/edtService.cc +++ b/src/edt/edt/edtService.cc @@ -68,6 +68,7 @@ Service::Service (db::Manager *manager, lay::LayoutViewBase *view, db::ShapeIter mp_view (view), mp_transient_marker (0), m_editing (false), m_immediate (false), + m_selection_maybe_invalid (true), m_transient_selection_maybe_invalid (true), m_cell_inst_service (false), m_flags (flags), m_move_sel (false), m_moving (false), @@ -90,6 +91,7 @@ Service::Service (db::Manager *manager, lay::LayoutViewBase *view) mp_view (view), mp_transient_marker (0), m_editing (false), m_immediate (false), + m_selection_maybe_invalid (true), m_transient_selection_maybe_invalid (true), m_cell_inst_service (true), m_flags (db::ShapeIterator::Nothing), m_move_sel (false), m_moving (false), @@ -108,12 +110,12 @@ Service::Service (db::Manager *manager, lay::LayoutViewBase *view) Service::~Service () { - for (std::vector::iterator r = m_markers.begin (); r != m_markers.end (); ++r) { - delete *r; + for (auto r = m_markers.begin (); r != m_markers.end (); ++r) { + delete r->second; } m_markers.clear (); - for (std::vector::iterator r = m_edit_markers.begin (); r != m_edit_markers.end (); ++r) { + for (auto r = m_edit_markers.begin (); r != m_edit_markers.end (); ++r) { delete *r; } m_edit_markers.clear (); @@ -249,8 +251,8 @@ Service::snap_marker_to_grid (const db::DVector &v, bool &snapped) const for (auto m = m_markers.begin (); m != m_markers.end () && count > 0; ++m) { - const lay::ShapeMarker *sm = dynamic_cast (*m); - const lay::InstanceMarker *im = dynamic_cast (*m); + const lay::ShapeMarker *sm = dynamic_cast (m->second); + const lay::InstanceMarker *im = dynamic_cast (m->second); if (sm) { update_vector_snapped_marker (sm, tt, vr, snapped, count); } else if (im) { @@ -413,18 +415,18 @@ Service::restore_highlights () } void -Service::highlight (const std::vector &n) +Service::highlight (const std::set &highlights) { m_highlights_selected = true; - m_selected_highlights = std::set (n.begin (), n.end ()); + m_selected_highlights = highlights; apply_highlights (); } void Service::apply_highlights () { - for (std::vector::iterator r = m_markers.begin (); r != m_markers.end (); ++r) { - (*r)->visible (! m_highlights_selected || m_selected_highlights.find (r - m_markers.begin ()) != m_selected_highlights.end ()); + for (auto r = m_markers.begin (); r != m_markers.end (); ++r) { + r->second->visible (! m_highlights_selected || m_selected_highlights.find (r->first) != m_selected_highlights.end ()); } } @@ -457,7 +459,7 @@ Service::copy_selected () unsigned int inst_mode = 0; if (m_hier_copy_mode < 0) { - for (objects::const_iterator r = m_selection.begin (); r != m_selection.end () && ! need_to_ask_for_copy_mode; ++r) { + for (objects::const_iterator r = selection ().begin (); r != selection ().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 ()) { @@ -500,7 +502,7 @@ Service::copy_selected (unsigned int inst_mode) // create one ClipboardData object per cv_index because, this one assumes that there is // only one source layout object. std::set cv_indices; - for (objects::const_iterator r = m_selection.begin (); r != m_selection.end (); ++r) { + for (objects::const_iterator r = selection ().begin (); r != selection ().end (); ++r) { cv_indices.insert (r->cv_index ()); } @@ -510,7 +512,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 = m_selection.begin (); r != m_selection.end (); ++r) { + for (objects::const_iterator r = selection ().begin (); r != selection ().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 ()); @@ -538,12 +540,12 @@ Service::begin_move (lay::Editable::MoveMode mode, const db::DPoint &p, lay::ang m_move_sel = true; // TODO: there is no "false". Remove this. m_moving = true; - for (std::vector::iterator r = m_markers.begin (); r != m_markers.end (); ++r) { + for (auto r = m_markers.begin (); r != m_markers.end (); ++r) { - (*r)->thaw (); + r->second->thaw (); // Show the inner structure of the instances - lay::InstanceMarker *inst_marker = dynamic_cast (*r); + lay::InstanceMarker *inst_marker = dynamic_cast (r->second); if (inst_marker) { inst_marker->set_draw_outline (! m_show_shapes_of_instances); inst_marker->set_max_shapes (m_show_shapes_of_instances ? m_max_shapes_of_instances : 0); @@ -612,7 +614,7 @@ Service::selection_bbox () lay::TextInfo text_info (view ()); db::DBox box; - for (objects::const_iterator r = m_selection.begin (); r != m_selection.end (); ++r) { + for (objects::const_iterator r = selection ().begin (); r != selection ().end (); ++r) { const lay::CellView &cv = view ()->cellview (r->cv_index ()); const db::Layout &layout = cv->layout (); @@ -694,9 +696,8 @@ Service::transform (const db::DCplxTrans &trans, const std::vector obj_ptrs; - obj_ptrs.reserve (m_selection.size ()); - n = 0; - for (objects::iterator r = m_selection.begin (); r != m_selection.end (); ++r, ++n) { + obj_ptrs.reserve (selection ().size ()); + for (objects::iterator r = selection ().begin (); r != selection ().end (); ++r) { obj_ptrs.push_back (r); } @@ -709,7 +710,7 @@ Service::transform (const db::DCplxTrans &trans, const std::vector >, std::vector > shapes_by_cell; n = 0; - for (objects::iterator r = m_selection.begin (); r != m_selection.end (); ++r, ++n) { + for (objects::iterator r = selection ().begin (); r != selection ().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 ())).first->second.push_back (n); } @@ -779,7 +780,7 @@ Service::transform (const db::DCplxTrans &trans, const std::vector, std::vector > insts_by_cell; n = 0; - for (objects::iterator r = m_selection.begin (); r != m_selection.end (); ++r, ++n) { + for (objects::iterator r = selection ().begin (); r != selection ().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 ())).first->second.push_back (n); } @@ -854,8 +855,8 @@ Service::move_cancel () { if (m_move_trans != db::DTrans () && m_moving) { - for (std::vector::iterator r = m_markers.begin (); r != m_markers.end (); ++r) { - (*r)->freeze (); + for (auto r = m_markers.begin (); r != m_markers.end (); ++r) { + r->second->freeze (); } m_move_trans = db::DTrans (); @@ -1031,7 +1032,7 @@ Service::del_selected () std::set needs_cleanup; // delete all shapes and instances. - for (objects::const_iterator r = m_selection.begin (); r != m_selection.end (); ++r) { + for (objects::const_iterator r = selection ().begin (); r != selection ().end (); ++r) { const lay::CellView &cv = view ()->cellview (r->cv_index ()); if (cv.is_valid ()) { db::Cell &cell = cv->layout ().cell (r->cell_index ()); @@ -1059,13 +1060,13 @@ Service::del_selected () bool Service::has_selection () { - return ! m_selection.empty (); + return ! selection ().empty (); } size_t Service::selection_size () { - return m_selection.size (); + return selection ().size (); } bool @@ -1317,11 +1318,11 @@ static std::string path_to_string (const db::Layout &layout, const lay::ObjectIn void Service::display_status (bool transient) { - const objects *selection = transient ? &m_transient_selection : &m_selection; + const objects *sel = transient ? &transient_selection () : &selection (); - if (selection->size () == 1) { + if (sel->size () == 1) { - objects::const_iterator r = selection->begin (); + objects::const_iterator r = sel->begin (); const db::Layout &layout = view ()->cellview (r->cv_index ())->layout (); if (m_cell_inst_service) { @@ -1518,7 +1519,7 @@ Service::select (const db::DBox &box, lay::Editable::SelectionMode mode) // clear the selection if it was consisting of a guiding shape before objects::const_iterator s0 = m_selection.begin (); - if (s0 != m_selection.end () && s0->layer () == view ()->cellview (s0->cv_index ())->layout ().guiding_shape_layer ()) { + if (s0 != m_selection.end () && s0->is_valid (view ()) && s0->layer () == view ()->cellview (s0->cv_index ())->layout ().guiding_shape_layer ()) { m_selection.clear (); } @@ -1550,6 +1551,19 @@ Service::select (const db::DBox &box, lay::Editable::SelectionMode mode) return any_selected; } +const std::set & +Service::selection () const +{ + if (m_selection_maybe_invalid) { + std::vector valid_sel; + get_selection (valid_sel); + m_selection = std::set (valid_sel.begin (), valid_sel.end ()); + m_selection_maybe_invalid = false; + } + + return m_selection; +} + void Service::get_selection (std::vector &sel) const { @@ -1558,11 +1572,40 @@ Service::get_selection (std::vector &sel) const // positions will hold a set of iterators that are to be erased for (std::set::const_iterator r = m_selection.begin (); r != m_selection.end (); ++r) { - sel.push_back (*r); + if (r->is_valid (view ())) { + sel.push_back (*r); + } } } -bool +const std::set & +Service::transient_selection () const +{ + if (m_transient_selection_maybe_invalid) { + std::vector valid_sel; + get_transient_selection (valid_sel); + m_transient_selection = std::set (valid_sel.begin (), valid_sel.end ()); + m_transient_selection_maybe_invalid = false; + } + + return m_transient_selection; +} + +void +Service::get_transient_selection (std::vector &sel) const +{ + sel.clear (); + sel.reserve (m_transient_selection.size ()); + + // positions will hold a set of iterators that are to be erased + for (std::set::const_iterator r = m_transient_selection.begin (); r != m_transient_selection.end (); ++r) { + if (r->is_valid (view ())) { + sel.push_back (*r); + } + } +} + +bool Service::select (const lay::ObjectInstPath &obj, lay::Editable::SelectionMode mode) { // allocate next sequence number @@ -1627,9 +1670,9 @@ Service::move_markers (const db::DTrans &t) view ()->message (pos); } - for (std::vector::iterator r = m_markers.begin (); r != m_markers.end (); ++r) { + for (auto r = m_markers.begin (); r != m_markers.end (); ++r) { - lay::GenericMarkerBase *marker = dynamic_cast (*r); + lay::GenericMarkerBase *marker = dynamic_cast (r->second); if (marker) { db::DCplxTrans dt = db::DCplxTrans (t) * db::DCplxTrans (m_move_trans).inverted (); marker->set_trans (dt * marker->trans ()); @@ -1662,11 +1705,15 @@ Service::selection_to_view () clear_transient_selection (); // the selection objects need to be recreated since we destroyed the old markers - for (std::vector::iterator r = m_markers.begin (); r != m_markers.end (); ++r) { - delete *r; + for (auto r = m_markers.begin (); r != m_markers.end (); ++r) { + delete r->second; } m_markers.clear (); + // selection may become invalid (issue-1145) + m_selection_maybe_invalid = true; + m_transient_selection_maybe_invalid = true; + dm_selection_to_view (); } @@ -1674,25 +1721,14 @@ void Service::do_selection_to_view () { // Hint: this is a lower bound: - m_markers.reserve (m_selection.size ()); + m_markers.reserve (selection ().size ()); // build the transformation variants cache TransformationVariants tv (view ()); - // Reduce the selection to valid paths (issue-1145) - std::vector::iterator> invalid_objects; - for (std::set::iterator r = m_selection.begin (); r != m_selection.end (); ++r) { - if (! r->is_valid (view ())) { - invalid_objects.push_back (r); - } - } - for (auto i = invalid_objects.begin (); i != invalid_objects.end (); ++i) { - m_selection.erase (*i); - } - // Build markers - for (std::set::iterator r = m_selection.begin (); r != m_selection.end (); ++r) { + for (std::set::iterator r = selection ().begin (); r != selection ().end (); ++r) { const lay::CellView &cv = view ()->cellview (r->cv_index ()); @@ -1722,7 +1758,7 @@ Service::do_selection_to_view () marker->set_dither_pattern (3); } marker->set (r->back ().inst_ptr, gt, *tv_list); - m_markers.push_back (marker); + m_markers.push_back (std::make_pair (r.operator-> (), marker)); } else { @@ -1735,7 +1771,7 @@ Service::do_selection_to_view () } db::box_convert bc (cv->layout ()); marker->set (bc (r->back ().inst_ptr.cell_inst ().object ()), gt * r->back ().inst_ptr.cell_inst ().complex_trans (*r->back ().array_inst), *tv_list); - m_markers.push_back (marker); + m_markers.push_back (std::make_pair (r.operator-> (), marker)); } @@ -1766,7 +1802,7 @@ Service::do_selection_to_view () marker->set_vertex_size (9 /*cross vertex size*/); } - m_markers.push_back (marker); + m_markers.push_back (std::make_pair (r.operator-> (), marker)); } @@ -1891,11 +1927,11 @@ bool Service::handle_guiding_shape_changes () { // just allow one guiding shape to be selected - if (m_selection.empty ()) { + if (selection ().empty ()) { return false; } - std::pair gs = handle_guiding_shape_changes (*m_selection.begin ()); + std::pair gs = handle_guiding_shape_changes (*selection ().begin ()); if (gs.first) { // remove superfluous proxies diff --git a/src/edt/edt/edtService.h b/src/edt/edt/edtService.h index 733be7298..e6a485cfd 100644 --- a/src/edt/edt/edtService.h +++ b/src/edt/edt/edtService.h @@ -109,7 +109,7 @@ public: /** * @brief Highlights a group of objects */ - void highlight (const std::vector &n); + void highlight (const std::set &highlights); /** * @brief "delete" operation @@ -200,11 +200,16 @@ public: virtual bool selection_applies (const lay::ObjectInstPath &sel) const; /** - * @brief Get the selection for the properties page + * @brief Get the (cleaned) selection for the properties page */ void get_selection (std::vector &selection) const; - /** + /** + * @brief Get the (cleaned) transient selection + */ + void get_transient_selection (std::vector &selection) const; + + /** * @brief "transform" operation with a transformation vector * * This version of the transformation operation allows one to specify a transformation per selected object. @@ -229,18 +234,12 @@ public: /** * @brief Get the selection container */ - const objects &selection () const - { - return m_selection; - } + const objects &selection () const; /** * @brief Get the transient selection container */ - const objects &transient_selection () const - { - return m_transient_selection; - } + const objects &transient_selection () const; /** * @brief Access to the view object @@ -588,7 +587,7 @@ private: lay::LayoutViewBase *mp_view; // The marker objects representing the selection - std::vector m_markers; + std::vector > m_markers; // Marker for the transient selection lay::ViewObject *mp_transient_marker; @@ -602,14 +601,20 @@ private: // True, if on the first mouse move an immediate do_begin_edit should be issued. bool m_immediate; - // The selection - objects m_selection; + // The selection (mutable because we clean it on the fly) + mutable objects m_selection; + + // A flag indicating that the selection may need cleanup + mutable bool m_selection_maybe_invalid; // The previous selection (used for cycling through different selections for single clicks) objects m_previous_selection; - // The transient selection - objects m_transient_selection; + // A flag indicating that the transient selection may need cleanup + mutable bool m_transient_selection_maybe_invalid; + + // The transient selection (mutable because we clean it on the fly) + mutable objects m_transient_selection; // True, if this service deals with cell instances bool m_cell_inst_service; @@ -644,7 +649,7 @@ private: // selective highlights bool m_highlights_selected; - std::set m_selected_highlights; + std::set m_selected_highlights; // Deferred method to update the selection tl::DeferredMethod dm_selection_to_view; From 9cec3f94310b946af1aa9c6b4c160929edfea7d4 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 9 Jun 2024 23:22:32 +0200 Subject: [PATCH 2/2] Simplified solution of for the invalid selection problem 1.) transient selection does not need to be handled 2.) restored original behavior or set_selection/get_selection (should not modify the selection) 3.) only geometry changes will trigger a selection cleanup --- src/edt/edt/edtService.cc | 79 ++++++++++++++++++++------------------- src/edt/edt/edtService.h | 21 +++++------ 2 files changed, 51 insertions(+), 49 deletions(-) diff --git a/src/edt/edt/edtService.cc b/src/edt/edt/edtService.cc index 53991397e..7260b7cb6 100644 --- a/src/edt/edt/edtService.cc +++ b/src/edt/edt/edtService.cc @@ -68,7 +68,7 @@ Service::Service (db::Manager *manager, lay::LayoutViewBase *view, db::ShapeIter mp_view (view), mp_transient_marker (0), m_editing (false), m_immediate (false), - m_selection_maybe_invalid (true), m_transient_selection_maybe_invalid (true), + m_selection_maybe_invalid (false), m_cell_inst_service (false), m_flags (flags), m_move_sel (false), m_moving (false), @@ -82,7 +82,7 @@ Service::Service (db::Manager *manager, lay::LayoutViewBase *view, db::ShapeIter m_highlights_selected (false), dm_selection_to_view (this, &edt::Service::do_selection_to_view) { - mp_view->geom_changed_event.add (this, &edt::Service::selection_to_view); + mp_view->geom_changed_event.add (this, &edt::Service::geometry_changing); } Service::Service (db::Manager *manager, lay::LayoutViewBase *view) @@ -91,7 +91,7 @@ Service::Service (db::Manager *manager, lay::LayoutViewBase *view) mp_view (view), mp_transient_marker (0), m_editing (false), m_immediate (false), - m_selection_maybe_invalid (true), m_transient_selection_maybe_invalid (true), + m_selection_maybe_invalid (false), m_cell_inst_service (true), m_flags (db::ShapeIterator::Nothing), m_move_sel (false), m_moving (false), @@ -105,7 +105,7 @@ Service::Service (db::Manager *manager, lay::LayoutViewBase *view) m_highlights_selected (false), dm_selection_to_view (this, &edt::Service::do_selection_to_view) { - mp_view->geom_changed_event.add (this, &edt::Service::selection_to_view); + mp_view->geom_changed_event.add (this, &edt::Service::geometry_changing); } Service::~Service () @@ -1446,6 +1446,7 @@ Service::select (const db::DBox &box, lay::Editable::SelectionMode mode) if (mode == lay::Editable::Replace) { if (! m_selection.empty ()) { m_selection.clear (); + m_selection_maybe_invalid = false; needs_update = true; } } @@ -1467,6 +1468,7 @@ Service::select (const db::DBox &box, lay::Editable::SelectionMode mode) if (mode == lay::Editable::Reset) { if (! m_selection.empty ()) { m_selection.clear (); + m_selection_maybe_invalid = false; needs_update = true; } } else { @@ -1510,6 +1512,7 @@ Service::select (const db::DBox &box, lay::Editable::SelectionMode mode) if (box.is_point () && f0 != finder.end () && f0->layer () == view ()->cellview (f0->cv_index ())->layout ().guiding_shape_layer ()) { m_selection.clear (); + m_selection_maybe_invalid = false; select (*f0, mode); m_previous_selection.insert (*f0); needs_update = true; @@ -1521,6 +1524,7 @@ Service::select (const db::DBox &box, lay::Editable::SelectionMode mode) objects::const_iterator s0 = m_selection.begin (); if (s0 != m_selection.end () && s0->is_valid (view ()) && s0->layer () == view ()->cellview (s0->cv_index ())->layout ().guiding_shape_layer ()) { m_selection.clear (); + m_selection_maybe_invalid = false; } // collect the founds from the finder @@ -1555,10 +1559,24 @@ const std::set & Service::selection () const { if (m_selection_maybe_invalid) { - std::vector valid_sel; - get_selection (valid_sel); - m_selection = std::set (valid_sel.begin (), valid_sel.end ()); + + bool any_invalid = false; + for (auto r = m_selection.begin (); r != m_selection.end () && ! any_invalid; ++r) { + any_invalid = ! r->is_valid (view ()); + } + + if (any_invalid) { + std::set valid_sel; + for (auto r = m_selection.begin (); r != m_selection.end (); ++r) { + if (r->is_valid (view ())) { + valid_sel.insert (*r); + } + } + m_selection.swap (valid_sel); + } + m_selection_maybe_invalid = false; + } return m_selection; @@ -1569,42 +1587,17 @@ Service::get_selection (std::vector &sel) const { sel.clear (); sel.reserve (m_selection.size ()); - - // positions will hold a set of iterators that are to be erased for (std::set::const_iterator r = m_selection.begin (); r != m_selection.end (); ++r) { - if (r->is_valid (view ())) { - sel.push_back (*r); - } + sel.push_back (*r); } } const std::set & Service::transient_selection () const { - if (m_transient_selection_maybe_invalid) { - std::vector valid_sel; - get_transient_selection (valid_sel); - m_transient_selection = std::set (valid_sel.begin (), valid_sel.end ()); - m_transient_selection_maybe_invalid = false; - } - return m_transient_selection; } -void -Service::get_transient_selection (std::vector &sel) const -{ - sel.clear (); - sel.reserve (m_transient_selection.size ()); - - // positions will hold a set of iterators that are to be erased - for (std::set::const_iterator r = m_transient_selection.begin (); r != m_transient_selection.end (); ++r) { - if (r->is_valid (view ())) { - sel.push_back (*r); - } - } -} - bool Service::select (const lay::ObjectInstPath &obj, lay::Editable::SelectionMode mode) { @@ -1698,22 +1691,31 @@ Service::tap (const db::DPoint & /*initial*/) // .. nothing here .. } +void +Service::geometry_changing () +{ + // selection may become invalid (issue-1145) + m_selection_maybe_invalid = true; + + // clear the previous selection for safety + clear_previous_selection (); + + selection_to_view (); +} + void Service::selection_to_view () { // we don't handle the transient selection properly, so clear it for safety reasons clear_transient_selection (); - // the selection objects need to be recreated since we destroyed the old markers + // clear markers for (auto r = m_markers.begin (); r != m_markers.end (); ++r) { delete r->second; } m_markers.clear (); - // selection may become invalid (issue-1145) - m_selection_maybe_invalid = true; - m_transient_selection_maybe_invalid = true; - + // postpone the actual work to collect multiple calls dm_selection_to_view (); } @@ -1818,6 +1820,7 @@ Service::set_selection (std::vector ::const_iterator s1, st { m_selection.clear (); m_selection.insert (s1, s2); + m_selection_maybe_invalid = false; selection_to_view (); } diff --git a/src/edt/edt/edtService.h b/src/edt/edt/edtService.h index e6a485cfd..ac2e5ad4c 100644 --- a/src/edt/edt/edtService.h +++ b/src/edt/edt/edtService.h @@ -200,15 +200,10 @@ public: virtual bool selection_applies (const lay::ObjectInstPath &sel) const; /** - * @brief Get the (cleaned) selection for the properties page + * @brief Get the selection for the properties page */ void get_selection (std::vector &selection) const; - /** - * @brief Get the (cleaned) transient selection - */ - void get_transient_selection (std::vector &selection) const; - /** * @brief "transform" operation with a transformation vector * @@ -402,6 +397,13 @@ protected: */ void selection_to_view (); + /** + * @brief Callback when any geometry is changing in the layout + * + * Will call selection_to_view() and invalidate the selection. + */ + void geometry_changing (); + /** * @brief starts editing at the given point. */ @@ -610,11 +612,8 @@ private: // The previous selection (used for cycling through different selections for single clicks) objects m_previous_selection; - // A flag indicating that the transient selection may need cleanup - mutable bool m_transient_selection_maybe_invalid; - - // The transient selection (mutable because we clean it on the fly) - mutable objects m_transient_selection; + // The transient selection + objects m_transient_selection; // True, if this service deals with cell instances bool m_cell_inst_service;