From e6ac66f8aa35a6e5e7015248e9180a2842d1151b Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Thu, 26 Dec 2024 23:11:59 +0100 Subject: [PATCH] Providing a more efficient event for property ID changes --- src/db/db/dbCell.cc | 3 ++ src/db/db/dbInstances.cc | 13 +++++++ src/db/db/dbInstances.h | 5 +++ src/db/db/dbLayout.cc | 9 +++-- src/db/db/dbLayoutStateModel.cc | 25 ++++++++++--- src/db/db/dbLayoutStateModel.h | 32 +++++++++++++++++ src/db/db/dbPropertiesRepository.cc | 9 ----- src/db/db/dbPropertiesRepository.h | 7 ---- src/db/db/dbShapes.cc | 14 ++++++++ src/db/db/dbShapes.h | 1 + src/db/unit_tests/dbLayoutTests.cc | 41 ++++++++++++++++++---- src/laybasic/laybasic/layCellView.cc | 3 +- src/laybasic/laybasic/layLayoutViewBase.cc | 31 ++++++++-------- src/laybasic/laybasic/layLayoutViewBase.h | 2 ++ 14 files changed, 150 insertions(+), 45 deletions(-) diff --git a/src/db/db/dbCell.cc b/src/db/db/dbCell.cc index 01993c8dc..6203d4a31 100644 --- a/src/db/db/dbCell.cc +++ b/src/db/db/dbCell.cc @@ -417,6 +417,9 @@ Cell::prop_id (db::properties_id_type id) if (manager () && manager ()->transacting ()) { manager ()->queue (this, new SetCellPropId (m_prop_id, id)); } + if (layout ()) { + layout ()->invalidate_prop_ids (); + } m_prop_id = id; } } diff --git a/src/db/db/dbInstances.cc b/src/db/db/dbInstances.cc index ceb5cc8cd..256020fbf 100644 --- a/src/db/db/dbInstances.cc +++ b/src/db/db/dbInstances.cc @@ -1041,6 +1041,16 @@ Instances::invalidate_insts () set_instance_by_cell_index_needs_made (true); set_instance_tree_needs_sort (true); + + invalidate_prop_ids (); +} + +void +Instances::invalidate_prop_ids () +{ + if (layout ()) { + layout ()->invalidate_prop_ids (); + } } template @@ -1600,6 +1610,9 @@ Instances::replace_prop_id (const instance_type &ref, db::properties_id_type pro } if (! ref.is_null ()) { + if (ref.prop_id () != prop_id) { + invalidate_prop_ids (); + } cell_inst_wp_array_type new_inst (ref.cell_inst (), prop_id); return replace (ref, new_inst); } else { diff --git a/src/db/db/dbInstances.h b/src/db/db/dbInstances.h index 6d853f6bc..168426d9a 100644 --- a/src/db/db/dbInstances.h +++ b/src/db/db/dbInstances.h @@ -1847,6 +1847,11 @@ private: */ void invalidate_insts (); + /** + * @brief Invalidates the properties IDs - called when some ID has changed + */ + void invalidate_prop_ids (); + /** * @brief Get the non-editable instance tree by instance type */ diff --git a/src/db/db/dbLayout.cc b/src/db/db/dbLayout.cc index 7a2cd8de3..cdaa30ce2 100644 --- a/src/db/db/dbLayout.cc +++ b/src/db/db/dbLayout.cc @@ -831,6 +831,7 @@ Layout::prop_id (db::properties_id_type id) if (manager () && manager ()->transacting ()) { manager ()->queue (this, new SetLayoutPropId (m_prop_id, id)); } + invalidate_prop_ids (); m_prop_id = id; } } @@ -1786,7 +1787,7 @@ Layout::force_update () void Layout::force_update_no_lock () const { - if (hier_dirty () || bboxes_dirty ()) { + if (hier_dirty () || bboxes_dirty () || prop_ids_dirty ()) { unsigned int invalid = m_invalid; @@ -1813,7 +1814,7 @@ Layout::update () const // NOTE: the assumption is that either one thread is writing or // multiple threads are reading. Hence, we do not need to lock hier_dirty() or bboxes_dirty(). // We still do double checking as another thread might do the update as well. - if (under_construction () || (! hier_dirty () && ! bboxes_dirty ())) { + if (under_construction () || (! hier_dirty () && ! bboxes_dirty () && ! prop_ids_dirty ())) { return; } @@ -1827,6 +1828,10 @@ Layout::update () const void Layout::do_update () { + if (! hier_dirty () && ! bboxes_dirty ()) { + return; + } + tl::SelfTimer timer (tl::verbosity () > layout_base_verbosity, tl::to_string (tr ("Sorting"))); // establish a progress report since this operation can take some time. diff --git a/src/db/db/dbLayoutStateModel.cc b/src/db/db/dbLayoutStateModel.cc index 80eecfd6c..c7e723b89 100644 --- a/src/db/db/dbLayoutStateModel.cc +++ b/src/db/db/dbLayoutStateModel.cc @@ -29,14 +29,14 @@ namespace db { LayoutStateModel::LayoutStateModel (bool busy) - : m_hier_dirty (false), m_hier_generation_id (0), m_all_bboxes_dirty (false), m_some_bboxes_dirty (false), m_busy (busy) + : m_hier_dirty (false), m_hier_generation_id (0), m_all_bboxes_dirty (false), m_some_bboxes_dirty (false), m_prop_ids_dirty (false), m_busy (busy) { // .. nothing yet .. } LayoutStateModel::LayoutStateModel (const LayoutStateModel &d) : m_hier_dirty (d.m_hier_dirty), m_hier_generation_id (d.m_hier_generation_id), m_bboxes_dirty (d.m_bboxes_dirty), - m_all_bboxes_dirty (d.m_all_bboxes_dirty), m_some_bboxes_dirty (d.m_some_bboxes_dirty), m_busy (d.m_busy) + m_all_bboxes_dirty (d.m_all_bboxes_dirty), m_some_bboxes_dirty (d.m_some_bboxes_dirty), m_prop_ids_dirty (d.m_prop_ids_dirty), m_busy (d.m_busy) { // .. nothing yet .. } @@ -49,6 +49,7 @@ LayoutStateModel::operator= (const LayoutStateModel &d) m_bboxes_dirty = d.m_bboxes_dirty; m_all_bboxes_dirty = d.m_all_bboxes_dirty; m_some_bboxes_dirty = d.m_some_bboxes_dirty; + m_prop_ids_dirty = d.m_prop_ids_dirty; m_busy = d.m_busy; return *this; } @@ -58,7 +59,13 @@ LayoutStateModel::~LayoutStateModel () // .. nothing yet .. } -void +void +LayoutStateModel::do_invalidate_prop_ids () +{ + prop_ids_changed_event (); +} + +void LayoutStateModel::do_invalidate_hier () { hier_changed_event (); @@ -97,15 +104,25 @@ LayoutStateModel::bboxes_dirty () const return m_some_bboxes_dirty || m_all_bboxes_dirty; } +void +LayoutStateModel::invalidate_prop_ids () +{ + if (! m_prop_ids_dirty) { + do_invalidate_prop_ids (); + m_prop_ids_dirty = true; + } +} + void LayoutStateModel::update () { - if (bboxes_dirty () || m_hier_dirty) { + if (bboxes_dirty () || m_hier_dirty || m_prop_ids_dirty) { do_update (); m_bboxes_dirty.clear (); m_some_bboxes_dirty = false; m_all_bboxes_dirty = false; m_hier_dirty = false; + m_prop_ids_dirty = false; } } diff --git a/src/db/db/dbLayoutStateModel.h b/src/db/db/dbLayoutStateModel.h index 538d30beb..7516aba3e 100644 --- a/src/db/db/dbLayoutStateModel.h +++ b/src/db/db/dbLayoutStateModel.h @@ -103,6 +103,14 @@ public: */ void invalidate_bboxes (unsigned int index); + /** + * @brief Invalidate the properties IDs + * + * This method is supposed to be called by shape containers for example if + * a property ID has been changed. + */ + void invalidate_prop_ids (); + /** * @brief Signal that the database unit has changed */ @@ -146,6 +154,17 @@ public: */ bool bboxes_dirty () const; + /** + * @brief The "dirty property IDs" attribute + * + * This attribute is true, if a properties ID has been changed on a shape, instance, cell or + * the layout itself. + */ + bool prop_ids_dirty () const + { + return m_prop_ids_dirty; + } + /** * @brief Sets or resets busy mode * @@ -196,15 +215,28 @@ public: tl::Event cell_name_changed_event; tl::Event layer_properties_changed_event; + /** + * @brief The "properties IDs change event" + * + * This event is issued before a change of properties IDs + * in shapes, instances, cells or the layout itself. + * Inserting a shape or instance implies a properties ID change. + * + * After such a change, "prop_ids_dirty" is true until "update" is called. + */ + tl::Event prop_ids_changed_event; + private: bool m_hier_dirty; size_t m_hier_generation_id; std::vector m_bboxes_dirty; bool m_all_bboxes_dirty, m_some_bboxes_dirty; + bool m_prop_ids_dirty; bool m_busy; void do_invalidate_hier (); void do_invalidate_bboxes (unsigned int index); + void do_invalidate_prop_ids (); }; } diff --git a/src/db/db/dbPropertiesRepository.cc b/src/db/db/dbPropertiesRepository.cc index 9d6f0d293..5d7638fd1 100644 --- a/src/db/db/dbPropertiesRepository.cc +++ b/src/db/db/dbPropertiesRepository.cc @@ -398,7 +398,6 @@ PropertiesRepository::properties_id (const PropertiesSet &props) } properties_id_type pid; - bool changed = false; { tl::MutexLocker locker (&m_lock); @@ -416,19 +415,11 @@ PropertiesRepository::properties_id (const PropertiesSet &props) m_properties_by_value_table [nv->second].insert (pid); } - changed = true; - } else { pid = db::properties_id_type (*pi); } } - if (changed) { - // signal the change of the properties ID's. This way for example, the layer views - // can recompute the property selectors - prop_ids_changed_event (); - } - return pid; } diff --git a/src/db/db/dbPropertiesRepository.h b/src/db/db/dbPropertiesRepository.h index da63bf283..3cd1e4360 100644 --- a/src/db/db/dbPropertiesRepository.h +++ b/src/db/db/dbPropertiesRepository.h @@ -439,13 +439,6 @@ public: db::mem_stat (stat, purpose, cat, m_properties_by_value_table, true, parent); } - /** - * @brief An event indicating a change in the properties IDs - * - * @@@ That is too global! Should be an event when a prop_id is changed on a Shape etc. - */ - tl::Event prop_ids_changed_event; - private: struct CompareNamePtrByValueForValues { diff --git a/src/db/db/dbShapes.cc b/src/db/db/dbShapes.cc index bb4aae1a1..ab83fe5b0 100644 --- a/src/db/db/dbShapes.cc +++ b/src/db/db/dbShapes.cc @@ -304,10 +304,20 @@ Shapes::invalidate_state () if (index != std::numeric_limits::max ()) { layout ()->invalidate_bboxes (index); } + // property ID change is implied + layout ()->invalidate_prop_ids (); } } } +void +Shapes::invalidate_prop_ids () +{ + if (layout ()) { + layout ()->invalidate_prop_ids (); + } +} + void Shapes::swap (Shapes &d) { @@ -730,6 +740,10 @@ Shapes::replace_prop_id (const Shapes::shape_type &ref, db::properties_id_type p if (ref.has_prop_id ()) { + if (ref.prop_id () != prop_id) { + invalidate_prop_ids (); + } + // this assumes we can simply patch the properties ID .. switch (ref.m_type) { case shape_type::Null: diff --git a/src/db/db/dbShapes.h b/src/db/db/dbShapes.h index 0b95e1da3..a92c0c0b0 100644 --- a/src/db/db/dbShapes.h +++ b/src/db/db/dbShapes.h @@ -1549,6 +1549,7 @@ private: db::Cell *mp_cell; // HINT: contains "dirty" in bit 0 and "editable" in bit 1 void invalidate_state (); + void invalidate_prop_ids (); void do_insert (const Shapes &d, unsigned int flags = db::ShapeIterator::All); void check_is_editable_for_undo_redo () const; diff --git a/src/db/unit_tests/dbLayoutTests.cc b/src/db/unit_tests/dbLayoutTests.cc index 5693cf0b8..d7b132b6c 100644 --- a/src/db/unit_tests/dbLayoutTests.cc +++ b/src/db/unit_tests/dbLayoutTests.cc @@ -437,8 +437,7 @@ TEST(4) g.dbu_changed_event.add (&el, &EventListener::dbu_changed); g.cell_name_changed_event.add (&el, &EventListener::cell_name_changed); - // @@@ - db::PropertiesRepository::instance ().prop_ids_changed_event.add (&el, &EventListener::property_ids_changed); + g.prop_ids_changed_event.add (&el, &EventListener::property_ids_changed); g.layer_properties_changed_event.add (&el, &EventListener::layer_properties_changed); EXPECT_EQ (el.dbu_dirty, false); @@ -470,17 +469,46 @@ TEST(4) g.rename_cell (top, "TAP"); EXPECT_EQ (el.cell_name_dirty, true); // but this is - // @@@ cannot be like that -> needs to trigger an event if some layout object has changed db::PropertiesSet ps; ps.insert (tl::Variant (1), tl::Variant ("XYZ")); - db::properties_id (ps); + db::properties_id_type pid1 = db::properties_id (ps); + + auto boxwp = g.cell (top).shapes (0).insert (db::BoxWithProperties (db::Box (0, 0, 100, 100), pid1)); + EXPECT_EQ (el.property_ids_dirty, true); el.reset (); + g.update (); // needed to enable new events from the layout - // @@@ cannot be like that -> needs to trigger an event if some layout object has changed ps.clear (); ps.insert (tl::Variant (1), tl::Variant ("XXX")); - db::properties_id (ps); + db::properties_id_type pid2 = db::properties_id (ps); + EXPECT_NE (pid1, pid2); + + boxwp.shapes ()->replace_prop_id (boxwp, pid2); + + EXPECT_EQ (el.property_ids_dirty, true); + + db::cell_index_type child = g.add_cell ("CHILD"); + db::Instance inst = g.cell (top).insert (db::CellInstArray (db::CellInst (child), db::Trans ())); + el.reset (); + g.update (); // needed to enable new events from the layout + + EXPECT_EQ (el.property_ids_dirty, false); + inst.instances ()->replace_prop_id (inst, pid2); + EXPECT_EQ (el.property_ids_dirty, true); + + el.reset (); + g.update (); // needed to enable new events from the layout + + EXPECT_EQ (el.property_ids_dirty, false); + g.cell (child).prop_id (pid1); + EXPECT_EQ (el.property_ids_dirty, true); + + el.reset (); + g.update (); // needed to enable new events from the layout + + EXPECT_EQ (el.property_ids_dirty, false); + g.prop_id (pid2); EXPECT_EQ (el.property_ids_dirty, true); el.layer_properties_dirty = false; @@ -488,6 +516,7 @@ TEST(4) EXPECT_EQ (el.layer_properties_dirty, false); g.get_layer (db::LayerProperties (42, 17)); EXPECT_EQ (el.layer_properties_dirty, true); // new layer got inserted + } static std::string l2s (const db::Layout &layout) diff --git a/src/laybasic/laybasic/layCellView.cc b/src/laybasic/laybasic/layCellView.cc index 670199b08..07e963176 100644 --- a/src/laybasic/laybasic/layCellView.cc +++ b/src/laybasic/laybasic/layCellView.cc @@ -88,8 +88,7 @@ LayoutHandle::LayoutHandle (db::Layout *layout, const std::string &filename) mp_layout->hier_changed_event.add (this, &LayoutHandle::layout_changed); mp_layout->bboxes_changed_any_event.add (this, &LayoutHandle::layout_changed); mp_layout->cell_name_changed_event.add (this, &LayoutHandle::layout_changed); - // @@@ - db::PropertiesRepository::instance ().prop_ids_changed_event.add (this, &LayoutHandle::layout_changed); + mp_layout->prop_ids_changed_event.add (this, &LayoutHandle::layout_changed); mp_layout->layer_properties_changed_event.add (this, &LayoutHandle::layout_changed); if (tl::verbosity () >= 30) { diff --git a/src/laybasic/laybasic/layLayoutViewBase.cc b/src/laybasic/laybasic/layLayoutViewBase.cc index f1dcd0a19..cf208de31 100644 --- a/src/laybasic/laybasic/layLayoutViewBase.cc +++ b/src/laybasic/laybasic/layLayoutViewBase.cc @@ -240,6 +240,7 @@ LayoutViewBase::LayoutViewBase (db::Manager *manager, bool editable, lay::Plugin : lay::Dispatcher (plugin_parent, false /*not standalone*/), mp_ui (0), dm_redraw (this, &LayoutViewBase::redraw), + dm_update_layer_sources (this, &LayoutViewBase::do_update_layer_sources), m_editable (editable), m_options (options), m_annotation_shapes (manager) @@ -254,6 +255,7 @@ LayoutViewBase::LayoutViewBase (lay::LayoutView *ui, db::Manager *manager, bool : lay::Dispatcher (plugin_parent, false /*not standalone*/), mp_ui (ui), dm_redraw (this, &LayoutViewBase::redraw), + dm_update_layer_sources (this, &LayoutViewBase::do_update_layer_sources), m_editable (editable), m_options (options), m_annotation_shapes (manager) @@ -534,13 +536,13 @@ void LayoutViewBase::update_event_handlers () } for (unsigned int i = 0; i < cellviews (); ++i) { - cellview (i)->layout ().hier_changed_event.add (this, &LayoutViewBase::signal_hier_changed); - cellview (i)->layout ().bboxes_changed_event.add (this, &LayoutViewBase::signal_bboxes_from_layer_changed, i); - cellview (i)->layout ().dbu_changed_event.add (this, &LayoutViewBase::signal_bboxes_changed); - // @@@ - db::PropertiesRepository::instance ().prop_ids_changed_event.add (this, &LayoutViewBase::signal_prop_ids_changed); - cellview (i)->layout ().layer_properties_changed_event.add (this, &LayoutViewBase::signal_layer_properties_changed); - cellview (i)->layout ().cell_name_changed_event.add (this, &LayoutViewBase::signal_cell_name_changed); + db::Layout &ly = cellview (i)->layout (); + ly.hier_changed_event.add (this, &LayoutViewBase::signal_hier_changed); + ly.bboxes_changed_event.add (this, &LayoutViewBase::signal_bboxes_from_layer_changed, i); + ly.dbu_changed_event.add (this, &LayoutViewBase::signal_bboxes_changed); + ly.prop_ids_changed_event.add (this, &LayoutViewBase::signal_prop_ids_changed); + ly.layer_properties_changed_event.add (this, &LayoutViewBase::signal_layer_properties_changed); + ly.cell_name_changed_event.add (this, &LayoutViewBase::signal_cell_name_changed); cellview (i)->apply_technology_with_sender_event.add (this, &LayoutViewBase::signal_apply_technology); } @@ -2344,18 +2346,17 @@ LayoutViewBase::signal_cell_name_changed () void LayoutViewBase::signal_layer_properties_changed () { - // recompute the source - // TODO: this is a side effect of this method - provide a special method for this purpose - for (unsigned int i = 0; i < layer_lists (); ++i) { - m_layer_properties_lists [i]->attach_view (this, i); - } - - // schedule a redraw request - since the layer views might have changed, this is necessary - redraw_later (); + dm_update_layer_sources (); } void LayoutViewBase::signal_prop_ids_changed () +{ + dm_update_layer_sources (); +} + +void +LayoutViewBase::do_update_layer_sources () { // inform the layer list observers that they need to recompute the property selectors layer_list_changed_event (1); diff --git a/src/laybasic/laybasic/layLayoutViewBase.h b/src/laybasic/laybasic/layLayoutViewBase.h index 5b85c33e0..ec9d99cce 100644 --- a/src/laybasic/laybasic/layLayoutViewBase.h +++ b/src/laybasic/laybasic/layLayoutViewBase.h @@ -2845,6 +2845,7 @@ private: private: lay::LayoutView *mp_ui; tl::DeferredMethod dm_redraw; + tl::DeferredMethod dm_update_layer_sources; bool m_editable; int m_disabled_edits; unsigned int m_options; @@ -2979,6 +2980,7 @@ private: void do_prop_changed (); void do_redraw (int layer); void do_redraw (); + void do_update_layer_sources (); void set_view_ops (); void background_color (tl::Color c);