#12925: Improve caf editors, label links and nested settings write

Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiLabel.cpp

@@ -62,4 +62,20 @@ bool PdmUiLabel::isUiGroup() const
     return false;
 }
 
+//--------------------------------------------------------------------------------------------------
+///
+//--------------------------------------------------------------------------------------------------
+void PdmUiLabel::setLinkActivatedCallback( const std::function<void( const QString& )>& callback )
+{
+    m_linkActivatedCallback = callback;
+}
+
+//--------------------------------------------------------------------------------------------------
+///
+//--------------------------------------------------------------------------------------------------
+const std::function<void( const QString& )>& PdmUiLabel::linkActivatedCallback() const
+{
+    return m_linkActivatedCallback;
+}
+
 } // End of namespace caf

Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafPdmUiLabel.h

@@ -38,6 +38,8 @@
 
 #include "cafPdmUiItem.h"
 
+#include <functional>
+
 namespace caf
 {
 //==================================================================================================
@@ -51,6 +53,13 @@ public:
     explicit PdmUiLabel( const QString& labelText );
 
     bool isUiGroup() const override;
+
+    // Called with the href when a hyperlink in a rich text label is clicked
+    void setLinkActivatedCallback( const std::function<void( const QString& )>& callback );
+    const std::function<void( const QString& )>& linkActivatedCallback() const;
+
+private:
+    std::function<void( const QString& )> m_linkActivatedCallback;
 };
 
 } // End of namespace caf

Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmSettings.cpp

@@ -117,14 +117,11 @@ void PdmSettings::writeFieldsToApplicationStore( const caf::PdmObjectHandle* obj
             caf::PdmObjectHandle* child = children[childIdx];
             if ( !child ) continue;
 
-            QString subContext;
-            if ( context.isEmpty() )
-            {
-                caf::PdmXmlObjectHandle* xmlObjHandle = xmlObj( child );
-                if ( !xmlObjHandle ) continue;
+            caf::PdmXmlObjectHandle* xmlObjHandle = xmlObj( child );
+            if ( !xmlObjHandle ) continue;
 
-                subContext = xmlObjHandle->classKeyword() + "/";
-            }
+            // Use the same nested context as when reading, to support children of children
+            const QString subContext = context + xmlObjHandle->classKeyword() + "/";
 
             writeFieldsToApplicationStore( child, subContext );
         }

Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests/CMakeLists.txt

@@ -20,9 +20,13 @@ include_directories(${CMAKE_CURRENT_SOURCE_DIR} # required for gtest-all.cpp
 )
 
 set(PROJECT_FILES
-    cafPdmXml_UnitTests.cpp gtest/gtest-all.cpp cafPdmXmlBasicTest.cpp
-    cafPdmAdvancedTemplateTest.cpp cafPdmXmlNumberTest.cpp
+    cafPdmXml_UnitTests.cpp
+    gtest/gtest-all.cpp
+    cafPdmXmlBasicTest.cpp
+    cafPdmAdvancedTemplateTest.cpp
+    cafPdmXmlNumberTest.cpp
     cafPdmPtrArrayTest.cpp
+    cafPdmSettingsTest.cpp
 )
 
 qt_add_executable(${PROJECT_NAME} MACOSX_BUNDLE ${PROJECT_FILES})

Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests/cafPdmSettingsTest.cpp

@@ -0,0 +1,94 @@
+#include "gtest/gtest.h"
+
+#include "cafPdmChildField.h"
+#include "cafPdmDataValueField.h"
+#include "cafPdmObjectHandle.h"
+#include "cafPdmSettings.h"
+#include "cafPdmXmlObjectHandle.h"
+#include "cafPdmXmlObjectHandleMacros.h"
+
+#include <QCoreApplication>
+#include <QDir>
+#include <QSettings>
+
+class SettingsLeaf : public caf::PdmObjectHandle, public caf::PdmXmlObjectHandle
+{
+    CAF_PDM_XML_HEADER_INIT;
+
+public:
+    SettingsLeaf()
+        : PdmObjectHandle()
+        , PdmXmlObjectHandle( this, false )
+    {
+        CAF_PDM_XML_InitField( &m_value, "Value" );
+    }
+
+    caf::PdmDataValueField<int> m_value;
+};
+CAF_PDM_XML_SOURCE_INIT( SettingsLeaf, "SettingsLeaf" );
+
+class SettingsMiddle : public caf::PdmObjectHandle, public caf::PdmXmlObjectHandle
+{
+    CAF_PDM_XML_HEADER_INIT;
+
+public:
+    SettingsMiddle()
+        : PdmObjectHandle()
+        , PdmXmlObjectHandle( this, false )
+    {
+        CAF_PDM_XML_InitField( &m_leaf, "Leaf" );
+        m_leaf = new SettingsLeaf;
+    }
+
+    ~SettingsMiddle() { delete m_leaf(); }
+
+    caf::PdmChildField<SettingsLeaf*> m_leaf;
+};
+CAF_PDM_XML_SOURCE_INIT( SettingsMiddle, "SettingsMiddle" );
+
+class SettingsRoot : public caf::PdmObjectHandle, public caf::PdmXmlObjectHandle
+{
+    CAF_PDM_XML_HEADER_INIT;
+
+public:
+    SettingsRoot()
+        : PdmObjectHandle()
+        , PdmXmlObjectHandle( this, false )
+    {
+        CAF_PDM_XML_InitField( &m_middle, "Middle" );
+        m_middle = new SettingsMiddle;
+    }
+
+    ~SettingsRoot() { delete m_middle(); }
+
+    caf::PdmChildField<SettingsMiddle*> m_middle;
+};
+CAF_PDM_XML_SOURCE_INIT( SettingsRoot, "SettingsRoot" );
+
+//--------------------------------------------------------------------------------------------------
+///
+//--------------------------------------------------------------------------------------------------
+TEST( PdmSettings, NestedChildrenRoundTrip )
+{
+    QSettings::setDefaultFormat( QSettings::IniFormat );
+    QSettings::setPath( QSettings::IniFormat, QSettings::UserScope, QDir::tempPath() );
+    QCoreApplication::setOrganizationName( "cafPdmSettingsTest" );
+    QCoreApplication::setApplicationName( "cafPdmSettingsTest" );
+    QSettings().clear();
+
+    {
+        SettingsRoot root;
+        root.m_middle()->m_leaf()->m_value = 42;
+        caf::PdmSettings::writeFieldsToApplicationStore( &root );
+    }
+
+    // Written with the same nested context as used when reading
+    EXPECT_TRUE( QSettings().contains( "SettingsMiddle/SettingsLeaf/Value" ) );
+
+    SettingsRoot restored;
+    restored.m_middle()->m_leaf()->m_value = 0;
+    caf::PdmSettings::readFieldsFromApplicationStore( &restored );
+    EXPECT_EQ( 42, restored.m_middle()->m_leaf()->m_value() );
+
+    QSettings().clear();
+}

Fwk/AppFwk/cafUserInterface/cafPdmUiFormLayoutObjectEditor.cpp

@@ -488,6 +488,13 @@ QLabel* caf::PdmUiFormLayoutObjectEditor::createLabel( QWidget* parent, const Pd
     qLabel->setText( label.uiName( uiConfigName ) );
     qLabel->setWordWrap( true );
 
+    if ( auto callback = label.linkActivatedCallback() )
+    {
+        qLabel->setTextFormat( Qt::RichText );
+        qLabel->setTextInteractionFlags( Qt::LinksAccessibleByMouse );
+        QObject::connect( qLabel, &QLabel::linkActivated, qLabel, [callback]( const QString& link ) { callback( link ); } );
+    }
+
     m_labels.push_back( qLabel );
     return qLabel;
 }
@@ -562,6 +569,14 @@ caf::PdmUiFieldEditorHandle* caf::PdmUiFormLayoutObjectEditor::findOrCreateField
 
     std::map<PdmFieldHandle*, PdmUiFieldEditorHandle*>::iterator it = m_fieldViews.find( field->fieldHandle() );
 
+    // The editor's field was deleted and a new field was allocated at the same address
+    if ( it != m_fieldViews.end() && it->second->uiField() != field )
+    {
+        delete it->second;
+        m_fieldViews.erase( it );
+        it = m_fieldViews.end();
+    }
+
     if ( it == m_fieldViews.end() )
     {
         fieldEditor = PdmUiFieldEditorHelper::createFieldEditorForField( field, uiConfigName );

Fwk/AppFwk/cafUserInterface/cafPdmUiListEditor.cpp

@@ -76,8 +76,24 @@ public:
 
     void setItemsEditable( bool isEditable ) { m_isItemsEditable = isEditable; }
 
+    void setIcons( const QList<QIcon>& icons ) { m_icons = icons; }
+
+    QVariant data( const QModelIndex& index, int role ) const override
+    {
+        if ( role == Qt::DecorationRole )
+        {
+            if ( index.isValid() && index.row() < m_icons.size() && !m_icons[index.row()].isNull() )
+            {
+                return m_icons[index.row()];
+            }
+            return QVariant();
+        }
+        return QStringListModel::data( index, role );
+    }
+
 private:
-    bool m_isItemsEditable;
+    bool         m_isItemsEditable;
+    QList<QIcon> m_icons;
 };
 
 //==================================================================================================
@@ -194,6 +210,19 @@ void PdmUiListEditor::configureAndUpdateUi( const QString& uiConfigName )
         strListModel->setItemsEditable( false );
         QModelIndex currentItem = m_listView->selectionModel()->currentIndex();
         QStringList texts       = PdmOptionItemInfo::extractUiTexts( options );
+
+        QList<QIcon> icons;
+        for ( const auto& option : options )
+        {
+            auto icon = option.icon();
+            icons.push_back( icon ? *icon : QIcon() );
+        }
+        strListModel->setIcons( icons );
+
+        if ( auto iconSize = uiField()->attribute<QSize>( Keys::ICON_SIZE, uiConfigName ) )
+        {
+            m_listView->setIconSize( *iconSize );
+        }
         strListModel->setStringList( texts );
 
         QVariant fieldValue = uiField()->uiValue();
@@ -247,6 +276,7 @@ void PdmUiListEditor::configureAndUpdateUi( const QString& uiConfigName )
         QVariant       fieldValue  = uiField()->uiValue();
         QStringList    texts       = fieldValue.toStringList();
         texts.push_back( "" );
+        strListModel->setIcons( {} );
         strListModel->setStringList( texts );
 
         strListModel->setItemsEditable( true );

Fwk/AppFwk/cafUserInterface/cafPdmUiListEditor.h

@@ -66,13 +66,15 @@ public:
         static inline const QString BASE_COLOR                  = QStringLiteral( "baseColor" );
         static inline const QString QSS_STATE                   = QStringLiteral( "qssState" );
         static inline const QString ALLOW_HORIZONTAL_SCROLL_BAR = QStringLiteral( "allowHorizontalScrollBar" );
+        static inline const QString ICON_SIZE                   = QStringLiteral( "iconSize" ); // QSize
     };
 
     // Set of all supported attributes for validation
     inline static const std::set<QString> SUPPORTED_ATTRIBUTES = { Keys::HEIGHT_HINT,
                                                                    Keys::BASE_COLOR,
                                                                    Keys::QSS_STATE,
-                                                                   Keys::ALLOW_HORIZONTAL_SCROLL_BAR };
+                                                                   Keys::ALLOW_HORIZONTAL_SCROLL_BAR,
+                                                                   Keys::ICON_SIZE };
 
 protected:
     QWidget* createEditorWidget( QWidget* parent ) override;

Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt

@@ -25,6 +25,7 @@ set(PROJECT_FILES
     cafPdmUiCheckBoxAndTextEditorTest.cpp
     cafPdmUiPropertyViewDialogTest.cpp
     cafPdmUiEditorHandleTest.cpp
+    cafPdmUiFormLayoutObjectEditorTest.cpp
     cafIconProviderTest.cpp
     cafProgressInfoTest.cpp
     gtest/gtest-all.cpp

Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafPdmUiFormLayoutObjectEditorTest.cpp

@@ -0,0 +1,97 @@
+#include "gtest/gtest.h"
+
+#include "cafPdmChildField.h"
+#include "cafPdmField.h"
+#include "cafPdmObject.h"
+#include "cafPdmUiOrdering.h"
+#include "cafPdmUiPropertyView.h"
+
+#include <new>
+
+using namespace caf;
+
+namespace
+{
+alignas( std::max_align_t ) unsigned char childStorage[4096];
+} // namespace
+
+//--------------------------------------------------------------------------------------------------
+/// Always allocated at the same address, to reproduce a deleted object being replaced by one at the same address
+//--------------------------------------------------------------------------------------------------
+class FormLayoutTestChild : public caf::PdmObject
+{
+    CAF_PDM_HEADER_INIT;
+
+public:
+    FormLayoutTestChild()
+    {
+        CAF_PDM_InitObject( "FormLayoutTestChild" );
+        CAF_PDM_InitField( &m_text, "Text", QString( "text" ), "Text" );
+    }
+
+    static void* operator new( size_t size )
+    {
+        EXPECT_LE( size, sizeof( childStorage ) );
+        return childStorage;
+    }
+    static void operator delete( void* ) {}
+
+    caf::PdmField<QString> m_text;
+};
+CAF_PDM_SOURCE_INIT( FormLayoutTestChild, "FormLayoutTestChild" );
+
+//--------------------------------------------------------------------------------------------------
+///
+//--------------------------------------------------------------------------------------------------
+class FormLayoutTestParent : public caf::PdmObject
+{
+    CAF_PDM_HEADER_INIT;
+
+public:
+    FormLayoutTestParent()
+    {
+        CAF_PDM_InitObject( "FormLayoutTestParent" );
+        CAF_PDM_InitFieldNoDefault( &m_child, "Child", "Child" );
+        m_child = new FormLayoutTestChild;
+    }
+
+    void replaceChild()
+    {
+        delete m_child();
+        m_child = new FormLayoutTestChild;
+    }
+
+    caf::PdmChildField<FormLayoutTestChild*> m_child;
+
+protected:
+    void defineUiOrdering( QString uiConfigName, caf::PdmUiOrdering& uiOrdering ) override
+    {
+        uiOrdering.add( &m_child->m_text );
+        uiOrdering.skipRemainingFields();
+    }
+};
+CAF_PDM_SOURCE_INIT( FormLayoutTestParent, "FormLayoutTestParent" );
+
+//--------------------------------------------------------------------------------------------------
+/// A field editor bound to a deleted field must not be reused for a new field at the same address
+//--------------------------------------------------------------------------------------------------
+TEST( PdmUiFormLayoutObjectEditorTest, FieldReplacedAtSameAddress )
+{
+    FormLayoutTestParent parent;
+
+    PdmUiPropertyView view;
+    view.showProperties( &parent );
+
+    auto oldField = &parent.m_child->m_text;
+    ASSERT_EQ( 1u, oldField->uiCapability()->connectedEditors().size() );
+
+    parent.replaceChild();
+    ASSERT_EQ( oldField, &parent.m_child->m_text );
+    ASSERT_TRUE( parent.m_child->m_text.uiCapability()->connectedEditors().empty() );
+
+    parent.updateConnectedEditors();
+
+    EXPECT_EQ( 1u, parent.m_child->m_text.uiCapability()->connectedEditors().size() );
+
+    view.showProperties( nullptr );
+}