Skip to content

fix(ofParameter): better default init() handling of non-comparable/non-copyable objects and containers #8325

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

Merged
ofTheo merged 7 commits into from
Mar 3, 2025
Merged

fix(ofParameter): better default init() handling of non-comparable/non-copyable objects and containers #8325

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9cf3a17
fix(ofParameter): correct init() handling of non-comparable objects
alexandreburton Feb 27, 2025
9433740
Merge branch 'openframeworks:master' into ofParameter-moreinit
artificiel Feb 28, 2025
44d52f2
Merge branch 'openframeworks:master' into ofParameter-moreinit
artificiel Mar 2, 2025
87323d1
refactor(ofParameter): default isInit switched from false to true
artificiel Mar 2, 2025
8e8f1c1
feat(ofParameter): prioritize assignment by value
alexandreburton Mar 3, 2025
bd538be
fix(ofParameter): correct return type for = const ParameterType & v
alexandreburton Mar 3, 2025
06c0e01
feat(ofParameter): expand the is_comparable trait to include actual c…
alexandreburton Mar 3, 2025
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
102 changes: 59 additions & 43 deletions libs/openFrameworks/types/ofParameter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 true; }
virtual void reInit() {};

virtual void setParent(ofParameterGroup & _parent) = 0;
std::vector<std::string> getGroupHierarchyNames() const;
Expand Down Expand Up @@ -351,6 +351,23 @@ ofReadOnlyParameter<ParameterType, Friend> & ofParameterGroup::getReadOnly(std::
/*! \cond PRIVATE */
namespace of {
namespace priv {

template<typename T, typename U = T>
constexpr auto test_comparable(int) -> decltype(std::declval<T>() == std::declval<U>(), std::true_type{});

template<typename T, typename U = T>
constexpr std::false_type test_comparable(...);

template<typename T>
struct is_comparable : decltype(test_comparable<T>(0)) {};

template<typename T>
struct is_comparable<std::vector<T>> : is_comparable<T> {};

template<typename T>
constexpr bool is_comparable_v = is_comparable<T>::value;


//----------------------------------------------------------------------
// Mechanism to provide min and max default values for types where it makes sense
template <typename T, bool B>
Expand Down Expand Up @@ -510,6 +527,9 @@ template <typename ParameterType>
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();
Expand Down Expand Up @@ -601,6 +621,8 @@ class ofParameter : public ofAbstractParameter {
void makeReferenceTo(ofParameter<ParameterType> & mom);

ofParameter<ParameterType> & operator=(const ofParameter<ParameterType> & v);

template<typename U = ParameterType, std::enable_if_t<!std::is_same_v<U, ofParameter<U>>, int> = 0>
const ParameterType & operator=(const ParameterType & v);

template <typename U = ParameterType>
Expand Down Expand Up @@ -671,39 +693,38 @@ class ofParameter : public ofAbstractParameter {
protected:
private:
class Value {
auto init_init(ParameterType &v) {
if constexpr (of::priv::is_comparable_v<ParameterType>) {
if (!init_opt_out) init = v;
}
}

public:
Value()
: init(of::priv::TypeInfo<ParameterType>::min())
, min(of::priv::TypeInfo<ParameterType>::min())
: min(of::priv::TypeInfo<ParameterType>::min())
, max(of::priv::TypeInfo<ParameterType>::max())
, bInNotify(false)
, serializable(true) { }

, serializable(true) { init_init(min); }
Value(ParameterType v)
: init(v)
, value(v)
: value(v)
, min(of::priv::TypeInfo<ParameterType>::min())
, max(of::priv::TypeInfo<ParameterType>::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<ParameterType>::min())
, max(of::priv::TypeInfo<ParameterType>::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;
Expand Down Expand Up @@ -762,9 +783,10 @@ inline ofParameter<ParameterType> & ofParameter<ParameterType>::operator=(const
}

template <typename ParameterType>
template<typename U, std::enable_if_t<!std::is_same_v<U, ofParameter<U>>, int>>
inline const ParameterType & ofParameter<ParameterType>::operator=(const ParameterType & v) {
set(v);
return obj->value;
return *this;
}

template <typename ParameterType>
Expand Down Expand Up @@ -891,7 +913,13 @@ ParameterType ofParameter<ParameterType>::getMax() const {

template <typename ParameterType>
void ofParameter<ParameterType>::setInit(const ParameterType & init) {
obj->init = init;
if constexpr (of::priv::is_comparable_v<ParameterType>) {
if (!init_opt_out) {
obj->init = init;
return;
}
}
ofLogWarning("ofParameter::setInit") << "called on a non-comparable (or opted-out) type";
}

template <typename ParameterType>
Expand All @@ -901,12 +929,24 @@ ParameterType ofParameter<ParameterType>::getInit() const {

template <typename ParameterType>
bool ofParameter<ParameterType>::isInit() const {
return obj->value == obj->init;
if constexpr (of::priv::is_comparable_v<ParameterType>) {
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 <typename ParameterType>
void ofParameter<ParameterType>::reInit() {
setMethod(obj->init);
if constexpr (of::priv::is_comparable_v<ParameterType>) {
if (!init_opt_out) {
setMethod(obj->init);
return;
}
}
ofLogWarning("ofParameter::reInit") << "called on a non-comparable (or opted-out) type => no-op";
}

template <typename ParameterType>
Expand Down Expand Up @@ -1276,9 +1316,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);

Expand Down Expand Up @@ -1560,27 +1597,6 @@ inline void ofReadOnlyParameter<ParameterType, Friend>::setMax(const ParameterTy
parameter.setMax(max);
}

template <typename ParameterType, typename Friend>
inline void ofReadOnlyParameter<ParameterType, Friend>::setInit(const ParameterType & init) {
parameter.setInit(init);
}

template <typename ParameterType, typename Friend>
inline bool ofReadOnlyParameter<ParameterType, Friend>::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<std::string>'
// 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 <typename ParameterType, typename Friend>
inline void ofReadOnlyParameter<ParameterType, Friend>::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 <typename ParameterType, typename Friend>
inline void ofReadOnlyParameter<ParameterType, Friend>::fromString(const std::string & str) {
parameter.fromString(str);
Expand Down