Better enum flag support

[adriengivry]

Problem this feature should fix With C++20, it's easier than ever to auto-generate operators for a given type. We should add a way to auto-generate typical flag operations for our enum class, to replace the previously copy-pasted mess.

Expected solution

Util:

// Define a concept for flag enums
template <typename E>
concept EnumType = std::is_enum_v<E>;

// Base template for enabling flag operations on enum classes
template <EnumType E>
struct EnableBitMaskOperators {
	static constexpr bool enable = false;
};

// Operator overloading using C++20 concepts
template <EnumType E>
	requires (EnableBitMaskOperators<E>::enable)
constexpr E operator|(E lhs, E rhs) {
	using underlying = std::underlying_type_t<E>;
	return static_cast<E>(
		static_cast<underlying>(lhs) |
		static_cast<underlying>(rhs)
		);
}

template <EnumType E>
	requires (EnableBitMaskOperators<E>::enable)
constexpr E operator&(E lhs, E rhs) {
	using underlying = std::underlying_type_t<E>;
	return static_cast<E>(
		static_cast<underlying>(lhs) &
		static_cast<underlying>(rhs)
		);
}

template <EnumType E>
	requires (EnableBitMaskOperators<E>::enable)
constexpr E operator^(E lhs, E rhs) {
	using underlying = std::underlying_type_t<E>;
	return static_cast<E>(
		static_cast<underlying>(lhs) ^
		static_cast<underlying>(rhs)
		);
}

template <EnumType E>
	requires (EnableBitMaskOperators<E>::enable)
constexpr E operator~(E rhs) {
	using underlying = std::underlying_type_t<E>;
	return static_cast<E>(
		~static_cast<underlying>(rhs)
		);
}

template <EnumType E>
	requires (EnableBitMaskOperators<E>::enable)
constexpr E& operator|=(E& lhs, E rhs) {
	return lhs = lhs | rhs;
}

template <EnumType E>
	requires (EnableBitMaskOperators<E>::enable)
constexpr E& operator&=(E& lhs, E rhs) {
	return lhs = lhs & rhs;
}

template <EnumType E>
	requires (EnableBitMaskOperators<E>::enable)
constexpr E& operator^=(E& lhs, E rhs) {
	return lhs = lhs ^ rhs;
}

// Function to check if a flag is set
template <EnumType E>
	requires (EnableBitMaskOperators<E>::enable)
constexpr bool has_flag(E value, E flag) {
	using underlying = std::underlying_type_t<E>;
	return (static_cast<underlying>(value) & static_cast<underlying>(flag))
		== static_cast<underlying>(flag);
}

Usage:

enum class FooFlags : uint8_t
{
    A = 0b0001,
    B = 0b0010,
    C = 0b0100,
}

// Enable flags for a given enum
template <>
struct OvTools::Utils::EnableBitMaskOperators<EnumTypeToEnable> {
	static constexpr bool enable = true;
};

[jstone-lucasfilm]

This is a great question, and I'm CC'ing @niklasharrysson, who worked closely on the original definition of this node in MaterialX.

This may end up being a question to resolve in a MaterialX TSC meeting, and I'll bet our colleagues on the Arnold, RenderMan, Karma, and OpenMoonray teams would have useful perspectives to provide.

[niklasharrysson]

@jstone-lucasfilm @boberfly The medium_vdf closure is actually not defined in the MaterialX specification yet. It was introduced in OSL as part of the work we did to formalize the MaterialX closures in OSL. But it was never ported back to the MaterialX spec AFAIK.

There is a long discussion of that work in this thread: https://github.com/AcademySoftwareFoundation/MaterialX/pull/614

The need for a new medium_vdf closure came up in that discussion. But the priority number's interpretation was not discussed there. Personally I just assumed a higher number would represent a higher priority. To my mind this is the most intuitive and practical interpretation, if you also allow negative numbers for setting lower priorities once a zero has been set.

But I agree with Jonathan this should be discussed and resolved in a TSC Meeting where all renderer teams can place their vote. Once it has been settled we should also add this node to the spec to have it defined there.