From 292f6f84c6b6491ea91d8e13242965ac5d4dd398 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 28 Sep 2025 16:14:11 +0200 Subject: [PATCH 1/3] Fixing issue #2162 (crash on selection + query) Problem was that changing the active cellview index by clicking on a specific item in the properties dialog cause a "clear_selection". That interfered with the code of the properties dialog that expects a static selection. Fixed by disabling the events that lead to "clear_selection". Side effect is that under these circumstances the active_cellview_changed event is not triggered which also prevents side effects due to scripts hooking into that event. --- src/edt/edt/edtShapeService.cc | 10 ++++++---- src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc | 1 + src/laybasic/laybasic/layLayoutViewBase.cc | 13 +++++++++++++ src/laybasic/laybasic/layLayoutViewBase.h | 6 ++++++ src/layui/layui/layHierarchyControlPanel.cc | 6 ++++-- src/layui/layui/layHierarchyControlPanel.h | 2 +- src/layview/layview/layLayoutView_qt.cc | 9 +++++---- 7 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/edt/edt/edtShapeService.cc b/src/edt/edt/edtShapeService.cc index 4664f777b..86dfadb5e 100644 --- a/src/edt/edt/edtShapeService.cc +++ b/src/edt/edt/edtShapeService.cc @@ -150,8 +150,7 @@ ShapeEditService::get_edit_layer () mp_layout = &(cv->layout ()); mp_cell = cv.cell (); - // fetches the last configuration for the given layer - view ()->set_active_cellview_index (cv_index); + view ()->set_active_cellview_index_silent (cv_index); } void @@ -173,8 +172,9 @@ ShapeEditService::change_edit_layer (const db::LayerProperties &lp) close_editor_hooks (false); } + view ()->set_active_cellview_index_silent (m_cv_index); + // fetches the last configuration for the given layer - view ()->set_active_cellview_index (m_cv_index); config_recent_for_layer (lp, m_cv_index); if (editing ()) { @@ -237,7 +237,9 @@ ShapeEditService::update_edit_layer (const lay::LayerPropertiesConstIterator &cl return; } - view ()->set_active_cellview_index (cv_index); + // NOTE: we don't want side effects during this operation - i.e. some that + // change the selection. Hence no events here. + view ()->set_active_cellview_index_silent (cv_index); const lay::ParsedLayerSource &source = cl->source (true /*real*/); int layer = cl->layer_index (); diff --git a/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc b/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc index 86892f3c1..c9f365c20 100644 --- a/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc +++ b/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc @@ -879,6 +879,7 @@ LAYBASIC_PUBLIC Class decl_LayoutViewBase (decl_Dispatcher, gsi::method ("active_cellview_index=|#active_setview_index=|#set_active_cellview_index", &lay::LayoutViewBase::set_active_cellview_index, gsi::arg ("index"), "@brief Makes the cellview with the given index the active one (shown in hierarchy browser)\n" "See \\active_cellview_index.\n" + "Note, that this changing the active cell view index has side effects such as terminating an editing operation.\n" "\n" "This method has been renamed from set_active_cellview_index to active_cellview_index= in version 0.25. " "The original name is still available, but is deprecated." diff --git a/src/laybasic/laybasic/layLayoutViewBase.cc b/src/laybasic/laybasic/layLayoutViewBase.cc index 50a160e5d..4300a9be8 100644 --- a/src/laybasic/laybasic/layLayoutViewBase.cc +++ b/src/laybasic/laybasic/layLayoutViewBase.cc @@ -4943,6 +4943,19 @@ LayoutViewBase::active_cellview_index () const return m_active_cellview_index; } +void +LayoutViewBase::set_active_cellview_index_silent (int index) +{ + enable_active_cellview_changed_event (false); + try { + set_active_cellview_index (index); + enable_active_cellview_changed_event (true, true /*silent*/); + } catch (...) { + enable_active_cellview_changed_event (true, true /*silent*/); + throw; + } +} + void LayoutViewBase::set_active_cellview_index (int index) { diff --git a/src/laybasic/laybasic/layLayoutViewBase.h b/src/laybasic/laybasic/layLayoutViewBase.h index eeda5619a..49edddaff 100644 --- a/src/laybasic/laybasic/layLayoutViewBase.h +++ b/src/laybasic/laybasic/layLayoutViewBase.h @@ -2151,6 +2151,12 @@ public: */ virtual void set_active_cellview_index (int index); + /** + * @brief Select a certain cellview for the active one + * This version does not emit any events while changing the cellview index + */ + void set_active_cellview_index_silent (int index); + /** * @brief An event triggered if the active cellview changes * This event is triggered after the active cellview changed. diff --git a/src/layui/layui/layHierarchyControlPanel.cc b/src/layui/layui/layHierarchyControlPanel.cc index 8c6ee9678..7a29ac11a 100644 --- a/src/layui/layui/layHierarchyControlPanel.cc +++ b/src/layui/layui/layHierarchyControlPanel.cc @@ -707,11 +707,13 @@ HierarchyControlPanel::update_required () } void -HierarchyControlPanel::select_active (int cellview_index) +HierarchyControlPanel::select_active (int cellview_index, bool silent) { if (cellview_index != m_active_index) { mp_selector->setCurrentIndex (cellview_index); - selection_changed (cellview_index); + if (! silent) { + selection_changed (cellview_index); + } } } diff --git a/src/layui/layui/layHierarchyControlPanel.h b/src/layui/layui/layHierarchyControlPanel.h index b67df7f09..0b5bf1d8e 100644 --- a/src/layui/layui/layHierarchyControlPanel.h +++ b/src/layui/layui/layHierarchyControlPanel.h @@ -140,7 +140,7 @@ public: * selects the active cellview by index. The index must be * a valid index within the context of the layout view. */ - void select_active (int cellview_index); + void select_active (int cellview_index, bool silent = false); /** * @brief Get the active cellview diff --git a/src/layview/layview/layLayoutView_qt.cc b/src/layview/layview/layLayoutView_qt.cc index 4502aec59..df1a40732 100644 --- a/src/layview/layview/layLayoutView_qt.cc +++ b/src/layview/layview/layLayoutView_qt.cc @@ -418,7 +418,7 @@ LayoutView::LayoutView (lay::LayoutView *source, db::Manager *manager, bool edit copy_from (source); bookmarks (source->bookmarks ()); - LayoutView::set_active_cellview_index (source->active_cellview_index ()); + set_active_cellview_index_silent (source->active_cellview_index ()); } LayoutView::LayoutView (db::Manager *manager, bool editable, lay::Plugin *plugin_parent, LayoutViewWidget *widget, unsigned int options) @@ -445,7 +445,7 @@ LayoutView::LayoutView (lay::LayoutView *source, db::Manager *manager, bool edit copy_from (source); bookmarks (source->bookmarks ()); - LayoutView::set_active_cellview_index (source->active_cellview_index ()); + set_active_cellview_index_silent (source->active_cellview_index ()); } bool @@ -1387,12 +1387,13 @@ LayoutView::active_cellview_index () const } } -void +void LayoutView::set_active_cellview_index (int index) { if (index >= 0 && index < int (cellviews ())) { if (mp_hierarchy_panel) { - mp_hierarchy_panel->select_active (index); + // NOTE: we don't send events from here, that is done in "LayoutViewBase::set_active_cellview_index" + mp_hierarchy_panel->select_active (index, true /*no events*/); } LayoutViewBase::set_active_cellview_index (index); } From 31ddbe24fab11fc49bff916b2d3b39cbe51787a5 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 28 Sep 2025 16:25:35 +0200 Subject: [PATCH 2/3] Fixed a potential crash on application exit that was discovered during debugging --- src/laybasic/laybasic/layLayoutViewBase.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/laybasic/laybasic/layLayoutViewBase.cc b/src/laybasic/laybasic/layLayoutViewBase.cc index 4300a9be8..6c03e7087 100644 --- a/src/laybasic/laybasic/layLayoutViewBase.cc +++ b/src/laybasic/laybasic/layLayoutViewBase.cc @@ -475,6 +475,12 @@ LayoutViewBase::shutdown () } } + // NOTE: this must happen before the services are deleted + mp_move_service = 0; + mp_selection_service = 0; + mp_tracker = 0; + mp_zoom_service = 0; + // delete all plugins std::vector plugins; plugins.swap (mp_plugins); From 17f4397f1b6ee3e83df851ff75d1eb0e741b8b28 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sun, 28 Sep 2025 20:17:52 +0200 Subject: [PATCH 3/3] Refining solution --- src/layui/layui/layHierarchyControlPanel.cc | 56 +++++++++++---------- src/layui/layui/layHierarchyControlPanel.h | 3 ++ src/layview/layview/layLayoutView_qt.cc | 4 +- 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/layui/layui/layHierarchyControlPanel.cc b/src/layui/layui/layHierarchyControlPanel.cc index 7a29ac11a..140f8fcae 100644 --- a/src/layui/layui/layHierarchyControlPanel.cc +++ b/src/layui/layui/layHierarchyControlPanel.cc @@ -711,42 +711,46 @@ HierarchyControlPanel::select_active (int cellview_index, bool silent) { if (cellview_index != m_active_index) { mp_selector->setCurrentIndex (cellview_index); + change_active_cellview (cellview_index); if (! silent) { - selection_changed (cellview_index); + emit active_cellview_changed (cellview_index); } } } +void +HierarchyControlPanel::change_active_cellview (int index) +{ + search_editing_finished (); + + m_active_index = index; + + bool split_mode = m_split_mode; + // for more than max_cellviews_in_split_mode cellviews, switch to overlay mode + if (int (m_cellviews.size ()) > max_cellviews_in_split_mode) { + split_mode = false; + } + + int i = 0; + for (std::vector ::const_iterator f = mp_cell_list_frames.begin (); f != mp_cell_list_frames.end (); ++f, ++i) { + (*f)->setVisible (i == index || split_mode); + if (i == index) { + mp_cell_lists [i]->setFocus (); + } + } + + i = 0; + for (std::vector ::const_iterator f = mp_cell_list_headers.begin (); f != mp_cell_list_headers.end (); ++f, ++i) { + (*f)->setChecked (i == index); + } +} + void HierarchyControlPanel::selection_changed (int index) { if (index != m_active_index) { - - search_editing_finished (); - - m_active_index = index; - - bool split_mode = m_split_mode; - // for more than max_cellviews_in_split_mode cellviews, switch to overlay mode - if (int (m_cellviews.size ()) > max_cellviews_in_split_mode) { - split_mode = false; - } - - int i = 0; - for (std::vector ::const_iterator f = mp_cell_list_frames.begin (); f != mp_cell_list_frames.end (); ++f, ++i) { - (*f)->setVisible (i == index || split_mode); - if (i == index) { - mp_cell_lists [i]->setFocus (); - } - } - - i = 0; - for (std::vector ::const_iterator f = mp_cell_list_headers.begin (); f != mp_cell_list_headers.end (); ++f, ++i) { - (*f)->setChecked (i == index); - } - + change_active_cellview (index); emit active_cellview_changed (index); - } } diff --git a/src/layui/layui/layHierarchyControlPanel.h b/src/layui/layui/layHierarchyControlPanel.h index 0b5bf1d8e..03a6630a7 100644 --- a/src/layui/layui/layHierarchyControlPanel.h +++ b/src/layui/layui/layHierarchyControlPanel.h @@ -346,6 +346,9 @@ private: // ask for cell copy mode bool ask_for_cell_copy_mode (const db::Layout &layout, const std::vector &paths, int &cell_copy_mode); + + // changes the active cellview + void change_active_cellview (int index); }; } // namespace lay diff --git a/src/layview/layview/layLayoutView_qt.cc b/src/layview/layview/layLayoutView_qt.cc index df1a40732..7fb6d316f 100644 --- a/src/layview/layview/layLayoutView_qt.cc +++ b/src/layview/layview/layLayoutView_qt.cc @@ -418,7 +418,7 @@ LayoutView::LayoutView (lay::LayoutView *source, db::Manager *manager, bool edit copy_from (source); bookmarks (source->bookmarks ()); - set_active_cellview_index_silent (source->active_cellview_index ()); + LayoutView::set_active_cellview_index (source->active_cellview_index ()); } LayoutView::LayoutView (db::Manager *manager, bool editable, lay::Plugin *plugin_parent, LayoutViewWidget *widget, unsigned int options) @@ -445,7 +445,7 @@ LayoutView::LayoutView (lay::LayoutView *source, db::Manager *manager, bool edit copy_from (source); bookmarks (source->bookmarks ()); - set_active_cellview_index_silent (source->active_cellview_index ()); + LayoutView::set_active_cellview_index (source->active_cellview_index ()); } bool