First attempt to implement a solution for issue #2016

The implementation will not update the PCell on
property sheet edits of the guiding shape
if lazy evaluation is requested. Still, changes
are committed to the PCell on committing the
property page.

src/ant/ant/antPropertiesPage.cc

@@ -517,7 +517,7 @@ PropertiesPage::readonly ()
 }
 
 void 
-PropertiesPage::apply ()
+PropertiesPage::apply (bool /*commit*/)
 {
   ant::Object obj;
   get_object (obj);

src/ant/ant/antPropertiesPage.h

@@ -50,7 +50,7 @@ public:
   virtual void update ();
   virtual void leave ();
   virtual bool readonly ();
-  virtual void apply ();
+  virtual void apply (bool commit);
 
 private slots:
   void swap_points_clicked ();

src/edt/edt/edtInstPropertiesPage.cc

@@ -925,7 +925,7 @@ InstPropertiesPage::do_apply (bool current_only, bool relative)
 }
 
 void 
-InstPropertiesPage::apply ()
+InstPropertiesPage::apply (bool /*commit*/)
 {
   do_apply (true, false);
 }
@@ -937,7 +937,7 @@ InstPropertiesPage::can_apply_to_all () const
 }
 
 void 
-InstPropertiesPage::apply_to_all (bool relative)
+InstPropertiesPage::apply_to_all (bool relative, bool /*commit*/)
 {
   do_apply (false, relative);
 }

src/edt/edt/edtInstPropertiesPage.h

@@ -66,8 +66,8 @@ protected:
   edt::PCellParametersPage *mp_pcell_parameters;
 
   virtual bool readonly ();
-  virtual void apply (); 
-  virtual void apply_to_all (bool relative);
+  virtual void apply (bool commit);
+  virtual void apply_to_all (bool relative, bool commit);
   virtual bool can_apply_to_all () const;
   void do_apply (bool current_only, bool relative);
   virtual ChangeApplicator *create_applicator (db::Cell &cell, const db::Instance &inst, double dbu);

src/edt/edt/edtPropertiesPages.cc

@@ -215,7 +215,7 @@ ShapePropertiesPage::recompute_selection_ptrs (const std::vector<lay::ObjectInst
 }
 
 void 
-ShapePropertiesPage::do_apply (bool current_only, bool relative)
+ShapePropertiesPage::do_apply (bool current_only, bool relative, bool commit)
 {
   if (m_indexes.empty ()) {
     return;
@@ -321,7 +321,7 @@ ShapePropertiesPage::do_apply (bool current_only, bool relative)
       }
 
       //  handle the case of guiding shape updates
-      std::pair<bool, lay::ObjectInstPath> gs = mp_service->handle_guiding_shape_changes (new_sel[index]);
+      std::pair<bool, lay::ObjectInstPath> gs = mp_service->handle_guiding_shape_changes (new_sel[index], commit);
       if (gs.first) {
 
         new_sel[index] = gs.second;
@@ -350,9 +350,9 @@ ShapePropertiesPage::do_apply (bool current_only, bool relative)
 }
 
 void 
-ShapePropertiesPage::apply ()
+ShapePropertiesPage::apply (bool commit)
 {
-  do_apply (true, false);
+  do_apply (true, false, commit);
 }
 
 bool
@@ -362,9 +362,9 @@ ShapePropertiesPage::can_apply_to_all () const
 }
 
 void 
-ShapePropertiesPage::apply_to_all (bool relative)
+ShapePropertiesPage::apply_to_all (bool relative, bool commit)
 {
-  do_apply (false, relative);
+  do_apply (false, relative, commit);
 }
 
 void 

src/edt/edt/edtPropertiesPages.h

@@ -63,10 +63,10 @@ protected:
 
 private:
   virtual void update ();
-  virtual void apply ();
-  virtual void apply_to_all (bool relative);
+  virtual void apply (bool commit);
+  virtual void apply_to_all (bool relative, bool commit);
   virtual bool can_apply_to_all () const;
-  virtual void do_apply (bool current_only, bool relative);
+  virtual void do_apply (bool current_only, bool relative, bool commit);
   void recompute_selection_ptrs (const std::vector<lay::ObjectInstPath> &new_sel);
 
 protected:

src/edt/edt/edtService.cc

@@ -76,6 +76,7 @@ Service::Service (db::Manager *manager, lay::LayoutViewBase *view, db::ShapeIter
     m_snap_to_objects (true),
     m_snap_objects_to_grid (true),
     m_top_level_sel (false), m_show_shapes_of_instances (true), m_max_shapes_of_instances (1000),
+    m_pcell_lazy_evaluation (0),
     m_hier_copy_mode (-1),
     m_indicate_secondary_selection (false),
     m_seq (0),
@@ -391,6 +392,10 @@ Service::configure (const std::string &name, const std::string &value)
     tl::from_string (value, m_hier_copy_mode);
     service_configuration_changed ();
 
+  } else if (name == cfg_edit_pcell_lazy_eval_mode) {
+
+    tl::from_string (value, m_pcell_lazy_evaluation);
+
   } else {
     lay::EditorServiceBase::configure (name, value);
   }
@@ -598,7 +603,7 @@ Service::end_move (const db::DPoint & /*p*/, lay::angle_constraint_type ac)
     transform (db::DCplxTrans (m_move_trans));
     move_cancel (); // formally this functionality fits here
     //  accept changes to guiding shapes
-    handle_guiding_shape_changes ();
+    handle_guiding_shape_changes (true);
   }
   m_alt_ac = lay::AC_Global;
 }
@@ -846,7 +851,7 @@ Service::transform (const db::DCplxTrans &trans, const std::vector<db::DCplxTran
 
   }
 
-  handle_guiding_shape_changes ();
+  handle_guiding_shape_changes (true);
   selection_to_view ();
 }
 
@@ -1860,7 +1865,7 @@ Service::add_selection (const lay::ObjectInstPath &sel)
 }
 
 std::pair<bool, lay::ObjectInstPath>
-Service::handle_guiding_shape_changes (const lay::ObjectInstPath &obj) const
+Service::handle_guiding_shape_changes (const lay::ObjectInstPath &obj, bool commit) const
 {
   unsigned int cv_index = obj.cv_index ();
   lay::CellView cv = view ()->cellview (cv_index);
@@ -1874,10 +1879,22 @@ Service::handle_guiding_shape_changes (const lay::ObjectInstPath &obj) const
     return std::make_pair (false, lay::ObjectInstPath ());
   }
 
-  if (! layout->is_pcell_instance (obj.cell_index ()).first) {
+  auto pcell_decl = layout->pcell_declaration_for_pcell_variant (obj.cell_index ());
+  if (! pcell_decl) {
     return std::make_pair (false, lay::ObjectInstPath ());
   }
 
+  //  Don't update unless we're committing or not in lazy PCell update mode
+  if (! commit) {
+    if (m_pcell_lazy_evaluation < 0) {
+      if (pcell_decl->wants_lazy_evaluation ()) {
+        return std::make_pair (false, lay::ObjectInstPath ());
+      }
+    } else if (m_pcell_lazy_evaluation > 0) {
+      return std::make_pair (false, lay::ObjectInstPath ());
+    }
+  }
+
   db::cell_index_type top_cell = std::numeric_limits<db::cell_index_type>::max ();
   db::cell_index_type parent_cell = std::numeric_limits<db::cell_index_type>::max ();
   db::Instance parent_inst;
@@ -1944,7 +1961,7 @@ Service::handle_guiding_shape_changes (const lay::ObjectInstPath &obj) const
 }
 
 bool 
-Service::handle_guiding_shape_changes ()
+Service::handle_guiding_shape_changes (bool commit)
 {
   EditableSelectionIterator s = begin_selection ();
 
@@ -1953,7 +1970,7 @@ Service::handle_guiding_shape_changes ()
     return false;
   }
 
-  std::pair<bool, lay::ObjectInstPath> gs = handle_guiding_shape_changes (*s);
+  std::pair<bool, lay::ObjectInstPath> gs = handle_guiding_shape_changes (*s, commit);
   if (gs.first) {
 
     //  remove superfluous proxies

src/edt/edt/edtService.h

@@ -398,17 +398,19 @@ public:
    *
    *  @return true, if PCells have been updated, indicating that our selection is no longer valid
    *
+   *  @param commit If true, changes are "final" (and PCells are updated also in lazy evaluation mode)
+   *
    *  This version assumes there is only one guiding shape selected and will update the selection.
    *  It will also call layout.cleanup() if required.
    */
-  bool handle_guiding_shape_changes ();
+  bool handle_guiding_shape_changes (bool commit);
 
   /**
    *  @brief Handle changes in a specific guiding shape, i.e. create new PCell variants if required
    *
    *  @return A pair of bool (indicating that the object path has changed) and the new guiding shape path
    */
-  std::pair<bool, lay::ObjectInstPath> handle_guiding_shape_changes (const lay::ObjectInstPath &obj) const;
+  std::pair<bool, lay::ObjectInstPath> handle_guiding_shape_changes (const lay::ObjectInstPath &obj, bool commit) const;
 
   /**
    *  @brief Gets a value indicating whether a move operation is ongoing
@@ -672,9 +674,12 @@ private:
   bool m_snap_to_objects;
   bool m_snap_objects_to_grid;
   db::DVector m_global_grid;
+
+  //  Other attributes
   bool m_top_level_sel;
   bool m_show_shapes_of_instances;
   unsigned int m_max_shapes_of_instances;
+  int m_pcell_lazy_evaluation;
 
   //  Hierarchical copy mode (-1: ask, 0: shallow, 1: deep)
   int m_hier_copy_mode;

src/img/img/imgPropertiesPage.cc

@@ -784,7 +784,7 @@ PropertiesPage::reverse_color_order ()
 }
 
 void 
-PropertiesPage::apply ()
+PropertiesPage::apply (bool /*commit*/)
 {
   bool has_error = false;
 
@@ -915,7 +915,7 @@ PropertiesPage::browse ()
 {
 BEGIN_PROTECTED
 
-  apply ();
+  apply (true);
 
   lay::FileDialog file_dialog (this, tl::to_string (QObject::tr ("Load Image File")), tl::to_string (QObject::tr ("All files (*)")));
 
@@ -941,7 +941,7 @@ PropertiesPage::save_pressed ()
 {
 BEGIN_PROTECTED
 
-  apply ();
+  apply (true);
 
   lay::FileDialog file_dialog (this, tl::to_string (QObject::tr ("Save As KLayout Image File")), tl::to_string (QObject::tr ("KLayout image files (*.lyimg);;All files (*)")));
 

src/img/img/imgPropertiesPage.h

@@ -57,7 +57,7 @@ public:
   virtual void update ();
   virtual void leave ();
   virtual bool readonly ();
-  virtual void apply (); 
+  virtual void apply (bool commit);
 
   void set_direct_image (img::Object *image);
 

src/img/img/imgService.cc

@@ -71,7 +71,7 @@ public:
     BEGIN_PROTECTED 
 
     properties_frame->set_direct_image (mp_image_object);
-    properties_frame->apply ();
+    properties_frame->apply (true);
 
     if (mp_image_object->is_empty ()) {
       throw tl::Exception (tl::to_string (tr ("No data loaded for that image")));

src/laybasic/laybasic/layProperties.h

@@ -154,8 +154,10 @@ public:
    *  Apply any changes to the current objects. If nothing was
    *  changed, the object may be left untouched.
    *  The dialog will start a transaction on the manager object.
+   *
+   *  @param commit Is true for the "final" changes (i.e. not during editing)
    */
-  virtual void apply () 
+  virtual void apply (bool /*commit*/)
   {
     //  default implementation is empty.
   }
@@ -174,8 +176,11 @@ public:
    *  Apply any changes to the current object plus all other objects of the same kind. 
    *  If nothing was changed, the objects may be left untouched.
    *  The dialog will start a transaction on the manager object.
+   *
+   *  @param relative Is true if relative mode is selected
+   *  @param commit Is true for the "final" changes (i.e. not during editing)
    */
-  virtual void apply_to_all (bool /*relative*/)
+  virtual void apply_to_all (bool /*relative*/, bool /*commit*/)
   {
     //  default implementation is empty.
   }

src/layui/layui/layPropertiesDialog.cc

@@ -197,7 +197,7 @@ PropertiesDialog::PropertiesDialog (QWidget * /*parent*/, db::Manager *manager,
   }
   for (size_t i = 0; i < mp_properties_pages.size (); ++i) {
     mp_stack->addWidget (mp_properties_pages [i]);
-    connect (mp_properties_pages [i], SIGNAL (edited ()), this, SLOT (apply ()));
+    connect (mp_properties_pages [i], SIGNAL (edited ()), this, SLOT (properties_edited ()));
   }
 
   //  Necessary to maintain the page order for UI regression testing of 0.18 vs. 0.19 (because tl::Collection has changed to order) ..
@@ -314,7 +314,7 @@ PropertiesDialog::current_index_changed (const QModelIndex &index, const QModelI
 
         db::Transaction t (mp_manager, tl::to_string (QObject::tr ("Apply changes")), m_transaction_id);
 
-        mp_properties_pages [m_index]->apply ();
+        mp_properties_pages [m_index]->apply (true);
 
         if (! t.is_empty ()) {
           m_transaction_id = t.id ();
@@ -437,7 +437,7 @@ BEGIN_PROTECTED
 
   if (! mp_properties_pages [m_index]->readonly ()) {
     db::Transaction t (mp_manager, tl::to_string (QObject::tr ("Apply changes")), m_transaction_id);
-    mp_properties_pages [m_index]->apply ();
+    mp_properties_pages [m_index]->apply (true);
     if (! t.is_empty ()) {
       m_transaction_id = t.id ();
     }
@@ -485,7 +485,7 @@ BEGIN_PROTECTED
 
   if (! mp_properties_pages [m_index]->readonly ()) {
     db::Transaction t (mp_manager, tl::to_string (QObject::tr ("Apply changes")), m_transaction_id);
-    mp_properties_pages [m_index]->apply ();
+    mp_properties_pages [m_index]->apply (true);
     if (! t.is_empty ()) {
       m_transaction_id = t.id ();
     }
@@ -567,7 +567,7 @@ PropertiesDialog::any_prev () const
 }
 
 void
-PropertiesDialog::apply ()
+PropertiesDialog::properties_edited ()
 {
 BEGIN_PROTECTED
 
@@ -580,9 +580,9 @@ BEGIN_PROTECTED
   try {
 
     if (mp_ui->apply_to_all_cbx->isChecked () && mp_properties_pages [m_index]->can_apply_to_all ()) {
-      mp_properties_pages [m_index]->apply_to_all (mp_ui->relative_cbx->isChecked ());
+      mp_properties_pages [m_index]->apply_to_all (mp_ui->relative_cbx->isChecked (), false);
     } else {
-      mp_properties_pages [m_index]->apply ();
+      mp_properties_pages [m_index]->apply (false);
     }
     mp_properties_pages [m_index]->update ();
 
@@ -632,7 +632,7 @@ BEGIN_PROTECTED
 
     db::Transaction t (mp_manager, tl::to_string (QObject::tr ("Apply changes")), m_transaction_id);
 
-    mp_properties_pages [m_index]->apply ();
+    mp_properties_pages [m_index]->apply (true);
     mp_properties_pages [m_index]->update ();
 
     if (! t.is_empty ()) {

src/layui/layui/layPropertiesDialog.h

@@ -105,7 +105,7 @@ private:
   void update_controls ();
 
 public slots:
-  void apply ();
+  void properties_edited ();
   void next_pressed ();
   void prev_pressed ();
   void cancel_pressed ();