From 09a2ef00cc846ebc5f091f5ae3dc52b3eeb51e18 Mon Sep 17 00:00:00 2001 From: joern274 Date: Tue, 17 Sep 2024 11:02:50 +0200 Subject: [PATCH] bugfix: whenever ModuleItem gets created or deleted maps must be updated accordingly --- .../include/gui/module_model/module_item.h | 12 +- .../include/gui/module_model/module_model.h | 1 + plugins/gui/src/module_model/module_item.cpp | 19 ++- plugins/gui/src/module_model/module_model.cpp | 141 ++++++++---------- .../selection_details_widget.cpp | 6 +- .../logic_evaluator_select_gates.h | 4 +- .../src/logic_evaluator_select_gates.cpp | 4 +- 7 files changed, 95 insertions(+), 92 deletions(-) diff --git a/plugins/gui/include/gui/module_model/module_item.h b/plugins/gui/include/gui/module_model/module_item.h index 537ffc27a10..d958cbfb828 100644 --- a/plugins/gui/include/gui/module_model/module_item.h +++ b/plugins/gui/include/gui/module_model/module_item.h @@ -36,6 +36,8 @@ namespace hal { + class ModuleModel; + /** * @ingroup gui * @brief An item in the ModuleModel. @@ -61,7 +63,14 @@ namespace hal * @param id - The id of the netlist item this ModuleItem represents * @param type - The type of the netlist item */ - ModuleItem(const u32 id, const TreeItemType type); + ModuleItem(const u32 id, const TreeItemType type, ModuleModel* model); + + /** + * Destructor. + * + * Must remove item from ModuleModel map as well. + */ + virtual ~ModuleItem(); /** * Given a set of ModuleItems (in a map [id]->[ModuleItem]) this function adds each ModuleItem of this set as @@ -142,5 +151,6 @@ namespace hal QString mModuleType; bool mHighlighted; + ModuleModel* mModuleModel; // reference to parent model }; } diff --git a/plugins/gui/include/gui/module_model/module_model.h b/plugins/gui/include/gui/module_model/module_model.h index b74c5c3fa69..9130123aa94 100644 --- a/plugins/gui/include/gui/module_model/module_model.h +++ b/plugins/gui/include/gui/module_model/module_model.h @@ -52,6 +52,7 @@ namespace hal */ class ModuleModel : public BaseTreeModel { + friend class ModuleItem; Q_OBJECT /** diff --git a/plugins/gui/src/module_model/module_item.cpp b/plugins/gui/src/module_model/module_item.cpp index f3a1e531f79..9fadba89b7c 100644 --- a/plugins/gui/src/module_model/module_item.cpp +++ b/plugins/gui/src/module_model/module_item.cpp @@ -1,4 +1,5 @@ #include "gui/module_model/module_item.h" +#include "gui/module_model/module_model.h" #include "hal_core/netlist/module.h" @@ -8,11 +9,12 @@ namespace hal { -ModuleItem::ModuleItem(const u32 id, const TreeItemType type) : +ModuleItem::ModuleItem(const u32 id, const TreeItemType type, ModuleModel *model) : BaseTreeItem(), mId(id), mItemType(type), - mHighlighted(false) + mHighlighted(false), + mModuleModel(model) { switch(type) { @@ -40,6 +42,19 @@ ModuleItem::ModuleItem(const u32 id, const TreeItemType type) : break; } } + mModuleModel->mModuleItemMaps[(int)mItemType]->insertMulti(id,this); + } + + ModuleItem::~ModuleItem() + { + auto it = mModuleModel->mModuleItemMaps[(int)mItemType]->lowerBound(mId); + while (it != mModuleModel->mModuleItemMaps[(int)mItemType]->upperBound(mId)) + { + if (it.value() == this) + it = mModuleModel->mModuleItemMaps[(int)mItemType]->erase(it); + else + ++it; + } } void ModuleItem::appendExistingChildIfAny(const QMap& moduleMap) diff --git a/plugins/gui/src/module_model/module_model.cpp b/plugins/gui/src/module_model/module_model.cpp index 5516cf0b1a1..ad95f58312f 100644 --- a/plugins/gui/src/module_model/module_model.cpp +++ b/plugins/gui/src/module_model/module_model.cpp @@ -130,14 +130,14 @@ namespace hal Module* parentModule = g->get_module(); ModuleItem* parentItem; bool insertToRoot = true; - ModuleItem* childItem = new ModuleItem(g->get_id(), ModuleItem::TreeItemType::Gate); + ModuleItem* childItem = new ModuleItem(g->get_id(), ModuleItem::TreeItemType::Gate, this); while (parentModule && insertToRoot) { parentItem = parentMap.value(parentModule); if (!parentItem) { - parentItem = new ModuleItem(parentModule->get_id(), ModuleItem::TreeItemType::Module); + parentItem = new ModuleItem(parentModule->get_id(), ModuleItem::TreeItemType::Module, this); parentMap.insert(parentModule, parentItem); } else @@ -172,10 +172,10 @@ namespace hal moduleAssignNets(); for(u32 id : gateIds) - newRootList.append(new ModuleItem(id, ModuleItem::TreeItemType::Gate)); + newRootList.append(new ModuleItem(id, ModuleItem::TreeItemType::Gate, this)); for(u32 id : netIds) - newRootList.append(new ModuleItem(id, ModuleItem::TreeItemType::Net)); + newRootList.append(new ModuleItem(id, ModuleItem::TreeItemType::Net, this)); for(auto item : newRootList) mRootItem->appendChild(item); @@ -232,50 +232,49 @@ namespace hal void ModuleModel::removeModule(const u32 id) { - auto it = mModuleMap.lowerBound(id); - while (it != mModuleMap.upperBound(id)) + QList trashcan; // access items to delete without map + + for (auto it = mModuleMap.lowerBound(id); it != mModuleMap.upperBound(id); ++it) + trashcan.append(it.value()); + + for (ModuleItem* item : trashcan) { - ModuleItem* item = it.value(); BaseTreeItem* parentItem = item->getParent(); - removeChildItem(item,parentItem); - - it = mModuleMap.erase(it); } } void ModuleModel::removeGate(const u32 id) { - auto it = mGateMap.lowerBound(id); - while (it != mGateMap.upperBound(id)) - { - ModuleItem* item = it.value(); - BaseTreeItem* parentItem = item->getParent(); + QList trashcan; // access items to delete without map - removeChildItem(item, parentItem); + for (auto it = mGateMap.lowerBound(id); it != mGateMap.upperBound(id); ++it) + trashcan.append(it.value()); - it = mGateMap.erase(it); + for (ModuleItem* item : trashcan) + { + BaseTreeItem* parentItem = item->getParent(); + removeChildItem(item,parentItem); } } void ModuleModel::removeNet(const u32 id) { - auto it = mNetMap.lowerBound(id); - while (it != mNetMap.upperBound(id)) - { - ModuleItem* item = it.value(); - BaseTreeItem* parentItem = item->getParent(); + QList trashcan; // access items to delete without map - removeChildItem(item, parentItem); + for (auto it = mNetMap.lowerBound(id); it != mNetMap.upperBound(id); ++it) + trashcan.append(it.value()); - it = mNetMap.erase(it); + for (ModuleItem* item : trashcan) + { + BaseTreeItem* parentItem = item->getParent(); + removeChildItem(item,parentItem); } } ModuleItem* ModuleModel::createChildItem(u32 id, ModuleItem::TreeItemType itemType, BaseTreeItem *parentItem) { - ModuleItem* retval = new ModuleItem(id, itemType); - mModuleItemMaps[(int)itemType]->insertMulti(id,retval); + ModuleItem* retval = new ModuleItem(id, itemType, this); if (!parentItem) parentItem = mRootItem; QModelIndex index = getIndexFromItem(parentItem); @@ -297,15 +296,6 @@ namespace hal while (itemToRemove->getChildCount()) { ModuleItem* childItem = static_cast(itemToRemove->getChildren().at(0)); - int ityp = static_cast(childItem->getType()); - auto it = mModuleItemMaps[ityp]->lowerBound(childItem->id()); - while (it != mModuleItemMaps[ityp]->upperBound(childItem->id())) - { - if (it.value() == childItem) - it = mModuleItemMaps[ityp]->erase(it); - else - ++it; - } removeChildItem(childItem,itemToRemove); } @@ -351,27 +341,25 @@ namespace hal void ModuleModel::handleModuleGateRemoved(Module* mod, u32 gateId) { + QList > trashcan; // item to delete, parent + if (mTempGateAssignment.isAccumulate()) mTempGateAssignment.removeGateFromModule(gateId,mod); else { - auto it = mGateMap.lowerBound(gateId); - while (it != mGateMap.upperBound(gateId)) + for (auto it = mGateMap.lowerBound(gateId); it != mGateMap.upperBound(gateId); ++it) { ModuleItem* item = it.value(); - if (!item->isToplevelItem()) - { - ModuleItem* parentItem = static_cast(item->getParent()); - if (parentItem->id() == mod->get_id()) - { - removeChildItem(item, parentItem); - it = mGateMap.erase(it); - continue; - } - } - ++it; + if (item->isToplevelItem()) continue;; + + ModuleItem* parentItem = static_cast(item->getParent()); + if (parentItem->id() == mod->get_id()) + trashcan.append(QPair(item, parentItem)); } } + + for (const QPair& trash : trashcan) + removeChildItem(trash.first, trash.second); } void ModuleModel::handleModuleGatesAssignBegin(Module* mod, u32 numberGates) @@ -540,8 +528,8 @@ namespace hal QSet parentsHandled; Q_ASSERT(gNetlist->get_gate_by_id(gateId)); - auto itGat = mGateMap.lowerBound(gateId); - while (itGat != mGateMap.upperBound(gateId)) + QList > trashcan; // item to delete, parent + for (auto itGat = mGateMap.lowerBound(gateId); itGat != mGateMap.upperBound(gateId); ++itGat) { ModuleItem* gatItem = itGat.value(); if (gatItem->isToplevelItem()) continue; @@ -550,16 +538,15 @@ namespace hal if (oldParentItem->id() != moduleId) { - removeChildItem(gatItem,oldParentItem); - itGat = mGateMap.erase(itGat); + trashcan.append(QPair(gatItem,oldParentItem)); } else { parentsHandled.insert(oldParentItem); - ++itGat; } - } + for (const QPair& trash : trashcan) + removeChildItem(trash.first, trash.second); if (!moduleId) return; for (auto itMod = mModuleMap.lowerBound(moduleId); itMod != mModuleMap.upperBound(moduleId); ++itMod) @@ -623,28 +610,30 @@ namespace hal newParentId = newParentModule->get_id(); } - auto itNet = mNetMap.lowerBound(netId); - while (itNet != mNetMap.upperBound(netId)) + QList > trashcan; // item to delete, parent + + for (auto itNet = mNetMap.lowerBound(netId); itNet != mNetMap.upperBound(netId); ++itNet) { if (itNet.value()->isToplevelItem()) continue; - + ModuleItem* netItem = itNet.value(); ModuleItem* oldParentItem = static_cast(netItem->getParent()); Q_ASSERT(oldParentItem); - if (newParentId == 0 || newParentId != oldParentItem->id()) { - removeChildItem(netItem,oldParentItem); - itNet = mNetMap.erase(itNet); + trashcan.append(QPair(netItem,oldParentItem)); + break; } else { parentsHandled.insert(oldParentItem); - ++itNet; } } + for (const QPair& trash : trashcan) + removeChildItem(trash.first, trash.second); + if (!newParentId) return; for (auto itMod = mModuleMap.lowerBound(newParentId); itMod != mModuleMap.upperBound(newParentId); ++itMod) { @@ -667,15 +656,13 @@ namespace hal u32 parentId = module->get_parent_module()->get_id(); Q_ASSERT(parentId > 0); - auto itSubm = mModuleMap.lowerBound(id); - while (itSubm != mModuleMap.upperBound(id)) + QList > trashcan; // item to delete, parent + + for (auto itSubm = mModuleMap.lowerBound(id); itSubm != mModuleMap.upperBound(id); ++itSubm) { ModuleItem* submItem = itSubm.value(); - if (submItem->isToplevelItem()) - { - ++itSubm; - continue; - } + if (submItem->isToplevelItem()) continue; + ModuleItem* oldParentItem = static_cast(submItem->getParent()); Q_ASSERT(oldParentItem); @@ -684,8 +671,7 @@ namespace hal if (moduleItemToBeMoved) { // remove tree item recursively - removeChildItem(submItem,oldParentItem); - itSubm = mModuleMap.erase(itSubm); + trashcan.append(QPair(submItem,oldParentItem)); } else { @@ -700,15 +686,15 @@ namespace hal oldParentItem->removeChild(submItem); endRemoveRows(); mIsModifying = false; - ++itSubm; } } else { parentsHandled.insert(oldParentItem); - ++itSubm; } } + for (const QPair& trash : trashcan) + removeChildItem(trash.first, trash.second); if (!parentId) return; for (auto itMod = mModuleMap.lowerBound(parentId); itMod != mModuleMap.upperBound(parentId); ++itMod) @@ -734,16 +720,7 @@ namespace hal if (moduleItemToBeMoved && !moduleItemReassigned) { - // stored item could not be reassigned, delete it - auto it = mModuleMap.lowerBound(id); - while (it != mModuleMap.upperBound(id)) - { - if (it.value() == moduleItemToBeMoved) - it = mModuleMap.erase(it); - else - ++it; - } - delete moduleItemToBeMoved; + removeChildItem(moduleItemToBeMoved, moduleItemToBeMoved->getParent()); } } diff --git a/plugins/gui/src/selection_details_widget/selection_details_widget.cpp b/plugins/gui/src/selection_details_widget/selection_details_widget.cpp index 9cd3a7c3dd4..e68a92fd901 100644 --- a/plugins/gui/src/selection_details_widget/selection_details_widget.cpp +++ b/plugins/gui/src/selection_details_widget/selection_details_widget.cpp @@ -410,17 +410,17 @@ namespace hal if (gSelectionRelay->numberSelectedModules()) { - ModuleItem sti(gSelectionRelay->selectedModulesList().at(0), ModuleItem::TreeItemType::Module); + ModuleItem sti(gSelectionRelay->selectedModulesList().at(0), ModuleItem::TreeItemType::Module, mModuleModel); singleSelectionInternal(&sti); } else if (gSelectionRelay->numberSelectedGates()) { - ModuleItem sti(gSelectionRelay->selectedGatesList().at(0), ModuleItem::TreeItemType::Gate); + ModuleItem sti(gSelectionRelay->selectedGatesList().at(0), ModuleItem::TreeItemType::Gate, mModuleModel); singleSelectionInternal(&sti); } else if (gSelectionRelay->numberSelectedNets()) { - ModuleItem sti(gSelectionRelay->selectedNetsList().at(0), ModuleItem::TreeItemType::Net); + ModuleItem sti(gSelectionRelay->selectedNetsList().at(0), ModuleItem::TreeItemType::Net, mModuleModel); singleSelectionInternal(&sti); } diff --git a/plugins/logic_evaluator/include/logic_evaluator/logic_evaluator_select_gates.h b/plugins/logic_evaluator/include/logic_evaluator/logic_evaluator_select_gates.h index 6810817bfa8..7c9b775786d 100644 --- a/plugins/logic_evaluator/include/logic_evaluator/logic_evaluator_select_gates.h +++ b/plugins/logic_evaluator/include/logic_evaluator/logic_evaluator_select_gates.h @@ -45,8 +45,8 @@ Qt::CheckState mState; bool mSelectable; public: - SelectGateItem(u32 id, ModuleItem::TreeItemType type, bool isSel = true) - : ModuleItem(id, type), mState(Qt::Unchecked), mSelectable(isSel) {;} + SelectGateItem(u32 id, ModuleItem::TreeItemType type, ModuleModel* model, bool isSel = true) + : ModuleItem(id, type, model), mState(Qt::Unchecked), mSelectable(isSel) {;} Qt::CheckState state() const { return mState; } bool isSelectable() const { return mSelectable; } void setSelectable(bool isSel) { mSelectable = isSel; } diff --git a/plugins/logic_evaluator/src/logic_evaluator_select_gates.cpp b/plugins/logic_evaluator/src/logic_evaluator_select_gates.cpp index 53c50ce0769..c8a02a30b38 100644 --- a/plugins/logic_evaluator/src/logic_evaluator_select_gates.cpp +++ b/plugins/logic_evaluator/src/logic_evaluator_select_gates.cpp @@ -91,13 +91,13 @@ namespace hal { int SelectGateModel::insertModuleRecursion(const Module* mod, SelectGateItem* parentItem) { int countCheckable = 0; - SelectGateItem* child = new SelectGateItem(mod->get_id(), ModuleItem::TreeItemType::Module); + SelectGateItem* child = new SelectGateItem(mod->get_id(), ModuleItem::TreeItemType::Module, this); for (const Module* subm : mod->get_submodules()) countCheckable += insertModuleRecursion(subm, child); for (const Gate* g : mod->get_gates(nullptr, false)) { bool isSel = GuiExtensionLogicEvaluator::acceptGate(g); - child->appendChild(new SelectGateItem(g->get_id(), ModuleItem::TreeItemType::Gate, isSel)); + child->appendChild(new SelectGateItem(g->get_id(), ModuleItem::TreeItemType::Gate, this, isSel)); if (isSel) ++countCheckable; } if (!countCheckable) child->setSelectable(false);