New data structures for node group interface declaration #109135

New Issue

Lukas Tönne · 2023-06-19T19:40:42+02:00

Lukas Tönne commented

2023-06-19 19:40:42 +02:00

A proposal for replacing the "interface" definition of node groups.

Currently the interface of a node group is defined by a list of bNodeSocket inputs and outputs. This is not good since the declaration of a socket is easy to confuse with instances of the socket (i.e. actual sockets on an instance of the group node). Because replacing these data structures has no immediate impact on users it is difficult to justify on its own.

However, we are going to add panels to (group) nodes, which requires changes to these node group declarations anyway. This proposal is describing data layout and API changes to enable panels and give node groups more flexibility for the future.

⚠️ Forward-compatibility

In order to have forward-compatibility, should we choose to require it, we need to keep the old inputs/outputs data around, and create matching socket declarations on blend file write.

Dropping forward compatibility requirement with 4.0 is the easier option.

`bNodeTree`

New struct bNodeTreeInterface to replace current node tree "interface".
inputs/outputs replaced with a single sockets list. panels list remains.
bNodeSocketDeclaration stores pointer to its panel.
bNodePanel stores pointer to its parent panel (future nesting support)
Sockets and panels lists are kept sorted by parent and nesting level. This way direct children can be returned as a simple span.

typedef struct bNodeSocketDeclaration {
  char *name;
  char *type;
  // etc.
} bNodeSocketDeclaration;

typedef struct bNodeTreeInterface {
  bNodeSocketDeclaration **sockets_array;
  int socket_num;
    
  bNodePanel **panels_array;
  int panels_num;
    
#ifdef __cplusplus
  Span<const bNodeSocketDeclaration *> sockets() const;
  MutableSpan<bNodeSocketDeclaration *> sockets();
  Span<const bNodeSocketDeclaration *> sockets_of_panel(const bNodePanel *panel) const;
  MutableSpan<bNodeSocketDeclaration *> sockets_of_panel(const bNodePanel *panel);

  Span<const bNodePanel *> panels() const;
  MutableSpan<bNodePanel *> panels();
  Span<const bNodePanel *> children_of_panel(const bNodePanel *panel) const;
  MutableSpan<bNodePanel *> children_of_panel(const bNodePanel *panel);
#endif
} bNodeTreeInterface;

`blender::nodes::NodeDeclaration`

Panels become part of node declaration.
Also stores a flat list of socket and panel declarations.
Straightforward conversion from bNodeTreeInterface.

Provide additional mappings between its sockets list and bNode inputs/outputs lists:

/* Panel that the socket is in. */
bNodePanel *panel_of_socket(const bNodeSocketDeclaration &socket_decl) const;
/**
* All socket declarations that are in a given panel.
* If \a panel is null then all sockets without a panel are returned. */
Span<bNodeSocketDeclaration *> *sockets_of_panel(const bNodePanel *panel) const;
/**
* Index of the bNodeSocket generated by \a socket.
* \return Index of the generated socket or -1 if no input socket is generated. */
int input_index(const bNodeSocketDeclaration *socket_decl) const;
/**
* Index of the bNodeSocket generated by \a socket.
* \return Index of the generated socket or -1 if no output socket is generated. */
int output_index(const bNodeSocketDeclaration *socket_decl) const;
/* Find the socket declaration that generated the input socket at index \a socket_index. */
bNodeSocketDeclaration *find_input_socket_declaration(int socket_index);
/* Find the socket declaration that generated the output socket at index \a socket_index. */
bNodeSocketDeclaration *find_output_socket_declaration(int socket_index);

`bNode`

inputs/outputs lists remain unchanged
UI rearrange sockets based on declaration

Panels are added based on declaration
Drawing loop pseudo code:

void draw_sockets(bNode &node, uiLayout &layout)
  for (const bNodePanel &panel : decl.panels()) {
    for (const bNodeSocketDeclaration *socket_decl : decl.sockets_of_panel(panel)) {
      const int input_index = decl.input_index(socket_decl);
      const int output_index = decl.output_index(socket_decl);
      if (input_index >= 0) {
        draw_socket(layout, node, node.inputs[input_index]);
      }
      if (output_index >= 0) {
        draw_socket(layout, node, node.outputs[output_index]);
      }
    }
  }
}

A proposal for replacing the "interface" definition of node groups. Currently the interface of a node group is defined by a list of `bNodeSocket` inputs and outputs. This is not good since the _declaration_ of a socket is easy to confuse with _instances_ of the socket (i.e. actual sockets on an instance of the group node). Because replacing these data structures has no immediate impact on users it is difficult to justify on its own. However, we are going to add panels to (group) nodes, which requires changes to these node group declarations anyway. This proposal is describing data layout and API changes to enable panels and give node groups more flexibility for the future. ## :warning: Forward-compatibility In order to have forward-compatibility, should we choose to require it, we need to keep the old inputs/outputs data around, and create matching socket declarations on blend file write. Dropping forward compatibility requirement with 4.0 is the easier option. ## `bNodeTree` - New struct `bNodeTreeInterface` to replace current node tree "interface". - `inputs`/`outputs` replaced with a single `sockets` list. `panels` list remains. - `bNodeSocketDeclaration` stores pointer to its panel. - `bNodePanel` stores pointer to its parent panel (future nesting support) - Sockets and panels lists are kept sorted by parent and nesting level. This way direct children can be returned as a simple span. ```c typedef struct bNodeSocketDeclaration { char *name; char *type; // etc. } bNodeSocketDeclaration; typedef struct bNodeTreeInterface { bNodeSocketDeclaration **sockets_array; int socket_num; bNodePanel **panels_array; int panels_num; #ifdef __cplusplus Span<const bNodeSocketDeclaration *> sockets() const; MutableSpan<bNodeSocketDeclaration *> sockets(); Span<const bNodeSocketDeclaration *> sockets_of_panel(const bNodePanel *panel) const; MutableSpan<bNodeSocketDeclaration *> sockets_of_panel(const bNodePanel *panel); Span<const bNodePanel *> panels() const; MutableSpan<bNodePanel *> panels(); Span<const bNodePanel *> children_of_panel(const bNodePanel *panel) const; MutableSpan<bNodePanel *> children_of_panel(const bNodePanel *panel); #endif } bNodeTreeInterface; ``` ## `blender::nodes::NodeDeclaration` - Panels become part of node declaration. - Also stores a flat list of socket and panel declarations. - Straightforward conversion from `bNodeTreeInterface`. - Provide additional mappings between its sockets list and `bNode` inputs/outputs lists: ```cpp /* Panel that the socket is in. */ bNodePanel *panel_of_socket(const bNodeSocketDeclaration &socket_decl) const; /** * All socket declarations that are in a given panel. * If \a panel is null then all sockets without a panel are returned. */ Span<bNodeSocketDeclaration *> *sockets_of_panel(const bNodePanel *panel) const; /** * Index of the bNodeSocket generated by \a socket. * \return Index of the generated socket or -1 if no input socket is generated. */ int input_index(const bNodeSocketDeclaration *socket_decl) const; /** * Index of the bNodeSocket generated by \a socket. * \return Index of the generated socket or -1 if no output socket is generated. */ int output_index(const bNodeSocketDeclaration *socket_decl) const; /* Find the socket declaration that generated the input socket at index \a socket_index. */ bNodeSocketDeclaration *find_input_socket_declaration(int socket_index); /* Find the socket declaration that generated the output socket at index \a socket_index. */ bNodeSocketDeclaration *find_output_socket_declaration(int socket_index); ``` ## `bNode` - `inputs`/`outputs` lists remain unchanged - UI rearrange sockets based on declaration - Panels are added based on declaration Drawing loop pseudo code: ```cpp void draw_sockets(bNode &node, uiLayout &layout) for (const bNodePanel &panel : decl.panels()) { for (const bNodeSocketDeclaration *socket_decl : decl.sockets_of_panel(panel)) { const int input_index = decl.input_index(socket_decl); const int output_index = decl.output_index(socket_decl); if (input_index >= 0) { draw_socket(layout, node, node.inputs[input_index]); } if (output_index >= 0) { draw_socket(layout, node, node.outputs[output_index]); } } } } ```

Lukas Tönne added the

Type

Design

label 2023-06-19 19:40:42 +02:00

Lukas Tönne added this to the Nodes & Physics project 2023-06-19 19:45:02 +02:00

Lukas Tönne self-assigned this 2023-06-19 19:45:07 +02:00

Lukas Tönne commented

2023-06-19 19:46:11 +02:00

UI design task for node panels: #108895

Jacques Lucke commented

2023-06-20 11:11:15 +02:00

I have a few initial thoughts after reading this:

Generally makes sense that the panels know about the contained sockets and not the other way around. In practice we may need a backpointer, but that can just be a cache.
I'd like to see more information for how this can be expanded to nested panels, even if we don't expose those in the UI initially (currently I still think we should support those from the beginning).
The term "declaration" may lead to confusion since we also have the run-time only node declaration system, is the goal to replace that or just to have something else that can be serialized?
I guess bNodeSocketDeclaration would have some type specific data as well, like the default value?

I have a few initial thoughts after reading this: * Generally makes sense that the panels know about the contained sockets and not the other way around. In practice we may need a backpointer, but that can just be a cache. * I'd like to see more information for how this can be expanded to nested panels, even if we don't expose those in the UI initially (currently I still think we should support those from the beginning). * The term "declaration" may lead to confusion since we also have the run-time only node declaration system, is the goal to replace that or just to have something else that can be serialized? * I guess `bNodeSocketDeclaration` would have some type specific data as well, like the default value?

Lukas Tönne commented

2023-06-20 12:19:21 +02:00

Generally makes sense that the panels know about the contained sockets and not the other way around. In practice we may need a backpointer, but that can just be a cache.

Yes, there would be a runtime cache to provide fast access to panel-of-socket and sockets-of-panel. This could be both in the DNA data and the NodeDeclaration, depending on where it is needed (e.g. if we use declarations for rendering panels on node instances).

I'd like to see more information for how this can be expanded to nested panels, even if we don't expose those in the UI initially (currently I still think we should support those from the beginning).

If nested panels need to be supported then the index-in-sockets approach alone won't work. With this way of describing sections the parent panels need to go into a separate list, which indexes the 1st layer of panels.

The model also does not allow gaps between panels of the same level: each panel extends until the next panel starts. For sockets this is part of the design (bare sockets first, then the panels) , but for higher-level panels this could be an unwanted limitation. The panel could store an optional length, using maximum extent by default.

Alternative is to store pointers to parent panels, both in sockets and in panels themselves. We probably want to keep the sockets and panels ordered topologically in this case.

The term "declaration" may lead to confusion since we also have the run-time only node declaration system, is the goal to replace that or just to have something else that can be serialized?

I was looking for a way to combine these two into the same system, but the fact that declarations are virtual classes makes that risky. Might play it safe and leave the DNA as separate class, and rename it some way. Suggestions welcome.

I guess bNodeSocketDeclaration would have some type specific data as well, like the default value?

Yes, i haven't fully worked this out yet. Derived structs for the major data types. I definitely want to avoid the mess of RNA types we have right now for sockets.

> * Generally makes sense that the panels know about the contained sockets and not the other way around. In practice we may need a backpointer, but that can just be a cache. Yes, there would be a runtime cache to provide fast access to panel-of-socket and sockets-of-panel. This could be both in the DNA data and the `NodeDeclaration`, depending on where it is needed (e.g. if we use declarations for rendering panels on node instances). > * I'd like to see more information for how this can be expanded to nested panels, even if we don't expose those in the UI initially (currently I still think we should support those from the beginning). If nested panels need to be supported then the index-in-sockets approach alone won't work. With this way of describing sections the parent panels need to go into a separate list, which indexes the 1st layer of panels. ![image](/attachments/1f0b4145-5e5f-47c2-a003-b46032cb90cb) The model also does not allow gaps between panels of the same level: each panel extends until the next panel starts. For sockets this is part of the design (bare sockets first, then the panels) , but for higher-level panels this could be an unwanted limitation. The panel could store an optional length, using maximum extent by default. Alternative is to store pointers to parent panels, both in sockets and in panels themselves. We probably want to keep the sockets and panels ordered topologically in this case. > * The term "declaration" may lead to confusion since we also have the run-time only node declaration system, is the goal to replace that or just to have something else that can be serialized? I was looking for a way to combine these two into the same system, but the fact that declarations are virtual classes makes that risky. Might play it safe and leave the DNA as separate class, and rename it some way. Suggestions welcome. > * I guess `bNodeSocketDeclaration` would have some type specific data as well, like the default value? Yes, i haven't fully worked this out yet. Derived structs for the major data types. I definitely want to avoid the mess of RNA types we have right now for sockets.

image.png

46 KiB

Hans Goudey commented

2023-06-20 15:35:29 +02:00

My main comment is that I'd like to avoid having a duplicate runtime and DNA version of the declaration system. That means the whole declaration system would be ported to DNA, and that builtin nodes and node groups would use the same declaration system. That's a nice simplification in my opinion, and it avoids the need to wrap the DNA system with the runtime system, for example. The problem of virtual classes doesn't seem big TBH, that could be solved with old fashioned polymorphism. We shouldn't need a socket type for each RNA subtype, which avoids the mess you mentioned in RNA.

inputs/outputs replaced with a single sockets list. panels list remains

What do you see as the benefit of this, for the code? In the UI it makes some sense, to simplify aligning sockets, but it seems like keeping a clear separation between inputs and outputs internally would be helpful.

bNodePanel stores a start socket index

What's the benefit of storing it this way, compared to the parent pointer approach? While the panel->socket connection you mention seems less redundant, it also has some limitations like not allowing gaps of top-level sockets between panels.

EDIT: Also, for forward compatibility, if it's required maybe the best way to handle it is patching 3.6 to allow reading the new format.

My main comment is that I'd like to avoid having a duplicate runtime and DNA version of the declaration system. That means the whole declaration system would be ported to DNA, and that builtin nodes and node groups would use the same declaration system. That's a nice simplification in my opinion, and it avoids the need to wrap the DNA system with the runtime system, for example. The problem of virtual classes doesn't seem big TBH, that could be solved with old fashioned polymorphism. We shouldn't need a socket type for each RNA subtype, which avoids the mess you mentioned in RNA. >- inputs/outputs replaced with a single sockets list. panels list remains What do you see as the benefit of this, for the code? In the UI it makes some sense, to simplify aligning sockets, but it seems like keeping a clear separation between inputs and outputs internally would be helpful. >bNodePanel stores a start socket index What's the benefit of storing it this way, compared to the parent pointer approach? While the panel->socket connection you mention seems less redundant, it also has some limitations like not allowing gaps of top-level sockets between panels. EDIT: Also, for forward compatibility, if it's required maybe the best way to handle it is patching 3.6 to allow reading the new format.

Lukas Tönne commented

2023-06-20 16:07:07 +02:00

What do you see as the benefit of this, for the code? In the UI it makes some sense, to simplify aligning sockets, but it seems like keeping a clear separation between inputs and outputs internally would be helpful.

Not sure if this has been stated explicitly, but the design looks like we want to mix inputs and outputs in arbitrary order. If inputs and outputs are kept in separate lists we'd need another mapping for final positions, somehow. If sockets are in the same lists then we already have the order.

There is also the concept of creating "pass-through" sockets, where both input and output is generated on the same row (UV in the example above). With a shared socket list this becomes a simple matter of setting both IN and OUT flag.

On the other hand, what do we actually need separate lists for? When looping over just inputs or just outputs we can easily skip the other kind. Note that actual sockets in bNode are not affected, they remain in separate lists (not sure how we are going to do alignment there, maybe each socket gets a vertical position slot)

bNodePanel stores a start socket index

I'm currently leaning towards dropping this idea and instead going for conventional parent pointer. This would be combined with some stable topological sorting (socket comes first if its parent comes first) and runtime caches for fast lookups.

The benefit of storing child start index would be simplicity: i don't need much further processing to get "all sockets in this panel". But as mentioned it is based on some assumptions about structure, like the "no gaps" requirement and a single parent level.

maybe the best way to handle it is patching 3.6 to allow reading the new format

But that would mean only 3.6 can load the newer files while older versions still break, right? I think we could construct old inputs/outputs lists on write that would keep files truly forward compatible, haven't tried yet.

> What do you see as the benefit of this, for the code? In the UI it makes some sense, to simplify aligning sockets, but it seems like keeping a clear separation between inputs and outputs internally would be helpful. Not sure if this has been stated explicitly, but the design looks like we want to mix inputs and outputs in arbitrary order. If inputs and outputs are kept in separate lists we'd need another mapping for final positions, somehow. If sockets are in the same lists then we already have the order. ![image](/attachments/e0beb3df-ce0c-4428-a489-3f6bfb1f9d48) There is also the concept of creating "pass-through" sockets, where both input and output is generated on the same row (`UV` in the example above). With a shared socket list this becomes a simple matter of setting both `IN` and `OUT` flag. On the other hand, what do we actually need separate lists for? When looping over just inputs or just outputs we can easily skip the other kind. Note that actual sockets in `bNode` are not affected, they remain in separate lists (not sure how we are going to do alignment there, maybe each socket gets a vertical position slot) > bNodePanel stores a start socket index I'm currently leaning towards dropping this idea and instead going for conventional parent pointer. This would be combined with some stable topological sorting (socket comes first if its parent comes first) and runtime caches for fast lookups. The benefit of storing child start index would be simplicity: i don't need much further processing to get "all sockets in this panel". But as mentioned it is based on some assumptions about structure, like the "no gaps" requirement and a single parent level. > maybe the best way to handle it is patching 3.6 to allow reading the new format But that would mean only 3.6 can load the newer files while older versions still break, right? I think we could construct old inputs/outputs lists on write that would keep files truly forward compatible, haven't tried yet.

image.png

22 KiB

Hans Goudey commented

2023-06-20 16:31:47 +02:00

On the other hand, what do we actually need separate lists for? When looping over just inputs or just outputs we can easily skip the other kind.

Okay, I'm convinced, thanks!

But that would mean only 3.6 can load the newer files while older versions still break, right?

That's already the case for meshes though, and means we don't constantly have to convert back and forth between the old and new versions when saving files for the foreseeable future, which is ugly and and error prone as well as slower.

>On the other hand, what do we actually need separate lists for? When looping over just inputs or just outputs we can easily skip the other kind. Okay, I'm convinced, thanks! >But that would mean only 3.6 can load the newer files while older versions still break, right? That's already the case for meshes though, and means we don't constantly have to convert back and forth between the old and new versions when saving files for the foreseeable future, which is ugly and and error prone as well as slower.

Lukas Tönne commented

2023-06-20 16:56:26 +02:00

That's already the case for meshes though, and means we don't constantly have to convert back and forth between the old and new versions when saving files for the foreseeable future, which is ugly and and error prone as well as slower.

Fair enough, i don't have a strong opinion on this. If forward compatibility for 3.6 is sufficient we can do it this way.

> That's already the case for meshes though, and means we don't constantly have to convert back and forth between the old and new versions when saving files for the foreseeable future, which is ugly and and error prone as well as slower. Fair enough, i don't have a strong opinion on this. If forward compatibility for 3.6 is sufficient we can do it this way.

Iliya Katushenock commented

2023-06-20 17:02:39 +02:00

declaration system would be ported to DNA

Is this a good decision in the long run? Having a version-free wrapper is quite useful.

> declaration system would be ported to DNA Is this a good decision in the long run? Having a version-free wrapper is quite useful.

Lukas Tönne commented

2023-06-21 13:13:07 +02:00

Renamed bNodeTreeDeclaration to bNodeTreeInterface
Changed panel assignment to a panel/parent pointer in sockets and panels respectively

* Renamed `bNodeTreeDeclaration` to `bNodeTreeInterface` * Changed panel assignment to a panel/parent pointer in sockets and panels respectively

Lukas Tönne commented

2023-06-21 16:55:55 +02:00

It looks like we're going to need a combined container for sockets and panels after all, if we want to be able to place sockets in front of a panel. An array is a way of defining relative order of items, but if we store panels and sockets in separate lists we have to make assumptions about their final order.

In the example below i'm storing two arrays. Drawing generally follows ordering in the array, with a general rule that panels come before sockets.

We might want to, for example, move socket 1 in front of panel 4, within their shared parent panel 1. There is currently no way to specify this. The same applies with only one panel layer (no nesting) when we want to move sockets to the top of the list.

It looks like we're going to need a combined container for sockets and panels after all, if we want to be able to place sockets in front of a panel. An array is a way of defining relative order of items, but if we store panels and sockets in separate lists we have to make assumptions about their final order. In the example below i'm storing two arrays. Drawing generally follows ordering in the array, with a general rule that panels come before sockets. We might want to, for example, move socket 1 in front of panel 4, within their shared parent panel 1. There is currently no way to specify this. The same applies with only one panel layer (no nesting) when we want to move sockets to the top of the list. ![image](/attachments/26b3bde0-cf6e-4659-8882-11522e3719a2)

image.png

79 KiB

Jacques Lucke commented

2023-06-21 17:04:08 +02:00

An array is a way of defining relative order of items, but if we store panels and sockets in separate lists we have to make assumptions about their final order.

Currently, I think we can make the simplifying assumption that within a panel (including the root "panel") sockets will always come first and then subpanels. This way a panel can could have separate lists for sockets and subpanels which are later drawn one after the other.

> An array is a way of defining relative order of items, but if we store panels and sockets in separate lists we have to make assumptions about their final order. Currently, I think we can make the simplifying assumption that within a panel (including the root "panel") sockets will always come first and then subpanels. This way a panel can could have separate lists for sockets and subpanels which are later drawn one after the other.

Brecht Van Lommel commented

2023-06-21 17:28:54 +02:00

Panels in the properties editor work like that anyway, subpanels are always at the bottom.

Regarding forward compatibility, see also #109151 for the policy we are trying to define for Blender as a whole. It's not clear to me how we can keep 3.6.0 compatible since we are so close to the release, would the idea be to add compatibility in 3.6.1?

Panels in the properties editor work like that anyway, subpanels are always at the bottom. Regarding forward compatibility, see also #109151 for the policy we are trying to define for Blender as a whole. It's not clear to me how we can keep 3.6.0 compatible since we are so close to the release, would the idea be to add compatibility in 3.6.1?

Lukas Tönne commented

2023-06-27 15:54:41 +02:00

I've settled on using a single flat array of items for the node group interface. Items can be sockets or panels. Sockets in turn have specialized subclasses for each data type (like bNodeSocket default_value classes but hopefully a less pervasive).

typedef struct bNodeTreeInterface {
  [...]
  bNodeTreeInterfaceItem **items_array;
  int items_num;
};

/** Type of interface item. */
typedef enum eNodeTreeInterfaceItemType {
  NODE_INTERFACE_SOCKET = 0,
  NODE_INTERFACE_PANEL = 1,
} eNodeTreeInterfaceItemType;

/** A socket or panel in the node tree interface. */
typedef struct bNodeTreeInterfaceItem {
  [...]
  /* eNodeTreeInterfaceItemType */
  char item_type;
};

The panel hierarchy is implemented with parent pointers in items that place an item inside a panel.

typedef struct bNodeTreeInterfaceItem {
  [...]
  /* Panel in which to display the item. */
  struct bNodeTreeInterfacePanel *parent;
};

This flat structure is nice for iterating over sockets when we don't care about the hierarchical structure (which is most of the time). The downside is that constructing a hierarchical layout requires some processing to determine root items, children of panels, etc.

To make this easier the interface code will keep the items list sorted topologically: Items appear in order of their nesting depth and the relative order of children from the same parent remains unchanged (i.e. stable sorting).

The interface items have utility functions for getting root items, children of a panel, and respective index ranges. These ranges are also calculated during topological sorting. For example, item_children(Panel7) would return Socket6, Socket7 Socket 8, item_children_range(Panel1) ranges from 6 to 8. The sorting makes sure that children can always be returned as a simple span.

typedef struct bNodeTreeInterface {
  [...]

  blender::Span<const bNodeTreeInterfaceItem *> items() const;
  blender::MutableSpan<bNodeTreeInterfaceItem *> items();

  blender::IndexRange root_items_range() const;
  blender::Span<const bNodeTreeInterfaceItem *> root_items() const;
  blender::MutableSpan<bNodeTreeInterfaceItem *> root_items();

  blender::IndexRange item_children_range(const bNodeTreeInterfaceItem &item) const;
  blender::Span<const bNodeTreeInterfaceItem *> item_children(const bNodeTreeInterfaceItem &item) const;
  blender::MutableSpan<bNodeTreeInterfaceItem *> item_children(const bNodeTreeInterfaceItem &item);

I've settled on using a single flat array of items for the node group interface. Items can be sockets or panels. Sockets in turn have specialized subclasses for each data type (like bNodeSocket default_value classes but hopefully a less pervasive). ```cpp typedef struct bNodeTreeInterface { [...] bNodeTreeInterfaceItem **items_array; int items_num; }; /** Type of interface item. */ typedef enum eNodeTreeInterfaceItemType { NODE_INTERFACE_SOCKET = 0, NODE_INTERFACE_PANEL = 1, } eNodeTreeInterfaceItemType; /** A socket or panel in the node tree interface. */ typedef struct bNodeTreeInterfaceItem { [...] /* eNodeTreeInterfaceItemType */ char item_type; }; ``` The panel hierarchy is implemented with `parent` pointers in items that place an item inside a panel. ```cpp typedef struct bNodeTreeInterfaceItem { [...] /* Panel in which to display the item. */ struct bNodeTreeInterfacePanel *parent; }; ``` This flat structure is nice for iterating over sockets when we don't care about the hierarchical structure (which is most of the time). The downside is that constructing a hierarchical layout requires some processing to determine root items, children of panels, etc. To make this easier the interface code will keep the items list sorted topologically: Items appear in order of their nesting depth and the relative order of children from the same parent remains unchanged (i.e. _stable sorting_). ![image](/attachments/186d083d-d012-4aa2-a4ec-1191c1cd7087) The interface items have utility functions for getting root items, children of a panel, and respective index ranges. These ranges are also calculated during topological sorting. For example, `item_children(Panel7)` would return `Socket6, Socket7 Socket 8`, `item_children_range(Panel1)` ranges from 6 to 8. The sorting makes sure that children can always be returned as a simple span. ```cpp typedef struct bNodeTreeInterface { [...] blender::Span<const bNodeTreeInterfaceItem *> items() const; blender::MutableSpan<bNodeTreeInterfaceItem *> items(); blender::IndexRange root_items_range() const; blender::Span<const bNodeTreeInterfaceItem *> root_items() const; blender::MutableSpan<bNodeTreeInterfaceItem *> root_items(); blender::IndexRange item_children_range(const bNodeTreeInterfaceItem &item) const; blender::Span<const bNodeTreeInterfaceItem *> item_children(const bNodeTreeInterfaceItem &item) const; blender::MutableSpan<bNodeTreeInterfaceItem *> item_children(const bNodeTreeInterfaceItem &item); ```

image.png

32 KiB

image.png

33 KiB

👍 1

Jacques Lucke commented

2023-06-28 09:25:08 +02:00

I think the general structure of having a base class for sockets and panels is ok if we don't want the limitation that sockets can't come after panels at the same hierarchy level. This is an ok thing to strive for, even if we might decide to limit this a bit at the user level.

I'm not convinced by the flat sorted list yet. The main argument for it seems to be that it makes iterating over sockets easier. However, I'd argue that this argument is not very strong for multiple reasons:

One still has to filter out panels during the iteration, so it's not trivially simple anyway.
The order of sockets in the sorted list does not match the order that the user sees.
We likely still need indices for the sockets. It seems very likely that independent of the approach we take here, we will cache an array of sockets to make socket iteration in the right order and index-based lookup even easier.

On top of these reasons, there is an argument against the flat sorted list: it is a non-obvious data structure which can lead to unnecessary complexity if there is not enough motivation for it. It feels like code that works on this data structure will also become less obvious. While it's not super difficult to work out, the code to move e.g. a socket or panel from one place in the hierarchy to another is not "trivial", at least I can't map it out easily by just looking at the image right now. Moving stuff around likely has to be done often in versioning code, which is best kept straight forward.

Unless additional motivation for this data structure is provided, I'd still vote for a more obvious structure that more explicitly encodes the hierarchy. What I mean is that a panel data structure just has it's own array of children, and the children have a back-pointer to the panel. Having everything in a single flat list is not important to me here, especially if it makes things less obvious. I wouldn't be so worried about iteration, because we can easily cache separate arrays for the things we want to iterate over easily.

I think the general structure of having a base class for sockets and panels is ok if we don't want the limitation that sockets can't come after panels at the same hierarchy level. This is an ok thing to strive for, even if we might decide to limit this a bit at the user level. I'm not convinced by the flat sorted list yet. The main argument for it seems to be that it makes iterating over sockets easier. However, I'd argue that this argument is not very strong for multiple reasons: * One still has to filter out panels during the iteration, so it's not trivially simple anyway. * The order of sockets in the sorted list does not match the order that the user sees. * We likely still need indices for the sockets. It seems very likely that independent of the approach we take here, we will cache an array of sockets to make socket iteration in the right order and index-based lookup even easier. On top of these reasons, there is an argument against the flat sorted list: it is a non-obvious data structure which can lead to unnecessary complexity if there is not enough motivation for it. It feels like code that works on this data structure will also become less obvious. While it's not super difficult to work out, the code to move e.g. a socket or panel from one place in the hierarchy to another is not "trivial", at least I can't map it out easily by just looking at the image right now. Moving stuff around likely has to be done often in versioning code, which is best kept straight forward. Unless additional motivation for this data structure is provided, I'd still vote for a more obvious structure that more explicitly encodes the hierarchy. What I mean is that a panel data structure just has it's own array of children, and the children have a back-pointer to the panel. Having everything in a single flat list is not important to me here, especially if it makes things less obvious. I wouldn't be so worried about iteration, because we can easily cache separate arrays for the things we want to iterate over easily.

👍 2

Lukas Tönne commented

2023-07-17 15:57:22 +02:00

RNA question: How important is the "mix-in" style of providing socket default values and the like?

We have those default_value storage pointers in sockets/interfaces for storing default value, subtype, limits, etc. That's nice because we can swap out the socket type in DNA without destroying the socket itself (which would invalidate all python objects, among other things). I'm using the same approach for the new group interfaces, using the existing bNodeSocketValueXXX structs but in a new bNodeTreeInterfaceSocket.

However, in RNA the socket value properties are integrated into the node socket itself, such that it looks like the properties are part of the node socket instead of a separate DNA struct. This is intuitive to users, but it requires a long list of specialized RNA types: one for each socket type and lots more if the subtype is also expected to affect property display.

We've discussed this and agreed that displaying values with the exact subtype is not required, at least for the interface declarations (nodes themselves keep their current behavior). That still leaves the question of specialized RNA types per socket type. I'd like to avoid generating all these RNA types.

Options i can think of:

Do it like current nodes: Generate an RNA type of the NodeTreeInterfaceSocket for each socket type:
NodeTreeInterfaceSocketFloat, NodeTreeInterfaceSocketVector, NodeTreeInterfaceSocketObject, etc.
Reflect DNA structure and create a socket_data pointer inside NodeTreeInterfaceSocket. Changing socket type invalidates this pointer but not the socket itself. There would be a RNA struct for each socket type (but not subtypes).

socket_interface = tree.interface.active
type(socket_interface) == NodeTreeInterfaceSocket
type(socket_interface.socket_data) == NodeSocketValueFloat

RNA question: _How important is the "mix-in" style of providing socket default values and the like?_ We have those `default_value` storage pointers in sockets/interfaces for storing default value, subtype, limits, etc. That's nice because we can swap out the socket type in DNA without destroying the socket itself (which would invalidate all python objects, among other things). I'm using the same approach for the new group interfaces, using the existing `bNodeSocketValueXXX` structs but in a new `bNodeTreeInterfaceSocket`. However, in RNA the socket value properties are integrated into the node socket itself, such that it looks like the properties are part of the node socket instead of a separate DNA struct. This is intuitive to users, but it requires a [long list of specialized RNA types](https://docs.blender.org/api/main/bpy.types.NodeSocketInterfaceBool.html): one for each socket type and lots more if the subtype is also expected to affect property display. We've discussed this and agreed that displaying values with the exact subtype is not required, at least for the interface declarations (nodes themselves keep their current behavior). That still leaves the question of specialized RNA types per socket type. I'd like to avoid generating all these RNA types. Options i can think of: 1. Do it like current nodes: Generate an RNA type of the `NodeTreeInterfaceSocket` for each socket type: `NodeTreeInterfaceSocketFloat`, `NodeTreeInterfaceSocketVector`, `NodeTreeInterfaceSocketObject`, etc. 2. Reflect DNA structure and create a `socket_data` pointer inside `NodeTreeInterfaceSocket`. Changing socket type invalidates this pointer but not the socket itself. There would be a RNA struct for each socket type (but not subtypes). ```py socket_interface = tree.interface.active type(socket_interface) == NodeTreeInterfaceSocket type(socket_interface.socket_data) == NodeSocketValueFloat ```

Brecht Van Lommel commented

2023-07-17 16:02:48 +02:00

I prefer (1), even all the subtypes don't bother me. I see no good reason to be inconsistent with NodeSocket* or break the API.

If you do (2) you now have to introduce a bunch of NodeSocketValue* types instead, or break the API really badly.

I prefer (1), even all the subtypes don't bother me. I see no good reason to be inconsistent with `NodeSocket*` or break the API. If you do (2) you now have to introduce a bunch of `NodeSocketValue*` types instead, or break the API really badly.

👍 1

Lukas Tönne referenced this issue

2023-07-17 19:14:57 +02:00

Nodes: Move node socket RNA definition into its own file. #110220

Lukas Tönne referenced this issue from a commit

2023-07-18 11:17:40 +02:00

Nodes: Move node socket RNA definition into its own file.

Lukas Tönne referenced this issue

2023-07-19 18:15:40 +02:00

WIP: Nodes: New node group interface declaration to support panels #110272

Campbell Barton referenced this issue from a commit

2023-07-20 05:06:29 +02:00

Nodes: Move node socket RNA definition into its own file.

Lukas Tönne commented

2023-07-20 12:01:03 +02:00

Quick draft of what the node declaration API might look like for panel support and aligned sockets. Sockets would also appear on nodes in the order they are declared, instead of the currently fixed outputs | buttons | inputs order. This will require a pass over all nodes to make sure they are consistent.

Buttons placement: does this require an explicit API method or do we just always draw them on top? Could also be optional: draw at the top by default, unless declaration explicitly adds them somewhere else. I'd maybe keep it simple for now and allow for explicit placement later. Also note that node layouts will change and might mess up existing node connection visuals in carefully layed out trees.

NodeDeclarationBuilder additions:

  /* Aligned dual-purpose socket, generates both input and output. */
  template<typename DeclType>
  typename DeclType::Builder &add_input_output(StringRef name, StringRef identifier = "");

  /* Opens a new panel. Subsequent sockets and panels are placed inside, until close_panel() is
   * called. Can be called multiple times to generate sub-panels. */
  PanelDeclarationBuilder &add_panel(StringRef name);
  /* Close the top panel, parent panels remain open. */
  void close_panel();
  /* Close all currently open panels. */
  void close_all_panels();

Example usage (raycast node):

static void node_declare(NodeDeclarationBuilder &b)
{
  b.add_input<decl::Geometry>("Target Geometry")
      .only_realized_data()
      .supported_type(GeometryComponent::Type::Mesh);

  // Opens a new panel. Subsequent sockets and panels are placed inside, until close_panel() is
  // called. Can be called multiple times to generate sub-panels.
  b.add_panel("Attributes").default_closed();

  // Aligned dual-purpose socket, generates both input and output.
  b.add_input_output<decl::Vector>("Attribute").hide_value().field_on_all();
  b.add_input_output<decl::Float>("Attribute", "Attribute_001").hide_value().field_on_all();
  b.add_input_output<decl::Color>("Attribute", "Attribute_002").hide_value().field_on_all();
  b.add_input_output<decl::Bool>("Attribute", "Attribute_003").hide_value().field_on_all();
  b.add_input_output<decl::Int>("Attribute", "Attribute_004").hide_value().field_on_all();
  b.add_input_output<decl::Rotation>("Attribute", "Attribute_005").hide_value().field_on_all();

  // Close the top panel. Possible parent panels remain open, call close_all_panels to close the
  // entire stack.
  b.close_panel();

  b.add_input<decl::Vector>("Source Position").implicit_field(implicit_field_inputs::position);
  b.add_input<decl::Vector>("Ray Direction").default_value({0.0f, 0.0f, -1.0f}).supports_field();
  b.add_input<decl::Float>("Ray Length")
      .default_value(100.0f)
      .min(0.0f)
      .subtype(PROP_DISTANCE)
      .supports_field();

  b.add_output<decl::Bool>("Is Hit").dependent_field({7, 8, 9});
  b.add_output<decl::Vector>("Hit Position").dependent_field({7, 8, 9});
  b.add_output<decl::Vector>("Hit Normal").dependent_field({7, 8, 9});
  b.add_output<decl::Float>("Hit Distance").dependent_field({7, 8, 9});
}

Quick draft of what the node declaration API might look like for panel support and aligned sockets. Sockets would also appear on nodes in the order they are declared, instead of the currently fixed _outputs | buttons | inputs_ order. This will require a pass over all nodes to make sure they are consistent. Buttons placement: does this require an explicit API method or do we just always draw them on top? Could also be optional: draw at the top by default, unless declaration explicitly adds them somewhere else. I'd maybe keep it simple for now and allow for explicit placement later. Also note that node layouts will change and might mess up existing node connection visuals in carefully layed out trees. `NodeDeclarationBuilder` additions: ```cpp /* Aligned dual-purpose socket, generates both input and output. */ template<typename DeclType> typename DeclType::Builder &add_input_output(StringRef name, StringRef identifier = ""); /* Opens a new panel. Subsequent sockets and panels are placed inside, until close_panel() is * called. Can be called multiple times to generate sub-panels. */ PanelDeclarationBuilder &add_panel(StringRef name); /* Close the top panel, parent panels remain open. */ void close_panel(); /* Close all currently open panels. */ void close_all_panels(); ``` Example usage (raycast node): ```cpp static void node_declare(NodeDeclarationBuilder &b) { b.add_input<decl::Geometry>("Target Geometry") .only_realized_data() .supported_type(GeometryComponent::Type::Mesh); // Opens a new panel. Subsequent sockets and panels are placed inside, until close_panel() is // called. Can be called multiple times to generate sub-panels. b.add_panel("Attributes").default_closed(); // Aligned dual-purpose socket, generates both input and output. b.add_input_output<decl::Vector>("Attribute").hide_value().field_on_all(); b.add_input_output<decl::Float>("Attribute", "Attribute_001").hide_value().field_on_all(); b.add_input_output<decl::Color>("Attribute", "Attribute_002").hide_value().field_on_all(); b.add_input_output<decl::Bool>("Attribute", "Attribute_003").hide_value().field_on_all(); b.add_input_output<decl::Int>("Attribute", "Attribute_004").hide_value().field_on_all(); b.add_input_output<decl::Rotation>("Attribute", "Attribute_005").hide_value().field_on_all(); // Close the top panel. Possible parent panels remain open, call close_all_panels to close the // entire stack. b.close_panel(); b.add_input<decl::Vector>("Source Position").implicit_field(implicit_field_inputs::position); b.add_input<decl::Vector>("Ray Direction").default_value({0.0f, 0.0f, -1.0f}).supports_field(); b.add_input<decl::Float>("Ray Length") .default_value(100.0f) .min(0.0f) .subtype(PROP_DISTANCE) .supports_field(); b.add_output<decl::Bool>("Is Hit").dependent_field({7, 8, 9}); b.add_output<decl::Vector>("Hit Position").dependent_field({7, 8, 9}); b.add_output<decl::Vector>("Hit Normal").dependent_field({7, 8, 9}); b.add_output<decl::Float>("Hit Distance").dependent_field({7, 8, 9}); } ```

Lukas Tönne commented

2023-07-20 12:43:56 +02:00

I'm not quite happy with the add_input_output method: it would limit the alignment feature (input + output in the same vertical slot) to only matching socket types/names.

A better option would be to keep the current add_input/add_output methods, but add a align_with_previous option (default false). By default adding a new socket will move to the next vertical position, leaving a gap when alternating inputs/outputs. When align_with_previous is set to true it will instead try and use the same vertical slot as the previous socket (ignored if the previous slot already has a socket of the same input/output kind). This would allow alignment of arbitrary sockets, provided there is enough space in the layout. It would be easier to use because the builders only describe one socket at a time instead of a "double socket".

template<typename SocketDecl>
class SocketDeclarationBuilder : public BaseSocketDeclarationBuilder {
 public:
  /* Try to use the same vertical layout slot as the previous socket.
   * If false the socket is moved to the next vertical slot.
   * Can be used to align input/output sockets. */
  Self &align_with_previous(bool value = true);
};

I'm not quite happy with the `add_input_output` method: it would limit the alignment feature (input + output in the same vertical slot) to only matching socket types/names. A better option would be to keep the current `add_input`/`add_output` methods, but add a `align_with_previous` option (default false). By default adding a new socket will move to the next vertical position, leaving a gap when alternating inputs/outputs. When `align_with_previous` is set to `true` it will instead try and use the same vertical slot as the previous socket (ignored if the previous slot already has a socket of the same input/output kind). This would allow alignment of arbitrary sockets, provided there is enough space in the layout. It would be easier to use because the builders only describe one socket at a time instead of a "double socket". ```cpp template<typename SocketDecl> class SocketDeclarationBuilder : public BaseSocketDeclarationBuilder { public: /* Try to use the same vertical layout slot as the previous socket. * If false the socket is moved to the next vertical slot. * Can be used to align input/output sockets. */ Self &align_with_previous(bool value = true); }; ```

Brecht Van Lommel commented

2023-07-20 13:22:00 +02:00

For panels, with the naming add/close it's not clear to me that they need to be paired. I suggest begin_panel and end_panel.

For alignment, an alternative would to have a b.add_output_aligned that doesn't take a name or identifier, because I guess we want to make sure those match?

For panels, with the naming add/close it's not clear to me that they need to be paired. I suggest `begin_panel` and `end_panel`. For alignment, an alternative would to have a `b.add_output_aligned` that doesn't take a name or identifier, because I guess we want to make sure those match?

Lukas Tönne commented

2023-07-20 13:59:04 +02:00

For panels, with the naming add/close it's not clear to me that they need to be paired. I suggest begin_panel and end_panel.

Yes, that's more clear. It would also be possible to put add_socket methods into the PanelDeclarationBuilder and create a hierarchy by adding sockets on the panel builder instead of the root NodeDeclarationBuilder. This is just to ensure correct scoping, the declaration output is still a flat list.

For alignment, an alternative would to have a b.add_output_aligned that doesn't take a name or identifier, because I guess we want to make sure those match?

Yes, that would be a good convenience method for the most common case.

The final output of the declaration would be a draw-order list of items (sockets + panels). We currently store inputs/outputs separate, but that way the order of declaration is lost and we can't alternate between inputs and outputs. Separate inputs/outputs isn't needed in declarations, it's easy enough to filter a single list by in_out type.

> For panels, with the naming add/close it's not clear to me that they need to be paired. I suggest begin_panel and end_panel. Yes, that's more clear. It would also be possible to put `add_socket` methods into the `PanelDeclarationBuilder` and create a hierarchy by adding sockets on the panel builder instead of the root `NodeDeclarationBuilder`. This is just to ensure correct scoping, the declaration output is still a flat list. > For alignment, an alternative would to have a b.add_output_aligned that doesn't take a name or identifier, because I guess we want to make sure those match? Yes, that would be a good convenience method for the most common case. The final output of the declaration would be a draw-order list of items (sockets + panels). We currently store inputs/outputs separate, but that way the order of declaration is lost and we can't alternate between inputs and outputs. Separate inputs/outputs isn't needed in declarations, it's easy enough to filter a single list by `in_out` type.

Jacques Lucke commented

2023-07-20 14:48:43 +02:00

Not really suggesting that we do this, but I wanted to share how I solved this when I tested aligned sockets: https://projects.blender.org/blender/blender/compare/main...JacquesLucke/blender:socket-align#diff-5348c1b18807bc1a143b9c75d173939b80882f8c
Note how I didn't do the alignment as part of the declaration but as part of the drawing function instead. Doing it as part of the declaration now is better though.

For alignment, an alternative would to have a b.add_output_aligned that doesn't take a name or identifier, because I guess we want to make sure those match?

I think this makes sense. For the time being we could also just match corresponding sockets using their identifiers (only applies to built-in nodes).

begin_panel and end_panel

I'm fine with those methods, however it seems like it would be better to use something that enforces scoping. That would most likely mean that we have an add_panel(FunctionRef<void()>) method on the builder. All sockets added within the callback are part of the panel.

Not really suggesting that we do this, but I wanted to share how I solved this when I tested aligned sockets: https://projects.blender.org/blender/blender/compare/main...JacquesLucke/blender:socket-align#diff-5348c1b18807bc1a143b9c75d173939b80882f8c Note how I didn't do the alignment as part of the declaration but as part of the drawing function instead. Doing it as part of the declaration now is better though. > For alignment, an alternative would to have a b.add_output_aligned that doesn't take a name or identifier, because I guess we want to make sure those match? I think this makes sense. For the time being we could also just match corresponding sockets using their identifiers (only applies to built-in nodes). > `begin_panel` and `end_panel` I'm fine with those methods, however it seems like it would be better to use something that enforces scoping. That would most likely mean that we have an `add_panel(FunctionRef<void()>)` method on the builder. All sockets added within the callback are part of the panel.

👍 1

Iliya Katushenock referenced this issue

2023-07-23 13:53:49 +02:00

Node overlay and hide scalable for socket amount #110397

Lukas Tönne commented

2023-07-25 10:58:41 +02:00

I've hit a snag when building in MSVC: the "interface" keyword is #defined there as "struct", which breaks much of my code ...
https://learn.microsoft.com/en-us/cpp/cpp/interface?view=msvc-170&redirectedfrom=MSDN

I've read /Za could disable all those language extensions, but haven't tried yet and not sure if it disables parts we rely on and it might be unrelated as well.
https://learn.microsoft.com/en-us/cpp/build/reference/za-ze-disable-language-extensions?view=msvc-170

I've hit a snag when building in MSVC: the "interface" keyword is #defined there as "struct", which breaks much of my code ... https://learn.microsoft.com/en-us/cpp/cpp/interface?view=msvc-170&redirectedfrom=MSDN I've read `/Za` could disable all those language extensions, but haven't tried yet and not sure if it disables parts we rely on and it might be unrelated as well. https://learn.microsoft.com/en-us/cpp/build/reference/za-ze-disable-language-extensions?view=msvc-170

Lukas Tönne commented

2023-07-25 12:21:11 +02:00

Regarding API: I could try and keep the existing API, but redirect the implementation to the new interface DNA. This should help avoid breaking scripts, but it also means there will be 2 separate APIs doing the same thing. The type names of the RNA structs will also change, but those is not usually needed in Python (we don't explicitly construct stuff other than through RNA methods).

Here's a comparison of common API uses:

Old interface

# Adding, removing, moving inputs/outputs
ntree.inputs.new("NodeSocketFloat", "Smoothness")
ntree.outputs.new("NodeSocketColor", "Diffuse Color")
ntree.inputs.remove(socket)
ntree.inputs.move(3, 1)

# Getting the active index/socket
active_input_index = ntree.active_input
active_output_index = ntree.active_output
active_input = ntree.inputs.get(ntree.active_input)
active_output = ntree.outputs.get(ntree.active_output)

# Socket details (type dependent)
ntree.inputs["Smoothness"].max_value = 50

# Iterate over sockets
for socket in ntree.inputs:
    ...

New interface

# Adding, removing, moving inputs/outputs
ntree.interface.new_socket("Smoothness", socket_type="NodeSocketFloat")
# Now also for panels
ntree.interface.new_panel("Advanced")
ntree.interface.remove(socket)
ntree.interface.move(socket, 1)

# Getting the active index/item
# There is only one active item for combined inputs/outputs
active_item_index = ntree.interface.active_index
active_item = ntree.interface.active

# Socket details (type dependent)
ntree.interface.inputs["Smoothness"].max_value = 50

# Iterate over sockets
for socket in ntree.interface.ui_items:
    # These can be sockets or panels
    if socket.item_type == 'SOCKET':
        ...

# New stuff!!

# Add an item inside a panel
panel = ntree.interface.new_panel("Advanced")
ntree.interface.new_socket("Smoothness", socket_type="NodeSocketFloat", parent=panel)
# Changing the parent later
socket = ntree.interface.new_socket("Smoothness", socket_type="NodeSocketFloat")
socket.parent = panel

Regarding API: I could try and keep the existing API, but redirect the implementation to the new interface DNA. This should help avoid breaking scripts, but it also means there will be 2 separate APIs doing the same thing. The type names of the RNA structs will also change, but those is not usually needed in Python (we don't explicitly construct stuff other than through RNA methods). Here's a comparison of common API uses: ### Old interface ```python # Adding, removing, moving inputs/outputs ntree.inputs.new("NodeSocketFloat", "Smoothness") ntree.outputs.new("NodeSocketColor", "Diffuse Color") ntree.inputs.remove(socket) ntree.inputs.move(3, 1) # Getting the active index/socket active_input_index = ntree.active_input active_output_index = ntree.active_output active_input = ntree.inputs.get(ntree.active_input) active_output = ntree.outputs.get(ntree.active_output) # Socket details (type dependent) ntree.inputs["Smoothness"].max_value = 50 # Iterate over sockets for socket in ntree.inputs: ... ``` ### New interface ```python # Adding, removing, moving inputs/outputs ntree.interface.new_socket("Smoothness", socket_type="NodeSocketFloat") # Now also for panels ntree.interface.new_panel("Advanced") ntree.interface.remove(socket) ntree.interface.move(socket, 1) # Getting the active index/item # There is only one active item for combined inputs/outputs active_item_index = ntree.interface.active_index active_item = ntree.interface.active # Socket details (type dependent) ntree.interface.inputs["Smoothness"].max_value = 50 # Iterate over sockets for socket in ntree.interface.ui_items: # These can be sockets or panels if socket.item_type == 'SOCKET': ... # New stuff!! # Add an item inside a panel panel = ntree.interface.new_panel("Advanced") ntree.interface.new_socket("Smoothness", socket_type="NodeSocketFloat", parent=panel) # Changing the parent later socket = ntree.interface.new_socket("Smoothness", socket_type="NodeSocketFloat") socket.parent = panel ```

Lukas Tönne referenced this issue

2023-07-25 16:12:39 +02:00

Breaking changes for 4.0 #105523

Brecht Van Lommel commented

2023-07-25 16:46:21 +02:00

I've hit a snag when building in MSVC: the "interface" keyword is #defined there as "struct", which breaks much of my code ...
https://learn.microsoft.com/en-us/cpp/cpp/interface?view=msvc-170&redirectedfrom=MSDN

I've read /Za could disable all those language extensions, but haven't tried yet and not sure if it disables parts we rely on and it might be unrelated as well.
https://learn.microsoft.com/en-us/cpp/build/reference/za-ze-disable-language-extensions?view=msvc-170

I recommend to just use another word, and avoid the added complexity of compiler flags.

Regarding API: I could try and keep the existing API, but redirect the implementation to the new interface DNA. This should help avoid breaking scripts, but it also means there will be 2 separate APIs doing the same thing. The type names of the RNA structs will also change, but those is not usually needed in Python (we don't explicitly construct stuff other than through RNA methods).

As discussed offline, I don't think we need to preserve API compatibility here. When we make significant changes to the actual functionality, it's fine to change the API along with that to reflect the changes.

The main thing is to have .blend file compatibility here.

> I've hit a snag when building in MSVC: the "interface" keyword is #defined there as "struct", which breaks much of my code ... > https://learn.microsoft.com/en-us/cpp/cpp/interface?view=msvc-170&redirectedfrom=MSDN > > I've read `/Za` could disable all those language extensions, but haven't tried yet and not sure if it disables parts we rely on and it might be unrelated as well. > https://learn.microsoft.com/en-us/cpp/build/reference/za-ze-disable-language-extensions?view=msvc-170 > I recommend to just use another word, and avoid the added complexity of compiler flags. > Regarding API: I could try and keep the existing API, but redirect the implementation to the new interface DNA. This should help avoid breaking scripts, but it also means there will be 2 separate APIs doing the same thing. The type names of the RNA structs will also change, but those is not usually needed in Python (we don't explicitly construct stuff other than through RNA methods). As discussed offline, I don't think we need to preserve API compatibility here. When we make significant changes to the actual functionality, it's fine to change the API along with that to reflect the changes. The main thing is to have .blend file compatibility here.

Lukas Tönne commented

2023-07-31 10:44:42 +02:00

API question:
It looks like the bNodeSocketType::interface_init_socket method is no longer used. It is intended to initialize a bNodeSocket based on the settings in the interface.

The new node declarations system replaced the direct creation and initialization of sockets from the interface, and it does not use this method any more. In particular for custom sockets this may have been an (unnoticed?) breaking change, but i don't recall seeing any bug reports. Maybe this part of the API is too arcane, but it looks like custom sockets are basically not customizable in node groups now. Any custom (python) socket will use the same default RNA values in each socket instance, not a per-group-node value defined by the node group interface.

The "reverse" method initializes a node group interface entry from an existing socket with the interface_from_socket callback. This is still in use.

Do we still need this method? Is there an alternative to defining these functions in a separate python class? Part of the problem is that we need 2 separate python classes to implement both the socket type and the matching interface:

# Defines properties and callbacks for actual sockets
class MyCustomSocket(NodeSocket):
    """Custom node socket type"""
    bl_idname = 'CustomSocketType'

    value : FloatProperty("Value", "Default value when the socket is not connected")
    max_value : FloatProperty("Max Value", "Maximum allowed value")

    # Drawing buttons on sockets
    def draw(self, context, layout, node, text):
        layout.prop(self, "value", text=text)


# Customizable interface properties to generate a socket from.
class MyCustomInterfaceSocket(NodeTreeInterfaceSocket):
    # The type of socket that is generated.
    bl_socket_idname = 'CustomSocketType'

    default_value : FloatProperty("Default Value", "Default value for new sockets")
    max_value : FloatProperty("Max Value", "Maximum allowed value")
    
    def draw_socket_properties(self, context, layout):
        layout.prop(self, "max_value")

    # This does not work any more!
    def init_socket(self, node, socket):
        socket.value = self.default_value
        socket.max_value = self.max_value

API question: It looks like the [`bNodeSocketType::interface_init_socket`](https://projects.blender.org/blender/blender/src/commit/52acf6a6ec902768e48a58a5739053c3f080850f/source/blender/blenkernel/BKE_node.h#L177) method is no longer used. It is intended to initialize a `bNodeSocket` based on the settings in the interface. The new node declarations system replaced the direct creation and initialization of sockets from the interface, and it does not use this method any more. In particular for custom sockets this may have been an (unnoticed?) breaking change, but i don't recall seeing any bug reports. Maybe this part of the API is too arcane, but it looks like custom sockets are basically not customizable in node groups now. Any custom (python) socket will use the same default RNA values in each socket instance, not a per-group-node value defined by the node group interface. The "reverse" method initializes a node group interface entry from an existing socket with the `interface_from_socket` callback. This is [still in use](https://projects.blender.org/blender/blender/src/commit/52acf6a6ec902768e48a58a5739053c3f080850f/source/blender/blenkernel/intern/node.cc#L3810). Do we still need this method? Is there an alternative to defining these functions in a separate python class? Part of the problem is that we need 2 separate python classes to implement both the socket type and the matching interface: ```python # Defines properties and callbacks for actual sockets class MyCustomSocket(NodeSocket): """Custom node socket type""" bl_idname = 'CustomSocketType' value : FloatProperty("Value", "Default value when the socket is not connected") max_value : FloatProperty("Max Value", "Maximum allowed value") # Drawing buttons on sockets def draw(self, context, layout, node, text): layout.prop(self, "value", text=text) # Customizable interface properties to generate a socket from. class MyCustomInterfaceSocket(NodeTreeInterfaceSocket): # The type of socket that is generated. bl_socket_idname = 'CustomSocketType' default_value : FloatProperty("Default Value", "Default value for new sockets") max_value : FloatProperty("Max Value", "Maximum allowed value") def draw_socket_properties(self, context, layout): layout.prop(self, "max_value") # This does not work any more! def init_socket(self, node, socket): socket.value = self.default_value socket.max_value = self.max_value ```

Lukas Tönne commented

2023-07-31 12:08:22 +02:00

Discussed this in chat:

Broken probably since node groups use declarations (~6 months) but worked before that:
a95bf1ac01/source/blender/nodes/intern/node_common.cc (L134)
Will try and make the Custom socket declaration use this callback again to restore ability to define custom socket interfaces.
Design for this should be revisited separately, it's not very convenient. Out of scope for this task though.

Discussed this in chat: * Broken probably since node groups use declarations (~6 months) but worked before that: https://projects.blender.org/blender/blender/src/commit/a95bf1ac01be7017007f6d1112a7a994db2e01fd/source/blender/nodes/intern/node_common.cc#L134 * Will try and make the `Custom` socket declaration use this callback again to restore ability to define custom socket interfaces. * Design for this should be revisited separately, it's not very convenient. Out of scope for this task though.

Lukas Tönne commented

2023-07-31 12:10:43 +02:00

Issue reported here some time ago:
#105283

Issue reported here some time ago: #105283

Lukas Tönne referenced this issue

2023-08-04 17:34:57 +02:00

Nodes: Remove the experimental "node panels" feature #110803

Lukas Tönne referenced this issue from a commit

2023-08-07 10:40:37 +02:00

Nodes: Remove the experimental "node panels" feature

Lukas Tönne referenced this issue

2023-08-07 12:28:34 +02:00