WIP: library browser - cleanup of unused cells in lib browser, some bug fixed, enhancements to parameter editor on drop

src/db/db/dbLayout.cc

@@ -1186,7 +1186,7 @@ Layout::allocate_new_cell ()
 }
 
 void
-Layout::cleanup ()
+Layout::cleanup (const std::set<db::cell_index_type> &keep)
 {
   //  deleting cells may create new top cells which need to be deleted as well, hence we iterate
   //  until there are no more cells to delete
@@ -1200,6 +1200,10 @@ Layout::cleanup ()
       }
     }
 
+    for (std::set<db::cell_index_type>::const_iterator k = keep.begin (); k != keep.end (); ++k) {
+      cells_to_delete.erase (*k);
+    }
+
     if (cells_to_delete.empty ()) {
       break;
     }

src/db/db/dbLayout.h

@@ -45,6 +45,7 @@
 
 #include <cstring>
 #include <map>
+#include <set>
 #include <string>
 #include <list>
 #include <vector>
@@ -1442,9 +1443,10 @@ public:
   /**
    *  @brief Cleans up the layout
    *
-   *  This method removes proxy objects which are no longer used
+   *  This method removes proxy objects which are no longer used.
+   *  It can be given a list of cells which need to be kept.
    */
-  void cleanup ();
+  void cleanup (const std::set<db::cell_index_type> &keep = std::set<db::cell_index_type> ());
 
   /**
    *  @brief Implementation of the undo operations

src/db/db/dbLibrary.cc

@@ -76,6 +76,33 @@ Library::unregister_proxy (db::LibraryProxy *lib_proxy, db::Layout *ly)
   }
 }
 
+void
+Library::retire_proxy (db::LibraryProxy *lib_proxy)
+{
+  m_retired_count.insert (std::make_pair (lib_proxy->library_cell_index (), 0)).first->second += 1;
+  retired_state_changed_event ();
+}
+
+void
+Library::unretire_proxy (db::LibraryProxy *lib_proxy)
+{
+  std::map<db::cell_index_type, int>::iterator c = m_retired_count.find (lib_proxy->library_cell_index ());
+  if (c != m_retired_count.end ()) {
+    if (! --c->second) {
+      m_retired_count.erase (c);
+    }
+    retired_state_changed_event ();
+  }
+}
+
+bool
+Library::is_retired (const db::cell_index_type library_cell_index) const
+{
+  std::map<db::cell_index_type, int>::const_iterator i = m_refcount.find (library_cell_index);
+  std::map<db::cell_index_type, int>::const_iterator j = m_retired_count.find (library_cell_index);
+  return (i != m_refcount.end () && j != m_retired_count.end () && i->second == j->second);
+}
+
 void
 Library::remap_to (db::Library *other)
 {

src/db/db/dbLibrary.h

@@ -163,6 +163,26 @@ public:
    */
   void unregister_proxy (db::LibraryProxy *lib_proxy, db::Layout *layout);
 
+  /**
+   *  @brief Retires a LibraryProxy in the given layout
+   *
+   *  A proxy becomes entirely retired if the refcount is equal to the
+   *  retired count. This feature is used to decide whether a proxy
+   *  is actually used or only present as a shadow object for the transaction
+   *  management.
+   */
+  void retire_proxy (db::LibraryProxy *lib_proxy);
+
+  /**
+   *  @brief Unretires the Library proxy
+   */
+  void unretire_proxy (db::LibraryProxy *lib_proxy);
+
+  /**
+   *  @brief Gets a value indicating whether a proxy is entirely retired
+   */
+  bool is_retired (const cell_index_type library_cell_index) const;
+
   /**
    *  @brief Remap the library proxies to a different library
    *
@@ -170,6 +190,11 @@ public:
    */
   void remap_to (db::Library *other);
 
+  /**
+   *  @brief This event is fired if proxies get retired on unretired
+   */
+  tl::Event retired_state_changed_event;
+
 private:
   std::string m_name;
   std::string m_description;
@@ -177,7 +202,7 @@ private:
   lib_id_type m_id;
   db::Layout m_layout;
   std::map<db::Layout *, int> m_referrers;
-  std::map<db::cell_index_type, int> m_refcount;
+  std::map<db::cell_index_type, int> m_refcount, m_retired_count;
 
   // no copying.
   Library &operator=(const Library &);

src/db/db/dbLibraryProxy.cc

@@ -59,6 +59,12 @@ LibraryProxy::unregister ()
   if (layout ()) {
     layout ()->unregister_lib_proxy (this);
   }
+  if (db::LibraryManager::initialized ()) {
+    db::Library *lib = db::LibraryManager::instance ().lib (m_lib_id);
+    if (lib) {
+      lib->retire_proxy (this);
+    }
+  }
 }
 
 void 
@@ -67,6 +73,12 @@ LibraryProxy::reregister ()
   if (layout ()) {
     layout ()->register_lib_proxy (this);
   }
+  if (db::LibraryManager::initialized ()) {
+    db::Library *lib = db::LibraryManager::instance ().lib (m_lib_id);
+    if (lib) {
+      lib->unretire_proxy (this);
+    }
+  }
 }
 
 void 

src/db/db/dbManager.h

@@ -262,10 +262,18 @@ class DB_PUBLIC Transaction
 {
 public:
   Transaction (db::Manager *manager, const std::string &desc)
-    : mp_manager (manager)
+    : mp_manager (manager), m_transaction_id (0)
   {
     if (mp_manager) {
-      mp_manager->transaction (desc);
+      m_transaction_id = mp_manager->transaction (desc);
+    }
+  }
+
+  Transaction (db::Manager *manager, const std::string &desc, db::Manager::transaction_id_t join_with)
+    : mp_manager (manager), m_transaction_id (0)
+  {
+    if (mp_manager) {
+      m_transaction_id = mp_manager->transaction (desc, join_with);
     }
   }
 
@@ -277,8 +285,14 @@ public:
     }
   }
 
+  db::Manager::transaction_id_t id () const
+  {
+    return m_transaction_id;
+  }
+
 private:
   db::Manager *mp_manager;
+  db::Manager::transaction_id_t m_transaction_id;
 
   //  no copying.
   Transaction (const Transaction &);

src/db/db/dbPCellDeclaration.cc

@@ -111,5 +111,22 @@ PCellDeclaration::map_parameters (const std::map<std::string, tl::Variant> &para
   return new_param;
 }
 
+std::map<std::string, tl::Variant>
+PCellDeclaration::named_parameters (const pcell_parameters_type &pv) const
+{
+  std::map<std::string, tl::Variant> np;
+
+  const std::vector<db::PCellParameterDeclaration> &pcp = parameter_declarations ();
+  for (std::vector<PCellParameterDeclaration>::const_iterator pd = pcp.begin (); pd != pcp.end (); ++pd) {
+    size_t index = pd - pcp.begin ();
+    if (index >= pv.size ()) {
+      break;
+    }
+    np.insert (std::make_pair (pd->get_name (), pv [index]));
+  }
+
+  return np;
+}
+
 }
 

src/db/db/dbPCellDeclaration.h

@@ -496,6 +496,11 @@ public:
    */
   pcell_parameters_type map_parameters (const std::map<std::string, tl::Variant> &named_parameters) const;
 
+  /**
+   *  @brief Converts a parameter vector to named parameters
+   */
+  std::map<std::string, tl::Variant> named_parameters (const pcell_parameters_type &pv) const;
+
 protected:
   /**
    *  @brief Gets a value indicating whether the PCell wants caching of the parameter declarations

src/edt/edt/PCellParametersDialog.ui

@@ -0,0 +1,73 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ui version="4.0">
+ <class>PCellParametersDialog</class>
+ <widget class="QDialog" name="PCellParametersDialog">
+  <property name="geometry">
+   <rect>
+    <x>0</x>
+    <y>0</y>
+    <width>469</width>
+    <height>429</height>
+   </rect>
+  </property>
+  <property name="windowTitle">
+   <string>Instantiation Path</string>
+  </property>
+  <layout class="QVBoxLayout" name="verticalLayout">
+   <item>
+    <widget class="edt::PCellParametersPage" name="parameters" native="true"/>
+   </item>
+   <item>
+    <widget class="QDialogButtonBox" name="buttons">
+     <property name="standardButtons">
+      <set>QDialogButtonBox::Apply|QDialogButtonBox::Cancel|QDialogButtonBox::Ok</set>
+     </property>
+    </widget>
+   </item>
+  </layout>
+ </widget>
+ <layoutdefault spacing="6" margin="11"/>
+ <customwidgets>
+  <customwidget>
+   <class>edt::PCellParametersPage</class>
+   <extends>QWidget</extends>
+   <header>edtPCellParametersPage.h</header>
+   <container>1</container>
+  </customwidget>
+ </customwidgets>
+ <resources/>
+ <connections>
+  <connection>
+   <sender>buttons</sender>
+   <signal>rejected()</signal>
+   <receiver>PCellParametersDialog</receiver>
+   <slot>reject()</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>321</x>
+     <y>405</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>337</x>
+     <y>423</y>
+    </hint>
+   </hints>
+  </connection>
+  <connection>
+   <sender>buttons</sender>
+   <signal>accepted()</signal>
+   <receiver>PCellParametersDialog</receiver>
+   <slot>accept()</slot>
+   <hints>
+    <hint type="sourcelabel">
+     <x>427</x>
+     <y>405</y>
+    </hint>
+    <hint type="destinationlabel">
+     <x>443</x>
+     <y>425</y>
+    </hint>
+   </hints>
+  </connection>
+ </connections>
+</ui>

src/edt/edt/edt.pro

@@ -20,7 +20,8 @@ HEADERS = \
   edtService.h \
   edtServiceImpl.h \
   edtUtils.h \
-    edtCommon.h
+    edtCommon.h \
+    edtPCellParametersDialog.h
 
 FORMS = \
   AlignOptionsDialog.ui \
@@ -41,6 +42,7 @@ FORMS = \
   PolygonPropertiesPage.ui \
   RoundCornerOptionsDialog.ui \
   TextPropertiesPage.ui \
+    PCellParametersDialog.ui
 
 SOURCES = \
   edtConfig.cc \
@@ -57,6 +59,7 @@ SOURCES = \
   edtServiceImpl.cc \
   edtUtils.cc \
   gsiDeclEdt.cc \
+    edtPCellParametersDialog.cc
 
 INCLUDEPATH += $$TL_INC $$GSI_INC $$LAYBASIC_INC $$DB_INC
 DEPENDPATH += $$TL_INC $$GSI_INC $$LAYBASIC_INC $$DB_INC

src/edt/edt/edtEditorOptionsPages.cc

@@ -28,6 +28,7 @@
 #include "edtEditorOptionsPages.h"
 #include "edtPCellParametersPage.h"
 #include "edtConfig.h"
+#include "edtService.h"
 #include "tlExceptions.h"
 #include "layPlugin.h"
 #include "layLayoutView.h"
@@ -643,15 +644,7 @@ EditorOptionsInst::apply (lay::Plugin *root)
     if (pc.first) {
       const db::PCellDeclaration *pc_decl = layout->pcell_declaration (pc.second);
       if (pc_decl) {
-        param += "!";  //  flags PCells
+        param = pcell_parameters_to_string (pc_decl->named_parameters (mp_pcell_parameters->get_parameters ()));
-        std::vector<tl::Variant> pv = mp_pcell_parameters->get_parameters ();
-        const std::vector<db::PCellParameterDeclaration> &pcp = pc_decl->parameter_declarations ();
-        for (size_t i = 0; i < std::min (pv.size (), pcp.size ()); ++i) {
-          param += tl::to_word_or_quoted_string (pcp [i].get_name ());
-          param += ":";
-          param += pv [i].to_parsable_string ();
-          param += ";";
-        }
       }
     }
   }

src/edt/edt/edtPCellParametersDialog.cc

@@ -0,0 +1,65 @@
+
+/*
+
+  KLayout Layout Viewer
+  Copyright (C) 2006-2019 Matthias Koefferlein
+
+  This program is free software; you can redistribute it and/or modify
+  it under the terms of the GNU General Public License as published by
+  the Free Software Foundation; either version 2 of the License, or
+  (at your option) any later version.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+
+*/
+
+
+#include "edtPCellParametersDialog.h"
+
+#include <QPushButton>
+
+namespace edt
+{
+
+PCellParametersDialog::PCellParametersDialog (QWidget *parent)
+  : QDialog (parent)
+{
+  setupUi (this);
+
+  connect (buttons->button (QDialogButtonBox::Apply), SIGNAL (clicked ()), this, SLOT (apply_pressed ()));
+}
+
+void
+PCellParametersDialog::apply_pressed ()
+{
+  emit parameters_changed ();
+  parameters_changed_event ();
+}
+
+std::vector<tl::Variant> 
+PCellParametersDialog::get_parameters () 
+{
+  return parameters->get_parameters ();
+}
+
+void 
+PCellParametersDialog::set_parameters (const std::vector<tl::Variant> &p)
+{
+  parameters->set_parameters (p);
+}
+
+int
+PCellParametersDialog::exec (const db::Layout *layout, lay::LayoutView *view, int cv_index, const db::PCellDeclaration *pcell_decl, const db::pcell_parameters_type &p)
+{
+  parameters->setup (layout, view, cv_index, pcell_decl, p);
+  return QDialog::exec ();
+}
+
+}

src/edt/edt/edtPCellParametersDialog.h

@@ -0,0 +1,86 @@
+
+/*
+
+  KLayout Layout Viewer
+  Copyright (C) 2006-2019 Matthias Koefferlein
+
+  This program is free software; you can redistribute it and/or modify
+  it under the terms of the GNU General Public License as published by
+  the Free Software Foundation; either version 2 of the License, or
+  (at your option) any later version.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+
+*/
+
+
+#ifndef HDR_edtPCellParametersDialog
+#define HDR_edtPCellParametersDialog
+
+#include "dbPCellDeclaration.h"
+#include "ui_PCellParametersDialog.h"
+
+#include <QDialog>
+
+namespace lay
+{
+  class LayoutView;
+}
+
+namespace edt
+{
+
+/**
+ *  @brief A QScrollArea that displays and allows editing PCell parameters
+ */
+class PCellParametersDialog
+  : public QDialog, private Ui::PCellParametersDialog
+{
+Q_OBJECT
+
+public:
+  /**
+   *  @brief Constructor: create a dialog showing the given parameters
+   *  @param parent The parent widget
+   */
+  PCellParametersDialog (QWidget *parent);
+
+  /**
+   *  @brief Executes the parameter dialog
+   *  @param layout The layout in which the PCell instance resides
+   *  @param view The layout view from which to take layers for example
+   *  @param cv_index The index of the cellview in "view"
+   *  @param pcell_decl The PCell declaration
+   *  @param parameters The parameter values to show (if empty, the default values are used)
+   */
+  int exec (const db::Layout *layout, lay::LayoutView *view, int cv_index, const db::PCellDeclaration *pcell_decl, const db::pcell_parameters_type &p);
+
+  /**
+   *  @brief Get the current parameters
+   */
+  std::vector<tl::Variant> get_parameters ();
+
+  /**
+   *  @brief Sets the given parameters as values
+   */
+  void set_parameters (const  std::vector<tl::Variant> &values);
+
+  tl::Event parameters_changed_event;
+
+signals:
+  void parameters_changed ();
+
+private slots:
+  void apply_pressed ();
+};
+
+}
+
+#endif

src/edt/edt/edtPCellParametersPage.cc

@@ -129,15 +129,30 @@ static void set_value (const db::PCellParameterDeclaration &p, const db::Layout
 }
 
 PCellParametersPage::PCellParametersPage (QWidget *parent, const db::Layout *layout, lay::LayoutView *view, int cv_index, const db::PCellDeclaration *pcell_decl, const db::pcell_parameters_type &parameters)
-  : QFrame (parent), mp_pcell_decl (pcell_decl), mp_layout (layout), mp_view (view), m_cv_index (cv_index)
+  : QFrame (parent)
 {
+  init ();
+  setup (layout, view, cv_index, pcell_decl, parameters);
+}
+
+PCellParametersPage::PCellParametersPage (QWidget *parent)
+  : QFrame (parent)
+{
+  init ();
+}
+
+void
+PCellParametersPage::init ()
+{
+  mp_pcell_decl = 0;
+  mp_layout = 0;
+  mp_view = 0;
+  m_cv_index = 0;
+  mp_parameters_area = 0;
+
   QGridLayout *frame_layout = new QGridLayout (this);
   setLayout (frame_layout);
 
-  mp_parameters_area = new QScrollArea (this);
-  frame_layout->addWidget (mp_parameters_area, 0, 0, 1, 2);
-  frame_layout->setRowStretch (0, 1);
-
   mp_error_icon = new QLabel (this);
   mp_error_icon->setPixmap (QPixmap (":/warn.png"));
   mp_error_icon->hide ();
@@ -153,9 +168,28 @@ PCellParametersPage::PCellParametersPage (QWidget *parent, const db::Layout *lay
   mp_error_label->hide ();
   frame_layout->addWidget (mp_error_label, 1, 1, 1, 1);
   frame_layout->setColumnStretch (1, 1);
+}
 
+void
+PCellParametersPage::setup (const db::Layout *layout, lay::LayoutView *view, int cv_index, const db::PCellDeclaration *pcell_decl, const db::pcell_parameters_type &parameters)
+{
+  mp_pcell_decl = pcell_decl;
+  mp_layout = layout;
+  mp_view = view;
+  m_cv_index = cv_index;
   m_parameters = parameters;
 
+  if (mp_parameters_area) {
+    delete mp_parameters_area;
+  }
+
+  m_widgets.clear ();
+
+  mp_parameters_area = new QScrollArea (this);
+  QGridLayout *frame_layout = dynamic_cast<QGridLayout *> (QFrame::layout ());
+  frame_layout->addWidget (mp_parameters_area, 0, 0, 1, 2);
+  frame_layout->setRowStretch (0, 1);
+
   QFrame *fi = new QFrame (mp_parameters_area);
   QWidget *inner_frame = fi;
   fi->setFrameShape (QFrame::NoFrame);

src/edt/edt/edtPCellParametersPage.h

@@ -57,7 +57,7 @@ public:
   };
 
   /**
-   *  @brief Constructor: create a page showing the given parameters
+   *  @brief Constructor: creates a page showing the given parameters
    *
    *  @param parent The parent widget
    *  @param layout The layout in which the PCell instance resides
@@ -68,6 +68,20 @@ public:
    */
   PCellParametersPage (QWidget *parent, const db::Layout *layout, lay::LayoutView *view, int cv_index, const db::PCellDeclaration *pcell_decl, const db::pcell_parameters_type &parameters);
 
+  /**
+   *  @brief Default constructor
+   *
+   *  Use "setup" to configure the page.
+   */
+  PCellParametersPage (QWidget *parent);
+
+  /**
+   *  @brief Delayed initialization
+   *
+   *  Use this method to setup when the arguments are not available in the constructor
+   */
+  void setup (const db::Layout *layout, lay::LayoutView *view, int cv_index, const db::PCellDeclaration *pcell_decl, const db::pcell_parameters_type &parameters);
+
   /**
    *  @brief Gets the pages current state
    */
@@ -110,6 +124,8 @@ private:
   lay::LayoutView *mp_view;
   int m_cv_index;
   db::pcell_parameters_type m_parameters;
+
+  void init ();
 };
 
 }

src/edt/edt/edtService.cc

@@ -60,6 +60,45 @@ ac_from_buttons (unsigned int buttons)
 
 // -------------------------------------------------------------
 
+std::string pcell_parameters_to_string (const std::map<std::string, tl::Variant> &parameters)
+{
+  std::string param;
+
+  param = "!";  //  flags PCells
+  for (std::map<std::string, tl::Variant>::const_iterator p = parameters.begin (); p != parameters.end (); ++p) {
+    param += tl::to_word_or_quoted_string (p->first);
+    param += ":";
+    param += p->second.to_parsable_string ();
+    param += ";";
+  }
+
+  return param;
+}
+
+std::map<std::string, tl::Variant> pcell_parameters_from_string (const std::string &s)
+{
+  tl::Extractor ex (s.c_str ());
+  std::map<std::string, tl::Variant> pm;
+
+  ex.test ("!");
+
+  try {
+    while (! ex.at_end ()) {
+      std::string n;
+      ex.read_word_or_quoted (n);
+      ex.test (":");
+      ex.read (pm.insert (std::make_pair (n, tl::Variant ())).first->second);
+      ex.test (";");
+    }
+  } catch (...) {
+    //  ignore errors
+  }
+
+  return pm;
+}
+
+// -------------------------------------------------------------
+
 Service::Service (db::Manager *manager, lay::LayoutView *view, db::ShapeIterator::flags_type flags)
   : lay::ViewService (view->view_object_widget ()), 
     lay::Editable (view),

src/edt/edt/edtService.h

@@ -57,6 +57,18 @@ extern lay::angle_constraint_type ac_from_buttons (unsigned int buttons);
 
 // -------------------------------------------------------------
 
+/**
+ *  @brief Utility function: serialize PCell parameters into a string
+ */
+std::string pcell_parameters_to_string (const std::map<std::string, tl::Variant> &parameters);
+
+/**
+ *  @brief Utility: deserialize PCell parameters from a string
+ */
+std::map<std::string, tl::Variant> pcell_parameters_from_string (const std::string &s);
+
+// -------------------------------------------------------------
+
 class EDT_PUBLIC Service
   : public lay::ViewService,
     public lay::Editable,

src/edt/edt/edtServiceImpl.cc

@@ -25,6 +25,8 @@
 #include "edtServiceImpl.h"
 #include "edtPropertiesPages.h"
 #include "edtInstPropertiesPage.h"
+#include "edtPCellParametersDialog.h"
+#include "edtService.h"
 #include "dbEdge.h"
 #include "dbLibrary.h"
 #include "dbLibraryManager.h"
@@ -1112,7 +1114,7 @@ InstService::InstService (db::Manager *manager, lay::LayoutView *view)
     m_row_x (0.0), m_row_y (0.0), m_column_x (0.0), m_column_y (0.0),
     m_place_origin (false), m_reference_transaction_id (0),
     m_needs_update (true), m_has_valid_cell (false), m_in_drag_drop (false), 
-    m_current_cell (0), m_cv_index (-1)
+    m_current_cell (0), mp_current_layout (0), mp_pcell_decl (0), m_cv_index (-1)
 { 
   //  .. nothing yet ..
 }
@@ -1166,13 +1168,14 @@ InstService::drag_enter_event (const db::DPoint &p, const lay::DragDropDataBase
     m_in_drag_drop = true;
 
     if (cd->library ()) {
+      if (m_lib_name != cd->library ()->get_name ()) {
         m_lib_name = cd->library ()->get_name ();
+        m_pcell_parameters.clear ();
+      }
     } else {
       m_lib_name.clear ();
     }
 
-    m_pcell_parameters.clear ();
-    m_cell_or_pcell_name.clear ();
     m_is_pcell = false;
 
     if (cd->is_pcell ()) {
@@ -1180,12 +1183,17 @@ InstService::drag_enter_event (const db::DPoint &p, const lay::DragDropDataBase
       const db::PCellDeclaration *pcell_decl = cd->layout ()->pcell_declaration (cd->cell_index ());
       if (pcell_decl) {
 
+        if (m_cell_or_pcell_name != pcell_decl->name ()) {
           m_cell_or_pcell_name = pcell_decl->name ();
+          m_pcell_parameters.clear ();
+        }
+
         m_is_pcell = true;
 
+        //  NOTE: we reuse previous parameters for convenience unless PCell or library has changed
         const std::vector<db::PCellParameterDeclaration> &pd = pcell_decl->parameter_declarations();
         for (std::vector<db::PCellParameterDeclaration>::const_iterator i = pd.begin (); i != pd.end (); ++i) {
-          if (i->get_type () == db::PCellParameterDeclaration::t_layer && i->get_default ().is_nil ()) {
+          if (i->get_type () == db::PCellParameterDeclaration::t_layer && !i->is_hidden () && !i->is_readonly () && i->get_default ().is_nil ()) {
             m_pcell_parameters.insert (std::make_pair (i->get_name (), get_default_layer_for_pcell ()));
           } else {
             m_pcell_parameters.insert (std::make_pair (i->get_name (), i->get_default ()));
@@ -1240,9 +1248,59 @@ bool
 InstService::drop_event (const db::DPoint & /*p*/, const lay::DragDropDataBase * /*data*/) 
 { 
   if (m_in_drag_drop) {
+
+    const lay::CellView &cv = view ()->cellview (m_cv_index);
+    if (! cv.is_valid ()) {
+      return false;
+    }
+
+    make_cell (cv);
+
+    bool accepted = true;
+
+    if (m_has_valid_cell && mp_pcell_decl) {
+
+      std::vector<tl::Variant> pv = mp_pcell_decl->map_parameters (m_pcell_parameters);
+
+      //  Turn off the drag cursor for the modal dialog
+      QApplication::restoreOverrideCursor ();
+
+      //  for PCells dragged show the parameter dialog for a chance to edit the initial parameters
+      if (! mp_pcell_parameters_dialog.get ()) {
+        mp_pcell_parameters_dialog.reset (new edt::PCellParametersDialog (view ()));
+        mp_pcell_parameters_dialog->parameters_changed_event.add (this, &InstService::apply_edits);
+      }
+
+      if (! mp_pcell_parameters_dialog->exec (mp_current_layout, view (), m_cv_index, mp_pcell_decl, pv)) {
+        accepted = false;
+      } else {
+        m_has_valid_cell = false;
+        m_pcell_parameters = mp_pcell_decl->named_parameters (mp_pcell_parameters_dialog->get_parameters ());
+      }
+
+    }
+
     set_edit_marker (0);
+
+    if (accepted) {
       do_finish_edit ();
+    } else {
+      do_cancel_edit ();
+    }
+
+    //  push the current setup to configuration so the instance dialog will take these as default
+    //  and "apply" of these instance properties doesn't fail because of insistency.
+    plugin_root ()->config_set (cfg_edit_inst_lib_name, m_lib_name);
+    plugin_root ()->config_set (cfg_edit_inst_cell_name, m_cell_or_pcell_name);
+    if (m_is_pcell) {
+      plugin_root ()->config_set (cfg_edit_inst_pcell_parameters, pcell_parameters_to_string (m_pcell_parameters));
+    } else {
+      plugin_root ()->config_set (cfg_edit_inst_pcell_parameters, std::string ());
+    }
+    plugin_root ()->config_end ();
+
     return true;
+
   } else {
     return false; 
   }
@@ -1307,80 +1365,75 @@ InstService::make_cell (const lay::CellView &cv)
     return std::make_pair (true, m_current_cell);
   }
 
+  //  NOTE: do this at the beginning: creating a transaction might delete transactions behind the
+  //  head transaction, hence releasing (thus: deleting) cells. To prevert interference, create
+  //  the transaction at the beginning.
+  db::Transaction tr (manager (), tl::to_string (QObject::tr ("Create reference cell")), m_reference_transaction_id);
+  m_reference_transaction_id = tr.id ();
+
   lay::LayerState layer_state = view ()->layer_snapshot ();
 
-  db::Layout *layout;
   db::Library *lib = db::LibraryManager::instance ().lib_ptr_by_name (m_lib_name);
 
   //  find the layout the cell has to be looked up: that is either the layout of the current instance or 
   //  the library selected
   if (lib) {
-    layout = &lib->layout ();
+    mp_current_layout = &lib->layout ();
   } else {
-    layout = &cv->layout ();
+    mp_current_layout = &cv->layout ();
   }
 
   std::pair<bool, db::cell_index_type> ci (false, db::cell_index_type (0));
   std::pair<bool, db::pcell_id_type> pci (false, db::pcell_id_type (0));
 
   if (! m_is_pcell) {
-    ci = layout->cell_by_name (m_cell_or_pcell_name.c_str ());
+    ci = mp_current_layout->cell_by_name (m_cell_or_pcell_name.c_str ());
   } else {
-    pci = layout->pcell_by_name (m_cell_or_pcell_name.c_str ());
+    pci = mp_current_layout->pcell_by_name (m_cell_or_pcell_name.c_str ());
   }
 
   if (! ci.first && ! pci.first) {
     return std::pair<bool, db::cell_index_type> (false, 0);
   }
 
-  m_reference_transaction_id = manager ()->transaction (tl::to_string (QObject::tr ("Create reference cell")), m_reference_transaction_id); 
   db::cell_index_type inst_cell_index = ci.second;
 
-  try {
+  mp_pcell_decl = 0;
 
   //  instantiate the PCell
   if (pci.first) {
 
     std::vector<tl::Variant> pv;
 
-      const db::PCellDeclaration *pc_decl = layout->pcell_declaration (pci.second);
+    mp_pcell_decl = mp_current_layout->pcell_declaration (pci.second);
-      if (pc_decl) {
+    if (mp_pcell_decl) {
 
-        const std::vector<db::PCellParameterDeclaration> &pcp = pc_decl->parameter_declarations ();
+      pv = mp_pcell_decl->map_parameters (m_pcell_parameters);
-        for (std::vector<db::PCellParameterDeclaration>::const_iterator pd = pcp.begin (); pd != pcp.end (); ++pd) {
-          std::map<std::string, tl::Variant>::const_iterator p = m_pcell_parameters.find (pd->get_name ());
-          if (p != m_pcell_parameters.end ()) {
-            pv.push_back (p->second);
-          } else {
-            pv.push_back (pd->get_default ());
-          }
-        }
 
       //  make the parameters fit (i.e. PCells may not define consistent default parameters)
-        pc_decl->coerce_parameters (*layout, pv);
+      mp_pcell_decl->coerce_parameters (*mp_current_layout, pv);
 
     }
 
-      inst_cell_index = layout->get_pcell_variant (pci.second, pv);
+    inst_cell_index = mp_current_layout->get_pcell_variant (pci.second, pv);
 
   }
 
   //  reference the library
   if (lib) {
-      layout = & cv->layout ();
+
-      layout->cleanup ();
+    mp_current_layout = & cv->layout ();
-      inst_cell_index = layout->get_lib_proxy (lib, inst_cell_index);
+    inst_cell_index = mp_current_layout->get_lib_proxy (lib, inst_cell_index);
+
+    //  remove unused references
+    std::set<db::cell_index_type> keep;
+    keep.insert (inst_cell_index);
+    mp_current_layout->cleanup (keep);
+
   }
 
   view ()->add_new_layers (layer_state);
 
-    manager ()->commit ();
-
-  } catch (...) {
-    manager ()->commit ();
-    throw;
-  }
-
   m_has_valid_cell = true;
   m_current_cell = inst_cell_index;
 
@@ -1487,11 +1540,6 @@ InstService::do_finish_edit ()
     m_has_valid_cell = false;
     m_in_drag_drop = false;
 
-    //  on a preconfigured PCell offer to edit the properties now
-    if (m_is_pcell) {
-      view ()->show_properties (QApplication::activeWindow ());
-    }
-
   } catch (...) {
     m_has_valid_cell = false;
     m_in_drag_drop = false;
@@ -1535,22 +1583,8 @@ InstService::configure (const std::string &name, const std::string &value)
 
   if (name == cfg_edit_inst_pcell_parameters) {
 
-    tl::Extractor ex (value.c_str ());
+    m_pcell_parameters = pcell_parameters_from_string (value);
-
+    m_is_pcell = ! value.empty ();
-    m_pcell_parameters.clear ();
-    m_is_pcell = ex.test ("!") || ! ex.at_end ();
-
-    try {
-      while (! ex.at_end ()) {
-        std::string n;
-        ex.read_word_or_quoted (n);
-        ex.test (":");
-        ex.read (m_pcell_parameters.insert (std::make_pair (n, tl::Variant ())).first->second);
-        ex.test (";");
-      }
-    } catch (...) {
-      //  ignore errors
-    }
 
     m_needs_update = true;
     return true; // taken
@@ -1638,6 +1672,17 @@ InstService::config_finalize ()
   edt::Service::config_finalize ();
 }
 
+void
+InstService::apply_edits()
+{
+  if (mp_pcell_decl && mp_pcell_parameters_dialog.get ()) {
+    m_pcell_parameters = mp_pcell_decl->named_parameters (mp_pcell_parameters_dialog->get_parameters ());
+  }
+
+  m_has_valid_cell = false;
+  update_marker ();
+}
+
 void
 InstService::update_marker ()
 {

src/edt/edt/edtServiceImpl.h

@@ -27,6 +27,8 @@
 #include "edtService.h"
 #include "edtConfig.h"
 
+#include <memory>
+
 namespace lay
 {
   class CellView;
@@ -35,6 +37,8 @@ namespace lay
 namespace edt
 {
 
+class PCellParametersDialog;
+
 /**
  *  @brief Implementation of the edt::Service for generic shape editing
  */
@@ -235,10 +239,14 @@ private:
   bool m_has_valid_cell;
   bool m_in_drag_drop;
   db::cell_index_type m_current_cell;
+  db::Layout *mp_current_layout;
+  const db::PCellDeclaration *mp_pcell_decl;
   int m_cv_index;
   db::ICplxTrans m_trans;
+  std::auto_ptr<edt::PCellParametersDialog> mp_pcell_parameters_dialog;
 
   void update_marker ();
+  void apply_edits ();
   bool get_inst (db::CellInstArray &inst);
   std::pair<bool, db::cell_index_type> make_cell (const lay::CellView &cv);
   tl::Variant get_default_layer_for_pcell ();

src/laybasic/laybasic/layCellTreeModel.cc

@@ -26,6 +26,7 @@
 #include "tlGlobPattern.h"
 #include "dbPCellHeader.h"
 #include "dbPCellVariant.h"
+#include "dbLibraryProxy.h"
 #include "dbLibrary.h"
 
 #include <QTreeView>
@@ -559,8 +560,13 @@ CellTreeModel::build_top_level ()
 
           const db::PCellHeader *pcell_header = mp_layout->pcell_header (pc->second);
           for (db::PCellHeader::variant_iterator v = pcell_header->begin (); v != pcell_header->end (); ++v) {
+            if (mp_library && mp_library->is_retired (v->second->cell_index ())) {
+              //  skip retired cells - this means we won't show variants which are just kept
+              //  as shadow variants for the transactions.
+            } else {
               item->add_child (new CellTreeItem (mp_layout, false, v->second->cell_index (), true, m_sorting));
             }
+          }
 
           item->finish_children ();
 

src/laybasic/laybasic/layLibrariesView.cc

@@ -716,6 +716,7 @@ LibrariesView::do_update_content (int lib_index)
   for (db::LibraryManager::iterator lib = db::LibraryManager::instance ().begin (); lib != db::LibraryManager::instance ().end (); ++lib) {
     libraries.push_back (db::LibraryManager::instance ().lib (lib->second));
     libraries.back ()->layout ().hier_changed_event.add (this, &LibrariesView::update_required);
+    libraries.back ()->retired_state_changed_event.add (this, &LibrariesView::update_required);
   }
 
   for (size_t i = imin; i < libraries.size () && i <= imax; ++i) {