Fix/module pins gui (#622)

joern274 · web-flow · commit 7ce30af74b50 · 2025-09-17T17:34:38.000+02:00
* Renamed files and classes to get a consistent naming scheme

* Bugfix: enable UNDO on delete pin group

* Move ActionPingroup helper routines into class

* Preserve order of pins when restoring deleted pin group

* Fixed internal bookkeeping of group start index allowing negative numbers

* Rename misleading function- and class names

* Assign pin to new group -&gt; Update indexes in source group as well

* Changelog updated

* Bugfix: make sure group exists when updating pin index
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,9 +16,10 @@ All notable changes to this project will be documented in this file.
 
 * pin groups
   * added getter for lowest and highest index of pin group
-  * added column for pin index in pin tree shown in selection details
-  * added context menu entry to toggle between ascending and descending
-  * changed drag&drop behavior, allowing to drop pin on pin when ordering
+  * added column for pin index in GUI module pin tree
+  * added GUI module pin tree context menu entry to toggle between ascending and descending
+  * fixed GUI undo function for group delete
+  * changed GUI module pin tree drag'n drop behavior, allow drop pin(-group) on pin
    
 * fixed availability of "save as" so that does not required modifications to be enabaled
 * added feature to unzip and open hal project by dropping zipped file on welcome screen
diff --git a/plugins/gui/include/gui/basic_tree_model/base_tree_item.h b/plugins/gui/include/gui/basic_tree_model/base_tree_item.h
@@ -87,7 +87,7 @@ namespace hal
          * @param index - The column to set the new data.
          * @param data - The new column data.
          */
-        virtual void setDataAtIndex(int index, QVariant& data) = 0;
+        virtual void setDataAtColumn(int column, QVariant& data) = 0;
 
         /**
          * Appends a new column to the item.
@@ -227,7 +227,7 @@ namespace hal
          * @param index - The column to set the new data.
          * @param data - The new column data.
          */
-        void setDataAtIndex(int index, QVariant& data) override;
+        void setDataAtColumn(int column, QVariant& data) override;
 
         /**
          * Appends a new column to the item.
diff --git a/plugins/gui/include/gui/context_manager_widget/models/context_tree_model.h b/plugins/gui/include/gui/context_manager_widget/models/context_tree_model.h
@@ -71,7 +71,7 @@ namespace hal
             ContextTreeItem(ContextDirectory* directory);
             QVariant getData(int column) const override;
             void setData(QList<QVariant> data) override;
-            void setDataAtIndex(int index, QVariant& data) override;
+            void setDataAtColumn(int column, QVariant& data) override;
             void appendData(QVariant data) override;
             int getColumnCount() const override;
             int row() const;
diff --git a/plugins/gui/include/gui/gatelibrary_management/gatelibrary_tab_widgets/gatelibrary_tab_pin.h b/plugins/gui/include/gui/gatelibrary_management/gatelibrary_tab_widgets/gatelibrary_tab_pin.h
@@ -26,8 +26,6 @@
 #pragma once
 
 #include "gatelibrary_tab_interface.h"
-#include "gui/selection_details_widget/gate_details_widget/pin_tree_model.h"
-#include "hal_core/defines.h"
 #include "hal_core/netlist/gate_library/gate_type.h"
 
 #include <QGridLayout>
diff --git a/plugins/gui/include/gui/module_model/module_item.h b/plugins/gui/include/gui/module_model/module_item.h
@@ -66,7 +66,7 @@ namespace hal
          * @param index - The column to set the new data. Either 0 (name) or 2(type). Other columns will be ignored.
          * @param data - The new column data.
          */
-        void setDataAtIndex(int index, QVariant& data) override;
+        void setDataAtColumn(int column, QVariant& data) override;
         /**
          * Unused dummy function overwritten from parent class.
          */
diff --git a/plugins/gui/include/gui/pin_model/pin_item.h b/plugins/gui/include/gui/pin_model/pin_item.h
@@ -50,7 +50,7 @@ namespace hal
         enum class TreeItemType {PinGroup, Pin, GroupCreator, PinCreator, InvalidPinGroup, InvalidPin};
 
         void setData(QList<QVariant> data) override;
-        void setDataAtIndex(int index, QVariant& data) override;
+        void setDataAtColumn(int column, QVariant& data) override;
         void appendData(QVariant data) override;
         int getColumnCount() const override;
 
diff --git a/plugins/gui/include/gui/selection_details_widget/gate_details_widget/gate_pin_tree.h b/plugins/gui/include/gui/selection_details_widget/gate_details_widget/gate_pin_tree.h
@@ -33,7 +33,7 @@ namespace hal
 {
     class GatePinsTreeModel;
     class Gate;
-    class PinTreeItem;
+    class GatePinsTreeItem;
     class GraphNavigationWidget;
 
     /**
@@ -98,8 +98,8 @@ namespace hal
         bool mClearSelection;
 
         //helper functions
-        void buildPythonMenuForPin(QMenu &menu, PinTreeItem* clickedPinItem);
-        void buildPythonMenuForPinGroup(QMenu &menu, PinTreeItem* clickedPinIGrouptem);
+        void buildPythonMenuForPin(QMenu &menu, GatePinsTreeItem* clickedPinItem);
+        void buildPythonMenuForPinGroup(QMenu &menu, GatePinsTreeItem* clickedPinIGrouptem);
         void addSourceOurDestinationToSelection(int netId, bool isInputPin);
         void handleNavigationCloseRequested();
         void handleNavigationJumpRequested(const Node& origin, const u32 via_net, const QSet<u32>& to_gates, const QSet<u32>& to_modules);
diff --git a/plugins/gui/include/gui/selection_details_widget/gate_details_widget/gate_pins_tree_model.h b/plugins/gui/include/gui/selection_details_widget/gate_details_widget/gate_pins_tree_model.h
@@ -36,7 +36,7 @@ namespace hal
 
     class Gate;
 
-    class PinTreeItem : public BaseTreeItem
+    class GatePinsTreeItem : public BaseTreeItem
     {
     public:
         enum Type {None, Pin, Group};
@@ -51,11 +51,11 @@ namespace hal
             int mIndex;
         public:
 
-            PinTreeItem(const std::string& pinName, QString pinDirection, QString pinTypee, QString netName, int inx);
-            PinTreeItem();
+            GatePinsTreeItem(const std::string& pinName, QString pinDirection, QString pinTypee, QString netName, int inx);
+            GatePinsTreeItem();
             QVariant getData(int column) const override;
             void setData(QList<QVariant> data) override;
-            void setDataAtIndex(int index, QVariant& data) override;
+            void setDataAtColumn(int column, QVariant& data) override;
             void appendData(QVariant data) override;
             int getColumnCount() const override;
             void setType(Type tp) { mType = tp; }
diff --git a/plugins/gui/include/gui/selection_details_widget/module_details_widget/module_pins_tree.h b/plugins/gui/include/gui/selection_details_widget/module_details_widget/module_pins_tree.h
diff --git a/plugins/gui/include/gui/selection_details_widget/module_details_widget/module_pins_tree_model.h b/plugins/gui/include/gui/selection_details_widget/module_details_widget/module_pins_tree_model.h
@@ -38,7 +38,7 @@ namespace hal
     class Net;
 
 
-    class PortTreeItem : public BaseTreeItem
+    class ModulePinsTreeItem : public BaseTreeItem
     {
     public:
         enum Type {None, Pin, Group};
@@ -54,11 +54,11 @@ namespace hal
 
         public:
 
-            PortTreeItem(Type itype, u32 id_, QString pinName, PinDirection dir, PinType ptype, int inx, QString netName = QString());
-            PortTreeItem() : mItemType(None), mId(0) {;}
+            ModulePinsTreeItem(Type itype, u32 id_, QString pinName, PinDirection dir, PinType ptype, int inx, QString netName = QString());
+            ModulePinsTreeItem() : mItemType(None), mId(0) {;}
             QVariant getData(int column) const override;
             void setData(QList<QVariant> data) override;
-            void setDataAtIndex(int index, QVariant& data) override;
+            void setDataAtColumn(int column, QVariant& data) override;
             void appendData(QVariant data) override;
             int getColumnCount() const override;
             void setItemType(Type tp) { mItemType = tp; }
@@ -127,7 +127,7 @@ namespace hal
          * @param item - The (port) item.
          * @return The net or nullptr.
          */
-        Net* getNetFromItem(PortTreeItem* item);
+        Net* getNetFromItem(ModulePinsTreeItem* item);
 
         /**
          * Get the id of the module that is currently represented.
@@ -162,18 +162,18 @@ namespace hal
         Module* mModule;
         //name is (hopefully) enough to identify
         QMap<QString, BaseTreeItem*> mNameToTreeItem;
-        QMap<int, PortTreeItem*> mIdToPinItem;
-        QMap<int, PortTreeItem*> mIdToGroupItem;
+        QMap<int, ModulePinsTreeItem*> mIdToPinItem;
+        QMap<int, ModulePinsTreeItem*> mIdToGroupItem;
 
-        void insertItem(PortTreeItem* item, BaseTreeItem* parent, int index);
-        void removeItem(PortTreeItem* item);
-        void updateGroupIndex(PortTreeItem* groupItem);
+        void insertItem(ModulePinsTreeItem* item, BaseTreeItem* parent, int index);
+        void removeItem(ModulePinsTreeItem* item);
+        void updateGroupIndex(ModulePinsTreeItem* groupItem);
 
         // helper functions for dnd for more clarity
         void dndGroupOnGroup(BaseTreeItem* droppedGroup, BaseTreeItem* onDroppedGroup, int row=-1);
-        void dndGroupBetweenGroup(PortTreeItem* droppedGroup, int row);
-        void dndPinOnGroup(PortTreeItem* droppedPin, BaseTreeItem* onDroppedGroup);
-        void dndPinBetweenPin(PortTreeItem* droppedPin, BaseTreeItem* onDroppedParent, int row);
-        void dndPinBetweenGroup(PortTreeItem* droppedPin, int row);
+        void dndGroupBetweenGroup(ModulePinsTreeItem* droppedGroup, int row);
+        void dndPinOnGroup(ModulePinsTreeItem* droppedPin, BaseTreeItem* onDroppedGroup);
+        void dndPinBetweenPin(ModulePinsTreeItem* droppedPin, BaseTreeItem* onDroppedParent, int row);
+        void dndPinBetweenGroup(ModulePinsTreeItem* droppedPin, int row);
     };
 }
diff --git a/plugins/gui/include/gui/user_action/action_pingroup.h b/plugins/gui/include/gui/user_action/action_pingroup.h
@@ -46,14 +46,6 @@ namespace hal
         static bool useExistingPin(Type tp);
     };
 
-    int pinGroupIndex(const Module* mod, const PinGroup<ModulePin>* pgrp);
-
-    int pinIndex2Row(const ModulePin* pin, int index);
-
-    int pinRow2Index(const ModulePin* pin, int row);
-
-    QString generateGroupName(const Module* mod, const ModulePin* pin);
-
     void dumpPingroups(Module* m = nullptr);
     /**
      * @ingroup user_action
@@ -66,8 +58,7 @@ namespace hal
      *                   negative ID: call constructor without ID, however,
      *                   ID will be used internally for subsequent commands related to crated group
      *     name     : name of group
-     *     value    : start index, assume ascending
-     *                   negative value: descending order starting with (-value-1)
+     *     value    : start index * 2, LSB : 0=descending 1=ascending
      *
      * GroupDelete
      *     ID       : ID of group to delete
@@ -135,6 +126,7 @@ namespace hal
 
         QHash<int,PinGroup<ModulePin>*> mPinGroups;
         QList<AtomicAction> mPinActions;
+        QList<QList<AtomicAction> > mTempUndoActions;
         Module* mParentModule;
         QMap<int,GroupRestore> mGroupRestore;
         QSet<u32> mPinsMoved;
@@ -144,7 +136,6 @@ namespace hal
         PinGroup<ModulePin>* getGroup(int grpId) const;
         void prepareUndoAction();
         void finalizeUndoAction();
-        void addUndoAction(PinActionType::Type tp, int id = 0, const QString& name=QString(), int value=0);
         static int pinGroupRow(const Module *m, PinGroup<ModulePin>* pgroup);
     public:
         ActionPingroup(PinActionType::Type tp = PinActionType::None, int id = 0, const QString& name=QString(), int value=0);
@@ -163,6 +154,11 @@ namespace hal
         static ActionPingroup* deletePinGroup(const Module* m, u32 grpId);
         static ActionPingroup* toggleAscendingGroup(const Module* m, u32 grpId);
         static ActionPingroup* changePinGroupType(const Module* m, u32 grpId, int ptype);
+
+        static int pinGroupIndex(const Module* mod, const PinGroup<ModulePin>* pgrp);
+        static int pinIndex2Row(const ModulePin* pin, int index);
+        static int pinRow2Index(const ModulePin* pin, int row);
+        static QString generateGroupName(const Module* mod, const ModulePin* pin);
     };
 
     /**
diff --git a/plugins/gui/src/basic_tree_model/base_tree_item.cpp b/plugins/gui/src/basic_tree_model/base_tree_item.cpp
@@ -114,11 +114,11 @@ namespace hal
         }
     }
 
-    void RootTreeItem::setDataAtIndex(int index, QVariant &data)
+    void RootTreeItem::setDataAtColumn(int column, QVariant &data)
     {
-        while (index >= mHeaderLabels.size())
+        while (column >= mHeaderLabels.size())
             mHeaderLabels << QString();
-        mHeaderLabels[index] = data.toString();
+        mHeaderLabels[column] = data.toString();
     }
 
     void RootTreeItem::appendData(QVariant data)
diff --git a/plugins/gui/src/basic_tree_model/base_tree_model.cpp b/plugins/gui/src/basic_tree_model/base_tree_model.cpp
@@ -114,7 +114,7 @@ namespace hal
         else
             for (int i=0; i<labels.size(); i++) {
                 QVariant qv = QVariant(labels.at(i));
-                mRootItem->setDataAtIndex(i, qv);
+                mRootItem->setDataAtColumn(i, qv);
             }
 
 
diff --git a/plugins/gui/src/context_manager_widget/models/context_tree_model.cpp b/plugins/gui/src/context_manager_widget/models/context_tree_model.cpp
@@ -94,9 +94,9 @@ namespace hal
 
     }
 
-    void ContextTreeItem::setDataAtIndex(int index, QVariant &data)
+    void ContextTreeItem::setDataAtColumn(int column, QVariant &data)
     {
-
+        // TODO: implement
     }
 
     void ContextTreeItem::appendData(QVariant data)
diff --git a/plugins/gui/src/module_model/module_item.cpp b/plugins/gui/src/module_model/module_item.cpp
@@ -106,9 +106,9 @@ ModuleItem::ModuleItem(const u32 id, const TreeItemType type, ModuleModel *model
             setModuleType(data.at(2).toString());
     }
 
-    void ModuleItem::setDataAtIndex(int index, QVariant &data)
+    void ModuleItem::setDataAtColumn(int column, QVariant &data)
     {
-        switch (index) {
+        switch (column) {
         case 0:
             setName(data.toString());
             return;
diff --git a/plugins/gui/src/pin_model/pin_item.cpp b/plugins/gui/src/pin_model/pin_item.cpp
@@ -73,9 +73,9 @@ namespace hal
         mId = newId;
     }
 
-    void PinItem::setDataAtIndex(int index, QVariant &data)
+    void PinItem::setDataAtColumn(int column, QVariant &data)
     {
-
+        // TODO : implement ?!
     }
 
     QString PinItem::getName() const
diff --git a/plugins/gui/src/selection_details_widget/gate_details_widget/gate_pin_tree.cpp b/plugins/gui/src/selection_details_widget/gate_details_widget/gate_pin_tree.cpp
@@ -1,5 +1,5 @@
 #include "gui/selection_details_widget/gate_details_widget/gate_pin_tree.h"
-#include "gui/selection_details_widget/gate_details_widget/pin_tree_model.h"
+#include "gui/selection_details_widget/gate_details_widget/gate_pins_tree_model.h"
 //#include "gui/gui_globals.h"
 #include "hal_core/netlist/gate.h"
 #include <QHeaderView>
@@ -69,8 +69,8 @@ namespace hal
         if(!idx.isValid())
             return;
 
-        PinTreeItem* clickedItem = dynamic_cast<PinTreeItem*>(mPinModel->getItemFromIndex(idx));
-        if(!clickedItem || clickedItem->type() != PinTreeItem::Pin)
+        GatePinsTreeItem* clickedItem = dynamic_cast<GatePinsTreeItem*>(mPinModel->getItemFromIndex(idx));
+        if(!clickedItem || clickedItem->type() != GatePinsTreeItem::Pin)
             return;
 
         auto netId = clickedItem->netIds().front();
@@ -96,7 +96,7 @@ namespace hal
         if(!idx.isValid())
             return;
 
-        PinTreeItem* clickedItem = dynamic_cast<PinTreeItem*>(mPinModel->getItemFromIndex(idx));
+        GatePinsTreeItem* clickedItem = dynamic_cast<GatePinsTreeItem*>(mPinModel->getItemFromIndex(idx));
         QMenu menu;
         bool isMiscSectionSet = false;//so that the misc-section is not set multiple times
 
@@ -117,7 +117,7 @@ namespace hal
         });
 
         //Check if jump to source or destination is possible
-        if(clickedItem->type() == PinTreeItem::Pin && clickedItem->netIds().size()==1)
+        if(clickedItem->type() == GatePinsTreeItem::Pin && clickedItem->netIds().size()==1)
         {
             auto netId = clickedItem->netIds().front();
             auto clickedNet = gNetlist->get_net_by_id(netId);
@@ -156,15 +156,15 @@ namespace hal
 
         //Add nets to selection if possible
         QList<u32> netIds;
-        if(clickedItem->type() == PinTreeItem::Pin)
+        if(clickedItem->type() == GatePinsTreeItem::Pin)
         {
             netIds = clickedItem->netIds();
         }
         else
         {
             for(auto childItem : clickedItem->getChildren())
             {
-                PinTreeItem* pti = dynamic_cast<PinTreeItem*>(childItem);
+                GatePinsTreeItem* pti = dynamic_cast<GatePinsTreeItem*>(childItem);
                 if (pti)
                     netIds.append(pti->netIds());
             }
@@ -194,7 +194,7 @@ namespace hal
 
         menu.addSection("Python");
 
-        if(clickedItem->type() == PinTreeItem::Pin)
+        if(clickedItem->type() == GatePinsTreeItem::Pin)
             buildPythonMenuForPin(menu, clickedItem);
         else
             buildPythonMenuForPinGroup(menu, clickedItem);
@@ -204,7 +204,7 @@ namespace hal
 
     }
 
-    void GatePinTree::buildPythonMenuForPin(QMenu &menu, PinTreeItem *clickedPinItem)
+    void GatePinTree::buildPythonMenuForPin(QMenu &menu, GatePinsTreeItem *clickedPinItem)
     {
         // 1.) NET-OBJECT
         QList<u32> netIdsOfItem = clickedPinItem->netIds();
@@ -246,7 +246,7 @@ namespace hal
 
     }
 
-    void GatePinTree::buildPythonMenuForPinGroup(QMenu &menu, PinTreeItem *clickedPinIGrouptem)
+    void GatePinTree::buildPythonMenuForPinGroup(QMenu &menu, GatePinsTreeItem *clickedPinIGrouptem)
     {
         // 1. PYTHON LIST OF PIN GROUPS
         QString pythonList = "[";
diff --git a/plugins/gui/src/selection_details_widget/gate_details_widget/gate_pins_tree_model.cpp b/plugins/gui/src/selection_details_widget/gate_details_widget/gate_pins_tree_model.cpp
diff --git a/plugins/gui/src/selection_details_widget/module_details_tab_widget.cpp b/plugins/gui/src/selection_details_widget/module_details_tab_widget.cpp
diff --git a/plugins/gui/src/selection_details_widget/module_details_widget/module_pins_tree.cpp b/plugins/gui/src/selection_details_widget/module_details_widget/module_pins_tree.cpp
diff --git a/plugins/gui/src/selection_details_widget/module_details_widget/module_pins_tree_model.cpp b/plugins/gui/src/selection_details_widget/module_details_widget/module_pins_tree_model.cpp
diff --git a/plugins/gui/src/user_action/action_pingroup.cpp b/plugins/gui/src/user_action/action_pingroup.cpp
