Bugfix for #954

The bug was that while iterating a Region during the gate traversal,
the "select_interacting" was triggering a sort() which changed the
order.

Solution is to pre-sort when iterators are issued also when the
iterator is non-region selecting. This way, plain and region query
iterators can be used together. In addition, the dirty flag scheme
of Cell+Shapes was cleaned up a little for bboxes.

src/db/db/dbCell.cc

@@ -306,7 +306,8 @@ Cell::update_bbox (unsigned int layers)
   //  update the bboxes of the shapes lists
   for (shapes_map::iterator s = m_shapes_map.begin (); s != m_shapes_map.end (); ++s) {
 
-    s->second.update_bbox ();
+    s->second.reset_bbox_dirty ();
+
     box_type sbox (s->second.bbox ());
 
     if (! sbox.empty ()) {

src/db/db/dbCell.h

@@ -65,7 +65,7 @@ class LayerMapping;
  *  a set of child cell instances and auxiliary information such as
  *  the parent instance list.
  *  A cell is identified through an index given to the cell upon instantiation.
- *  The cell index is valid in the context of a cell graph object which
+ *  The cell index is valid in the context of a layout object which
  *  must issue the cell index.
  */
 
@@ -483,7 +483,7 @@ public:
   bool is_shape_bbox_dirty () const;
 
   /** 
-   *  @brief Update the bbox 
+   *  @brief Updates the bbox
    *
    *  This will update the bbox from the shapes and instances.
    *  This requires the bboxes of the child cells to be computed
@@ -496,8 +496,9 @@ public:
    *  @return true, if the bounding box has changed.
    */
   bool update_bbox (unsigned int layers);
+
   /**
-   *  @brief Sort the shapes lists 
+   *  @brief Sorts the shapes lists
    *
    *  This will sort the shapes lists for query of regions 
    *  on a per-shape basis. Since sorting of the shapes is
@@ -511,7 +512,7 @@ public:
    *
    *  Before the bounding box can be retrieved, it must have
    *  been computed using update_bbox. This is performed by 
-   *  requesting an update from the graph.
+   *  requesting an update from the layout.
    *
    *  @return The bounding box that was computed by update_bbox
    */
@@ -522,7 +523,7 @@ public:
    *
    *  Before the bounding box can be retrieved, it must have
    *  been computed using update_bbox. This is performed by 
-   *  requesting an update from the graph.
+   *  requesting an update from the layout.
    *
    *  @return The bounding box that was computed by update_bbox
    */
@@ -1026,10 +1027,10 @@ protected:
   /**
    *  @brief Standard constructor: create an empty cell object
    * 
-   *  Takes the manager object from the graph object.
+   *  Takes the manager object from the layout object.
    *
    *  @param ci The index of the cell
-   *  @param g A reference to the graph object that owns the cell
+   *  @param g A reference to the layout object that owns the cell
    */
   Cell (cell_index_type ci, db::Layout &g);
 
@@ -1065,7 +1066,7 @@ private:
   //  linked list, used by Layout
   Cell *mp_last, *mp_next;
 
-  //  clear the shapes without telling the graph
+  //  clear the shapes without telling the layout
   void clear_shapes_no_invalidate ();
 
   //  helper function for computing the number of hierarchy levels

src/db/db/dbFlatEdgePairs.cc

@@ -91,7 +91,6 @@ size_t FlatEdgePairs::hier_count () const
 
 Box FlatEdgePairs::compute_bbox () const
 {
-  mp_edge_pairs->update_bbox ();
   return mp_edge_pairs->bbox ();
 }
 

src/db/db/dbFlatEdges.cc

@@ -181,7 +181,6 @@ bool FlatEdges::is_merged () const
 
 Box FlatEdges::compute_bbox () const
 {
-  mp_edges->update_bbox ();
   return mp_edges->bbox ();
 }
 

src/db/db/dbFlatRegion.cc

@@ -190,7 +190,6 @@ bool FlatRegion::is_merged () const
 
 Box FlatRegion::compute_bbox () const
 {
-  mp_polygons->update_bbox ();
   return mp_polygons->bbox ();
 }
 

src/db/db/dbFlatTexts.cc

@@ -91,7 +91,6 @@ size_t FlatTexts::hier_count () const
 
 Box FlatTexts::compute_bbox () const
 {
-  mp_texts->update_bbox ();
   return mp_texts->bbox ();
 }
 

src/db/db/dbGenericShapeIterator.h

@@ -158,8 +158,14 @@ class DB_PUBLIC generic_shapes_iterator_delegate
 {
 public:
   generic_shapes_iterator_delegate (const db::Shapes *shapes)
-    : mp_shapes (shapes), m_iter (mp_shapes->begin (shape_flags<T> ()))
+    : mp_shapes (shapes)
   {
+    //  NOTE: to allow multiple iterators acting on the same Shapes container at once, we always sort before we deliver the iterator -
+    //  also in the non-region case. Without this, sorting may happen while another iterator is progressing.
+    if (mp_shapes->is_bbox_dirty ()) {
+      const_cast<db::Shapes *> (mp_shapes)->update ();
+    }
+    m_iter = mp_shapes->begin (shape_flags<T> ());
     m_is_addressable = shape_flags<T> () == shape_flags_pure<T> () || mp_shapes->begin (shape_flags<T> () - shape_flags_pure<T> ()).at_end ();
     set ();
   }
@@ -171,17 +177,17 @@ public:
 
   virtual void do_reset (const db::Box &box, bool overlapping)
   {
+    //  NOTE: to allow multiple iterators acting on the same Shapes container at once, we always sort before we deliver the iterator -
+    //  also in the non-region case. Without this, sorting may happen while another iterator is progressing.
+    if (mp_shapes->is_bbox_dirty ()) {
+      const_cast<db::Shapes *> (mp_shapes)->update ();
+    }
     if (box == db::Box::world ()) {
       m_iter = mp_shapes->begin (shape_flags<T> ());
+    } else if (overlapping) {
+      m_iter = mp_shapes->begin_overlapping (box, shape_flags<T> ());
     } else {
-      if (mp_shapes->is_bbox_dirty ()) {
-        const_cast<db::Shapes *> (mp_shapes)->update ();
-      }
-      if (overlapping) {
-        m_iter = mp_shapes->begin_overlapping (box, shape_flags<T> ());
-      } else {
-        m_iter = mp_shapes->begin_touching (box, shape_flags<T> ());
-      }
+      m_iter = mp_shapes->begin_touching (box, shape_flags<T> ());
     }
 
     set ();
@@ -214,9 +220,6 @@ public:
 
   virtual db::Box bbox () const
   {
-    if (mp_shapes->is_bbox_dirty ()) {
-      const_cast<db::Shapes *> (mp_shapes)->update ();
-    }
     return mp_shapes->bbox ();
   }
 

src/db/db/dbLayer.h

@@ -459,13 +459,21 @@ struct layer
   }
 
   /**
-   *  @brief Return true if the bounding box is "dirty"
+   *  @brief Return true if the bounding box needs update
    */
   bool is_bbox_dirty () const
   {
     return m_bbox_dirty;
   }
 
+  /**
+   *  @brief Return true if the tree needs update
+   */
+  bool is_tree_dirty () const
+  {
+    return m_tree_dirty;
+  }
+
   /**
    *  @brief Reserve a certain number of elements
    */

src/db/db/dbLayout.cc

@@ -1748,6 +1748,7 @@ Layout::do_update ()
           cp.sort_shapes ();
         }
       }
+
     }
 
     //  sort the instance trees now, since we have computed the bboxes

src/db/db/dbShapes.cc

@@ -1016,15 +1016,12 @@ Shapes::clear ()
   }
 }
 
-void Shapes::update_bbox ()
+void Shapes::reset_bbox_dirty ()
 {
-  for (tl::vector<LayerBase *>::const_iterator l = m_layers.begin (); l != m_layers.end (); ++l) {
-    (*l)->update_bbox ();
-  }
   set_dirty (false);
 }
 
-void Shapes::update () 
+void Shapes::update ()
 {
   for (tl::vector<LayerBase *>::const_iterator l = m_layers.begin (); l != m_layers.end (); ++l) {
     (*l)->sort ();
@@ -1039,7 +1036,7 @@ bool Shapes::is_bbox_dirty () const
     return true;
   }
   for (tl::vector<LayerBase *>::const_iterator l = m_layers.begin (); l != m_layers.end (); ++l) {
-    if ((*l)->is_bbox_dirty ()) {
+    if ((*l)->is_tree_dirty ()) {
       return true;
     }
   }
@@ -1050,6 +1047,9 @@ Shapes::box_type Shapes::bbox () const
 {
   box_type box;
   for (tl::vector<LayerBase *>::const_iterator l = m_layers.begin (); l != m_layers.end (); ++l) {
+    if ((*l)->is_bbox_dirty ()) {
+      (*l)->update_bbox ();
+    }
     box += (*l)->bbox ();
   }
   return box;

src/db/db/dbShapes.h

@@ -485,6 +485,7 @@ public:
   virtual bool is_bbox_dirty () const = 0;
   virtual size_t size () const = 0;
   virtual bool empty () const = 0;
+  virtual bool is_tree_dirty () const = 0;
   virtual void sort () = 0;
   virtual LayerBase *clone () const = 0;
   virtual bool is_same_type (const LayerBase *other) const = 0;
@@ -1177,28 +1178,20 @@ public:
   shape_type transform (const shape_type &ref, const Trans &t);
 
   /**
-   *  @brief updates the bbox and sorts if necessary
-   *
-   *  This is equivalent to calling sort () and update_bbox () in this order.
+   *  @brief Updates the quad trees (sort ()) and resets the dirty flag
    */
   void update ();
 
   /**
-   *  @brief updates the bbox 
-   *
-   *  Updating the bbox is required after insert operations
-   *  and is performed only as far as necessary.
-   */
-  void update_bbox ();
-
-  /**
-   *  @brief check if the bounding box needs update
-   *
-   *  Returns true if the bounding box of the shapes has changed and 
-   *  requires an update.
+   *  @brief Returns a value indicating whether the shape container is modified and needs update
    */
   bool is_bbox_dirty () const;
 
+  /**
+   *  @brief Resets the "dirty bbox" condition (see is_bbox_dirty)
+   */
+  void reset_bbox_dirty ();
+
   /**
    *  @brief Retrieve the bbox 
    *

src/db/db/dbShapes2.h

@@ -126,6 +126,11 @@ public:
     return m_layer.is_bbox_dirty ();
   }
 
+  virtual bool is_tree_dirty () const
+  {
+    return m_layer.is_tree_dirty ();
+  }
+
   size_t size () const
   {
     return m_layer.size ();

src/db/unit_tests/dbNetlistExtractorTests.cc

@@ -3176,3 +3176,128 @@ TEST(14_JoinNets)
   db::compare_layouts (_this, ly, au);
 }
 
+
+TEST(100_issue954)
+{
+  db::Layout ly;
+  db::LayerMap lmap;
+
+  unsigned int active     = define_layer (ly, lmap, 1);
+  unsigned int poly       = define_layer (ly, lmap, 2);
+
+  {
+    db::LoadLayoutOptions options;
+    options.get_options<db::CommonReaderOptions> ().layer_map = lmap;
+    options.get_options<db::CommonReaderOptions> ().create_other_layers = false;
+
+    std::string fn (tl::testdata ());
+    fn = tl::combine_path (fn, "algo");
+    fn = tl::combine_path (fn, "device_extract_issue954.gds");
+
+    tl::InputStream stream (fn);
+    db::Reader reader (stream);
+    reader.read (ly, options);
+  }
+
+  db::Cell &tc = ly.cell (*ly.begin_top_down ());
+
+  db::DeepShapeStore dss;
+  dss.set_text_enlargement (1);
+  dss.set_text_property_name (tl::Variant ("LABEL"));
+
+  //  original layers
+  db::Region ractive (db::RecursiveShapeIterator (ly, tc, active), dss);
+  db::Region rpoly (db::RecursiveShapeIterator (ly, tc, poly), dss);
+
+  //  derived regions
+
+  db::Region rpgate   = ractive & rpoly;
+  db::Region rpsd     = ractive - rpgate;
+
+  //  Global
+
+  db::Region bulk (dss);
+
+  //  return the computed layers into the original layout and write it for debugging purposes
+
+  unsigned int lgate  = ly.insert_layer (db::LayerProperties (10, 0));      // 10/0 -> Gate
+  unsigned int lsd    = ly.insert_layer (db::LayerProperties (11, 0));      // 11/0 -> Source/Drain
+  unsigned int lpdiff = ly.insert_layer (db::LayerProperties (12, 0));      // 12/0 -> P Diffusion
+
+  rpgate.insert_into (&ly, tc.cell_index (), lgate);
+  rpsd.insert_into (&ly, tc.cell_index (), lsd);
+  rpsd.insert_into (&ly, tc.cell_index (), lpdiff);
+
+  //  perform the extraction
+
+  db::Netlist nl;
+  db::hier_clusters<db::NetShape> cl;
+
+  db::NetlistDeviceExtractorMOS4Transistor pmos_ex ("PMOS");
+
+  db::NetlistDeviceExtractor::input_layers dl;
+
+  dl["SD"] = &rpsd;
+  dl["G"] = &rpgate;
+  dl["W"] = &bulk;
+  dl["P"] = &rpoly;  //  not needed for extraction but to return terminal shapes
+  pmos_ex.extract (dss, 0, dl, nl, cl);
+
+  //  perform the net extraction
+
+  db::NetlistExtractor net_ex;
+
+  db::Connectivity conn;
+
+  //  Global nets
+  conn.connect_global (bulk, "BULK");
+
+  //  Intra-layer
+  conn.connect (rpsd);
+  conn.connect (rpoly);
+
+  //  extract the nets
+
+  std::list<std::set<std::string> > jn;
+
+  jn.push_back (std::set<std::string> ());
+  jn.back ().insert ("BULK");
+  jn.back ().insert ("VSS");
+
+  jn.push_back (std::set<std::string> ());
+  jn.back ().insert ("NWELL");
+  jn.back ().insert ("VDD");
+
+  net_ex.set_joined_nets ("INV2", jn);
+
+  std::list<tl::GlobPattern> gp;
+  gp.push_back (tl::GlobPattern ("NEXT"));
+  gp.push_back (tl::GlobPattern ("FB"));
+  net_ex.set_joined_net_names (gp);
+
+  net_ex.extract_nets (dss, 0, conn, nl, cl);
+
+  EXPECT_EQ (all_net_names_unique (nl), true);
+
+  //  debug layers produced for nets
+  //    200/0 -> Poly
+  //    201/0 -> N source/drain
+  //    202/0 -> Bulk
+  std::map<unsigned int, unsigned int> dump_map;
+  dump_map [layer_of (bulk)      ] = ly.insert_layer (db::LayerProperties (202, 0));
+  dump_map [layer_of (rpsd)      ] = ly.insert_layer (db::LayerProperties (201, 0));
+  dump_map [layer_of (rpoly)     ] = ly.insert_layer (db::LayerProperties (200, 0));
+
+  //  write nets to layout
+  db::CellMapping cm = dss.cell_mapping_to_original (0, &ly, tc.cell_index ());
+  dump_nets_to_layout (nl, cl, ly, dump_map, cm, true);
+
+  //  compare the collected test data
+
+  std::string au = tl::testdata ();
+  au = tl::combine_path (au, "algo");
+  au = tl::combine_path (au, "device_extract_issue954_au.gds");
+
+  db::compare_layouts (_this, ly, au);
+}
+

src/db/unit_tests/dbShapeArrayTests.cc

@@ -119,7 +119,6 @@ TEST(2)
   shapes.insert (p2);
   shapes.insert (db::SimplePolygonRef (p2, *rep));
   shapes.sort ();
-  shapes.update_bbox ();
 
   EXPECT_EQ (shapes.bbox () == db::Box (100,-5200,2300,2000), true);
 

src/db/unit_tests/dbShapesTests.cc

@@ -34,26 +34,21 @@ TEST(1)
   db::Shapes s (&m, 0, db::default_editable_mode ());
   db::Box b_empty;
 
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), b_empty);
 
   db::Box b (0, 100, 1000, 1200);
   s.insert (b);
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), b);
 
   db::Edge e (-100, -200, 0, 0);
   s.insert (e);
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), db::Box (-100, -200, 1000, 1200));
   
   db::Shapes s2 (s);
-  s2.update_bbox ();
   EXPECT_EQ (s2.bbox (), db::Box (-100, -200, 1000, 1200));
 
   if (db::default_editable_mode ()) {
     s2.erase (db::Box::tag (), db::stable_layer_tag (), s2.begin (db::Box::tag (), db::stable_layer_tag ()));
-    s2.update_bbox ();
     EXPECT_EQ (s2.bbox (), db::Box (-100, -200, 0, 0));
   }
 }
@@ -64,25 +59,20 @@ TEST(1a)
   db::Shapes s (&m, 0, true);
   db::Box b_empty;
 
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), b_empty);
 
   db::Box b (0, 100, 1000, 1200);
   s.insert (b);
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), b);
 
   db::Edge e (-100, -200, 0, 0);
   s.insert (e);
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), db::Box (-100, -200, 1000, 1200));
   
   db::Shapes s2 (s);
-  s2.update_bbox ();
   EXPECT_EQ (s2.bbox (), db::Box (-100, -200, 1000, 1200));
 
   s2.erase (db::Box::tag (), db::stable_layer_tag (), s2.begin (db::Box::tag (), db::stable_layer_tag ()));
-  s2.update_bbox ();
   EXPECT_EQ (s2.bbox (), db::Box (-100, -200, 0, 0));
 }
 
@@ -92,21 +82,17 @@ TEST(1b)
   db::Shapes s (&m, 0, false);
   db::Box b_empty;
 
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), b_empty);
 
   db::Box b (0, 100, 1000, 1200);
   s.insert (b);
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), b);
 
   db::Edge e (-100, -200, 0, 0);
   s.insert (e);
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), db::Box (-100, -200, 1000, 1200));
   
   db::Shapes s2 (s);
-  s2.update_bbox ();
   EXPECT_EQ (s2.bbox (), db::Box (-100, -200, 1000, 1200));
 }
 
@@ -3206,12 +3192,10 @@ TEST(23)
   db::Shapes s (&m, 0, db::default_editable_mode ());
   db::Box b_empty;
 
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), b_empty);
 
   db::EdgePair ep (db::Edge (-100, -200, 0, 0), db::Edge (0, -100, 100, 100));
   s.insert (ep);
-  s.update_bbox ();
   EXPECT_EQ (s.bbox (), db::Box (-100, -200, 100, 100));
 
   db::ShapeIterator si = s.begin (db::ShapeIterator::EdgePairs);