Conversation
…nt/pdmVariantEqual Introduces free function customization points to replace PdmValueFieldSpecialization. Default implementations handle Qt-metatype-registered types. Overloads provided for std::vector, std::pair, PdmPointer, AppEnum, FilePath, float, and double.
…riant free functions Replace PdmValueFieldSpecialization class templates in Vec3d, Mat4d, Color3f, Mat3d headers with free function overloads pdmToVariant/pdmFromVariant and full specializations of pdmVariantEqual. Update all call sites in unit tests and application code accordingly.
…free functions Replace PdmValueFieldSpecialization<T>::convert/setFromVariant calls with pdmToVariant/pdmFromVariant free functions in PdmDataValueField, PdmProxyValueField, PdmUiCheckBoxAndTextEditor, and PdmUiValueRangeEditor.
…tEqual free functions Replace all PdmValueFieldSpecialization<T>::convert, setFromVariant, and isEqual calls in PdmUiFieldSpecializationForValueSpec, std::vector/pair/optional specializations, and PdmFieldUiCap::isQVariantDataEqual.
…ining call sites Delete the old policy class header. Update unit tests, comment references, and CMakeLists. All consumers now use pdmToVariant/pdmFromVariant/pdmVariantEqual from cafPdmFieldTraits.h.
- PdmUiFieldSpecialization<PdmPointer<T>> and <AppEnum<T>> now delegate convert/setFromVariant to pdmToVariant/pdmFromVariant instead of duplicating the same logic - Add list.reserve() in pdmToVariant(std::vector<T>) to avoid repeated reallocations
…n build - Add cafPdmFieldTraitsTest.cpp with 33 tests covering round-trips and pdmVariantEqual for int, double, float, bool, QString, FilePath, AppEnum, vector, pair, and PdmPointer types - Add VariantEqualTest to cafPdmCoreVec3dTest.cpp and cafPdmCoreColor3fTest.cpp - Include cafPdmCvf_UnitTests in RESINSIGHT_INCLUDE_APPFWK_TESTS build
Add std::optional overloads to cafPdmFieldTraits.h so optional fields round-trip correctly and pdmVariantEqual<std::optional<T>> uses the has-value/empty-QVariant convention rather than falling back to a.value<std::optional<T>>() which always returns nullopt. Simplify PdmUiFieldSpecialization<std::optional<T>> to delegate to the free functions. Add 7 tests covering round-trip and equality.
Move type-specific free function overloads into the headers that define those types (cafPdmPointer.h, cafAppEnum.h, cafFilePath.h), so a simple PdmField<int> no longer forces every TU to parse those heavy headers. Replace if constexpr dispatch in pdmVariantEqual with PdmVariantEqualImpl<T> class template partial specializations, which also fixes the extensibility for nested containers (e.g. std::vector<cvf::Vec3d>) since class template partial specialization is allowed where function template partial specialization is not. cafPdmFieldTraits.h now only requires <QVariant> and standard library headers. Each type's equality and conversion logic lives next to its type.
- Remove redundant forward declarations for PdmPointer/AppEnum in
cafInternalPdmFieldTypeSpecializations.h (full headers now included)
- Fix std::list<T> specialization to use pdmToVariant/pdmFromVariant
instead of raw QVariant(*it)/lst[i].value<T>() — consistent with vector
- Remove redundant #include <QVariant> from cafAppEnum.h and cafFilePath.h
(already provided by cafPdmFieldTraits.h)
- Simplify PdmVariantEqualImpl<optional<T>>::equal to use !a.isValid()
instead of allocating toString().remove('"').isEmpty() — pdmToVariant
for nullopt always produces QVariant() (invalid)
…ter and std::optional PdmUiFieldSpecialization<PdmPointer<T>> and PdmUiFieldSpecialization<std::optional<T>> were pure pass-throughs to pdmToVariant/pdmFromVariant/pdmVariantEqual, identical in behavior to the primary template. Removing them lets the primary template handle these types via ADL. Also removes the cafPdmObjectHandle.h include which was only needed for the PdmPointer specialization, reducing include weight for all TUs using cafPdmUiFieldSpecialization.h.
Adds cafPdm_CompileBench, an OBJECT library with five translation units that each define 20 PdmObject subclasses using different field types: basic (int/double/bool/QString), AppEnum, containers (vector/optional/pair), FilePath, and a mixed file covering all types. Unity build is explicitly disabled on the target so each file compiles independently, giving per-file timing via ninja -j1.
…NCLUDE_APPFWK_TESTS
Move <QIcon> and <QPixmap> out of cafIconProvider.h into cafIconProvider.cpp by declaring the destructor out-of-line. This prevents these heavy Qt GUI headers from being pulled into every translation unit that includes cafPdmObject.h. Remove cafPdmOptionItemInfo.h from cafPdmUiItem.h — PdmUiItem does not use PdmOptionItemInfo directly. Move the include to cafPdmUiFieldHandle.h and cafPdmUiObjectHandle.h which actually require it. Fix downstream headers that relied on the transitive include (cafPdmUiItemInfo.h, cafPdmUiFieldEditorHandle.h). Measured improvement over dev branch: 3–7% reduction in bench file compile times, stacking on top of the cafPdmFieldTraits.h include-weight reduction.
Restore cafPdmOptionItemInfo.h in cafPdmUiItem.h — the prior commit removed it too aggressively, breaking ~70 downstream headers that relied on the transitive include. Re-adding it is still cheaper than before because cafIconProvider.h no longer pulls in <QIcon>/<QPixmap>. Add direct includes to files that used CAF_ASSERT, PdmObjectHandle, PdmFieldHandle, PdmUiFieldHandle, QIcon, QRect, or PdmOptionItemInfo through the now-broken transitive chain: - cafPdmUiCore: cafPdmUiItemInfo.cpp, cafPdmOptionItemInfo.cpp, cafPdmUiItem.cpp — add <QIcon> - cafPdmUiCore: cafPdmUiEditorHandle.cpp — add cafAssert.h - cafPdmUiCore: cafPdmUiFieldHandle.cpp — add cafPdmObjectHandle.h - cafPdmUiCore: cafFontTools.cpp — add cafPdmOptionItemInfo.h - cafPdmUiCore: cafPdmUiFieldEditorHandle.h — forward-declare PdmFieldHandle - cafPdmUiCore: cafPdmUi3dObjectEditorHandle.h — forward-declare PdmUiFieldHandle - cafUserInterface: cafPdmUiLineEditor.h — add cafPdmOptionItemInfo.h - cafUserInterface: cafPdmUiFormLayoutObjectEditor.h — forward-declare PdmFieldHandle and PdmUiFieldHandle - cafUserInterface: cafPdmUiTableViewEditor.h — forward-declare PdmObjectHandle - cafUserInterface: cafPdmUiTreeAttributes.h — add <QRect>, forward-declare PdmObjectHandle - cafUserInterface: cafPdmUiTreeSelectionQModel.h — add cafPdmOptionItemInfo.h, remove now-redundant forward declaration - cafTestApplication: CustomObjectEditor.cpp — add cafAssert.h (pre-existing bug) - ApplicationLibCode: RiaOptionItemFactory.h, RiaQDateTimeTools.cpp, RimDataSourceSteppingTools.h — add cafPdmOptionItemInfo.h
Review Summary by QodoReplace PdmValueFieldSpecialization with ADL-based pdmToVariant/pdmFromVariant free functions
WalkthroughsDescription• Replace PdmValueFieldSpecialization class templates with ADL-based free functions - Introduces pdmToVariant, pdmFromVariant, pdmVariantEqual customization points - Simplifies type specialization and improves code maintainability • Migrate all call sites to new free function API - Updates PDM field classes, UI editors, and application code - Removes old cafInternalPdmValueFieldSpecializations.h header • Add comprehensive test coverage for field traits - New cafPdmFieldTraitsTest.cpp with 40+ test cases - Tests round-trip conversions and equality comparisons • Add compile-time benchmarks for PDM field types - Five benchmark files measuring instantiation cost - Helps track performance impact of field specializations Diagramflowchart LR
A["PdmValueFieldSpecialization<br/>class templates"] -->|"Replaced by"| B["pdmToVariant<br/>pdmFromVariant<br/>pdmVariantEqual<br/>free functions"]
B -->|"Enables"| C["Type specializations<br/>via overloads"]
C -->|"Supports"| D["Basic types<br/>Containers<br/>Custom types"]
E["cafPdmFieldTraits.h<br/>new header"] -->|"Provides"| B
F["Removed<br/>cafInternalPdmValueFieldSpecializations.h"] -->|"Replaced by"| E
File Changes1. Fwk/AppFwk/cafProjectDataModel/cafPdmCore/cafPdmFieldTraits.h
|
Code Review by Qodo
1. Missing Vec3d traits include
|
Inconsistency: pdmToVariant(nullopt) returns an invalid QVariant(), but
pdmFromVariant was detecting nullopt via toString().remove('"').isEmpty(),
which is fragile and allocates unnecessarily. Use !v.isValid() to match
the documented convention.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| 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 ); |
There was a problem hiding this comment.
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
1 participant
No description provided.