From aa124487470be45c4b680c2a4a25cfe18327e083 Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 5 Apr 2025 20:35:11 +0200 Subject: [PATCH] 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 | 2 +- src/ant/ant/antPropertiesPage.h | 2 +- src/edt/edt/edtInstPropertiesPage.cc | 4 ++-- src/edt/edt/edtInstPropertiesPage.h | 4 ++-- src/edt/edt/edtPropertiesPages.cc | 12 +++++------ src/edt/edt/edtPropertiesPages.h | 6 +++--- src/edt/edt/edtService.cc | 29 ++++++++++++++++++++------ src/edt/edt/edtService.h | 9 ++++++-- src/img/img/imgPropertiesPage.cc | 6 +++--- src/img/img/imgPropertiesPage.h | 2 +- src/img/img/imgService.cc | 2 +- src/laybasic/laybasic/layProperties.h | 9 ++++++-- src/layui/layui/layPropertiesDialog.cc | 16 +++++++------- src/layui/layui/layPropertiesDialog.h | 2 +- 14 files changed, 66 insertions(+), 39 deletions(-) diff --git a/src/ant/ant/antPropertiesPage.cc b/src/ant/ant/antPropertiesPage.cc index 6b081bf21..15e19315e 100644 --- a/src/ant/ant/antPropertiesPage.cc +++ b/src/ant/ant/antPropertiesPage.cc @@ -517,7 +517,7 @@ PropertiesPage::readonly () } void -PropertiesPage::apply () +PropertiesPage::apply (bool /*commit*/) { ant::Object obj; get_object (obj); diff --git a/src/ant/ant/antPropertiesPage.h b/src/ant/ant/antPropertiesPage.h index 9b9438d71..c98c42d91 100644 --- a/src/ant/ant/antPropertiesPage.h +++ b/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 (); diff --git a/src/edt/edt/edtInstPropertiesPage.cc b/src/edt/edt/edtInstPropertiesPage.cc index da06cb814..de27a4d90 100644 --- a/src/edt/edt/edtInstPropertiesPage.cc +++ b/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); } diff --git a/src/edt/edt/edtInstPropertiesPage.h b/src/edt/edt/edtInstPropertiesPage.h index c33d7f3d6..f00a1a659 100644 --- a/src/edt/edt/edtInstPropertiesPage.h +++ b/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); diff --git a/src/edt/edt/edtPropertiesPages.cc b/src/edt/edt/edtPropertiesPages.cc index 365ca1c63..26b1def30 100644 --- a/src/edt/edt/edtPropertiesPages.cc +++ b/src/edt/edt/edtPropertiesPages.cc @@ -215,7 +215,7 @@ ShapePropertiesPage::recompute_selection_ptrs (const std::vector gs = mp_service->handle_guiding_shape_changes (new_sel[index]); + std::pair 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 diff --git a/src/edt/edt/edtPropertiesPages.h b/src/edt/edt/edtPropertiesPages.h index 7d8d5b644..60d844f50 100644 --- a/src/edt/edt/edtPropertiesPages.h +++ b/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 &new_sel); protected: diff --git a/src/edt/edt/edtService.cc b/src/edt/edt/edtService.cc index 75fbf5428..aeb4223d1 100644 --- a/src/edt/edt/edtService.cc +++ b/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 -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::max (); db::cell_index_type parent_cell = std::numeric_limits::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 gs = handle_guiding_shape_changes (*s); + std::pair gs = handle_guiding_shape_changes (*s, commit); if (gs.first) { // remove superfluous proxies diff --git a/src/edt/edt/edtService.h b/src/edt/edt/edtService.h index 7e2d34801..e782bbfb5 100644 --- a/src/edt/edt/edtService.h +++ b/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 handle_guiding_shape_changes (const lay::ObjectInstPath &obj) const; + std::pair 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; diff --git a/src/img/img/imgPropertiesPage.cc b/src/img/img/imgPropertiesPage.cc index 4ff7a954b..056e7e5ce 100644 --- a/src/img/img/imgPropertiesPage.cc +++ b/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 (*)"))); diff --git a/src/img/img/imgPropertiesPage.h b/src/img/img/imgPropertiesPage.h index b9ccedc33..e715f99cd 100644 --- a/src/img/img/imgPropertiesPage.h +++ b/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); diff --git a/src/img/img/imgService.cc b/src/img/img/imgService.cc index 0dc06ae74..3b1c85589 100644 --- a/src/img/img/imgService.cc +++ b/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"))); diff --git a/src/laybasic/laybasic/layProperties.h b/src/laybasic/laybasic/layProperties.h index ef4b8ec3c..09b16c2b9 100644 --- a/src/laybasic/laybasic/layProperties.h +++ b/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. } diff --git a/src/layui/layui/layPropertiesDialog.cc b/src/layui/layui/layPropertiesDialog.cc index b714742e8..edd85844b 100644 --- a/src/layui/layui/layPropertiesDialog.cc +++ b/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 ()) { diff --git a/src/layui/layui/layPropertiesDialog.h b/src/layui/layui/layPropertiesDialog.h index 153002127..01e09db45 100644 --- a/src/layui/layui/layPropertiesDialog.h +++ b/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 ();