Merge pull request #1299 from KLayout/issue-1249

Implemented solution for issue #1249 (persist layer properties in ses…

src/laybasic/laybasic/gsiDeclLayLayers.cc

@@ -1189,6 +1189,16 @@ Class<lay::LayerPropertiesNode> decl_LayerPropertiesNode (
     "Unlike the name suggests, this node will still contain a hierarchy of nodes below if the original "
     "node did so."
   ) +
+  method ("is_expanded?", &lay::LayerPropertiesNode::expanded,
+    "@brief Gets a value indicating whether the layer tree node is expanded.\n"
+    "This predicate has been introduced in version 0.28.6."
+  ) +
+  method ("expanded=", &lay::LayerPropertiesNode::set_expanded, gsi::arg ("ex"),
+    "@brief Set a value indicating whether the layer tree node is expanded.\n"
+    "Setting this value to 'true' will expand (open) the tree node. Setting it to 'false' will collapse the node.\n"
+    "\n"
+    "This predicate has been introduced in version 0.28.6."
+  ) +
   method_ext ("add_child", &add_child0,
     "@brief Add a child entry\n"
     "@return A reference to the node created\n"

src/laybasic/laybasic/layLayerProperties.cc

@@ -591,6 +591,12 @@ LayerProperties::need_realize (unsigned int flags, bool /*force*/)
   }
 }
 
+void
+LayerProperties::expanded_state_changed ()
+{
+  //  .. no effect ..
+}
+
 void 
 LayerProperties::do_realize (const LayoutViewBase *view) const
 {
@@ -651,7 +657,7 @@ static unsigned int s_unique_id = 0;
 
 LayerPropertiesNode::LayerPropertiesNode ()
   : LayerProperties (),
-    m_list_index (0)
+    m_list_index (0), m_expanded (false)
 {
   m_id = ++s_unique_id;
 }
@@ -663,7 +669,7 @@ LayerPropertiesNode::~LayerPropertiesNode ()
 
 LayerPropertiesNode::LayerPropertiesNode (const LayerProperties &d)
   : LayerProperties (d),
-    m_list_index (0)
+    m_list_index (0), m_expanded (false)
 {
   m_id = ++s_unique_id;
 }
@@ -671,6 +677,7 @@ LayerPropertiesNode::LayerPropertiesNode (const LayerProperties &d)
 LayerPropertiesNode::LayerPropertiesNode (const LayerPropertiesNode &d)
   : LayerProperties (d), tl::Object (),
     m_list_index (0),
+    m_expanded (d.m_expanded),
     m_children (d.m_children),
     m_id (d.m_id)
 {
@@ -687,6 +694,7 @@ LayerPropertiesNode::operator= (const LayerPropertiesNode &d)
     LayerProperties::operator= (d);
 
     m_children = d.m_children;
+    m_expanded = d.m_expanded;
     m_id = d.m_id;
 
     for (iterator c = m_children.begin (); c != m_children.end (); ++c) {
@@ -705,7 +713,7 @@ LayerPropertiesNode::operator== (const LayerPropertiesNode &d) const
   if (! LayerProperties::operator== (d)) {
     return false;
   }
-  return m_children == d.m_children;
+  return m_children == d.m_children && m_expanded == d.m_expanded;
 }
 
 LayoutViewBase *LayerPropertiesNode::view() const
@@ -713,6 +721,15 @@ LayoutViewBase *LayerPropertiesNode::view() const
   return const_cast<lay::LayoutViewBase *> (mp_view.get ());
 }
 
+void
+LayerPropertiesNode::set_expanded (bool ex)
+{
+  if (expanded () != ex) {
+    m_expanded = ex;
+    expanded_state_changed ();
+  }
+}
+
 unsigned int
 LayerPropertiesNode::list_index () const
 {
@@ -740,6 +757,12 @@ LayerPropertiesNode::realize_source () const
   do_realize (mp_view.get ());
 }
 
+void
+LayerPropertiesNode::expanded_state_changed ()
+{
+  touch ();
+}
+
 void
 LayerPropertiesNode::need_realize (unsigned int flags, bool force)
 {
@@ -1763,8 +1786,9 @@ struct LineStyleIndexConverter
 static const tl::XMLElementList layer_element = tl::XMLElementList (
   //  HINT: these make_member calls want to be qualified: otherwise an internal error
   //  was observed ..
-  tl::make_member<tl::color_t, LayerPropertiesNode> (&LayerPropertiesNode::frame_color_loc,      &LayerPropertiesNode::set_frame_color_code, "frame-color",        UIntColorConverter ()) + 
-  tl::make_member<tl::color_t, LayerPropertiesNode> (&LayerPropertiesNode::fill_color_loc,       &LayerPropertiesNode::set_fill_color_code,  "fill-color",         UIntColorConverter ()) + 
+  tl::make_member<bool, LayerPropertiesNode>         (&LayerPropertiesNode::expanded,             &LayerPropertiesNode::set_expanded,         "expanded") +
+  tl::make_member<tl::color_t, LayerPropertiesNode>  (&LayerPropertiesNode::frame_color_loc,      &LayerPropertiesNode::set_frame_color_code, "frame-color",        UIntColorConverter ()) +
+  tl::make_member<tl::color_t, LayerPropertiesNode>  (&LayerPropertiesNode::fill_color_loc,       &LayerPropertiesNode::set_fill_color_code,  "fill-color",         UIntColorConverter ()) +
   tl::make_member<int, LayerPropertiesNode>          (&LayerPropertiesNode::frame_brightness_loc, &LayerPropertiesNode::set_frame_brightness, "frame-brightness") + 
   tl::make_member<int, LayerPropertiesNode>          (&LayerPropertiesNode::fill_brightness_loc,  &LayerPropertiesNode::set_fill_brightness,  "fill-brightness") + 
   tl::make_member<int, LayerPropertiesNode>          (&LayerPropertiesNode::dither_pattern_loc,   &LayerPropertiesNode::set_dither_pattern,   "dither-pattern",     DitherPatternIndexConverter ()) + 
@@ -2061,6 +2085,16 @@ LayerPropertiesNodeRef::need_realize (unsigned int flags, bool force)
   }
 }
 
+void
+LayerPropertiesNodeRef::expanded_state_changed ()
+{
+  LayerPropertiesNode::expanded_state_changed ();
+
+  if (is_valid ()) {
+    view ()->set_layer_node_expanded (m_iter, expanded ());
+  }
+}
+
 void
 LayerPropertiesNodeRef::refresh () const
 {

src/laybasic/laybasic/layLayerProperties.h

@@ -919,6 +919,11 @@ protected:
    */
   virtual void need_realize (unsigned int flags, bool force = false);
 
+  /**
+   *  @brief indicates a change of the collapsed/expanded state
+   */
+  virtual void expanded_state_changed ();
+
   /**
    *  @brief Fetches the current status from the original properties for the LayerPropertiesNodeRef implementation
    */
@@ -1069,6 +1074,20 @@ public:
     return r;
   }
 
+  /**
+   *  @brief Sets the expanded state of the layer properties tree node
+   */
+  void set_expanded (bool ex);
+
+  /**
+   *  @brief Gets the expanded state of the layer properties node
+   */
+  bool expanded () const
+  {
+    refresh ();
+    return m_expanded;
+  }
+
   /**
    *  @brief Child layers: begin iterator
    */
@@ -1207,14 +1226,21 @@ public:
   virtual void realize_source () const;
   virtual void realize_visual () const;
 
+  void set_expanded_silent (bool ex)
+  {
+    m_expanded = ex;
+  }
+
 protected: 
   virtual void need_realize (unsigned int flags, bool force);
+  virtual void expanded_state_changed ();
   void set_parent (const LayerPropertiesNode *);
 
 private:
   //  A reference to the view 
   tl::weak_ptr<lay::LayoutViewBase> mp_view;
   unsigned int m_list_index;
+  bool m_expanded;
   //  the parent node
   tl::weak_ptr<LayerPropertiesNode> mp_parent;
   //  the list of children
@@ -2009,8 +2035,9 @@ private:
   tl::weak_ptr<LayerPropertiesNode> mp_node;
   size_t m_synched_gen_id;
 
-  void need_realize (unsigned int flags, bool force);
-  void refresh () const;
+  virtual void need_realize (unsigned int flags, bool force);
+  virtual void expanded_state_changed ();
+  virtual void refresh () const;
 };
 
 }

src/laybasic/laybasic/layLayoutViewBase.cc

@@ -1880,6 +1880,21 @@ LayoutViewBase::replace_layer_node (unsigned int index, const LayerPropertiesCon
   }
 }
 
+void
+LayoutViewBase::set_layer_node_expanded (unsigned int index, const LayerPropertiesConstIterator &iter, bool ex)
+{
+  if (ex != iter->expanded ()) {
+
+    LayerPropertiesIterator non_const_iter (get_properties (index), iter.uint ());
+    non_const_iter->set_expanded (ex);
+
+    if (index == current_layer_list ()) {
+      layer_list_changed_event (8 /*expanded state needs update*/);
+    }
+
+  }
+}
+
 void 
 LayoutViewBase::set_properties (unsigned int index, const LayerPropertiesConstIterator &iter, const LayerProperties &props)
 {

src/laybasic/laybasic/layLayoutViewBase.h

@@ -335,6 +335,17 @@ public:
     set_properties (current_layer_list (), iter, props);
   }
 
+  /**
+   *  @brief Sets a value indicating whether the given node is expanded in the layer tree
+   *
+   *  @param iter Points to the layer node to be modified
+   *  @param ex True if the layer node shall be expanded, false if it shall be collapsed
+   */
+  void set_layer_node_expanded (const LayerPropertiesConstIterator &iter, bool ex)
+  {
+    set_layer_node_expanded (current_layer_list (), iter, ex);
+  }
+
   /**
    *  @brief Set the layer properties of a layer with the given position (by iterator) for the layer list with the given index
    *
@@ -344,6 +355,15 @@ public:
    */
   void set_properties (unsigned int index, const LayerPropertiesConstIterator &iter, const LayerProperties &props);
 
+  /**
+   *  @brief Sets a value indicating whether the given node is expanded in the layer tree
+   *
+   *  @param index The layer list's index
+   *  @param iter Points to the layer node to be modified
+   *  @param ex True if the layer node shall be expanded, false if it shall be collapsed
+   */
+  void set_layer_node_expanded (unsigned int index, const LayerPropertiesConstIterator &iter, bool ex);
+
   /**
    *  @brief Expand the layer properties of all tabs 
    *

src/laybasic/unit_tests/layLayerProperties.cc

@@ -135,6 +135,7 @@ TEST (1)
     "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
     "<layer-properties>\n"
     " <properties>\n"
+    "  <expanded>false</expanded>\n"
     "  <frame-color/>\n"
     "  <fill-color/>\n"
     "  <frame-brightness>0</frame-brightness>\n"

src/layui/layui/layLayerControlPanel.cc

@@ -205,6 +205,7 @@ LayerControlPanel::LayerControlPanel (lay::LayoutViewBase *view, db::Manager *ma
     db::Object (manager),
     mp_view (view), 
     m_needs_update (true), 
+    m_expanded_state_needs_update (false),
     m_tabs_need_update (true), 
     m_hidden_flags_need_update (true),
     m_in_update (false),
@@ -1696,6 +1697,7 @@ LayerControlPanel::cancel_updates ()
 {
   m_in_update = false;
   m_needs_update = false;
+  m_expanded_state_needs_update = false;
   m_hidden_flags_need_update = false;
   m_tabs_need_update = false;
 }
@@ -1874,11 +1876,13 @@ LayerControlPanel::do_update_content ()
   }
 
   if (m_hidden_flags_need_update) {
-
     do_update_hidden_flags ();
-
     m_hidden_flags_need_update = false;
+  }
 
+  if (m_expanded_state_needs_update) {
+    restore_expanded ();
+    m_expanded_state_needs_update = false;
   }
 }
 
@@ -1993,6 +1997,11 @@ LayerControlPanel::signal_li_changed (int)
 void 
 LayerControlPanel::update_required (int f)
 {
+  //  the name of a layer list has changed
+  if ((f & 8) != 0) {
+    m_expanded_state_needs_update = true;
+  }
+
   //  the name of a layer list has changed
   if ((f & 4) != 0) {
     m_tabs_need_update = true;
@@ -2032,18 +2041,18 @@ LayerControlPanel::current_index_changed (const QModelIndex &index)
 void
 LayerControlPanel::group_collapsed (const QModelIndex &index)
 {
-  lay::LayerPropertiesConstIterator iter = mp_model->iterator (index);
+  auto iter = mp_model->iterator_nc (index);
   if (! iter.is_null () && ! iter.at_end ()) {
-    m_expanded.erase (iter->id ());
+    iter->set_expanded_silent (false);
   }
 }
 
 void
 LayerControlPanel::group_expanded (const QModelIndex &index)
 {
-  lay::LayerPropertiesConstIterator iter = mp_model->iterator (index);
+  auto iter = mp_model->iterator_nc (index);
   if (! iter.is_null () && ! iter.at_end ()) {
-    m_expanded.insert (iter->id ());
+    iter->set_expanded_silent (true);
   }
 }
 
@@ -2052,33 +2061,16 @@ LayerControlPanel::restore_expanded ()
 {
   mp_layer_list->blockSignals (true);
 
-#if 1
-  //  By keeping m_expanded, we can preserve the expansion state of different tabs.
-  //  However we will always continue filling m_expanded.
   lay::LayerPropertiesConstIterator l = mp_view->begin_layers ();
   while (! l.at_end ()) {
-    if (m_expanded.find (l->id ()) != m_expanded.end ()) {
-      QModelIndex index = mp_model->index (l, 0);
+    QModelIndex index = mp_model->index (l, 0);
+    if (l->expanded ()) {
       mp_layer_list->expand (index);
+    } else {
+      mp_layer_list->collapse (index);
     }
     ++l;
   }
-#else
-  //  this solution will forget the other tab's expansion flags.
-  std::set<unsigned int> new_expanded;
-
-  lay::LayerPropertiesConstIterator l = mp_view->begin_layers ();
-  while (! l.at_end ()) {
-    if (m_expanded.find (l->id ()) != m_expanded.end ()) {
-      new_expanded.insert (l->id ());
-      QModelIndex index = mp_model->index (l, 0);
-      mp_layer_list->expand (index);
-    }
-    ++l;
-  }
-
-  m_expanded.swap (new_expanded);
-#endif
 
   mp_layer_list->blockSignals (false);
 }
@@ -2287,7 +2279,7 @@ public:
 
     menu_entries.push_back (lay::menu_item ("cm_lv_select_all", "select_all", at, tl::to_string (QObject::tr ("Select All"))));
     //  It is not sure, whether "expandAll" destabilizes the tree widget:
-    //  menu_entries.push_back (lay::menu_item ("cm_lv_expand_all", "expand_all", at, tl::to_string (QObject::tr ("Expand All")));
+    //  menu_entries.push_back (lay::menu_item ("cm_lv_expand_all", "expand_all", at, tl::to_string (QObject::tr ("Expand All"))));
     menu_entries.push_back (lay::separator ("tab_group", at));
     menu_entries.push_back (lay::submenu ("tab_menu", at, tl::to_string (QObject::tr ("Tabs"))));
 

src/layui/layui/layLayerControlPanel.h

@@ -352,6 +352,7 @@ private:
   LayerTreeModel *mp_model;
   lay::LayoutViewBase *mp_view;
   bool m_needs_update;
+  bool m_expanded_state_needs_update;
   bool m_tabs_need_update;
   bool m_hidden_flags_need_update;
   bool m_in_update;
@@ -360,7 +361,6 @@ private:
   int m_oversampling;
   bool m_hrm;
   tl::DeferredMethod<LayerControlPanel> m_do_update_content_dm;
-  std::set<unsigned int> m_expanded;
   bool m_no_stipples;
   QLabel *m_no_stipples_label;
   lay::DecoratedLineEdit *mp_search_edit_box;

src/layui/layui/layLayerTreeModel.cc

@@ -756,7 +756,20 @@ LayerTreeModel::iterator (const QModelIndex &index) const
   return lay::LayerPropertiesConstIterator ();
 }
 
-QModelIndex 
+lay::LayerPropertiesIterator
+LayerTreeModel::iterator_nc (const QModelIndex &index)
+{
+  if (index.isValid ()) {
+    size_t iter_index = size_t (index.internalPointer ());
+    if (mp_view->layer_lists () > 0 && iter_index >= m_id_start && iter_index < m_id_end) {
+      return lay::LayerPropertiesIterator (mp_view->get_properties (), iter_index - m_id_start);
+    }
+  }
+
+  return lay::LayerPropertiesIterator ();
+}
+
+QModelIndex
 LayerTreeModel::index (lay::LayerPropertiesConstIterator iter, int column) const
 {
   try {

src/layui/layui/layLayerTreeModel.h

@@ -51,6 +51,7 @@ namespace lay
 
 class LayoutViewBase;
 class LayerPropertiesConstIterator;
+class LayerPropertiesIterator;
 
 /**
  *  @brief A helper class implementing a cache for the "test shapes in view" feature
@@ -107,6 +108,16 @@ public:
   virtual QModelIndex index (int row, int column, const QModelIndex &parent) const;
   virtual QModelIndex parent (const QModelIndex &index) const;
 
+  /**
+   *  @brief Sets the expanded state for a given model index
+   */
+  void set_expanded (const QModelIndex &index, bool ex);
+
+  /**
+   *  @brief Gets the expanded state for a given model index
+   */
+  bool expanded (const QModelIndex &index) const;
+
   /**
    *  @brief Provides an icon for a given layer style
    */
@@ -127,6 +138,11 @@ public:
    */
   lay::LayerPropertiesConstIterator iterator (const QModelIndex &index) const;
 
+  /**
+   *  @brief Converts a QModelIndex to an iterator (non-const)
+   */
+  lay::LayerPropertiesIterator iterator_nc (const QModelIndex &index);
+
   /**
    *  @brief Gets a flag indicating that an entry is hidden
    */

testdata/ruby/layLayers.rb

@@ -667,12 +667,18 @@ class LAYLayers_TestClass < TestBase
     p.clear_line_style
     assert_equal( p.has_line_style?, false )
 
+    assert_equal( p.is_expanded?, false )
+    p.expanded = true
+    assert_equal( p.is_expanded?, true )
+
     pp = RBA::LayerPropertiesNode::new
     assert_equal( pp == p, false )
     assert_equal( pp != p, true )
+    assert_equal( pp.is_expanded?, false )
     pp = p.dup
     assert_equal( pp == p, true )
     assert_equal( pp != p, false )
+    assert_equal( pp.is_expanded?, true )
 
   end