From 088b5513a7cdf397f7d0f0dc3b8381aa3d40b1aa Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Fri, 3 May 2019 11:11:39 +0200 Subject: [PATCH] #4388 AppFwk : Remove problematic setUiField() calls setUiField() call used to keep track of which fields that is retained when updating an editor. Replace this workflow with a set of the ones that are used, and use this set to clean up unused editors afterwards. --- .../cafPdmUiFieldEditorHelper.cpp | 2 +- .../cafPdmUiFieldEditorHelper.h | 2 +- .../cafPdmUiFormLayoutObjectEditor.cpp | 32 +++++++++---------- .../cafPdmUiFormLayoutObjectEditor.h | 1 + .../cafPdmUiTableViewQModel.cpp | 20 +++++------- .../cafPdmUiToolBarEditor.cpp | 4 +-- 6 files changed, 29 insertions(+), 32 deletions(-) diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiFieldEditorHelper.cpp b/Fwk/AppFwk/cafUserInterface/cafPdmUiFieldEditorHelper.cpp index 174afc9c6d..935051ad6b 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiFieldEditorHelper.cpp +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiFieldEditorHelper.cpp @@ -47,7 +47,7 @@ //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- -caf::PdmUiFieldEditorHandle* caf::PdmUiFieldEditorHelper::fieldEditorForField(caf::PdmUiFieldHandle* field, const QString& uiConfigName) +caf::PdmUiFieldEditorHandle* caf::PdmUiFieldEditorHelper::createFieldEditorForField(caf::PdmUiFieldHandle* field, const QString& uiConfigName) { caf::PdmUiFieldEditorHandle* fieldEditor = nullptr; diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiFieldEditorHelper.h b/Fwk/AppFwk/cafUserInterface/cafPdmUiFieldEditorHelper.h index a0d01bd451..1e78a04722 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiFieldEditorHelper.h +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiFieldEditorHelper.h @@ -50,7 +50,7 @@ namespace caf class PdmUiFieldEditorHelper { public: - static PdmUiFieldEditorHandle* fieldEditorForField(PdmUiFieldHandle* fieldHandle, const QString& uiConfigName); + static PdmUiFieldEditorHandle* createFieldEditorForField(PdmUiFieldHandle* fieldHandle, const QString& uiConfigName); }; diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp b/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp index 232a09c136..1a91c54e0d 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp @@ -176,16 +176,18 @@ int caf::PdmUiFormLayoutObjectEditor::recursivelyConfigureAndUpdateUiOrderingInG } else { - // Also assign required item space that isn't taken up by field and label - spareColumnsToAssign += minimumItemColumnSpan - (minimumLabelColumnSpan + minimumFieldColumnSpan); - + PdmUiFieldEditorHandle* fieldEditor = nullptr; PdmUiFieldHandle* field = dynamic_cast(currentItem); - PdmUiFieldEditorHandle* fieldEditor = findOrCreateFieldEditor(containerWidgetWithGridLayout, field, uiConfigName); + if (field) fieldEditor = findOrCreateFieldEditor(containerWidgetWithGridLayout, field, uiConfigName); if (fieldEditor) { - fieldEditor->setUiField(field); + // Mark this field as used in the editor + m_usedFields.insert(field->fieldHandle()); + + // Also assign required item space that isn't taken up by field and label + spareColumnsToAssign += minimumItemColumnSpan - (minimumLabelColumnSpan + minimumFieldColumnSpan); // Place the widget(s) into the correct parent and layout QWidget* fieldCombinedWidget = fieldEditor->combinedWidget(); @@ -390,11 +392,12 @@ caf::PdmUiFieldEditorHandle* caf::PdmUiFormLayoutObjectEditor::findOrCreateField if (it == m_fieldViews.end()) { - fieldEditor = PdmUiFieldEditorHelper::fieldEditorForField(field, uiConfigName); + fieldEditor = PdmUiFieldEditorHelper::createFieldEditorForField(field, uiConfigName); if (fieldEditor) { m_fieldViews[field->fieldHandle()] = fieldEditor; + fieldEditor->setUiField(field); fieldEditor->createWidgets(parent); } else @@ -497,11 +500,8 @@ void caf::PdmUiFormLayoutObjectEditor::configureAndUpdateUi(const QString& uiCon } // Set all fieldViews to be unvisited - std::map::iterator it; - for (it = m_fieldViews.begin(); it != m_fieldViews.end(); ++it) - { - it->second->setUiField(nullptr); - } + + m_usedFields.clear(); // Set all group Boxes to be unvisited m_newGroupBoxes.clear(); @@ -511,13 +511,13 @@ void caf::PdmUiFormLayoutObjectEditor::configureAndUpdateUi(const QString& uiCon // Remove all fieldViews not mentioned by the configuration from the layout std::vector fvhToRemoveFromMap; - for (it = m_fieldViews.begin(); it != m_fieldViews.end(); ++it) + for (auto oldFvIt = m_fieldViews.begin(); oldFvIt != m_fieldViews.end(); ++oldFvIt) { - if (it->second->uiField() == nullptr) + if (m_usedFields.count(oldFvIt->first) == 0) { - PdmUiFieldEditorHandle* fvh = it->second; - delete fvh; - fvhToRemoveFromMap.push_back(it->first); + // The old field editor is not present anymore, get rid of it + delete oldFvIt->second; + fvhToRemoveFromMap.push_back(oldFvIt->first); } } diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.h b/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.h index 586c094059..077a13556a 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.h +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.h @@ -110,6 +110,7 @@ private: private: std::map m_fieldViews; + std::set m_usedFields; ///< used temporarily to store the new(complete) set of used fields std::map > m_groupBoxes; std::map > m_newGroupBoxes; ///< used temporarily to store the new(complete) set of group boxes std::map > m_objectKeywordGroupUiNameExpandedState; diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiTableViewQModel.cpp b/Fwk/AppFwk/cafUserInterface/cafPdmUiTableViewQModel.cpp index a6162756c9..7362b06e6d 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiTableViewQModel.cpp +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiTableViewQModel.cpp @@ -396,12 +396,8 @@ void PdmUiTableViewQModel::setArrayFieldAndBuildEditors(PdmChildArrayFieldHandle const std::vector& uiItems = configForFirstObject.uiItems(); - // Set all fieldViews to be unvisited - std::map::iterator it; - for (it = m_fieldEditors.begin(); it != m_fieldEditors.end(); ++it) - { - it->second->setUiField(nullptr); - } + + std::set usedFieldKeywords; m_modelColumnIndexToFieldIndex.clear(); @@ -416,14 +412,15 @@ void PdmUiTableViewQModel::setArrayFieldAndBuildEditors(PdmChildArrayFieldHandle PdmUiFieldEditorHandle* fieldEditor = nullptr; // Find or create FieldEditor - it = m_fieldEditors.find(field->fieldHandle()->keyword()); + auto it = m_fieldEditors.find(field->fieldHandle()->keyword()); if (it == m_fieldEditors.end()) { - fieldEditor = PdmUiFieldEditorHelper::fieldEditorForField(field, configName); + fieldEditor = PdmUiFieldEditorHelper::createFieldEditorForField(field, configName); if (fieldEditor) { + fieldEditor->setUiField(field); m_fieldEditors[field->fieldHandle()->keyword()] = fieldEditor; } } @@ -434,7 +431,7 @@ void PdmUiTableViewQModel::setArrayFieldAndBuildEditors(PdmChildArrayFieldHandle if (fieldEditor) { - fieldEditor->setUiField(field); + usedFieldKeywords.insert(field->fieldHandle()->keyword()); //TODO: Create/update is not required at this point, as UI is recreated in getEditorWidgetAndTransferOwnership() // Can be moved, but a move will require changes in PdmUiFieldEditorHandle @@ -447,13 +444,12 @@ void PdmUiTableViewQModel::setArrayFieldAndBuildEditors(PdmChildArrayFieldHandle } } - // Remove all fieldViews not mentioned by the configuration from the layout std::vector< QString > fvhToRemoveFromMap; - for (it = m_fieldEditors.begin(); it != m_fieldEditors.end(); ++it) + for (auto it = m_fieldEditors.begin(); it != m_fieldEditors.end(); ++it) { - if (it->second->uiField() == nullptr) + if (usedFieldKeywords.count(it->first) == 0) { PdmUiFieldEditorHandle* fvh = it->second; delete fvh; diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiToolBarEditor.cpp b/Fwk/AppFwk/cafUserInterface/cafPdmUiToolBarEditor.cpp index 4967f6aeb3..5577cde98f 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiToolBarEditor.cpp +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiToolBarEditor.cpp @@ -147,7 +147,7 @@ void PdmUiToolBarEditor::configureAndUpdateUi(const QString& uiConfigName) } else { - fieldEditor = caf::PdmUiFieldEditorHelper::fieldEditorForField(field->uiCapability(), uiConfigName); + fieldEditor = caf::PdmUiFieldEditorHelper::createFieldEditorForField(field->uiCapability(), uiConfigName); addSpace = true; } @@ -156,6 +156,7 @@ void PdmUiToolBarEditor::configureAndUpdateUi(const QString& uiConfigName) if (fieldEditor) { m_fieldViews[field->keyword()] = fieldEditor; + fieldEditor->setUiField(uiFieldHandle); fieldEditor->createWidgets(nullptr); m_actions.push_back(m_toolbar->addWidget(fieldEditor->editorWidget())); @@ -166,7 +167,6 @@ void PdmUiToolBarEditor::configureAndUpdateUi(const QString& uiConfigName) m_toolbar->addWidget(widget); } - fieldEditor->setUiField(uiFieldHandle); fieldEditor->updateUi(uiConfigName); } }