Skip to content

Simplify field specializations #849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
magnesj wants to merge 21 commits into from
Closed

Simplify field specializations #849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1ecc456
#834 Add cafPdmFieldTraits.h with ADL-based pdmToVariant/pdmFromVaria…
magnesj Apr 2, 2026
77e5b03
#834 Migrate cafPdmCvf types and call sites to pdmToVariant/pdmFromVa…
magnesj Apr 2, 2026
f02fbee
#834 Switch PdmDataValueField, PdmProxyValueField, and UI editors to …
magnesj Apr 2, 2026
411b98f
#834 Update UI framework to use pdmToVariant/pdmFromVariant/pdmVarian…
magnesj Apr 2, 2026
1ffbc4b
#834 Remove cafInternalPdmValueFieldSpecializations.h and update rema…
magnesj Apr 2, 2026
6061217
#834 Fix duplicate logic and add vector reserve
magnesj Apr 3, 2026
0b3db7c
Add unit tests for ADL field traits and include cafPdmCvf_UnitTests i…
magnesj Apr 3, 2026
c811102
Fix std::optional support in pdmToVariant/pdmFromVariant/pdmVariantEqual
magnesj Apr 3, 2026
2554579
Delete generated XML test files after ReadWrite test completes
magnesj Apr 3, 2026
596ec6f
Fixes by clang-format
magnesj Apr 3, 2026
fe5260e
Reduce include weight of cafPdmFieldTraits.h for faster PDM compilation
magnesj Apr 3, 2026
21cb1a3
Simplify and clean up field traits after /simplify review
magnesj Apr 3, 2026
bfd12be
Remove redundant PdmUiFieldSpecialization specializations for PdmPoin…
magnesj Apr 3, 2026
34512cd
Add PDM compile-time benchmark target
magnesj Apr 3, 2026
4681de1
Add build-bench to .gitignore
magnesj Apr 3, 2026
77b107f
Hook cafPdm_CompileBench into top-level CMakeLists under RESINSIGHT_I…
magnesj Apr 3, 2026
5d326dc
Reduce include weight of cafIconProvider.h and cafPdmUiItem.h
magnesj Apr 3, 2026
53888de
Fix build breakage from include-weight reduction in cafPdmUiItem.h
magnesj Apr 4, 2026
4a4617e
Fixes by cmake-format
magnesj Apr 4, 2026
2087b58
Fixes by clang-format
magnesj Apr 4, 2026
c7ec12f
Fix pdmFromVariant<optional<T>> to use isValid() instead of string check
magnesj Apr 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Ankh.NoLoad
.vs
out
build
build-bench
CMakeSettings.json
enc_temp_folder
CMakeUserPresets.json
Expand Down
1 change: 1 addition & 0 deletions ApplicationLibCode/Application/Tools/RiaOptionItemFactory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include "RifEclipseSummaryAddressDefines.h"

#include "cafPdmOptionItemInfo.h"
#include "cafPdmUiItem.h"

#include <QList>
Expand Down
1 change: 1 addition & 0 deletions ApplicationLibCode/Application/Tools/RiaQDateTimeTools.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <QString>
#include <QTime>

#include "cafPdmOptionItemInfo.h"
#include "cafPdmUiItem.h"

#include <cvfAssert.h>
Expand Down
6 changes: 3 additions & 3 deletions ApplicationLibCode/Commands/3dView/RicApplyUserDefinedCameraFeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void RicApplyUserDefinedCameraFeature::readCameraFromSettings( cvf::Vec3d& eye,
QVariant upVariant = settings.value( RicStoreUserDefinedCameraFeature::upName() );
if ( eyeVariant.isNull() || vrpVariant.isNull() || upVariant.isNull() ) return;

caf::PdmValueFieldSpecialization<cvf::Vec3d>::setFromVariant( eyeVariant, eye );
caf::PdmValueFieldSpecialization<cvf::Vec3d>::setFromVariant( vrpVariant, vrp );
caf::PdmValueFieldSpecialization<cvf::Vec3d>::setFromVariant( upVariant, up );
caf::pdmFromVariant( eyeVariant, eye );
caf::pdmFromVariant( vrpVariant, vrp );
caf::pdmFromVariant( upVariant, up );
}
6 changes: 3 additions & 3 deletions ApplicationLibCode/Commands/3dView/RicStoreUserDefinedCameraFeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@ void RicStoreUserDefinedCameraFeature::onActionTriggered( bool isChecked )

camera->toLookAt( &eye, &vrp, &up );

QVariant eyeVariant = caf::PdmValueFieldSpecialization<cvf::Vec3d>::convert( eye );
QVariant eyeVariant = caf::pdmToVariant( eye );
settings.setValue( RicStoreUserDefinedCameraFeature::eyeName(), eyeVariant );

QVariant vrpVariant = caf::PdmValueFieldSpecialization<cvf::Vec3d>::convert( vrp );
QVariant vrpVariant = caf::pdmToVariant( vrp );
settings.setValue( RicStoreUserDefinedCameraFeature::viewReferencePointName(), vrpVariant );

QVariant upVariant = caf::PdmValueFieldSpecialization<cvf::Vec3d>::convert( up );
QVariant upVariant = caf::pdmToVariant( up );
settings.setValue( RicStoreUserDefinedCameraFeature::upName(), upVariant );
Comment on lines +107 to 114
Copy link
Copy Markdown

@qodo-code-review qodo-code-review bot Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Missing vec3d traits include 🐞 Bug ≡ Correctness

RicStoreUserDefinedCameraFeature.cpp and RicApplyUserDefinedCameraFeature.cpp call
caf::pdmToVariant/pdmFromVariant for cvf::Vec3d but do not include the header that declares the
cvf::Vec3d overloads, which will likely fail to compile. Even if another header incidentally
provides the generic template, it would store cvf::Vec3d as a metatype QVariant instead of the
intended QString format, breaking persistence via QSettings.
Agent Prompt 
### Issue description
`RicStoreUserDefinedCameraFeature.cpp` and `RicApplyUserDefinedCameraFeature.cpp` now call `caf::pdmToVariant` / `caf::pdmFromVariant` for `cvf::Vec3d`, but they don’t include the header that declares the `cvf::Vec3d` overloads. This is likely a compile error (unresolved symbol at compile-time), and it also risks changing the persisted QSettings format if the generic template overload is chosen.

### Issue Context
The `cvf::Vec3d` overloads live in `Fwk/AppFwk/cafPdmCvf/cafPdmCoreVec3d.h` and convert to/from a `QString`-backed `QVariant`, which is appropriate for `QSettings`.

### Fix Focus Areas
- ApplicationLibCode/Commands/3dView/RicStoreUserDefinedCameraFeature.cpp[19-33]
- ApplicationLibCode/Commands/3dView/RicApplyUserDefinedCameraFeature.cpp[19-32]

### Suggested change
Add:
```cpp
#include "cafPdmCoreVec3d.h"
```
(in both translation units, or in a common header they both include) so the intended overloads are visible.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


RiuMainWindow::instance()->refreshViewActions();
Expand Down
2 changes: 1 addition & 1 deletion ApplicationLibCode/Commands/AnnotationCommands/RicTextAnnotation3dEditor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ void RicTextAnnotation3dEditor::updatePoint( caf::PdmUiFieldHandle* uiField, con

cvf::Vec3d domainPos = dispXf->transformToDomainCoord( newPos );
domainPos.z() = -domainPos.z();
QVariant originVariant = caf::PdmValueFieldSpecialization<cvf::Vec3d>::convert( domainPos );
QVariant originVariant = caf::pdmToVariant( domainPos );

caf::PdmUiCommandSystemProxy::instance()->setUiValueToField( uiField, originVariant );
}
2 changes: 1 addition & 1 deletion ...onLibCode/Commands/WellPathCommands/PointTangentManipulator/RicPolylineTarget3dEditor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ void RicPolylineTarget3dEditor::slotUpdated( const cvf::Vec3d& origin, const cvf

cvf::Vec3d domainOrigin = dispXf->transformToDomainCoord( origin );
domainOrigin.z() = -domainOrigin.z();
QVariant originVariant = caf::PdmValueFieldSpecialization<cvf::Vec3d>::convert( domainOrigin );
QVariant originVariant = caf::pdmToVariant( domainOrigin );

caf::PdmUiCommandSystemProxy::instance()->setUiValueToField( target->targetPointUiCapability(), originVariant );
}
Expand Down
1 change: 1 addition & 0 deletions ApplicationLibCode/ProjectDataModel/RimDataSourceSteppingTools.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include "RifEclipseSummaryAddressDefines.h"

#include "cafPdmOptionItemInfo.h"
#include "cafPdmUiItem.h"
#include "cafPdmValueField.h"

Expand Down
2 changes: 1 addition & 1 deletion ApplicationLibCode/ProjectDataModel/WellPath/RimWellPathGeometryDef.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ void RimWellPathGeometryDef::fieldChangedByUi( const caf::PdmFieldHandle* change
linkedDefs.end() );

cvf::Vec3d oldPos;
caf::PdmValueFieldSpecialization<cvf::Vec3d>::setFromVariant( oldValue, oldPos );
caf::pdmFromVariant( oldValue, oldPos );

auto delta = m_referencePointUtmXyd() - oldPos;

Expand Down
5 changes: 5 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,9 @@ option(RESINSIGHT_INCLUDE_APPFWK_TESTS "Enable AppFwk Tests" OFF)
mark_as_advanced(FORCE RESINSIGHT_INCLUDE_APPFWK_TESTS)

if(RESINSIGHT_INCLUDE_APPFWK_TESTS)
# Compile-time benchmark
add_subdirectory(Fwk/AppFwk/cafProjectDataModel/cafPdm_CompileBench)

# Unit Tests
add_subdirectory(Fwk/AppFwk/cafProjectDataModel/cafProjectDataModel_UnitTests)
add_subdirectory(
Expand All @@ -997,6 +1000,7 @@ if(RESINSIGHT_INCLUDE_APPFWK_TESTS)
add_subdirectory(Fwk/AppFwk/cafProjectDataModel/cafPdmXml/cafPdmXml_UnitTests)
add_subdirectory(Fwk/AppFwk/cafPdmScripting/cafPdmScripting_UnitTests)
add_subdirectory(Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests)
add_subdirectory(Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests)

# Executables
add_subdirectory(Fwk/AppFwk/cafTests/cafTestApplication)
Expand All @@ -1010,6 +1014,7 @@ if(RESINSIGHT_INCLUDE_APPFWK_TESTS)
cafPdmXml_UnitTests
cafPdmScripting_UnitTests
cafUserInterface_UnitTests
cafPdmCvf_UnitTests
cafTestApplication
cafTestCvfApplication
)
Expand Down
39 changes: 15 additions & 24 deletions Fwk/AppFwk/cafPdmCvf/cafPdmCoreColor3f.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,40 +39,31 @@
#include "cvfBase.h"
#include "cvfColor3.h"

#include "cafInternalPdmValueFieldSpecializations.h"
#include "cafPdmFieldTraits.h"
#include "cafPdmXmlColor3f.h"

#include <QColor>

namespace caf
{
//==================================================================================================
/// Partial specialization for PdmValueFieldSpecialization< cvf::Color3f >
//==================================================================================================

template <>
class PdmValueFieldSpecialization<cvf::Color3f>
inline QVariant pdmToVariant( const cvf::Color3f& value )
{
public:
static QVariant convert( const cvf::Color3f& value )
{
QColor col;
col.setRgbF( value.r(), value.g(), value.b() );

return col;
}

static void setFromVariant( const QVariant& variantValue, cvf::Color3f& value )
{
QColor col = variantValue.value<QColor>();
QColor col;
col.setRgbF( value.r(), value.g(), value.b() );
return col;
}

value.set( col.redF(), col.greenF(), col.blueF() );
}
inline void pdmFromVariant( const QVariant& v, cvf::Color3f& out )
{
QColor col = v.value<QColor>();
out.set( col.redF(), col.greenF(), col.blueF() );
}

static bool isEqual( const QVariant& variantValue, const QVariant& variantValue2 )
{
return variantValue == variantValue2;
}
template <>
struct PdmVariantEqualImpl<cvf::Color3f>
{
static bool equal( const QVariant& a, const QVariant& b ) { return a.value<QColor>() == b.value<QColor>(); }
};

} // end namespace caf
45 changes: 18 additions & 27 deletions Fwk/AppFwk/cafPdmCvf/cafPdmCoreMat4d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,43 +39,34 @@
#include "cvfBase.h"
#include "cvfMatrix4.h"

#include "cafInternalPdmValueFieldSpecializations.h"
#include "cafPdmFieldTraits.h"

#include "cafPdmXmlMat4d.h"

#include <QTextStream>

namespace caf
{
template <>
class PdmValueFieldSpecialization<cvf::Mat4d>
{
public:
/// Convert the field value into a QVariant
static QVariant convert( const cvf::Mat4d& value )
{
QString str;

QTextStream textStream( &str );
textStream << value;

return QVariant( str );
}

/// Set the field value from a QVariant
static void setFromVariant( const QVariant& variantValue, cvf::Mat4d& value )
{
QString str = variantValue.toString();

QTextStream textStream( &str );
inline QVariant pdmToVariant( const cvf::Mat4d& value )
{
QString str;
QTextStream textStream( &str );
textStream << value;
return QVariant( str );
}

textStream >> value;
}
inline void pdmFromVariant( const QVariant& v, cvf::Mat4d& out )
{
QString str = v.toString();
QTextStream textStream( &str );
textStream >> out;
}

static bool isEqual( const QVariant& variantValue, const QVariant& variantValue2 )
{
return variantValue == variantValue2;
}
template <>
struct PdmVariantEqualImpl<cvf::Mat4d>
{
static bool equal( const QVariant& a, const QVariant& b ) { return a.toString() == b.toString(); }
};

} // end namespace caf
45 changes: 18 additions & 27 deletions Fwk/AppFwk/cafPdmCvf/cafPdmCoreVec3d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
#include "cvfBase.h"
#include "cvfVector3.h"

#include "cafInternalPdmValueFieldSpecializations.h"
#include "cafPdmFieldTraits.h"

#include "cafPdmXmlVec3d.h"

Expand All @@ -49,35 +49,26 @@ Q_DECLARE_METATYPE( cvf::Vec3d );

namespace caf
{
template <>
class PdmValueFieldSpecialization<cvf::Vec3d>
{
public:
/// Convert the field value into a QVariant
static QVariant convert( const cvf::Vec3d& value )
{
QString str;

QTextStream textStream( &str );
textStream << value;

return QVariant( str );
}

/// Set the field value from a QVariant
static void setFromVariant( const QVariant& variantValue, cvf::Vec3d& value )
{
QString str = variantValue.toString();

QTextStream textStream( &str );
inline QVariant pdmToVariant( const cvf::Vec3d& value )
{
QString str;
QTextStream textStream( &str );
textStream << value;
return QVariant( str );
}

textStream >> value;
}
inline void pdmFromVariant( const QVariant& v, cvf::Vec3d& out )
{
QString str = v.toString();
QTextStream textStream( &str );
textStream >> out;
}

static bool isEqual( const QVariant& variantValue, const QVariant& variantValue2 )
{
return variantValue == variantValue2;
}
template <>
struct PdmVariantEqualImpl<cvf::Vec3d>
{
static bool equal( const QVariant& a, const QVariant& b ) { return a.toString() == b.toString(); }
};

} // end namespace caf
18 changes: 16 additions & 2 deletions Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreColor3fTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,30 @@ TEST( VariantTest, PdmCoreColor3f )
float b = 0.18f;
cvf::Color3f myColor( r, g, b );

QVariant myVariant = caf::PdmValueFieldSpecialization<cvf::Color3f>::convert( myColor );
QVariant myVariant = caf::pdmToVariant( myColor );

cvf::Color3f decoded;
caf::PdmValueFieldSpecialization<cvf::Color3f>::setFromVariant( myVariant, decoded );
caf::pdmFromVariant( myVariant, decoded );

EXPECT_FLOAT_EQ( myColor.r(), decoded.r() );
EXPECT_FLOAT_EQ( myColor.g(), decoded.g() );
EXPECT_NEAR( myColor.b(), decoded.b(), 0.01 ); // For some reason, 0.18 is not close enough to use EXPECT_FLOAT_EQ
}

TEST( VariantEqualTest, PdmCoreColor3f )
{
cvf::Color3f a( 0.4f, 0.2f, 0.18f );
cvf::Color3f b( 0.4f, 0.2f, 0.18f );
cvf::Color3f c( 0.4f, 0.2f, 0.5f );

QVariant va = caf::pdmToVariant( a );
QVariant vb = caf::pdmToVariant( b );
QVariant vc = caf::pdmToVariant( c );

EXPECT_TRUE( caf::pdmVariantEqual<cvf::Color3f>( va, vb ) );
EXPECT_FALSE( caf::pdmVariantEqual<cvf::Color3f>( va, vc ) );
}

TEST( SerializeSeveralTest, PdmCoreColor3f )
{
float r = 0.4f;
Expand Down
4 changes: 2 additions & 2 deletions Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreMat4dTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ TEST( VariantTest, PdmCoreMat4d )
{
cvf::Mat4d myMatrix = createMatrix();

QVariant myVariant = caf::PdmValueFieldSpecialization<cvf::Mat4d>::convert( myMatrix );
QVariant myVariant = caf::pdmToVariant( myMatrix );

cvf::Mat4d decoded;
caf::PdmValueFieldSpecialization<cvf::Mat4d>::setFromVariant( myVariant, decoded );
caf::pdmFromVariant( myVariant, decoded );

EXPECT_TRUE( decoded.equals( myMatrix ) );
}
Expand Down
18 changes: 16 additions & 2 deletions Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreVec3dTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,28 @@ TEST( VariantTest, PdmCoreVec3d )

cvf::Vec3d myVector( a, b, c );

QVariant myVariant = caf::PdmValueFieldSpecialization<cvf::Vec3d>::convert( myVector );
QVariant myVariant = caf::pdmToVariant( myVector );

cvf::Vec3d decoded;
caf::PdmValueFieldSpecialization<cvf::Vec3d>::setFromVariant( myVariant, decoded );
caf::pdmFromVariant( myVariant, decoded );

EXPECT_TRUE( decoded.equals( myVector ) );
}

TEST( VariantEqualTest, PdmCoreVec3d )
{
cvf::Vec3d a( 1.0, 2.0, 3.0 );
cvf::Vec3d b( 1.0, 2.0, 3.0 );
cvf::Vec3d c( 1.0, 2.0, 4.0 );

QVariant va = caf::pdmToVariant( a );
QVariant vb = caf::pdmToVariant( b );
QVariant vc = caf::pdmToVariant( c );

EXPECT_TRUE( caf::pdmVariantEqual<cvf::Vec3d>( va, vb ) );
EXPECT_FALSE( caf::pdmVariantEqual<cvf::Vec3d>( va, vc ) );
}

TEST( SerializeSeveralTest, PdmCoreVec3d )
{
double a = 2.4;
Expand Down
Loading
Loading