Bugfix: redrawing issues when multiple layers are affected

This is how to reproduce the bug: have a layout with two
layers. Select two shapes of different layers and delete them.
One layer is not updated and only after zooming/panning the
shape will disappear on this layer.

src/db/db/dbLayoutStateModel.cc

@@ -23,17 +23,19 @@
 
 #include "dbLayoutStateModel.h"
 
+#include <limits>
+
 namespace db 
 {
 
 LayoutStateModel::LayoutStateModel (bool busy)
-  : m_hier_dirty (false), m_bboxes_dirty (false), m_busy (busy)
+  : m_hier_dirty (false), m_all_bboxes_dirty (false), m_busy (busy)
 {
   //  .. nothing yet ..
 }
 
 LayoutStateModel::LayoutStateModel (const LayoutStateModel &d)
-  : m_hier_dirty (d.m_hier_dirty), m_bboxes_dirty (d.m_bboxes_dirty), m_busy (d.m_busy)
+  : m_hier_dirty (d.m_hier_dirty), m_bboxes_dirty (d.m_bboxes_dirty), m_all_bboxes_dirty (d.m_all_bboxes_dirty), m_busy (d.m_busy)
 {
   //  .. nothing yet ..
 }
@@ -43,6 +45,7 @@ LayoutStateModel::operator= (const LayoutStateModel &d)
 {
   m_hier_dirty = d.m_hier_dirty;
   m_bboxes_dirty = d.m_bboxes_dirty;
+  m_all_bboxes_dirty = d.m_all_bboxes_dirty;
   m_busy = d.m_busy;
   return *this;
 }
@@ -65,5 +68,41 @@ LayoutStateModel::do_invalidate_bboxes (unsigned int index)
   bboxes_changed_any_event ();
 }
 
+void
+LayoutStateModel::invalidate_bboxes (unsigned int index)
+{
+  if (index == std::numeric_limits<unsigned int>::max ()) {
+    if (! m_all_bboxes_dirty || m_busy) {
+      do_invalidate_bboxes (index);  //  must be called before the bboxes are invalidated (stopping of redraw thread requires this)
+      m_all_bboxes_dirty = true;
+    }
+  } else {
+    if (index >= (unsigned int) m_bboxes_dirty.size ()) {
+      m_bboxes_dirty.resize (index + 1, false);
+    }
+    if ((! m_all_bboxes_dirty && ! m_bboxes_dirty [index]) || m_busy) {
+      do_invalidate_bboxes (index);  //  must be called before the bboxes are invalidated (stopping of redraw thread requires this)
+      m_bboxes_dirty [index] = true;
+    }
+  }
+}
+
+bool
+LayoutStateModel::bboxes_dirty () const
+{
+  return ! m_bboxes_dirty.empty () || m_all_bboxes_dirty;
+}
+
+void
+LayoutStateModel::update ()
+{
+  if (bboxes_dirty () || m_hier_dirty) {
+    do_update ();
+    m_bboxes_dirty.clear ();
+    m_all_bboxes_dirty = false;
+    m_hier_dirty = false;
+  }
+}
+
 }
 

src/db/db/dbLayoutStateModel.h

@@ -100,13 +100,7 @@ public:
    *  If the index is std::numeric_limits<unsigned int>::max, this method
    *  applies to all layers.
    */
-  void invalidate_bboxes (unsigned int index)
-  {
-    if (! m_bboxes_dirty || m_busy) {
-      do_invalidate_bboxes (index);  //  must be called before the bboxes are invalidated (stopping of redraw thread requires this)
-      m_bboxes_dirty = true;
-    }
-  }
+  void invalidate_bboxes (unsigned int index);
 
   /**
    *  @brief Signal that the database unit has changed
@@ -121,14 +115,7 @@ public:
    *
    *  This method will call do_update if necessary and reset the invalid flags. 
    */
-  void update ()
-  {
-    if (m_bboxes_dirty || m_hier_dirty) {
-      do_update ();
-      m_bboxes_dirty = false;
-      m_hier_dirty = false;
-    }
-  }
+  void update ();
 
   /**
    *  @brief The "dirty hierarchy" attribute
@@ -145,10 +132,7 @@ public:
    *
    *  This attribute is true, if the bounding boxes have changed since the last "update" call
    */
-  bool bboxes_dirty () const
-  {
-    return m_bboxes_dirty;
-  }
+  bool bboxes_dirty () const;
 
   /**
    *  @brief Sets or resets busy mode
@@ -211,7 +195,8 @@ public:
 
 private:
   bool m_hier_dirty;
-  bool m_bboxes_dirty;
+  std::vector<bool> m_bboxes_dirty;
+  bool m_all_bboxes_dirty;
   bool m_busy;
 
   void do_invalidate_hier ();

src/db/unit_tests/dbLayout.cc

@@ -202,3 +202,265 @@ TEST(1)
   EXPECT_EQ (ex, true);
 }
 
+namespace
+{
+
+struct EventListener
+  : public tl::Object
+{
+  EventListener ()
+    : flags (0),
+      bboxes_dirty (false),
+      bboxes_all_dirty (false),
+      hier_dirty (false),
+      dbu_dirty (true),
+      cell_name_dirty (true),
+      property_ids_dirty (true),
+      layer_properties_dirty (true)
+  { }
+
+  void reset () { *this = EventListener (); }
+
+  void bboxes_changed (unsigned int i)
+  {
+    if (i < 31) {
+      flags |= (1 << i);
+    } else {
+      bboxes_all_dirty = true;
+    }
+  }
+
+  void bboxes_any_changed () { bboxes_dirty = true; }
+  void hier_changed () { hier_dirty = true; }
+  void dbu_changed () { dbu_dirty = true; }
+  void cell_name_changed () { cell_name_dirty = true; }
+  void property_ids_changed () { property_ids_dirty = true; }
+  void layer_properties_changed () { layer_properties_dirty = true; }
+
+  unsigned int flags;
+  bool bboxes_dirty, bboxes_all_dirty, hier_dirty;
+  bool dbu_dirty, cell_name_dirty, property_ids_dirty, layer_properties_dirty;
+};
+
+}
+
+TEST(2)
+{
+  //  LayoutStateModel hierarchy events
+
+  db::Layout g;
+  EventListener el;
+
+  g.hier_changed_event.add (&el, &EventListener::hier_changed);
+  g.bboxes_changed_any_event.add (&el, &EventListener::bboxes_any_changed);
+  g.bboxes_changed_event.add (&el, &EventListener::bboxes_changed);
+
+  db::cell_index_type ci;
+  db::Cell *top;
+
+  ci = g.add_cell ("TOP");
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);
+  EXPECT_EQ (el.bboxes_dirty, false);
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, true);
+
+  el.reset ();
+  top = &g.cell (ci);
+  ci = g.add_cell ("A");
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);
+  EXPECT_EQ (el.bboxes_dirty, false);
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, false);  //  needs g.update() before being issues again
+
+  el.reset ();
+  top->insert (db::CellInstArray (ci, db::Trans ()));
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);
+  EXPECT_EQ (el.bboxes_dirty, true);
+  EXPECT_EQ (el.bboxes_all_dirty, true);
+  EXPECT_EQ (el.hier_dirty, false);  //  needs g.update() before being issues again
+
+  g.clear ();
+  g.update ();
+  el.reset ();
+
+  ci = g.add_cell ("TOP");
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);
+  EXPECT_EQ (el.bboxes_dirty, false);
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, true);
+
+  el.reset ();
+  g.update ();
+  top = &g.cell (ci);
+  ci = g.add_cell ("A");
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);
+  EXPECT_EQ (el.bboxes_dirty, false);
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, true);  //  OK - see above
+
+  el.reset ();
+  g.update ();
+  top->insert (db::CellInstArray (ci, db::Trans ()));
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);
+  EXPECT_EQ (el.bboxes_dirty, true);
+  EXPECT_EQ (el.bboxes_all_dirty, true);
+  EXPECT_EQ (el.hier_dirty, true);  //  OK - see above
+
+  //  busy mode will make events issued always
+  g.clear ();
+  g.set_busy (true);
+  el.reset ();
+
+  ci = g.add_cell ("TOP");
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);
+  EXPECT_EQ (el.bboxes_dirty, false);
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, true);
+
+  el.reset ();
+  top = &g.cell (ci);
+  ci = g.add_cell ("A");
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);
+  EXPECT_EQ (el.bboxes_dirty, false);
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, true);  //  OK - see above
+
+  el.reset ();
+  top->insert (db::CellInstArray (ci, db::Trans ()));
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);
+  EXPECT_EQ (el.bboxes_dirty, true);
+  EXPECT_EQ (el.bboxes_all_dirty, true);
+  EXPECT_EQ (el.hier_dirty, true);  //  OK - see above
+
+}
+
+TEST(3)
+{
+  //  LayoutStateModel bbox events
+
+  db::Layout g;
+  EventListener el;
+
+  g.insert_layer (0);
+  g.insert_layer (1);
+
+  g.hier_changed_event.add (&el, &EventListener::hier_changed);
+  g.bboxes_changed_any_event.add (&el, &EventListener::bboxes_any_changed);
+  g.bboxes_changed_event.add (&el, &EventListener::bboxes_changed);
+
+  db::cell_index_type ci;
+  db::Cell *top;
+
+  ci = g.add_cell ("TOP");
+  top = &g.cell (ci);
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);
+  EXPECT_EQ (el.bboxes_dirty, false);
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, true);
+
+  el.reset ();
+  g.update ();
+
+  top->shapes (0).insert (db::Box (0, 0, 10, 20));
+  top->shapes (1).insert (db::Box (0, 0, 10, 20));
+
+  EXPECT_EQ (el.flags, (unsigned int) 3);
+  EXPECT_EQ (el.bboxes_dirty, true);
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, false);
+
+  el.reset ();
+
+  top->shapes (0).insert (db::Box (0, 0, 10, 20));
+
+  EXPECT_EQ (el.flags, (unsigned int) 0);  //  g.update () is missing -> no new events
+  EXPECT_EQ (el.bboxes_dirty, false);   //  g.update () is missing -> no new events
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, false);
+
+  el.reset ();
+  g.update ();
+
+  top->shapes (0).insert (db::Box (0, 0, 10, 20));
+
+  EXPECT_EQ (el.flags, (unsigned int) 1);  //  voila
+  EXPECT_EQ (el.bboxes_dirty, true);   //  :-)
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, false);
+
+  top->shapes (1).insert (db::Box (0, 0, 10, 20));
+
+  EXPECT_EQ (el.flags, (unsigned int) 3);  //  and yet another one
+  EXPECT_EQ (el.bboxes_dirty, true);
+  EXPECT_EQ (el.bboxes_all_dirty, false);
+  EXPECT_EQ (el.hier_dirty, false);
+}
+
+TEST(4)
+{
+  //  Other events
+
+  db::Layout g;
+  EventListener el;
+
+  g.insert_layer (0);
+  g.insert_layer (1);
+  db::cell_index_type top = g.add_cell ("TOP");
+
+  g.dbu_changed_event.add (&el, &EventListener::dbu_changed);
+  g.cell_name_changed_event.add (&el, &EventListener::cell_name_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);
+  EXPECT_EQ (el.cell_name_dirty, false);
+  EXPECT_EQ (el.property_ids_dirty, false);
+  EXPECT_EQ (el.layer_properties_dirty, false);
+
+  g.set_properties (0, db::LayerProperties (1, 0));
+  EXPECT_EQ (el.layer_properties_dirty, true);
+  el.reset ();
+  g.set_properties (0, db::LayerProperties (1, 0));
+  EXPECT_EQ (el.layer_properties_dirty, false);  //  no change
+  g.set_properties (0, db::LayerProperties (1, 1));
+  EXPECT_EQ (el.layer_properties_dirty, true);  //  but this is
+
+  g.dbu (1.0);
+  EXPECT_EQ (el.dbu_dirty, true);
+  el.reset ();
+  g.dbu (1.0);
+  EXPECT_EQ (el.dbu_dirty, false);  //  no change
+  g.dbu (0.5);
+  EXPECT_EQ (el.dbu_dirty, true);  //  but this is
+
+  g.rename_cell (top, "TIP");
+  EXPECT_EQ (el.cell_name_dirty, true);
+  el.reset ();
+  g.rename_cell (top, "TIP");
+  EXPECT_EQ (el.cell_name_dirty, false);  //  no change
+  g.rename_cell (top, "TAP");
+  EXPECT_EQ (el.cell_name_dirty, true);  //  but this is
+
+  db::properties_id_type prop_id;
+
+  db::PropertiesRepository::properties_set ps;
+  ps.insert (std::make_pair (g.properties_repository ().prop_name_id (tl::Variant (1)), tl::Variant ("XYZ")));
+  prop_id = g.properties_repository ().properties_id (ps);
+  EXPECT_EQ (el.property_ids_dirty, true);
+  el.reset ();
+
+  ps.clear ();
+  ps.insert (std::make_pair (g.properties_repository ().prop_name_id (tl::Variant (1)), tl::Variant ("XXX")));
+  prop_id = g.properties_repository ().properties_id (ps);
+  EXPECT_EQ (el.property_ids_dirty, true);
+}

src/laybasic/laybasic/layLayoutCanvas.cc

@@ -40,6 +40,7 @@
 #include "layBitmapsToImage.h"
 
 #include <sstream>
+#include <algorithm>
 
 namespace lay
 {
@@ -1149,7 +1150,9 @@ LayoutCanvas::redraw_selected (const std::vector<int> &layers)
   }
 
   m_need_redraw = true;
-  m_need_redraw_layer = layers;
+  m_need_redraw_layer.insert (m_need_redraw_layer.end (), layers.begin (), layers.end ());
+  std::sort (m_need_redraw_layer.begin (), m_need_redraw_layer.end ());
+  m_need_redraw_layer.erase (std::unique (m_need_redraw_layer.begin (), m_need_redraw_layer.end ()), m_need_redraw_layer.end ());
   m_redraw_force_update = true;
 
   update (); // produces a paintEvent()
@@ -1166,7 +1169,6 @@ LayoutCanvas::change_visibility (const std::vector <bool> &visible)
 
   if (! m_need_redraw) {
     m_redraw_clearing = false;
-    m_need_redraw_layer.clear ();
   }
 
   m_need_redraw = true;

src/laybasic/laybasic/layLayoutView.cc

@@ -2126,7 +2126,7 @@ LayoutView::signal_bboxes_from_layer_changed (unsigned int cv_index, unsigned in
 
     //  redraw only the layers required for redrawing
     for (std::vector<lay::RedrawLayerInfo>::const_iterator l = mp_canvas->get_redraw_layers ().begin (); l != mp_canvas->get_redraw_layers ().end (); ++l) {
-      if (l->cellview_index == int (cv_index) && (layer_index == std::numeric_limits<unsigned int>::max () || l->layer_index == int (layer_index))) {
+      if (l->cellview_index == int (cv_index) && l->layer_index == int (layer_index)) {
         redraw_layer ((unsigned int) (l - mp_canvas->get_redraw_layers ().begin ()));
       }
     }