From 167bcbcc5f64f90897d2feed8a6ee4a410232ca3 Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Mon, 25 Jul 2022 21:06:56 +0200 Subject: [PATCH 01/10] Enabling MSVC debug builds with iterator debugging on --- src/db/db/dbCircuit.cc | 11 ++--- src/db/db/dbDevice.cc | 7 ++-- src/db/db/dbGenericShapeIterator.h | 4 ++ src/db/db/dbHierarchyBuilder.cc | 6 +-- src/db/db/dbSubCircuit.cc | 9 +++-- src/klayout.pri | 23 +++++++++-- src/lay/lay/lay.pro | 11 ----- src/tl/tl/tlIteratorUtils.h | 64 ++++++++++++++++++++++++++++++ 8 files changed, 106 insertions(+), 29 deletions(-) create mode 100644 src/tl/tl/tlIteratorUtils.h diff --git a/src/db/db/dbCircuit.cc b/src/db/db/dbCircuit.cc index bf11738b2..bb2d4281e 100644 --- a/src/db/db/dbCircuit.cc +++ b/src/db/db/dbCircuit.cc @@ -23,6 +23,7 @@ #include "dbCircuit.h" #include "dbNetlist.h" #include "dbLayout.h" +#include "tlIteratorUtils.h" #include @@ -169,7 +170,7 @@ const Pin *Circuit::pin_by_id (size_t id) const return 0; } else { pin_list::iterator pi = m_pin_by_id [id]; - if (pi == pin_list::iterator ()) { + if (tl::is_null_iterator (pi)) { return 0; } else { return pi.operator-> (); @@ -179,7 +180,7 @@ const Pin *Circuit::pin_by_id (size_t id) const void Circuit::rename_pin (size_t id, const std::string &name) { - if (id < m_pin_by_id.size () && m_pin_by_id [id] != pin_list::iterator ()) { + if (id < m_pin_by_id.size () && ! tl::is_null_iterator (m_pin_by_id [id])) { m_pin_by_id [id]->set_name (name); } } @@ -327,7 +328,7 @@ Pin &Circuit::add_pin (const std::string &name) void Circuit::remove_pin (size_t id) { - if (id < m_pin_by_id.size () && m_pin_by_id [id] != pin_list::iterator ()) { + if (id < m_pin_by_id.size () && ! tl::is_null_iterator (m_pin_by_id [id])) { m_pins.erase (m_pin_by_id [id]); m_pin_by_id [id] = pin_list::iterator (); } @@ -653,7 +654,7 @@ const Net *Circuit::net_for_pin (size_t pin_id) const { if (pin_id < m_pin_refs.size ()) { Net::pin_iterator p = m_pin_refs [pin_id]; - if (p != Net::pin_iterator ()) { + if (! tl::is_null_iterator (p)) { return p->net (); } } @@ -668,7 +669,7 @@ void Circuit::connect_pin (size_t pin_id, Net *net) if (pin_id < m_pin_refs.size ()) { Net::pin_iterator p = m_pin_refs [pin_id]; - if (p != Net::pin_iterator () && p->net ()) { + if (! tl::is_null_iterator (p) && p->net ()) { p->net ()->erase_pin (p); } m_pin_refs [pin_id] = Net::pin_iterator (); diff --git a/src/db/db/dbDevice.cc b/src/db/db/dbDevice.cc index 3440c143f..eadffc772 100644 --- a/src/db/db/dbDevice.cc +++ b/src/db/db/dbDevice.cc @@ -23,6 +23,7 @@ #include "dbDevice.h" #include "dbCircuit.h" #include "dbDeviceClass.h" +#include "tlIteratorUtils.h" namespace db { @@ -39,7 +40,7 @@ Device::Device () Device::~Device () { for (std::vector::const_iterator t = m_terminal_refs.begin (); t != m_terminal_refs.end (); ++t) { - if (*t != Net::terminal_iterator () && (*t)->net ()) { + if (! tl::is_null_iterator (*t) && (*t)->net ()) { (*t)->net ()->erase_terminal (*t); } } @@ -125,7 +126,7 @@ const Net *Device::net_for_terminal (size_t terminal_id) const { if (terminal_id < m_terminal_refs.size ()) { Net::terminal_iterator p = m_terminal_refs [terminal_id]; - if (p != Net::terminal_iterator ()) { + if (! tl::is_null_iterator (p)) { return p->net (); } } @@ -140,7 +141,7 @@ void Device::connect_terminal (size_t terminal_id, Net *net) if (terminal_id < m_terminal_refs.size ()) { Net::terminal_iterator p = m_terminal_refs [terminal_id]; - if (p != Net::terminal_iterator () && p->net ()) { + if (! tl::is_null_iterator (p) && p->net ()) { p->net ()->erase_terminal (p); } m_terminal_refs [terminal_id] = Net::terminal_iterator (); diff --git a/src/db/db/dbGenericShapeIterator.h b/src/db/db/dbGenericShapeIterator.h index 6b55a2ea3..3de6953b9 100644 --- a/src/db/db/dbGenericShapeIterator.h +++ b/src/db/db/dbGenericShapeIterator.h @@ -94,7 +94,11 @@ public: virtual bool equals (const generic_shape_iterator_delegate_base *other) const { const generic_shape_iterator_delegate2 *o = dynamic_cast *> (other); +#if defined(_MSC_VER) && defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL >= 2 + return o && o->m_iter._Unwrapped () == m_iter._Unwrapped (); +#else return o && o->m_iter == m_iter; +#endif } private: diff --git a/src/db/db/dbHierarchyBuilder.cc b/src/db/db/dbHierarchyBuilder.cc index 0093bd9bb..77e5ea235 100644 --- a/src/db/db/dbHierarchyBuilder.cc +++ b/src/db/db/dbHierarchyBuilder.cc @@ -25,14 +25,14 @@ #include "dbClip.h" #include "dbRegion.h" #include "dbPolygonTools.h" +#include "tlIteratorUtils.h" namespace db { static HierarchyBuilderShapeInserter def_inserter; -static HierarchyBuilder::cell_map_type null_map; -static HierarchyBuilder::cell_map_type::const_iterator null_iterator = null_map.end (); +static HierarchyBuilder::cell_map_type::const_iterator null_iterator = HierarchyBuilder::cell_map_type::const_iterator (); // ------------------------------------------------------------------------------------------- @@ -286,7 +286,7 @@ HierarchyBuilder::end (const RecursiveShapeIterator *iter) void HierarchyBuilder::enter_cell (const RecursiveShapeIterator * /*iter*/, const db::Cell * /*cell*/, const db::Box & /*region*/, const box_tree_type * /*complex_region*/) { - tl_assert (m_cm_entry != m_cell_map.end () && m_cm_entry != null_iterator); + tl_assert (! tl::is_equal_iterator_unchecked (m_cm_entry, null_iterator) && m_cm_entry != m_cell_map.end ()); m_cells_seen.insert (m_cm_entry->first); diff --git a/src/db/db/dbSubCircuit.cc b/src/db/db/dbSubCircuit.cc index 72a37221b..851a27647 100644 --- a/src/db/db/dbSubCircuit.cc +++ b/src/db/db/dbSubCircuit.cc @@ -22,6 +22,7 @@ #include "dbSubCircuit.h" #include "dbCircuit.h" +#include "tlIteratorUtils.h" namespace db { @@ -38,7 +39,7 @@ SubCircuit::SubCircuit () SubCircuit::~SubCircuit() { for (std::vector::const_iterator p = m_pin_refs.begin (); p != m_pin_refs.end (); ++p) { - if (*p != Net::subcircuit_pin_iterator () && (*p)->net ()) { + if (! tl::is_null_iterator (*p) && (*p)->net ()) { (*p)->net ()->erase_subcircuit_pin (*p); } } @@ -112,7 +113,7 @@ const Net *SubCircuit::net_for_pin (size_t pin_id) const { if (pin_id < m_pin_refs.size ()) { Net::subcircuit_pin_iterator p = m_pin_refs [pin_id]; - if (p != Net::subcircuit_pin_iterator ()) { + if (! tl::is_null_iterator (p)) { return p->net (); } } @@ -123,7 +124,7 @@ const NetSubcircuitPinRef *SubCircuit::netref_for_pin (size_t pin_id) const { if (pin_id < m_pin_refs.size ()) { Net::subcircuit_pin_iterator p = m_pin_refs [pin_id]; - if (p != Net::subcircuit_pin_iterator ()) { + if (! tl::is_null_iterator (p)) { return p.operator-> (); } } @@ -138,7 +139,7 @@ void SubCircuit::connect_pin (size_t pin_id, Net *net) if (pin_id < m_pin_refs.size ()) { Net::subcircuit_pin_iterator p = m_pin_refs [pin_id]; - if (p != Net::subcircuit_pin_iterator () && p->net ()) { + if (! tl::is_null_iterator (p) && p->net ()) { p->net ()->erase_subcircuit_pin (p); } m_pin_refs [pin_id] = Net::subcircuit_pin_iterator (); diff --git a/src/klayout.pri b/src/klayout.pri index ad0504dd2..48ddb352c 100644 --- a/src/klayout.pri +++ b/src/klayout.pri @@ -145,9 +145,10 @@ msvc { QMAKE_CXXFLAGS_WARN_ON += \ - # as we're using default-constructed iterators as "null" we can't have - # checked iterators with MSVC - DEFINES += _ITERATOR_DEBUG_LEVEL=0 + # for stack trace support: + # lpsapi for GetModuleFileName and others + # dbghelp for SymFromAddr and other + LIBS += -lpsapi -ldbghelp } else { @@ -171,9 +172,25 @@ msvc { QMAKE_CXXFLAGS += -std=c++11 win32 { + QMAKE_LFLAGS += -Wl,--exclude-all-symbols + + # for stack trace support: + # lpsapi for GetModuleFileName and others + # dbghelp for SymFromAddr and other + LIBS += -lpsapi -ldbghelp + + QMAKE_CXXFLAGS += -Wa,-mbig-obj + } else { + QMAKE_CXXFLAGS += -fvisibility=hidden + + } + + *bsd* { + # stack trace support + LIBS += -lexecinfo } } diff --git a/src/lay/lay/lay.pro b/src/lay/lay/lay.pro index c93131cc1..3a6001cc0 100644 --- a/src/lay/lay/lay.pro +++ b/src/lay/lay/lay.pro @@ -181,17 +181,6 @@ INCLUDEPATH += $$TL_INC $$GSI_INC $$DB_INC $$RDB_INC $$LAYBASIC_INC $$LAYUI_INC DEPENDPATH += $$TL_INC $$GSI_INC $$DB_INC $$RDB_INC $$LAYBASIC_INC $$LAYUI_INC $$LAYVIEW_INC $$ANT_INC $$IMG_INC $$EDT_INC $$LYM_INC LIBS += -L$$DESTDIR -lklayout_tl -lklayout_gsi -lklayout_db -lklayout_rdb -lklayout_lym -lklayout_laybasic -lklayout_layview -lklayout_layui -lklayout_ant -lklayout_img -lklayout_edt -win32 { - # for stack trace support: - # lpsapi for GetModuleFileName and others - # dbghelp for SymFromAddr and other - LIBS += -lpsapi -ldbghelp -} - -*bsd* { - LIBS += -lexecinfo -} - INCLUDEPATH += $$QTBASIC_INC DEPENDPATH += $$QTBASIC_INC diff --git a/src/tl/tl/tlIteratorUtils.h b/src/tl/tl/tlIteratorUtils.h new file mode 100644 index 000000000..89aa7874a --- /dev/null +++ b/src/tl/tl/tlIteratorUtils.h @@ -0,0 +1,64 @@ + +/* + + KLayout Layout Viewer + Copyright (C) 2006-2022 Matthias Koefferlein + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + +*/ + + +#ifndef HDR_tlIteratorUtils +#define HDR_tlIteratorUtils + +namespace tl +{ + +/** + * @brief Checks an iterator against the default-constructed value + * + * This function takes care of using the right way of comparing iterators + * suitable for MSVC with iterator debugging on. + */ +template +static inline bool is_null_iterator (const Iter &iter) +{ +#if defined(_MSC_VER) && defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL >= 2 + return iter._Unwrapped () == Iter ()._Unwrapped (); +#else + return iter == Iter (); +#endif +} + +/** + * @brief Checks an iterator against another one without iterator checking + * + * This function takes care of using the right way of comparing iterators + * suitable for MSVC with iterator debugging on. + */ +template +static inline bool is_equal_iterator_unchecked (const Iter &a, const Iter &b) +{ +#if defined(_MSC_VER) && defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL >= 2 + return a._Unwrapped () == b._Unwrapped (); +#else + return a == b; +#endif +} + +} + +#endif From da398e77fddf3f935ed88cf16394b05040163cfe Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Mon, 25 Jul 2022 22:03:01 +0200 Subject: [PATCH 02/10] [Consider merging] avoid a memory corruption issue in Netlist::flatten --- src/db/db/dbNetlist.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/db/db/dbNetlist.cc b/src/db/db/dbNetlist.cc index 7f0a04077..3cc519076 100644 --- a/src/db/db/dbNetlist.cc +++ b/src/db/db/dbNetlist.cc @@ -505,6 +505,8 @@ void Netlist::flatten_circuit (Circuit *circuit) void Netlist::flatten () { + db::NetlistLocker locker (this); + std::set top_circuits; size_t ntop = top_circuit_count (); for (db::Netlist::top_down_circuit_iterator tc = begin_top_down (); tc != end_top_down () && ntop > 0; ++tc) { From 50a863bb2685f01c656f97df3254de681808b172 Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Tue, 26 Jul 2022 00:13:26 +0200 Subject: [PATCH 03/10] [Consider merging] Fixed a potential memory corruption problem --- src/db/db/dbStreamLayers.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db/db/dbStreamLayers.cc b/src/db/db/dbStreamLayers.cc index c29945d1f..7c8625ce4 100644 --- a/src/db/db/dbStreamLayers.cc +++ b/src/db/db/dbStreamLayers.cc @@ -787,7 +787,7 @@ LayerMap::unmap (const LDPair &p1, const LDPair &p2) if (db::is_static_ld (p1.layer) && db::is_static_ld (p2.layer)) { m_ld_map.add (p1.layer, p2.layer + 1, LayerMap::datatype_map (), op); } else { - m_ld_map.add (m_ld_map.begin ()->first.first, m_ld_map.end ()->first.second, LayerMap::datatype_map (), op); + m_ld_map.add (m_ld_map.begin ()->first.first, (--m_ld_map.end ())->first.second, LayerMap::datatype_map (), op); } } From 0401f3b89cb3564db1fc8f600c2e245b5b633b28 Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Tue, 26 Jul 2022 08:28:22 +0200 Subject: [PATCH 04/10] Fixed a non-problem that pops up with iterator assertions --- src/layui/layui/layNetlistBrowserModel.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/layui/layui/layNetlistBrowserModel.cc b/src/layui/layui/layNetlistBrowserModel.cc index 39c3031d1..c70e6455d 100644 --- a/src/layui/layui/layNetlistBrowserModel.cc +++ b/src/layui/layui/layNetlistBrowserModel.cc @@ -457,6 +457,7 @@ static std::vectorfirst, j->first)); nn2.erase (j); found = true; + break; } } From 5a9e3f91882a948685dd05f3fd07ad1eaa6c2647 Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Tue, 26 Jul 2022 21:02:17 +0200 Subject: [PATCH 05/10] [Consider merging] One more iterator issue fixed --- src/db/db/gsiDeclDbNetlist.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/db/db/gsiDeclDbNetlist.cc b/src/db/db/gsiDeclDbNetlist.cc index fdaab035a..83f4a429f 100644 --- a/src/db/db/gsiDeclDbNetlist.cc +++ b/src/db/db/gsiDeclDbNetlist.cc @@ -197,10 +197,10 @@ static bool is_combined_device (const db::Device *device) return ! device->reconnected_terminals ().empty (); } +static std::vector empty; + static std::vector::const_iterator begin_reconnected_terminals_for (const db::Device *device, size_t terminal_id) { - static std::vector empty; - const std::vector *ti = device->reconnected_terminals_for ((unsigned int) terminal_id); if (! ti) { return empty.begin (); @@ -211,8 +211,6 @@ static std::vector::const_iterator begin_reconnec static std::vector::const_iterator end_reconnected_terminals_for (const db::Device *device, size_t terminal_id) { - static std::vector empty; - const std::vector *ti = device->reconnected_terminals_for ((unsigned int) terminal_id); if (! ti) { return empty.end (); From bca7486082446a2d910754494d8d16ee67dff9fd Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Tue, 26 Jul 2022 21:02:48 +0200 Subject: [PATCH 06/10] [Consider merging] object lifetime problem with QLatin1String and GSI fixed --- src/gsi/gsi/gsiSerialisation.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gsi/gsi/gsiSerialisation.h b/src/gsi/gsi/gsiSerialisation.h index 6feb7d8cf..4b89dfc62 100644 --- a/src/gsi/gsi/gsiSerialisation.h +++ b/src/gsi/gsi/gsiSerialisation.h @@ -1036,11 +1036,12 @@ public: return m_s_utf8.constData (); } - virtual void set (const char *c_str, size_t s, tl::Heap &) + virtual void set (const char *c_str, size_t s, tl::Heap &heap) { if (! m_is_const) { - m_latin1_holder = QString::fromUtf8 (c_str, int (s)).toLatin1 (); - *mp_s = QLatin1String (m_latin1_holder.constData (), m_latin1_holder.size ()); + QByteArray *latin1_holder = new QByteArray (QString::fromUtf8 (c_str, int (s)).toLatin1 ()); + heap.push (latin1_holder); + *mp_s = QLatin1String (latin1_holder->constData (), latin1_holder->size ()); } } @@ -1048,7 +1049,6 @@ private: QLatin1String *mp_s; bool m_is_const; QLatin1String m_s; - QByteArray m_latin1_holder; mutable QByteArray m_s_utf8; }; From 1b728f1067320f4e97921728d832a18483a12a87 Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Thu, 28 Jul 2022 20:52:19 +0200 Subject: [PATCH 07/10] Removed duplicated method declarations in LayoutViewBase --- .../laybasic/gsiDeclLayLayoutViewBase.cc | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc b/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc index fbbbb4ed9..1a4e28170 100644 --- a/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc +++ b/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc @@ -553,18 +553,6 @@ LAYBASIC_PUBLIC Class decl_LayoutViewBase (QT_EXTERNAL_BASE "\n" "This constant has been introduced in version 0.27.\n" ) + - gsi::method ("call_menu", static_cast (&lay::LayoutViewBase::menu_activated), - "@brief Calls the menu item with the provided symbol.\n" - "To obtain all symbols, use get_menu_symbols.\n" - "\n" - "This method has been introduced in version 0.27." - ) + - gsi::method ("menu_symbols", &lay::LayoutViewBase::menu_symbols, - "@brief Gets all available menu symbols (see \\call_menu).\n" - "NOTE: currently this method delivers a superset of all available symbols. Depending on the context, no all symbols may trigger actual functionality.\n" - "\n" - "This method has been introduced in version 0.27." - ) + gsi::method ("stop_redraw", static_cast (&lay::LayoutViewBase::stop_redraw), "@brief Stops the redraw thread\n" "\n" @@ -993,13 +981,13 @@ LAYBASIC_PUBLIC Class decl_LayoutViewBase (QT_EXTERNAL_BASE "@brief Calls the menu item with the provided symbol.\n" "To obtain all symbols, use \\menu_symbols.\n" "\n" - "This method has been introduced in version 0.28." + "This method has been introduced in version 0.27." ) + gsi::method ("menu_symbols", &lay::LayoutViewBase::menu_symbols, - "@brief Gets all available menu symbols.\n" + "@brief Gets all available menu symbols (see \\call_menu).\n" "NOTE: currently this method delivers a superset of all available symbols. Depending on the context, no all symbols may trigger actual functionality.\n" "\n" - "This method has been introduced in version 0.28." + "This method has been introduced in version 0.27." ) + gsi::method ("cancel", &lay::LayoutViewBase::cancel, "@brief Cancels all edit operations\n" 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 08/10] 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 09/10] 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 10/10] 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