Page MenuHome
Differential D8644 Diff 28152 intern/cycles/graph/node.h

Changeset View

Standalone View

View Options

intern/cycles/graph/node.h

Show First 20 Lines • Show All 48 Lines • ▼ Show 20 Lines const type_ &get_##name() const \
return get_socket_value<type_>(this, *socket); \ return get_socket_value<type_>(this, *socket); \
} \ } \
void set_##name(type_ value) \ void set_##name(type_ value) \
{ \ { \
const SocketType *socket = type->find_input(ustring(#name)); \ const SocketType *socket = type->find_input(ustring(#name)); \
this->set(*socket, value); \ this->set(*socket, value); \
} }
#define NODE_PUBLIC_API_STRUCT_MEMBER(type_, name, member) \
public: \
brechtUnsubmitted
Not Done
Inline Actions

I wouldn't add public as part of the macro, since it affects further struct members in a hidden way.

brecht: I wouldn't add `public` as part of the macro, since it affects further struct members in a…
bool name##_##member##_is_modified() const \
{ \
const SocketType *socket = type->find_input(ustring(#name"."#member)); \
return socket_is_modified(*socket); \
} \
const type_ &get_##name##_##member() const \
{ \
const SocketType *socket = type->find_input(ustring(#name"."#member)); \
return get_socket_value<type_>(this, *socket); \
} \
void set_##name##_##member(type_ value) \
{ \
const SocketType *socket = type->find_input(ustring(#name"."#member)); \
this->set(*socket, value); \
}
brechtUnsubmitted
Not Done
Inline Actions

Constructing a ustring and calling find_input for every set or get seems too slow to me.

Maybe just using static const SocketType *socket is enough? It would only need to be initialized once for each socket type, and checking if it has been initialized afterwards is probably done pretty efficiently by the compiler.

I would then also put static variable in a shared function, so we don't have 3 copies.

brecht: Constructing a `ustring` and calling `find_input` for every `set` or `get` seems too slow to me.
/* Node */ /* Node */
struct Node { struct Node {
explicit Node(const NodeType *type, ustring name = ustring()); explicit Node(const NodeType *type, ustring name = ustring());
virtual ~Node() = 0; virtual ~Node() = 0;
/* set values */ /* set values */
void set(const SocketType &input, bool value); void set(const SocketType &input, bool value);
▲ Show 20 Lines • Show All 52 Lines • ▼ Show 20 Linesstruct Node {
void hash(MD5Hash &md5); void hash(MD5Hash &md5);
/* Get total size of this node. */ /* Get total size of this node. */
size_t get_total_size_in_bytes() const; size_t get_total_size_in_bytes() const;
/* Type testing, taking into account base classes. */ /* Type testing, taking into account base classes. */
bool is_a(const NodeType *type); bool is_a(const NodeType *type);
bool socket_is_modified(const SocketType &input) const;
brechtUnsubmitted
Done
Inline Actions

No need to explicitly use inline.

brecht: No need to explicitly use `inline`.
bool need_update_();
void clear_modified();
kevindietrichAuthorUnsubmitted
Not Done
Inline Actions

This is need_update_ (with an underscore at the end) because other nodes have a need_update (with no trailing underscore) member which conflicts, it can be renamed or we can rename the members and remove the trailing underscore (the need_update members will be replaced by the flags anyway at some point in the future).

kevindietrich: This is `need_update_` (with an underscore at the end) because other nodes have a `need_update`…
brechtUnsubmitted
Not Done
Inline Actions

Maybe rename need_update_() to is_modified()? Just to keep the naming consistent.

brecht: Maybe rename `need_update_()` to `is_modified()`? Just to keep the naming consistent.
void print_modified_sockets() const;
ustring name; ustring name;
const NodeType *type; const NodeType *type;
SocketModifiedFlag socket_modified;
brechtUnsubmitted
Done
Inline Actions

Define it like this, making clear this is per socket and ensuring every place use the same amount of bits in case we want to increase it later:

typedef uint32_t SocketModifiedFlags;
SocketUpdateFlags socket_modified;

We also use "modified" rather than "changed" in some places already, I'd remain consistent with that naming.

Also I suggest "modified" rather than "update" since the node base class only tracks what has been modified, and what that then means in terms of updates is up to the specific node implementations and managers.

brecht: Define it like this, making clear this is per socket and ensuring every place use the same…
template<typename T> static T &get_socket_value(const Node *node, const SocketType &socket) template<typename T> static T &get_socket_value(const Node *node, const SocketType &socket)
{ {
return (T &)*(((char *)node) + socket.struct_offset); return (T &)*(((char *)node) + socket.struct_offset);
} }
private:
template <typename T>
void set_if_different(const SocketType &input, T value);
template <typename T>
void set_if_different(const SocketType &input, array<T> &value);
kevindietrichAuthorUnsubmitted
Done
Inline Actions

Those functions are here to avoid typo errors while writing redundant code, and basically implement the logic to set the value, replacing the body of Node::set. I think Node::set could be a template instead, we can use some template magic to handle the runtime assertions, e.g. :

template <typename T>
struct SocketDataTypeInfo;

template <>
struct SocketDataTypeInfo<int> {
    static bool is_value_of_type(SocketType::Type type)
    {
        return type == SocketType::INT;
    }
};

template <>
struct SocketDataTypeInfo<float3> {
    static bool is_value_of_type(SocketType::Type type)
    {
        return type == SocketType::COLOR || type == SocketType::POINT ||
               type == SocketType::VECTOR || type == SocketType::NORMAL;
    }
};

template <typename T>
void Node::set(const SocketType &input, T value)
{
    assert(SocketDataTypeInfo<T>::is_value_of_type(input.type));

    if (value != get_socket_value(this, input)) {
        return;
    }

    get_socket_value(this, input) = value;
    socket_modified |= input.modified_flag_bit;
}

Those SocketDataTypeInfo could also be used to have type safety for getting or setting structures in one go if we choose to have structure sockets.

kevindietrich: Those functions are here to avoid typo errors while writing redundant code, and basically…
}; };
CCL_NAMESPACE_END CCL_NAMESPACE_END