From 99565f62f27bcf33b2d659c086be82c958379ae8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20St=C3=B8ren?= Date: Fri, 29 Jun 2018 16:21:54 +0200 Subject: [PATCH] AppFwk: Fix missing type guard in multi select field edit code. Added Assert in updateUI to prevent accidental recursive calling. --- .../cafPdmUiCommandSystemProxy.cpp | 7 ++++- .../cafPdmUiCore/cafPdmUiEditorHandle.cpp | 27 ++++++++++++++++++- .../cafPdmUiCore/cafPdmUiEditorHandle.h | 23 +++++++--------- .../cafPdmUiCore/cafPdmUiFieldHandle.cpp | 2 ++ .../cafPdmUiWidgetBasedObjectEditor.cpp | 7 +++-- 5 files changed, 48 insertions(+), 18 deletions(-) diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiCommandSystemProxy.cpp b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiCommandSystemProxy.cpp index a49d30bba0..0ec301f050 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiCommandSystemProxy.cpp +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiCommandSystemProxy.cpp @@ -86,10 +86,15 @@ void PdmUiCommandSystemProxy::setUiValueToField(PdmUiFieldHandle* uiFieldHandle, { // Handle editing multiple objects when several objects are selected PdmFieldHandle* editorField = uiFieldHandle->fieldHandle(); + const std::type_info& fieldOwnerTypeId = typeid( *editorField->ownerObject()); + std::vector fieldsToUpdate; fieldsToUpdate.push_back(editorField); // For current selection, find all fields with same keyword + // Todo: Should traverse the ui ordering and find all fields with same keyword and same ownerobject type. + // Until we do, fields embedded into the proerty panel fram a different object will not work with multiselection edit + // For now we only makes sure we have same ownerobject type { std::vector items; SelectionManager::instance()->selectedItems(items, SelectionManager::CURRENT); @@ -98,7 +103,7 @@ void PdmUiCommandSystemProxy::setUiValueToField(PdmUiFieldHandle* uiFieldHandle, for (size_t i = 0; i < items.size(); i++) { PdmObjectHandle* objectHandle = dynamic_cast(items[i]); - if (objectHandle) + if (objectHandle && typeid( *objectHandle) == fieldOwnerTypeId) { // An object is selected, find field with same keyword as the current field being edited PdmFieldHandle* fieldHandle = objectHandle->findField(editorField->keyword()); diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.cpp b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.cpp index 74009476b4..cc3153de1e 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.cpp +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.cpp @@ -43,7 +43,9 @@ namespace caf //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- -PdmUiEditorHandle::PdmUiEditorHandle() : m_pdmItem(nullptr) +PdmUiEditorHandle::PdmUiEditorHandle() + : m_pdmItem(nullptr) + , m_isConfiguringUi(false) { } @@ -56,6 +58,29 @@ PdmUiEditorHandle::~PdmUiEditorHandle() if (m_pdmItem) m_pdmItem->removeFieldEditor(this); } +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void PdmUiEditorHandle::updateUi(const QString& uiConfigName) +{ + CAF_ASSERT(!m_isConfiguringUi); + m_isConfiguringUi = true; + m_currentConfigName = uiConfigName; + this->configureAndUpdateUi(uiConfigName); + m_isConfiguringUi = false; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void PdmUiEditorHandle::updateUi() +{ + CAF_ASSERT(!m_isConfiguringUi); + m_isConfiguringUi = true; + this->configureAndUpdateUi(m_currentConfigName); + m_isConfiguringUi = false; +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.h b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.h index a5c4e23f99..a9301fc404 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.h +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiEditorHandle.h @@ -57,22 +57,15 @@ public: virtual ~PdmUiEditorHandle(); public: + + void updateUi(const QString& uiConfigName);; + + void updateUi();; + +protected: + // Interface to override: /// Virtual method to be overridden. Needs to set up the supplied widget /// with all signals etc to make it communicate with this object - - void updateUi(const QString& uiConfigName) - { - m_currentConfigName = uiConfigName; - this->configureAndUpdateUi(uiConfigName); - }; - - void updateUi() - { - this->configureAndUpdateUi(m_currentConfigName); - }; - -protected: // Interface to override: - /// Supposed to update all parts of the widgets, both visibility, sensitivity, decorations and field data virtual void configureAndUpdateUi(const QString& uiConfigName) = 0; @@ -86,6 +79,8 @@ private: friend PdmUiItem::~PdmUiItem(); PdmUiItem* m_pdmItem; QString m_currentConfigName; + + bool m_isConfiguringUi; }; diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiFieldHandle.cpp b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiFieldHandle.cpp index 1f6686a5dd..a912b29671 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiFieldHandle.cpp +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiFieldHandle.cpp @@ -24,6 +24,8 @@ PdmUiFieldHandle::PdmUiFieldHandle(PdmFieldHandle* owner, bool giveOwnership): //-------------------------------------------------------------------------------------------------- void PdmUiFieldHandle::notifyFieldChanged(const QVariant& oldFieldValue, const QVariant& newFieldValue) { + // Todo : Should use a virtual version of isElementEqual. The variant != operation will not work on user types + if (oldFieldValue != newFieldValue) { PdmFieldHandle* fieldHandle = this->fieldHandle(); diff --git a/Fwk/AppFwk/cafUserInterface/cafPdmUiWidgetBasedObjectEditor.cpp b/Fwk/AppFwk/cafUserInterface/cafPdmUiWidgetBasedObjectEditor.cpp index efcf94aaaa..5126c00ae1 100644 --- a/Fwk/AppFwk/cafUserInterface/cafPdmUiWidgetBasedObjectEditor.cpp +++ b/Fwk/AppFwk/cafUserInterface/cafPdmUiWidgetBasedObjectEditor.cpp @@ -390,9 +390,12 @@ void caf::PdmUiWidgetBasedObjectEditor::configureAndUpdateUi(const QString& uiCo } //-------------------------------------------------------------------------------------------------- -/// +/// Unused. Should probably remove //-------------------------------------------------------------------------------------------------- -void caf::PdmUiWidgetBasedObjectEditor::recursiveVerifyUniqueNames(const std::vector& uiItems, const QString& uiConfigName, std::set* fieldKeywordNames, std::set* groupNames) +void caf::PdmUiWidgetBasedObjectEditor::recursiveVerifyUniqueNames(const std::vector& uiItems, + const QString& uiConfigName, + std::set* fieldKeywordNames, + std::set* groupNames) { for (size_t i = 0; i < uiItems.size(); ++i) {