From e8223485b12cd6aeba4b2d4ede19612526c267a6 Mon Sep 17 00:00:00 2001 From: Magne Sjaastad Date: Wed, 8 Apr 2026 23:12:49 +0200 Subject: [PATCH] Fix ADL lookup for cvf pdmToVariant/pdmFromVariant overloads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move pdmToVariant/pdmFromVariant overloads for cvf::Color3f, Vec3d, Mat4d, and Mat3d from namespace caf to namespace cvf. Templates in cafInternalPdmFieldTypeSpecializations.h use "using caf::pdmToVariant" followed by an unqualified call, so ADL searches the argument's namespace (cvf). With the overloads only in caf, the default template was selected, causing cvf::Color3f to be stored raw in a QVariant instead of as QColor — breaking color field editing and producing QVariant::save warnings. All call sites now use the unqualified "using caf::pdmXxx; pdmXxx(value)" pattern so ADL finds the correct overload without depending on cvf:: as a visible prefix. Add AdlVariantTest to cafPdmCoreColor3fTest.cpp that exercises this exact pattern. Adds caf::toVariant and caf::fromVariant to cafPdmFieldTraits.h as thin wrappers that encapsulate the ADL two-step internally. Client code can now call caf::toVariant(x) directly instead of requiring a using-declaration at every call site. --- .../RicApplyUserDefinedCameraFeature.cpp | 6 ++-- .../RicStoreUserDefinedCameraFeature.cpp | 11 ++----- .../RicTextAnnotation3dEditor.cpp | 2 +- .../RicPolylineTarget3dEditor.cpp | 2 +- .../WellPath/RimWellPathGeometryDef.cpp | 2 +- Fwk/AppFwk/cafPdmCvf/cafPdmCoreColor3f.h | 13 ++++++-- Fwk/AppFwk/cafPdmCvf/cafPdmCoreMat4d.h | 13 ++++++-- Fwk/AppFwk/cafPdmCvf/cafPdmCoreVec3d.h | 13 ++++++-- .../cafPdmCoreColor3fTest.cpp | 33 ++++++++++++++++--- .../cafPdmCoreMat4dTest.cpp | 4 +-- .../cafPdmCoreVec3dTest.cpp | 10 +++--- .../cafPdmCvf/cafPdmMat3d/cafPdmCoreMat3d.h | 13 ++++++-- .../cafPdmCore/cafPdmFieldTraits.h | 23 +++++++++++++ 13 files changed, 107 insertions(+), 38 deletions(-) diff --git a/ApplicationLibCode/Commands/3dView/RicApplyUserDefinedCameraFeature.cpp b/ApplicationLibCode/Commands/3dView/RicApplyUserDefinedCameraFeature.cpp index 62d466744b..f209c46732 100644 --- a/ApplicationLibCode/Commands/3dView/RicApplyUserDefinedCameraFeature.cpp +++ b/ApplicationLibCode/Commands/3dView/RicApplyUserDefinedCameraFeature.cpp @@ -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::pdmFromVariant( eyeVariant, eye ); - caf::pdmFromVariant( vrpVariant, vrp ); - caf::pdmFromVariant( upVariant, up ); + caf::fromVariant( eyeVariant, eye ); + caf::fromVariant( vrpVariant, vrp ); + caf::fromVariant( upVariant, up ); } diff --git a/ApplicationLibCode/Commands/3dView/RicStoreUserDefinedCameraFeature.cpp b/ApplicationLibCode/Commands/3dView/RicStoreUserDefinedCameraFeature.cpp index df84e68d3f..0aaa9649f6 100644 --- a/ApplicationLibCode/Commands/3dView/RicStoreUserDefinedCameraFeature.cpp +++ b/ApplicationLibCode/Commands/3dView/RicStoreUserDefinedCameraFeature.cpp @@ -104,14 +104,9 @@ void RicStoreUserDefinedCameraFeature::onActionTriggered( bool isChecked ) camera->toLookAt( &eye, &vrp, &up ); - QVariant eyeVariant = caf::pdmToVariant( eye ); - settings.setValue( RicStoreUserDefinedCameraFeature::eyeName(), eyeVariant ); - - QVariant vrpVariant = caf::pdmToVariant( vrp ); - settings.setValue( RicStoreUserDefinedCameraFeature::viewReferencePointName(), vrpVariant ); - - QVariant upVariant = caf::pdmToVariant( up ); - settings.setValue( RicStoreUserDefinedCameraFeature::upName(), upVariant ); + settings.setValue( RicStoreUserDefinedCameraFeature::eyeName(), caf::toVariant( eye ) ); + settings.setValue( RicStoreUserDefinedCameraFeature::viewReferencePointName(), caf::toVariant( vrp ) ); + settings.setValue( RicStoreUserDefinedCameraFeature::upName(), caf::toVariant( up ) ); RiuMainWindow::instance()->refreshViewActions(); } diff --git a/ApplicationLibCode/Commands/AnnotationCommands/RicTextAnnotation3dEditor.cpp b/ApplicationLibCode/Commands/AnnotationCommands/RicTextAnnotation3dEditor.cpp index 2f89c25262..d76e466f2d 100644 --- a/ApplicationLibCode/Commands/AnnotationCommands/RicTextAnnotation3dEditor.cpp +++ b/ApplicationLibCode/Commands/AnnotationCommands/RicTextAnnotation3dEditor.cpp @@ -177,7 +177,7 @@ void RicTextAnnotation3dEditor::updatePoint( caf::PdmUiFieldHandle* uiField, con cvf::Vec3d domainPos = dispXf->transformToDomainCoord( newPos ); domainPos.z() = -domainPos.z(); - QVariant originVariant = caf::pdmToVariant( domainPos ); + QVariant originVariant = caf::toVariant( domainPos ); caf::PdmUiCommandSystemProxy::instance()->setUiValueToField( uiField, originVariant ); } diff --git a/ApplicationLibCode/Commands/WellPathCommands/PointTangentManipulator/RicPolylineTarget3dEditor.cpp b/ApplicationLibCode/Commands/WellPathCommands/PointTangentManipulator/RicPolylineTarget3dEditor.cpp index 716372a2b3..3ad7579026 100644 --- a/ApplicationLibCode/Commands/WellPathCommands/PointTangentManipulator/RicPolylineTarget3dEditor.cpp +++ b/ApplicationLibCode/Commands/WellPathCommands/PointTangentManipulator/RicPolylineTarget3dEditor.cpp @@ -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::pdmToVariant( domainOrigin ); + QVariant originVariant = caf::toVariant( domainOrigin ); caf::PdmUiCommandSystemProxy::instance()->setUiValueToField( target->targetPointUiCapability(), originVariant ); } diff --git a/ApplicationLibCode/ProjectDataModel/WellPath/RimWellPathGeometryDef.cpp b/ApplicationLibCode/ProjectDataModel/WellPath/RimWellPathGeometryDef.cpp index 3878a6abe4..d58ac8a538 100644 --- a/ApplicationLibCode/ProjectDataModel/WellPath/RimWellPathGeometryDef.cpp +++ b/ApplicationLibCode/ProjectDataModel/WellPath/RimWellPathGeometryDef.cpp @@ -520,7 +520,7 @@ void RimWellPathGeometryDef::fieldChangedByUi( const caf::PdmFieldHandle* change linkedDefs.end() ); cvf::Vec3d oldPos; - caf::pdmFromVariant( oldValue, oldPos ); + caf::fromVariant( oldValue, oldPos ); auto delta = m_referencePointUtmXyd() - oldPos; diff --git a/Fwk/AppFwk/cafPdmCvf/cafPdmCoreColor3f.h b/Fwk/AppFwk/cafPdmCvf/cafPdmCoreColor3f.h index df04072f92..ada5e3998a 100644 --- a/Fwk/AppFwk/cafPdmCvf/cafPdmCoreColor3f.h +++ b/Fwk/AppFwk/cafPdmCvf/cafPdmCoreColor3f.h @@ -44,22 +44,29 @@ #include -namespace caf +// pdmToVariant/pdmFromVariant in namespace cvf so ADL finds them when called via caf::toVariant/ +// caf::fromVariant — ADL searches the argument's namespace (cvf) in addition to caf. +namespace cvf { -inline QVariant pdmToVariant( const cvf::Color3f& value ) +inline QVariant pdmToVariant( const Color3f& value ) { QColor col; col.setRgbF( value.r(), value.g(), value.b() ); return col; } -inline void pdmFromVariant( const QVariant& v, cvf::Color3f& out ) +inline void pdmFromVariant( const QVariant& v, Color3f& out ) { QColor col = v.value(); out.set( col.redF(), col.greenF(), col.blueF() ); } +} // end namespace cvf + +namespace caf +{ + template <> struct PdmVariantEqualImpl { diff --git a/Fwk/AppFwk/cafPdmCvf/cafPdmCoreMat4d.h b/Fwk/AppFwk/cafPdmCvf/cafPdmCoreMat4d.h index c75109586d..ef778d8497 100644 --- a/Fwk/AppFwk/cafPdmCvf/cafPdmCoreMat4d.h +++ b/Fwk/AppFwk/cafPdmCvf/cafPdmCoreMat4d.h @@ -45,10 +45,12 @@ #include -namespace caf +// pdmToVariant/pdmFromVariant in namespace cvf so ADL finds them when called via caf::toVariant/ +// caf::fromVariant — ADL searches the argument's namespace (cvf) in addition to caf. +namespace cvf { -inline QVariant pdmToVariant( const cvf::Mat4d& value ) +inline QVariant pdmToVariant( const Mat4d& value ) { QString str; QTextStream textStream( &str ); @@ -56,13 +58,18 @@ inline QVariant pdmToVariant( const cvf::Mat4d& value ) return QVariant( str ); } -inline void pdmFromVariant( const QVariant& v, cvf::Mat4d& out ) +inline void pdmFromVariant( const QVariant& v, Mat4d& out ) { QString str = v.toString(); QTextStream textStream( &str ); textStream >> out; } +} // end namespace cvf + +namespace caf +{ + template <> struct PdmVariantEqualImpl { diff --git a/Fwk/AppFwk/cafPdmCvf/cafPdmCoreVec3d.h b/Fwk/AppFwk/cafPdmCvf/cafPdmCoreVec3d.h index 082adc45b9..5f6c4a7fe2 100644 --- a/Fwk/AppFwk/cafPdmCvf/cafPdmCoreVec3d.h +++ b/Fwk/AppFwk/cafPdmCvf/cafPdmCoreVec3d.h @@ -47,10 +47,12 @@ Q_DECLARE_METATYPE( cvf::Vec3d ); -namespace caf +// pdmToVariant/pdmFromVariant in namespace cvf so ADL finds them when called via caf::toVariant/ +// caf::fromVariant — ADL searches the argument's namespace (cvf) in addition to caf. +namespace cvf { -inline QVariant pdmToVariant( const cvf::Vec3d& value ) +inline QVariant pdmToVariant( const Vec3d& value ) { QString str; QTextStream textStream( &str ); @@ -58,13 +60,18 @@ inline QVariant pdmToVariant( const cvf::Vec3d& value ) return QVariant( str ); } -inline void pdmFromVariant( const QVariant& v, cvf::Vec3d& out ) +inline void pdmFromVariant( const QVariant& v, Vec3d& out ) { QString str = v.toString(); QTextStream textStream( &str ); textStream >> out; } +} // end namespace cvf + +namespace caf +{ + template <> struct PdmVariantEqualImpl { diff --git a/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreColor3fTest.cpp b/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreColor3fTest.cpp index bf3d7cb6ed..6d284cf7fc 100644 --- a/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreColor3fTest.cpp +++ b/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreColor3fTest.cpp @@ -1,7 +1,11 @@ #include "cafPdmCoreColor3f.h" +#include "cafPdmFieldTraits.h" + #include "gtest/gtest.h" +#include + TEST( SerializeTest, PdmCoreColor3f ) { float r = 0.4f; @@ -33,10 +37,10 @@ TEST( VariantTest, PdmCoreColor3f ) float b = 0.18f; cvf::Color3f myColor( r, g, b ); - QVariant myVariant = caf::pdmToVariant( myColor ); + QVariant myVariant = caf::toVariant( myColor ); cvf::Color3f decoded; - caf::pdmFromVariant( myVariant, decoded ); + caf::fromVariant( myVariant, decoded ); EXPECT_FLOAT_EQ( myColor.r(), decoded.r() ); EXPECT_FLOAT_EQ( myColor.g(), decoded.g() ); @@ -49,14 +53,33 @@ TEST( VariantEqualTest, PdmCoreColor3f ) 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 ); + QVariant va = caf::toVariant( a ); + QVariant vb = caf::toVariant( b ); + QVariant vc = caf::toVariant( c ); EXPECT_TRUE( caf::pdmVariantEqual( va, vb ) ); EXPECT_FALSE( caf::pdmVariantEqual( va, vc ) ); } +// Verifies that caf::toVariant dispatches to the cvf::Color3f overload via ADL, +// producing a QColor variant rather than a raw cvf::Color3f. +TEST( AdlVariantTest, PdmCoreColor3f ) +{ + cvf::Color3f original( 0.4f, 0.2f, 0.18f ); + + QVariant variant = caf::toVariant( original ); + + // The variant must hold a QColor, not a raw cvf::Color3f + EXPECT_TRUE( variant.canConvert() ); + + cvf::Color3f decoded; + caf::fromVariant( variant, decoded ); + + EXPECT_FLOAT_EQ( original.r(), decoded.r() ); + EXPECT_FLOAT_EQ( original.g(), decoded.g() ); + EXPECT_NEAR( original.b(), decoded.b(), 0.01 ); +} + TEST( SerializeSeveralTest, PdmCoreColor3f ) { float r = 0.4f; diff --git a/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreMat4dTest.cpp b/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreMat4dTest.cpp index 8c17fbcb8c..49aa9b8b0c 100644 --- a/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreMat4dTest.cpp +++ b/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreMat4dTest.cpp @@ -53,10 +53,10 @@ TEST( VariantTest, PdmCoreMat4d ) { cvf::Mat4d myMatrix = createMatrix(); - QVariant myVariant = caf::pdmToVariant( myMatrix ); + QVariant myVariant = caf::toVariant( myMatrix ); cvf::Mat4d decoded; - caf::pdmFromVariant( myVariant, decoded ); + caf::fromVariant( myVariant, decoded ); EXPECT_TRUE( decoded.equals( myMatrix ) ); } diff --git a/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreVec3dTest.cpp b/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreVec3dTest.cpp index 545c9a7072..1b55e85251 100644 --- a/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreVec3dTest.cpp +++ b/Fwk/AppFwk/cafPdmCvf/cafPdmCvf_UnitTests/cafPdmCoreVec3dTest.cpp @@ -37,10 +37,10 @@ TEST( VariantTest, PdmCoreVec3d ) cvf::Vec3d myVector( a, b, c ); - QVariant myVariant = caf::pdmToVariant( myVector ); + QVariant myVariant = caf::toVariant( myVector ); cvf::Vec3d decoded; - caf::pdmFromVariant( myVariant, decoded ); + caf::fromVariant( myVariant, decoded ); EXPECT_TRUE( decoded.equals( myVector ) ); } @@ -51,9 +51,9 @@ TEST( VariantEqualTest, PdmCoreVec3d ) 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 ); + QVariant va = caf::toVariant( a ); + QVariant vb = caf::toVariant( b ); + QVariant vc = caf::toVariant( c ); EXPECT_TRUE( caf::pdmVariantEqual( va, vb ) ); EXPECT_FALSE( caf::pdmVariantEqual( va, vc ) ); diff --git a/Fwk/AppFwk/cafPdmCvf/cafPdmMat3d/cafPdmCoreMat3d.h b/Fwk/AppFwk/cafPdmCvf/cafPdmMat3d/cafPdmCoreMat3d.h index 15ae8ba7fa..5d77ba52fb 100644 --- a/Fwk/AppFwk/cafPdmCvf/cafPdmMat3d/cafPdmCoreMat3d.h +++ b/Fwk/AppFwk/cafPdmCvf/cafPdmMat3d/cafPdmCoreMat3d.h @@ -45,10 +45,12 @@ #include -namespace caf +// pdmToVariant/pdmFromVariant in namespace cvf so ADL finds them when called via caf::toVariant/ +// caf::fromVariant — ADL searches the argument's namespace (cvf) in addition to caf. +namespace cvf { -inline QVariant pdmToVariant( const cvf::Mat3d& value ) +inline QVariant pdmToVariant( const Mat3d& value ) { QString str; QTextStream textStream( &str ); @@ -56,13 +58,18 @@ inline QVariant pdmToVariant( const cvf::Mat3d& value ) return QVariant( str ); } -inline void pdmFromVariant( const QVariant& v, cvf::Mat3d& out ) +inline void pdmFromVariant( const QVariant& v, Mat3d& out ) { QString str = v.toString(); QTextStream textStream( &str ); textStream >> out; } +} // end namespace cvf + +namespace caf +{ + template <> struct PdmVariantEqualImpl { diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmCore/cafPdmFieldTraits.h b/Fwk/AppFwk/cafProjectDataModel/cafPdmCore/cafPdmFieldTraits.h index e5cd4fda5d..32f178d911 100644 --- a/Fwk/AppFwk/cafProjectDataModel/cafPdmCore/cafPdmFieldTraits.h +++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmCore/cafPdmFieldTraits.h @@ -120,6 +120,29 @@ void pdmFromVariant( const QVariant& v, std::optional& out ) out = underlying; } +//================================================================================================== +/// toVariant / fromVariant — public API wrappers +/// +/// Simple qualified-call entry points that encapsulate the ADL two-step internally. +/// Prefer these over calling pdmToVariant/pdmFromVariant with a using-declaration at the call site. +/// Custom types are still extended by adding pdmToVariant/pdmFromVariant overloads in the type's +/// own namespace; these wrappers just hide that detail from callers. +//================================================================================================== + +template +QVariant toVariant( const T& value ) +{ + using caf::pdmToVariant; + return pdmToVariant( value ); +} + +template +void fromVariant( const QVariant& v, T& out ) +{ + using caf::pdmFromVariant; + pdmFromVariant( v, out ); +} + //================================================================================================== /// PdmVariantEqualImpl ///