From 9cf3a17be4df0089427e1f598c0679fb9439014b Mon Sep 17 00:00:00 2001 From: alexandre burton Date: Thu, 27 Feb 2025 16:56:08 -0500 Subject: [PATCH 1/5] fix(ofParameter): correct init() handling of non-comparable objects --- libs/openFrameworks/types/ofParameter.h | 92 ++++++++++++++----------- 1 file changed, 50 insertions(+), 42 deletions(-) diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index 9623619b26a..d2737f0aa7e 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -38,8 +38,8 @@ class ofAbstractParameter { virtual std::string getEscapedName() const; virtual std::string valueType() const = 0; - virtual bool isInit() const = 0; - virtual void reInit() = 0; + virtual bool isInit() const { return false; } + virtual void reInit() {}; virtual void setParent(ofParameterGroup & _parent) = 0; std::vector getGroupHierarchyNames() const; @@ -351,6 +351,18 @@ ofReadOnlyParameter & ofParameterGroup::getReadOnly(std:: /*! \cond PRIVATE */ namespace of { namespace priv { + +template +struct is_comparable : std::false_type {}; + +template +struct is_comparable() == std::declval())>> + : std::true_type {}; + +template +inline constexpr bool is_comparable_v = is_comparable::value; + + //---------------------------------------------------------------------- // Mechanism to provide min and max default values for types where it makes sense template @@ -510,6 +522,9 @@ template class ofParameter : public ofAbstractParameter { public: + /// \brief flag to opt-out of the isInit mechanism (in-complement to auto-detection of incomparability) + static inline bool init_opt_out { false }; + /// \brief constructs a default ofParameter of type ParameterType /// \tparam ParameterType the type of the Value held by the ofParameter ofParameter(); @@ -671,39 +686,38 @@ class ofParameter : public ofAbstractParameter { protected: private: class Value { + auto init_init(ParameterType &v) { + if constexpr (of::priv::is_comparable()) { + if (!init_opt_out) init = v; + } + } + public: Value() - : init(of::priv::TypeInfo::min()) - , min(of::priv::TypeInfo::min()) + : min(of::priv::TypeInfo::min()) , max(of::priv::TypeInfo::max()) , bInNotify(false) - , serializable(true) { } - + , serializable(true) { init_init(min); } Value(ParameterType v) - : init(v) - , value(v) + : value(v) , min(of::priv::TypeInfo::min()) , max(of::priv::TypeInfo::max()) , bInNotify(false) - , serializable(true) { } - + , serializable(true) { init_init(v); } Value(std::string name, ParameterType v) : name(name) - , init(v) , value(v) , min(of::priv::TypeInfo::min()) , max(of::priv::TypeInfo::max()) , bInNotify(false) - , serializable(true) { } - + , serializable(true) { init_init(v); } Value(std::string name, ParameterType v, ParameterType min, ParameterType max) : name(name) - , init(v) , value(v) , min(min) , max(max) , bInNotify(false) - , serializable(true) { } + , serializable(true) { init_init(v); } std::string name; ParameterType init, value, min, max; @@ -891,7 +905,13 @@ ParameterType ofParameter::getMax() const { template void ofParameter::setInit(const ParameterType & init) { - obj->init = init; + if constexpr (of::priv::is_comparable()) { + if (!init_opt_out) { + obj->init = init; + return; + } + } + ofLogWarning("ofParameter::setInit") << "called on a non-comparable (or opted-out) type"; } template @@ -901,12 +921,24 @@ ParameterType ofParameter::getInit() const { template bool ofParameter::isInit() const { - return obj->value == obj->init; + if constexpr (of::priv::is_comparable()) { + if (!init_opt_out) { + return obj->value == obj->init; + } + } + ofLogWarning("ofParameter::isInit") << "called on a non-comparable (or opted-out) type => always true"; + return true; } template void ofParameter::reInit() { - setMethod(obj->init); + if constexpr (of::priv::is_comparable()) { + if (!init_opt_out) { + setMethod(obj->init); + return; + } + } + ofLogWarning("ofParameter::reInit") << "called on a non-comparable (or opted-out) type => no-op"; } template @@ -1276,9 +1308,6 @@ class ofReadOnlyParameter : public ofAbstractParameter { void setMin(const ParameterType & min); void setMax(const ParameterType & max); - void setInit(const ParameterType & init); - bool isInit() const; - void reInit(); void fromString(const std::string & str); @@ -1560,27 +1589,6 @@ inline void ofReadOnlyParameter::setMax(const ParameterTy parameter.setMax(max); } -template -inline void ofReadOnlyParameter::setInit(const ParameterType & init) { - parameter.setInit(init); -} - -template -inline bool ofReadOnlyParameter::isInit() const { - // not sure what the expected behaviour for isInit() would be for ReadOnlyParameter - // as-is, it fails with : No member named 'value' in 'ofParameter' - // returning true while informing with a log msg seems sane - ofLogVerbose("ofReadOnlyParameter::isInit()") << "isInit() called on ofReadOnlyParameter, where it always returns true"; - return true; -} - -template -inline void ofReadOnlyParameter::reInit() { - // not sure what the expected behaviour for reInit() would be for ReadOnlyParameter - // informing with a log msg seems sane - ofLogVerbose("ofReadOnlyParameter::reInit()") << "reInit() called on ofReadOnlyParameter, where it is a no-op"; -} - template inline void ofReadOnlyParameter::fromString(const std::string & str) { parameter.fromString(str); From 87323d121a5bffc7ef9859d141b59c38c1b9a228 Mon Sep 17 00:00:00 2001 From: alexandre burton Date: Sun, 2 Mar 2025 13:01:36 -0500 Subject: [PATCH 2/5] refactor(ofParameter): default isInit switched from false to true --- libs/openFrameworks/types/ofParameter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index d2737f0aa7e..5daf14854eb 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -38,7 +38,7 @@ class ofAbstractParameter { virtual std::string getEscapedName() const; virtual std::string valueType() const = 0; - virtual bool isInit() const { return false; } + virtual bool isInit() const { return true; } virtual void reInit() {}; virtual void setParent(ofParameterGroup & _parent) = 0; From 8e8f1c1904016d83c2804fd38ff622a6006e0cb5 Mon Sep 17 00:00:00 2001 From: alexandre burton Date: Mon, 3 Mar 2025 15:09:35 -0500 Subject: [PATCH 3/5] feat(ofParameter): prioritize assignment by value --- libs/openFrameworks/types/ofParameter.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index 5daf14854eb..eb1155e103f 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -616,6 +616,8 @@ class ofParameter : public ofAbstractParameter { void makeReferenceTo(ofParameter & mom); ofParameter & operator=(const ofParameter & v); + + template>, int> = 0> const ParameterType & operator=(const ParameterType & v); template @@ -776,6 +778,7 @@ inline ofParameter & ofParameter::operator=(const } template +template>, int>> inline const ParameterType & ofParameter::operator=(const ParameterType & v) { set(v); return obj->value; From bd538be4ff9f1235e3aff2aed02b9e367c7ffe0b Mon Sep 17 00:00:00 2001 From: alexandre burton Date: Mon, 3 Mar 2025 15:10:14 -0500 Subject: [PATCH 4/5] fix(ofParameter): correct return type for = const ParameterType & v --- libs/openFrameworks/types/ofParameter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index eb1155e103f..eeeaddb1ec6 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -781,7 +781,7 @@ template template>, int>> inline const ParameterType & ofParameter::operator=(const ParameterType & v) { set(v); - return obj->value; + return *this; } template From 06c0e017fe8e0965a6fb0dd7b956a0a3d1d91fe8 Mon Sep 17 00:00:00 2001 From: alexandre burton Date: Mon, 3 Mar 2025 15:11:02 -0500 Subject: [PATCH 5/5] feat(ofParameter): expand the is_comparable trait to include actual comparability --- libs/openFrameworks/types/ofParameter.h | 27 +++++++++++++++---------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index eeeaddb1ec6..f26625d2118 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -352,15 +352,20 @@ ofReadOnlyParameter & ofParameterGroup::getReadOnly(std:: namespace of { namespace priv { -template -struct is_comparable : std::false_type {}; +template +constexpr auto test_comparable(int) -> decltype(std::declval() == std::declval(), std::true_type{}); -template -struct is_comparable() == std::declval())>> - : std::true_type {}; +template +constexpr std::false_type test_comparable(...); -template -inline constexpr bool is_comparable_v = is_comparable::value; +template +struct is_comparable : decltype(test_comparable(0)) {}; + +template +struct is_comparable> : is_comparable {}; + +template +constexpr bool is_comparable_v = is_comparable::value; //---------------------------------------------------------------------- @@ -689,7 +694,7 @@ class ofParameter : public ofAbstractParameter { private: class Value { auto init_init(ParameterType &v) { - if constexpr (of::priv::is_comparable()) { + if constexpr (of::priv::is_comparable_v) { if (!init_opt_out) init = v; } } @@ -908,7 +913,7 @@ ParameterType ofParameter::getMax() const { template void ofParameter::setInit(const ParameterType & init) { - if constexpr (of::priv::is_comparable()) { + if constexpr (of::priv::is_comparable_v) { if (!init_opt_out) { obj->init = init; return; @@ -924,7 +929,7 @@ ParameterType ofParameter::getInit() const { template bool ofParameter::isInit() const { - if constexpr (of::priv::is_comparable()) { + if constexpr (of::priv::is_comparable_v) { if (!init_opt_out) { return obj->value == obj->init; } @@ -935,7 +940,7 @@ bool ofParameter::isInit() const { template void ofParameter::reInit() { - if constexpr (of::priv::is_comparable()) { + if constexpr (of::priv::is_comparable_v) { if (!init_opt_out) { setMethod(obj->init); return;