From e2f9015c2671e7d01b61e9155c6b71361cb42143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20K=C3=B6fferlein?= Date: Mon, 1 Aug 2022 18:47:20 +0200 Subject: [PATCH 1/3] Fixed issue #1132 by taking the first point of paths and polygons instead of center (center is still taken if the polygon is a rectangle) (#1134) --- .../lefdef/db_plugin/dbLEFImporter.cc | 32 +++++++++++++++--- .../lefdef/unit_tests/dbLEFDEFImportTests.cc | 12 +++++++ testdata/lefdef/issue-1132/au.oas.gz | Bin 0 -> 220 bytes testdata/lefdef/issue-1132/test.lef | 20 +++++++++++ 4 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 testdata/lefdef/issue-1132/au.oas.gz create mode 100644 testdata/lefdef/issue-1132/test.lef diff --git a/src/plugins/streamers/lefdef/db_plugin/dbLEFImporter.cc b/src/plugins/streamers/lefdef/db_plugin/dbLEFImporter.cc index 5d6625ad4..5f458d7c8 100644 --- a/src/plugins/streamers/lefdef/db_plugin/dbLEFImporter.cc +++ b/src/plugins/streamers/lefdef/db_plugin/dbLEFImporter.cc @@ -135,6 +135,26 @@ static db::Shape insert_shape (db::Cell &cell, unsigned int layer_id, const Shap } } +static db::Box box_for_label (const db::Polygon &p) +{ + if (p.is_box ()) { + return p.box (); + } else if (p.begin_hull () != p.end_hull ()) { + return db::Box (*p.begin_hull (), *p.begin_hull ()); + } else { + return db::Box (); + } +} + +static db::Box box_for_label (const db::Path &p) +{ + if (p.begin () != p.end ()) { + return db::Box (*p.begin (), *p.begin ()); + } else { + return db::Box (); + } +} + void LEFImporter::read_geometries (GeometryBasedLayoutGenerator *lg, double dbu, LayerPurpose purpose, std::map *collect_boxes_for_labels, db::properties_id_type prop_id) { @@ -201,7 +221,7 @@ LEFImporter::read_geometries (GeometryBasedLayoutGenerator *lg, double dbu, Laye db::Path pt = p.transformed (*t); lg->add_path (layer_name, purpose, pt, mask, prop_id); if (collect_boxes_for_labels) { - collect_boxes_for_labels->insert (std::make_pair (layer_name, db::Box ())).first->second = pt.box (); + collect_boxes_for_labels->insert (std::make_pair (layer_name, db::Box ())).first->second = box_for_label (pt); } } @@ -209,7 +229,7 @@ LEFImporter::read_geometries (GeometryBasedLayoutGenerator *lg, double dbu, Laye lg->add_path (layer_name, purpose, p, mask, prop_id); if (collect_boxes_for_labels) { - collect_boxes_for_labels->insert (std::make_pair (layer_name, db::Box ())).first->second = p.box (); + collect_boxes_for_labels->insert (std::make_pair (layer_name, db::Box ())).first->second = box_for_label (p); } } @@ -249,7 +269,7 @@ LEFImporter::read_geometries (GeometryBasedLayoutGenerator *lg, double dbu, Laye db::Polygon pt = p.transformed (*t); lg->add_polygon (layer_name, purpose, pt, mask, prop_id); if (collect_boxes_for_labels) { - collect_boxes_for_labels->insert (std::make_pair (layer_name, db::Box ())).first->second = pt.box (); + collect_boxes_for_labels->insert (std::make_pair (layer_name, db::Box ())).first->second = box_for_label (pt); } } @@ -257,7 +277,7 @@ LEFImporter::read_geometries (GeometryBasedLayoutGenerator *lg, double dbu, Laye lg->add_polygon (layer_name, purpose, p, mask, prop_id); if (collect_boxes_for_labels) { - collect_boxes_for_labels->insert (std::make_pair (layer_name, db::Box ())).first->second = p.box (); + collect_boxes_for_labels->insert (std::make_pair (layer_name, db::Box ())).first->second = box_for_label (p); } } @@ -883,7 +903,9 @@ LEFImporter::read_macro (Layout &layout) read_geometries (mg, layout.dbu (), LEFPins, &boxes_for_labels, prop_id); for (std::map ::const_iterator b = boxes_for_labels.begin (); b != boxes_for_labels.end (); ++b) { - mg->add_text (b->first, LEFLabel, db::Text (label.c_str (), db::Trans (b->second.center () - db::Point ())), 0, 0); + if (! b->second.empty ()) { + mg->add_text (b->first, LEFLabel, db::Text (label.c_str (), db::Trans (b->second.center () - db::Point ())), 0, 0); + } } } else { diff --git a/src/plugins/streamers/lefdef/unit_tests/dbLEFDEFImportTests.cc b/src/plugins/streamers/lefdef/unit_tests/dbLEFDEFImportTests.cc index fd49e97d2..e6a8bcba7 100644 --- a/src/plugins/streamers/lefdef/unit_tests/dbLEFDEFImportTests.cc +++ b/src/plugins/streamers/lefdef/unit_tests/dbLEFDEFImportTests.cc @@ -930,3 +930,15 @@ TEST(203_regionsAndMapfileConcat) run_test (_this, "map_regions", "map:'test.map,test.add.map'+lef:test.lef+def:test.def", "au.oas.gz", lefdef_opt, false); } +// issue 1132 +TEST(204_concave_pins) +{ + db::LEFDEFReaderOptions lefdef_opt = default_options (); + lefdef_opt.set_lef_pins_datatype (12); + lefdef_opt.set_lef_pins_suffix (".LEFPIN"); + lefdef_opt.set_lef_labels_datatype (11); + lefdef_opt.set_lef_labels_suffix (".LEFLABEL"); + + run_test (_this, "issue-1132", "read:test.lef", "au.oas.gz", lefdef_opt, false); +} + diff --git a/testdata/lefdef/issue-1132/au.oas.gz b/testdata/lefdef/issue-1132/au.oas.gz new file mode 100644 index 0000000000000000000000000000000000000000..08e39f76bfbfc387a799e43676af7f17e03b003e GIT binary patch literal 220 zcmV<203-h&iwFoI(&l0S17US8Z((x)Qw?_Y_0;uu4E7A>JUCcncx52cp?=sD$cS{X;{1JpEjm8JQV)kU8Akxv3>4dOoghAdO7SJUr+E z+`KRWA4exwAD|F752g?=gGe$X!-vV72WIhz8Z+@QvMiXx`0xtz1iuf9SRPzq7A|IB W;s(-84I>#aFaQASBCo#|0ssIRG-DzF literal 0 HcmV?d00001 diff --git a/testdata/lefdef/issue-1132/test.lef b/testdata/lefdef/issue-1132/test.lef new file mode 100644 index 000000000..9f787aba7 --- /dev/null +++ b/testdata/lefdef/issue-1132/test.lef @@ -0,0 +1,20 @@ +VERSION 5.7 ; +DIVIDERCHAR "/" ; +BUSBITCHARS "[]" ; + +MACRO POLYGON_PIN + FOREIGN POLYGON_PIN ; + ORIGIN 0.000 0.000 ; + SIZE 150.000 BY 200.000 ; + PIN VDD + DIRECTION INOUT ; + USE POWER ; + PORT + LAYER met4 ; + POLYGON 0.000 10.000 30.000 10.000 30.000 0.000 35.000 0.000 35.000 15.000 0.000 15.0000 ; + END + END VDD +END POLYGON_PIN + +END LIBRARY + From 801ef78990738f36cc2a838bd695764696f26db9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20K=C3=B6fferlein?= Date: Mon, 1 Aug 2022 18:49:42 +0200 Subject: [PATCH 2/3] Fixed issue-1131 (do not show non-existing files in MRU lists) (#1133) * Fixed issue-1131 (do not show files in MRU lists which do no longer exist) The solution consists of an extension of the Action system allowing to dynamically hide or disable items. This currently works for menu items only. This feature is used to dynamically *disable* (as of now, not hiding) items from the four MRU lists corresponding to non-existing files. In addition, a "clear list" menu has been added to the MRU lists. * Small enhancement: file names can be URIs --- src/lay/lay/layMainWindow.cc | 148 +++++++++++++---------- src/lay/lay/layMainWindow.h | 4 + src/laybasic/laybasic/gsiDeclLayMenu.cc | 46 ++++++- src/laybasic/laybasic/layAbstractMenu.cc | 108 ++++++++++++----- src/laybasic/laybasic/layAbstractMenu.h | 36 +++++- testdata/ruby/layMenuTest.rb | 50 +++++++- 6 files changed, 296 insertions(+), 96 deletions(-) diff --git a/src/lay/lay/layMainWindow.cc b/src/lay/lay/layMainWindow.cc index be6e1bb57..129c913fc 100644 --- a/src/lay/lay/layMainWindow.cc +++ b/src/lay/lay/layMainWindow.cc @@ -51,6 +51,8 @@ #include "tlStream.h" #include "tlExceptions.h" #include "tlExpression.h" +#include "tlFileUtils.h" +#include "tlUri.h" #include "dbMemStatistics.h" #include "dbManager.h" #include "dbStream.h" @@ -2902,29 +2904,28 @@ MainWindow::add_mru (const std::string &fn_rel) add_mru (fn_rel, m_initial_technology); } -const size_t max_mru = 16; +const size_t max_mru = 16; // TODO: make configurable? void MainWindow::add_mru (const std::string &fn_rel, const std::string &tech) { - std::vector > new_mru (m_mru); + std::vector > new_mru; std::string fn (tl::InputStream::absolute_path (fn_rel)); - for (std::vector >::iterator mru = new_mru.begin (); mru != new_mru.end (); ++mru) { - if (mru->first == fn) { - new_mru.erase (mru); - break; + for (auto mru = m_mru.begin (); mru != m_mru.end (); ++mru) { + if (mru->first != fn /* delete non-existing files: && tl::is_readable (mru->first) */) { + new_mru.push_back (*mru); } } new_mru.push_back (std::make_pair (fn, tech)); if (new_mru.size () > max_mru) { - new_mru.erase (new_mru.begin ()); + new_mru.erase (new_mru.begin (), new_mru.end () - max_mru); } std::string config_str; - for (std::vector >::const_iterator mru = new_mru.begin (); mru != new_mru.end (); ++mru) { + for (auto mru = new_mru.begin (); mru != new_mru.end (); ++mru) { if (! config_str.empty ()) { config_str += " "; } @@ -2952,20 +2953,19 @@ MainWindow::add_to_other_mru (const std::string &fn_rel, const std::string &cfg) tl_assert (false); } - std::vector new_mru = *mru_ptr; + std::vector new_mru; std::string fn (tl::InputStream::absolute_path (fn_rel)); - for (std::vector::iterator mru = new_mru.begin (); mru != new_mru.end (); ++mru) { - if (*mru == fn) { - new_mru.erase (mru); - break; + for (auto mru = mru_ptr->begin (); mru != mru_ptr->end (); ++mru) { + if (*mru != fn /* delete non-existing files: && tl::is_readable (*mru) */) { + new_mru.push_back (*mru); } } new_mru.push_back (fn); if (new_mru.size () > max_mru) { - new_mru.erase (new_mru.begin ()); + new_mru.erase (new_mru.begin (), new_mru.end () - max_mru); } std::string config_str; @@ -2986,72 +2986,45 @@ class OpenRecentAction : public lay::Action { public: - OpenRecentAction (lay::MainWindow *mw, size_t n) - : lay::Action (), mp_mw (mw), m_n (n) + OpenRecentAction (lay::MainWindow *mw, size_t n, void (lay::MainWindow::*open_meth) (size_t), bool (lay::MainWindow::*avail_meth) (size_t)) + : lay::Action (), mp_mw (mw), m_n (n), m_open_meth (open_meth), m_avail_meth (avail_meth) { } void triggered () { - mp_mw->open_recent (m_n); + (mp_mw->*m_open_meth) (m_n); + } + + bool wants_enabled () const + { + return (mp_mw->*m_avail_meth) (m_n); } private: lay::MainWindow *mp_mw; size_t m_n; + void (lay::MainWindow::*m_open_meth) (size_t); + bool (lay::MainWindow::*m_avail_meth) (size_t); }; -class OpenRecentSessionAction +class ClearRecentAction : public lay::Action { public: - OpenRecentSessionAction (lay::MainWindow *mw, size_t n) - : lay::Action (), mp_mw (mw), m_n (n) - { } + ClearRecentAction (lay::MainWindow *mw, const std::string &cfg) + : lay::Action (), mp_mw (mw), m_cfg (cfg) + { + set_title (tl::to_string (tr ("Clear List"))); + } void triggered () { - mp_mw->open_recent_session (m_n); + mp_mw->configure (m_cfg, std::string ()); } private: lay::MainWindow *mp_mw; - size_t m_n; -}; - -class OpenRecentLayerPropertiesAction - : public lay::Action -{ -public: - OpenRecentLayerPropertiesAction (lay::MainWindow *mw, size_t n) - : lay::Action (), mp_mw (mw), m_n (n) - { } - - void triggered () - { - mp_mw->open_recent_layer_properties (m_n); - } - -private: - lay::MainWindow *mp_mw; - size_t m_n; -}; - -class OpenRecentBookmarksAction - : public lay::Action -{ -public: - OpenRecentBookmarksAction (lay::MainWindow *mw, size_t n) - : lay::Action (), mp_mw (mw), m_n (n) - { } - - void triggered () - { - mp_mw->open_recent_bookmarks (m_n); - } - -private: - lay::MainWindow *mp_mw; - size_t m_n; + std::string m_cfg; }; } @@ -3074,11 +3047,14 @@ MainWindow::do_update_mru_menus () for (std::vector >::iterator mru = m_mru.end (); mru != m_mru.begin (); ) { --mru; size_t i = std::distance (m_mru.begin (), mru); - Action *action = new OpenRecentAction (this, i); + Action *action = new OpenRecentAction (this, i, &lay::MainWindow::open_recent, &lay::MainWindow::is_available_recent); action->set_title (mru->first); menu ()->insert_item (mru_menu + ".end", tl::sprintf ("open_recent_%d", i + 1), action); } + menu ()->insert_separator (mru_menu + ".end", "clear_sep"); + menu ()->insert_item (mru_menu + ".end", "clear_recent", new ClearRecentAction (this, cfg_mru)); + } else { open_recent_action->set_enabled (false); } @@ -3100,11 +3076,14 @@ MainWindow::do_update_mru_menus () for (std::vector::iterator mru = m_mru_sessions.end (); mru != m_mru_sessions.begin (); ) { --mru; size_t i = std::distance (m_mru_sessions.begin (), mru); - Action *action = new OpenRecentSessionAction (this, i); + Action *action = new OpenRecentAction (this, i, &lay::MainWindow::open_recent_session, &lay::MainWindow::is_available_recent_session); action->set_title (*mru); menu ()->insert_item (mru_menu + ".end", tl::sprintf ("open_recent_%d", i + 1), action); } + menu ()->insert_separator (mru_menu + ".end", "clear_sep"); + menu ()->insert_item (mru_menu + ".end", "clear_recent", new ClearRecentAction (this, cfg_mru_sessions)); + } else { open_recent_action->set_enabled (false); } @@ -3126,11 +3105,14 @@ MainWindow::do_update_mru_menus () for (std::vector::iterator mru = m_mru_layer_properties.end (); mru != m_mru_layer_properties.begin (); ) { --mru; size_t i = std::distance (m_mru_layer_properties.begin (), mru); - Action *action = new OpenRecentLayerPropertiesAction (this, i); + Action *action = new OpenRecentAction (this, i, &lay::MainWindow::open_recent_layer_properties, &lay::MainWindow::is_available_recent_layer_properties); action->set_title (*mru); menu ()->insert_item (mru_menu + ".end", tl::sprintf ("open_recent_%d", i + 1), action); } + menu ()->insert_separator (mru_menu + ".end", "clear_sep"); + menu ()->insert_item (mru_menu + ".end", "clear_recent", new ClearRecentAction (this, cfg_mru_layer_properties)); + } else { open_recent_action->set_enabled (false); } @@ -3152,11 +3134,14 @@ MainWindow::do_update_mru_menus () for (std::vector::iterator mru = m_mru_bookmarks.end (); mru != m_mru_bookmarks.begin (); ) { --mru; size_t i = std::distance (m_mru_bookmarks.begin (), mru); - Action *action = new OpenRecentBookmarksAction (this, i); + Action *action = new OpenRecentAction (this, i, &lay::MainWindow::open_recent_bookmarks, &lay::MainWindow::is_available_recent_bookmarks); action->set_title (*mru); menu ()->insert_item (mru_menu + ".end", tl::sprintf ("open_recent_%d", i + 1), action); } + menu ()->insert_separator (mru_menu + ".end", "clear_sep"); + menu ()->insert_item (mru_menu + ".end", "clear_recent", new ClearRecentAction (this, cfg_mru_bookmarks)); + } else { open_recent_action->set_enabled (false); } @@ -3218,6 +3203,25 @@ MainWindow::open_recent (size_t n) END_PROTECTED } +static bool +is_file_available (const std::string &fn) +{ + tl::URI uri (fn); + if (uri.scheme ().empty ()) { + return tl::is_readable (fn); + } else if (uri.scheme () == "file") { + return tl::is_readable (uri.path ()); + } else { + return true; + } +} + +bool +MainWindow::is_available_recent (size_t n) +{ + return (n < m_mru.size () && is_file_available (m_mru [n].first)); +} + void MainWindow::open_recent_session (size_t n) { @@ -3232,6 +3236,12 @@ MainWindow::open_recent_session (size_t n) END_PROTECTED } +bool +MainWindow::is_available_recent_session (size_t n) +{ + return (n < m_mru_sessions.size () && is_file_available (m_mru_sessions [n])); +} + void MainWindow::open_recent_layer_properties (size_t n) { @@ -3246,6 +3256,12 @@ MainWindow::open_recent_layer_properties (size_t n) END_PROTECTED } +bool +MainWindow::is_available_recent_layer_properties (size_t n) +{ + return (n < m_mru_layer_properties.size () && is_file_available (m_mru_layer_properties [n])); +} + void MainWindow::open_recent_bookmarks (size_t n) { @@ -3264,6 +3280,12 @@ MainWindow::open_recent_bookmarks (size_t n) END_PROTECTED } +bool +MainWindow::is_available_recent_bookmarks (size_t n) +{ + return (n < m_mru_bookmarks.size () && is_file_available (m_mru_bookmarks [n])); +} + void MainWindow::open (int mode) { diff --git a/src/lay/lay/layMainWindow.h b/src/lay/lay/layMainWindow.h index 362894ab2..21f6d9996 100644 --- a/src/lay/lay/layMainWindow.h +++ b/src/lay/lay/layMainWindow.h @@ -637,6 +637,10 @@ public slots: void open_recent_session (size_t n); void open_recent_layer_properties (size_t n); void open_recent_bookmarks (size_t n); + bool is_available_recent(size_t n); + bool is_available_recent_session (size_t n); + bool is_available_recent_layer_properties (size_t n); + bool is_available_recent_bookmarks (size_t n); void view_selected (int index); void view_title_changed (lay::LayoutView *view); diff --git a/src/laybasic/laybasic/gsiDeclLayMenu.cc b/src/laybasic/laybasic/gsiDeclLayMenu.cc index 559ddf8e7..51f3c244e 100644 --- a/src/laybasic/laybasic/gsiDeclLayMenu.cc +++ b/src/laybasic/laybasic/gsiDeclLayMenu.cc @@ -39,7 +39,27 @@ public: on_triggered_event (); } + virtual bool wants_visible () const + { + if (wants_visible_cb.can_issue ()) { + return wants_visible_cb.issue (&lay::Action::wants_visible); + } else { + return true; + } + } + + virtual bool wants_enabled () const + { + if (wants_enabled_cb.can_issue ()) { + return wants_enabled_cb.issue (&lay::Action::wants_enabled); + } else { + return true; + } + } + gsi::Callback triggered_cb; + gsi::Callback wants_visible_cb; + gsi::Callback wants_enabled_cb; tl::Event on_triggered_event; }; @@ -291,10 +311,16 @@ Class decl_ActionBase ("lay", "ActionBase", ) + method ("is_effective_visible?", &lay::Action::is_effective_visible, "@brief Gets a value indicating whether the item is really visible\n" - "This is the combined visibility from \\is_visible? and \\is_hidden?." + "This is the combined visibility from \\is_visible? and \\is_hidden? and dynamic visibility (\\wants_visible)." "\n" "This attribute has been introduced in version 0.25.\n" ) + + method ("is_effective_enabled?", &lay::Action::is_effective_enabled, + "@brief Gets a value indicating whether the item is really enabled\n" + "This is the combined value from \\is_enabled? and dynamic value (\\wants_enabled)." + "\n" + "This attribute has been introduced in version 0.28.\n" + ) + method ("separator=", &lay::Action::set_separator, gsi::arg ("separator"), "@brief Makes an item a separator or not\n" "\n" @@ -368,6 +394,24 @@ Class decl_Action (decl_ActionBase, "lay", "Action", gsi::callback ("triggered", &ActionStub::triggered, &ActionStub::triggered_cb, "@brief This method is called if the menu item is selected" ) + + gsi::callback ("wants_visible", &ActionStub::wants_visible, &ActionStub::wants_visible_cb, + "@brief Returns a value whether the action wants to become visible\n" + "This is a dynamic query for visibility which the system uses to dynamically show or hide " + "menu items, for example in the MRU lists. This visibility information is evaluated in addition " + "to \\is_visible? and \\is_hidden? and contributes to the effective visibility status from " + "\\is_effective_visible?.\n" + "\n" + "This feature has been introduced in version 0.28.\n" + ) + + gsi::callback ("wants_enabled", &ActionStub::wants_enabled, &ActionStub::wants_enabled_cb, + "@brief Returns a value whether the action wants to become enabled\n" + "This is a dynamic query for enabled state which the system uses to dynamically show or hide " + "menu items. This information is evaluated in addition " + "to \\is_enabled? and contributes to the effective enabled status from " + "\\is_effective_enabled?.\n" + "\n" + "This feature has been introduced in version 0.28.\n" + ) + gsi::event ("on_triggered", &ActionStub::on_triggered_event, "@brief This event is called if the menu item is selected\n" "\n" diff --git a/src/laybasic/laybasic/layAbstractMenu.cc b/src/laybasic/laybasic/layAbstractMenu.cc index 9e264218d..8a6f6888c 100644 --- a/src/laybasic/laybasic/layAbstractMenu.cc +++ b/src/laybasic/laybasic/layAbstractMenu.cc @@ -387,7 +387,7 @@ Action::Action () : #if defined(HAVE_QT) // catch the destroyed signal to tell if the QAction object is deleted. if (mp_action) { - connect (mp_action, SIGNAL (destroyed (QObject *)), this, SLOT (destroyed (QObject *))); + connect (mp_action, SIGNAL (destroyed (QObject *)), this, SLOT (was_destroyed (QObject *))); connect (mp_action, SIGNAL (triggered ()), this, SLOT (qaction_triggered ())); } #endif @@ -413,7 +413,7 @@ Action::Action (QAction *action, bool owned) sp_actionHandles->insert (this); // catch the destroyed signal to tell if the QAction object is deleted. - connect (mp_action, SIGNAL (destroyed (QObject *)), this, SLOT (destroyed (QObject *))); + connect (mp_action, SIGNAL (destroyed (QObject *)), this, SLOT (was_destroyed (QObject *))); connect (mp_action, SIGNAL (triggered ()), this, SLOT (qaction_triggered ())); } @@ -436,7 +436,8 @@ Action::Action (QMenu *menu, bool owned) sp_actionHandles->insert (this); // catch the destroyed signal to tell if the QAction object is deleted. - connect (mp_menu, SIGNAL (destroyed (QObject *)), this, SLOT (destroyed (QObject *))); + connect (mp_menu, SIGNAL (destroyed (QObject *)), this, SLOT (was_destroyed (QObject *))); + connect (mp_menu, SIGNAL (aboutToShow ()), this, SLOT (menu_about_to_show ())); connect (mp_action, SIGNAL (triggered ()), this, SLOT (qaction_triggered ())); } #endif @@ -466,7 +467,7 @@ Action::Action (const std::string &title) : #if defined(HAVE_QT) // catch the destroyed signal to tell if the QAction object is deleted. if (mp_action) { - connect (mp_action, SIGNAL (destroyed (QObject *)), this, SLOT (destroyed (QObject *))); + connect (mp_action, SIGNAL (destroyed (QObject *)), this, SLOT (was_destroyed (QObject *))); connect (mp_action, SIGNAL (triggered ()), this, SLOT (qaction_triggered ())); } #endif @@ -536,6 +537,30 @@ Action::configure_from_title (const std::string &s) } } +#if defined(HAVE_QT) +void +Action::menu_about_to_show () +{ + BEGIN_PROTECTED + + if (! mp_dispatcher || ! mp_dispatcher->menu ()) { + return; + } + AbstractMenuItem *item = mp_dispatcher->menu ()->find_item_for_action (this); + if (! item ) { + return; + } + + for (auto i = item->children.begin (); i != item->children.end (); ++i) { + if (i->action ()) { + i->action ()->sync_qaction (); + } + } + + END_PROTECTED +} +#endif + #if defined(HAVE_QT) void Action::qaction_triggered () @@ -578,7 +603,7 @@ Action::menu () const } void -Action::destroyed (QObject *obj) +Action::was_destroyed (QObject *obj) { if (obj == mp_action) { mp_action = 0; @@ -591,17 +616,24 @@ Action::destroyed (QObject *obj) } #endif +void +Action::sync_qaction () +{ +#if defined(HAVE_QT) + if (mp_action) { + mp_action->setVisible (is_effective_visible ()); + mp_action->setShortcut (get_key_sequence ()); + mp_action->setEnabled (is_effective_enabled ()); + } +#endif +} + void Action::set_visible (bool v) { if (m_visible != v) { m_visible = v; -#if defined(HAVE_QT) - if (mp_action) { - mp_action->setVisible (is_effective_visible ()); - mp_action->setShortcut (get_key_sequence ()); - } -#endif + sync_qaction (); } } @@ -610,12 +642,7 @@ Action::set_hidden (bool h) { if (m_hidden != h) { m_hidden = h; -#if defined(HAVE_QT) - if (mp_action) { - mp_action->setVisible (is_effective_visible ()); - mp_action->setShortcut (get_key_sequence ()); - } -#endif + sync_qaction (); } } @@ -634,7 +661,7 @@ Action::is_hidden () const bool Action::is_effective_visible () const { - return m_visible && !m_hidden; + return m_visible && !m_hidden && wants_visible (); } void @@ -792,11 +819,7 @@ Action::is_checked () const bool Action::is_enabled () const { -#if defined(HAVE_QT) - return qaction () ? qaction ()->isEnabled () : m_enabled; -#else return m_enabled; -#endif } bool @@ -808,12 +831,16 @@ Action::is_separator () const void Action::set_enabled (bool b) { -#if defined(HAVE_QT) - if (qaction ()) { - qaction ()->setEnabled (b); + if (m_enabled != b) { + m_enabled = b; + sync_qaction (); } -#endif - m_enabled = b; +} + +bool +Action::is_effective_enabled () const +{ + return m_enabled && wants_enabled (); } void @@ -1508,6 +1535,33 @@ AbstractMenu::delete_items (Action *action) } } +const AbstractMenuItem * +AbstractMenu::find_item_for_action (const Action *action, const AbstractMenuItem *from) const +{ + return (const_cast (this))->find_item_for_action (action, const_cast (from)); +} + +AbstractMenuItem * +AbstractMenu::find_item_for_action (const Action *action, AbstractMenuItem *from) +{ + if (! from) { + from = const_cast (&root ()); + } + + if (from->action () == action) { + return from; + } + + for (auto i = from->children.begin (); i != from->children.end (); ++i) { + AbstractMenuItem *item = find_item_for_action (action, i.operator-> ()); + if (item) { + return item; + } + } + + return 0; +} + const AbstractMenuItem * AbstractMenu::find_item_exact (const std::string &path) const { diff --git a/src/laybasic/laybasic/layAbstractMenu.h b/src/laybasic/laybasic/layAbstractMenu.h index 1a51ff398..ebecc4eba 100644 --- a/src/laybasic/laybasic/layAbstractMenu.h +++ b/src/laybasic/laybasic/layAbstractMenu.h @@ -318,6 +318,34 @@ public: return false; } + /** + * @brief Gets a value indicating the action is visible (dynamic calls) + * In addition to static visibility (visible/hidden), an Action object can request to + * become invisible dynamically based on conditions. This will work for menu-items + * for which the system will query the status before the menu is shown. + */ + virtual bool wants_visible () const + { + return true; + } + + /** + * @brief Gets a value indicating the action is enabled (dynamic calls) + * In addition to static visibility (visible/hidden), an Action object can request to + * become invisible dynamically based on conditions. This will work for menu-items + * for which the system will query the status before the menu is shown. + */ + virtual bool wants_enabled () const + { + return true; + } + + /** + * @brief Gets the effective enabled state + * This is the combined value from is_enabled and wants_enabled. + */ + bool is_effective_enabled () const; + #if defined(HAVE_QT) /** * @brief Get the underlying QAction object @@ -342,8 +370,9 @@ public: #if defined(HAVE_QT) protected slots: - void destroyed (QObject *obj); + void was_destroyed (QObject *obj); void qaction_triggered (); + void menu_about_to_show (); #endif private: @@ -381,6 +410,7 @@ private: #endif void configure_from_title (const std::string &s); + void sync_qaction (); // no copying Action (const Action &); @@ -801,7 +831,7 @@ public: * @param group The group name * @param A vector of all members (as actions) of the group */ - std::vector group_actions(const std::string &name); + std::vector group_actions (const std::string &name); /** * @brief Get the configure actions for a given configuration name @@ -844,6 +874,8 @@ private: std::vector::iterator> > find_item (tl::Extractor &extr); const AbstractMenuItem *find_item_exact (const std::string &path) const; AbstractMenuItem *find_item_exact (const std::string &path); + const AbstractMenuItem *find_item_for_action (const Action *action, const AbstractMenuItem *from = 0) const; + AbstractMenuItem *find_item_for_action (const Action *action, AbstractMenuItem *from = 0); #if defined(HAVE_QT) void build (QMenu *menu, std::list &items); void build (QToolBar *tbar, std::list &items); diff --git a/testdata/ruby/layMenuTest.rb b/testdata/ruby/layMenuTest.rb index 256349b09..0df95a72a 100644 --- a/testdata/ruby/layMenuTest.rb +++ b/testdata/ruby/layMenuTest.rb @@ -227,20 +227,20 @@ RESULT $a1.visible = false assert_equal( menu.action( "my_menu.new_item" ).is_visible?, false ) assert_equal( menu.action( "my_menu.new_item" ).is_checked?, false ) - assert_equal( menu.action( "my_menu.new_item" ).is_enabled?, false ) + assert_equal( menu.action( "my_menu.new_item" ).is_enabled?, true ) $a1.checked = true assert_equal( menu.action( "file_menu.#3" ).is_visible?, false ) assert_equal( menu.action( "file_menu.#3" ).is_checked?, false ) assert_equal( menu.action( "file_menu.#3" ).is_checkable?, false ) - assert_equal( menu.action( "file_menu.#3" ).is_enabled?, false ) + assert_equal( menu.action( "file_menu.#3" ).is_enabled?, true ) $a1.checked = false $a1.checkable = true; assert_equal( menu.action( "my_menu.new_item" ).is_visible?, false ) assert_equal( menu.action( "my_menu.new_item" ).is_checked?, false ) assert_equal( menu.action( "my_menu.new_item" ).is_checkable?, true ) - assert_equal( menu.action( "my_menu.new_item" ).is_enabled?, false ) + assert_equal( menu.action( "my_menu.new_item" ).is_enabled?, true ) $a1.checked = true assert_equal( menu.action( "file_menu.#0" ).is_checked?, true ) @@ -440,6 +440,50 @@ RESULT end + class MyAction < RBA::Action + attr_accessor :dyn_visible, :dyn_enabled + def initialize + self.dyn_visible = true + self.dyn_enabled = true + end + def wants_visible + self.dyn_visible + end + def wants_enabled + self.dyn_enabled + end + end + + def test_7 + + a = MyAction::new + + assert_equal(a.is_effective_visible?, true) + a.hidden = true + assert_equal(a.is_effective_visible?, false) + a.hidden = false + assert_equal(a.is_effective_visible?, true) + a.visible = false + assert_equal(a.is_effective_visible?, false) + a.visible = true + assert_equal(a.is_effective_visible?, true) + a.dyn_visible = false + assert_equal(a.is_effective_visible?, false) + a.dyn_visible = true + assert_equal(a.is_effective_visible?, true) + + assert_equal(a.is_effective_enabled?, true) + a.enabled = false + assert_equal(a.is_effective_enabled?, false) + a.enabled = true + assert_equal(a.is_effective_enabled?, true) + a.dyn_enabled = false + assert_equal(a.is_effective_enabled?, false) + a.dyn_enabled = true + assert_equal(a.is_effective_enabled?, true) + + end + end load("test_epilogue.rb") From 716369de632e82375687d232eb290480034471ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20K=C3=B6fferlein?= Date: Mon, 1 Aug 2022 18:50:07 +0200 Subject: [PATCH 3/3] Issue 1126 (#1128) * First attempt to fix. Rather experimental. * Debugging and bug fixing The basic issue was a missing break However, correct computation of cache results for instance-to-instance cluster interaction is implemented Plus: identical and overlapping instances are no longer ignored except in the case of exact duplicates. Otherwise these instance generate dead nets which are not connected elsewhere. * Added tests, fixed duplicate cells test, added missing files. * Code simplification (removed invariant from transformation in cluster-to-cluster interaction cache) * Skipping cell instance duplicates as some real-world testcases mandate so * Updated test data --- src/db/db/dbHierNetworkProcessor.cc | 19 ++++++-- .../unit_tests/dbHierNetworkProcessorTests.cc | 45 ++++++++++++++++++ src/db/unit_tests/dbLayoutToNetlistTests.cc | 1 + testdata/algo/issue-1126.gds.gz | Bin 0 -> 421 bytes testdata/algo/issue-1126_au.gds | Bin 0 -> 14700 bytes 5 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 testdata/algo/issue-1126.gds.gz create mode 100644 testdata/algo/issue-1126_au.gds diff --git a/src/db/db/dbHierNetworkProcessor.cc b/src/db/db/dbHierNetworkProcessor.cc index 46fb1e3c1..fd6554b5f 100644 --- a/src/db/db/dbHierNetworkProcessor.cc +++ b/src/db/db/dbHierNetworkProcessor.cc @@ -1582,8 +1582,7 @@ private: * @param interacting_clusters_out Receives the cluster interaction descriptors * * "interacting_clusters_out" will be cluster interactions in the parent instance space of i1 and i2 respectively. - * Cluster ID will be valid in the parent cells containing i1 and i2 and the cluster instances property ID will be set to p1 and p2 - * respectively. + * Cluster ID will be valid in the parent cells containing i1 and i2. */ void consider_instance_pair (const box_type &common, const db::Instance &i1, const db::ICplxTrans &t1, const db::CellInstArray::iterator &i1element, @@ -1629,6 +1628,7 @@ private: db::ICplxTrans tt2 = t2 * i2t; db::ICplxTrans cache_norm = tt1.inverted (); + ii_key = InstanceToInstanceInteraction ((! i1element.at_end () || i1.size () == 1) ? 0 : i1.cell_inst ().delegate (), (! i2element.at_end () || i2.size () == 1) ? 0 : i2.cell_inst ().delegate (), cache_norm, cache_norm * tt2); @@ -1695,9 +1695,14 @@ private: db::ICplxTrans i2t = i2.complex_trans (*ii2); db::ICplxTrans tt2 = t2 * i2t; + if (i1.cell_index () == i2.cell_index () && tt1 == tt2) { // skip interactions between identical instances (duplicate instance removal) - continue; + if (! i2element.at_end ()) { + break; + } else { + continue; + } } box_type ib2 = bb2.transformed (tt2); @@ -1903,6 +1908,7 @@ private: for (std::list >::iterator ii = ii_interactions.begin (); ii != ii_interactions.end (); ++ii) { propagate_cluster_inst (ii->second, i.cell_index (), tt2, i.prop_id ()); } + interacting_clusters.splice (interacting_clusters.end (), ii_interactions, ii_interactions.begin (), ii_interactions.end ()); } @@ -1974,6 +1980,7 @@ private: for (typename std::list::iterator ii = ci_interactions.begin (); ii != ci_interactions.end (); ++ii) { propagate_cluster_inst (ii->other_ci, i2.cell_index (), i2t, i2.prop_id ()); } + interactions_out.splice (interactions_out.end (), ci_interactions, ci_interactions.begin (), ci_interactions.end ()); } @@ -2088,6 +2095,10 @@ private: * * After calling this method, the cluster instance in ci is guaranteed to have connections from all * parent cells. One of these connections represents the instance ci. + * + * Returns false if the connection was already there in the same place (indicating duplicate instances). + * In this case, the cluster instance should be skipped. In the other case, the cluster instance is + * updated to reflect the connected cluster. */ void propagate_cluster_inst (ClusterInstance &ci, db::cell_index_type pci, const db::ICplxTrans &trans, db::properties_id_type prop_id) const { @@ -2178,7 +2189,7 @@ hier_clusters::propagate_cluster_inst (const db::Layout &layout, const db::Ce if (parent_cluster > 0) { - // taken parent + // take parent id_new = parent_cluster; } else { diff --git a/src/db/unit_tests/dbHierNetworkProcessorTests.cc b/src/db/unit_tests/dbHierNetworkProcessorTests.cc index 0c06713ec..d403c281a 100644 --- a/src/db/unit_tests/dbHierNetworkProcessorTests.cc +++ b/src/db/unit_tests/dbHierNetworkProcessorTests.cc @@ -1353,3 +1353,48 @@ TEST(200_issue609) EXPECT_EQ (root_nets (hc.clusters_per_cell (*td)), size_t (0)); } } + +// issue #1126 +TEST(201_issue1126) +{ + { + db::Layout ly; + unsigned int l1 = 0; + + { + db::LayerProperties p; + db::LayerMap lmap; + + p.layer = 1; + p.datatype = 0; + lmap.map (db::LDPair (p.layer, p.datatype), l1 = ly.insert_layer ()); + ly.set_properties (l1, p); + + db::LoadLayoutOptions options; + options.get_options ().layer_map = lmap; + options.get_options ().create_other_layers = false; + + std::string fn (tl::testdata ()); + fn += "/algo/issue-1126.gds.gz"; + tl::InputStream stream (fn); + db::Reader reader (stream); + reader.read (ly, options); + } + + std::vector strings; + normalize_layer (ly, strings, l1); + + // connect 1 to 1 + db::Connectivity conn; + conn.connect (l1, l1); + + db::hier_clusters hc; + hc.build (ly, ly.cell (*ly.begin_top_down ()), conn); + + // should not assert until here + } + + // detailed test: + run_hc_test (_this, "issue-1126.gds.gz", "issue-1126_au.gds"); +} + diff --git a/src/db/unit_tests/dbLayoutToNetlistTests.cc b/src/db/unit_tests/dbLayoutToNetlistTests.cc index 05d70654a..5f843ecdd 100644 --- a/src/db/unit_tests/dbLayoutToNetlistTests.cc +++ b/src/db/unit_tests/dbLayoutToNetlistTests.cc @@ -2846,6 +2846,7 @@ TEST(11_DuplicateInstances) // compare netlist as string CHECKPOINT (); db::compare_netlist (_this, *l2n.netlist (), + // suppressing duplicate cells: "circuit RINGO ();\n" " subcircuit INV2 $1 (IN=$I12,$2=FB,OUT=OSC,$4=VSS,$5=VDD);\n" " subcircuit INV2 $2 (IN=FB,$2=$I25,OUT=$I1,$4=VSS,$5=VDD);\n" diff --git a/testdata/algo/issue-1126.gds.gz b/testdata/algo/issue-1126.gds.gz new file mode 100644 index 0000000000000000000000000000000000000000..1cf858d49a1d23e06cb9da97a0ee4a3b09cc4e5d GIT binary patch literal 421 zcmV;W0b2eaiwFP!000001C5fuOF~f?hTn7U55?+oD!*lO@&+~ofzy}?i z?;^yw=M5eN5kM41_dC2?-^wHazoKo-JYHPcJ6~&adsFr8n+1fl!Je+`$tYAWppS7d z=mrO{Xe!O-iv`3@VkQP`op&hnxCwO}gZyT*R3qMh2(Lq#mul|*&pY)In?zrt%mvB! z;NM%uppLYhdk$s3 zq`Lb@^F(WtJV}`!&Aa>MH1X+xJV}|C)2=?}IifG$_8iKb=Z2oNWoOc2V$v*{E(WDN zUEZV(UM=>JeW!-5z5>-p)tCROkBXUXEybu(mG1w2b1wEy8}-g!=cN5-L~6E&%zxsX PQC0B^fp56(lmq|(1#{Ho literal 0 HcmV?d00001 diff --git a/testdata/algo/issue-1126_au.gds b/testdata/algo/issue-1126_au.gds new file mode 100644 index 0000000000000000000000000000000000000000..d1c79fdbbbefb75307ad87160112033f1e9dfa38 GIT binary patch literal 14700 zcmeHO&ubiY6n{Ir*=%DQqKOJ6)*sL++KXvWL{Zw1ltRUW7Z2VlRM0=52M;+|FyKkV zi>E3=vC{n)~;*<&-pT&@OL6zs;>R?Q-+v z(4G$!@uxMO2>wov)kR);MrsefeGd#Zb-D+?mqv`gTQH>7rWC_~1#&6npN zI1=)o`#9ti89#8OJb&?c$bbL4kWXZM@pyUu>PpDJFck8MjIXYg=GS&LEWh^f4$DuG z@m-Db{6pJA{`}sMPh|Yi_VWCtxsd66yIT-SLaN2b0 zv?(}kKDetUb#qs1W<`<{i;~CMrP+LS^}n(?l-AwPi%fqL@R7D``uwyAk#WW5eKx;4 zn=|V4Y$od$3tY*YP2)^4pw`3VSzWqD=dNkKu8GI~4mm(@9UadQT%F^+aIIIbKa1Mys$-nG z8kzP)#uc-=yl-Q2{aUC?S1YRvs}$-=k#WUtb>+Emoxo)XF1>!n1!UZD`RnMo^g3A% z5L|j4Qv_G%cve?;E@)oK9xJLBwKcrP+Mg|YRd+tSp1LENSBZ=(W}fgfl;<*!>SqnX z0gn1K$%F@3pRoT&B66cc6XVUnQRC2kqC{<1fY8*1J;km)@<~ zY!ew*>^9qZE}LontRXls+eF44m%ol1#s$4_ejw_>Y!eypwXTRayC(NTb{~En|t}xsBL}~uQlO$$4MaC7o&32v(I)Td&Tsqsv1!UZD`Pp_{ z+MDG7!KL$WlC_(bs3f_e4a-6}#2-dyz|fFi!0a-(Mv}#!Gta2d>qkUg{syOMPW} z5gB*>^7H7p7Cw*!IY8xTo&PLD#vPaUz2iEt6u5xkT0WH_xH`u(Pn2-MU&sNnHzMP% zu96ZLpVQB>#2Q;K;^J~o5;B1@@ur?!VDn?2oCJEM82GzIUi#Y}rWcWM#mx79o%pH1h4+2!K3d;jwfkryZ8jh5Ik$0F`51dd2K9CRa?oVsgdQJ|)Ue(`mYBb~@;#`Z3 literal 0 HcmV?d00001