Providing a more efficient event for property ID changes

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;
   }
 }

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 <class Tag, class ET, class I>
@@ -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 {

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
    */

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.

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;
   }
 }
 

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<bool> 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 ();
 };
 
 }

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;
 }
 

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
   {

src/db/db/dbShapes.cc

@@ -304,10 +304,20 @@ Shapes::invalidate_state ()
       if (index != std::numeric_limits<unsigned int>::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:

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;
 

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)

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) {

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);

src/laybasic/laybasic/layLayoutViewBase.h

@@ -2845,6 +2845,7 @@ private:
 private:
   lay::LayoutView *mp_ui;
   tl::DeferredMethod<lay::LayoutViewBase> dm_redraw;
+  tl::DeferredMethod<lay::LayoutViewBase> 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);