tmaq
/
tornavis
1
0
Fork 0
Code Issues 2 Pull Requests 29 Packages Projects 24 Releases Wiki Activity

Nodes: Use index instead of reordering for draw order 

Currently nodes are reordered so that the "on top" nodes are last in
the list. Node order changing for simple operations like selection
means we either have to reevaluate the node tree data-block on
selections or accept that the evaluated order can be different from the
original. Currently we do the latter (see d76a0e98ba), but
makes it complex to access nodes by index, and is hard to reason about.

Instead of reordering nodes, store the ui order in the node itself
and sort the nodes before drawing them or doing any processing
that depends on the "depth."

The "selected_nodes" list in the context is no longer ordered by the
recent selection.

Pull Request: https://projects.blender.org/blender/blender/pulls/113419
This commit is contained in:
Hans Goudey 2023-10-10 10:57:51 +02:00 committed by Hans Goudey
parent 203559757a
commit 1ccba4d9fe
9 changed files with 70 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
source/blender/blenkernel/BKE_node_tree_update.h
View File

@ -35,7 +35,6 @@ void BKE_ntree_update_tag_all(struct bNodeTree *ntree);
void BKE_ntree_update_tag_node_property(struct bNodeTree *ntree, struct bNode *node);
void BKE_ntree_update_tag_node_new(struct bNodeTree *ntree, struct bNode *node);
void BKE_ntree_update_tag_node_removed(struct bNodeTree *ntree);
void BKE_ntree_update_tag_node_reordered(struct bNodeTree *ntree);
void BKE_ntree_update_tag_node_mute(struct bNodeTree *ntree, struct bNode *node);
void BKE_ntree_update_tag_node_internal_link(struct bNodeTree *ntree, struct bNode *node);

9
source/blender/blenkernel/intern/node_tree_update.cc
View File

@ -1287,15 +1287,6 @@ void BKE_ntree_update_tag_node_removed(bNodeTree *ntree)
add_tree_tag(ntree, NTREE_CHANGED_REMOVED_NODE);
}
void BKE_ntree_update_tag_node_reordered(bNodeTree *ntree)
{
/* Don't add a tree update tag to avoid reevaluations for trivial operations like selection or
* parenting that typically influence the node order. This means the node order can be different
* for original and evaluated trees. A different solution might avoid sorting nodes based on UI
* states like selection, which would require not tying the node order to the drawing order. */
ntree->runtime->topology_cache_mutex.tag_dirty();
}
void BKE_ntree_update_tag_node_mute(bNodeTree *ntree, bNode *node)
{
add_node_tag(ntree, node, NTREE_CHANGED_NODE_PROPERTY);

53
source/blender/editors/space_node/node_draw.cc
View File

@ -287,28 +287,39 @@ static bool compare_node_depth(const bNode *a, const bNode *b)
return false;
}
void node_sort(bNodeTree &ntree)
void tree_draw_order_update(bNodeTree &ntree)
{
Array<bNode *> sort_nodes = ntree.all_nodes();
std::stable_sort(sort_nodes.begin(), sort_nodes.end(), compare_node_depth);
/* If nothing was changed, exit early. Otherwise the node tree's runtime
* node vector needs to be rebuilt, since it cannot be reordered in place. */
if (sort_nodes == ntree.all_nodes()) {
return;
}
BKE_ntree_update_tag_node_reordered(&ntree);
ntree.runtime->nodes_by_id.clear();
BLI_listbase_clear(&ntree.nodes);
for (const int i : sort_nodes.index_range()) {
BLI_addtail(&ntree.nodes, sort_nodes[i]);
ntree.runtime->nodes_by_id.add_new(sort_nodes[i]);
sort_nodes[i]->runtime->index_in_tree = i;
sort_nodes[i]->ui_order = i;
}
}
Array<bNode *> tree_draw_order_calc_nodes(bNodeTree &ntree)
{
Array<bNode *> nodes = ntree.all_nodes();
if (nodes.is_empty()) {
return {};
}
std::sort(nodes.begin(), nodes.end(), [](const bNode *a, const bNode *b) {
return a->ui_order < b->ui_order;
});
return nodes;
}
Array<bNode *> tree_draw_order_calc_nodes_reversed(bNodeTree &ntree)
{
Array<bNode *> nodes = ntree.all_nodes();
if (nodes.is_empty()) {
return {};
}
std::sort(nodes.begin(), nodes.end(), [](const bNode *a, const bNode *b) {
return a->ui_order > b->ui_order;
});
return nodes;
}
static Array<uiBlock *> node_uiblocks_init(const bContext &C, const Span<bNode *> nodes)
{
Array<uiBlock *> blocks(nodes.size());
@ -3304,13 +3315,9 @@ int node_get_resize_cursor(NodeResizeDirection directions)
static const bNode *find_node_under_cursor(SpaceNode &snode, const float2 &cursor)
{
const Span<bNode *> nodes = snode.edittree->all_nodes();
if (nodes.is_empty()) {
return nullptr;
}
for (int i = nodes.index_range().last(); i >= 0; i--) {
if (BLI_rctf_isect_pt(&nodes[i]->runtime->totr, cursor[0], cursor[1])) {
return nodes[i];
for (const bNode *node : tree_draw_order_calc_nodes_reversed(*snode.edittree)) {
if (BLI_rctf_isect_pt(&node->runtime->totr, cursor[0], cursor[1])) {
return node;
}
}
return nullptr;
@ -4029,7 +4036,7 @@ static void draw_nodetree(const bContext &C,
SpaceNode *snode = CTX_wm_space_node(&C);
ntree.ensure_topology_cache();
const Span<bNode *> nodes = ntree.all_nodes();
Array<bNode *> nodes = tree_draw_order_calc_nodes(ntree);
Array<uiBlock *> blocks = node_uiblocks_init(C, nodes);

19
source/blender/editors/space_node/node_edit.cc
View File

@ -846,9 +846,9 @@ namespace blender::ed::space_node {
static bool socket_is_occluded(const float2 &location,
const bNode &node_the_socket_belongs_to,
const SpaceNode &snode)
const Span<bNode *> sorted_nodes)
{
LISTBASE_FOREACH_BACKWARD (bNode *, node, &snode.edittree->nodes) {
for (bNode *node : sorted_nodes) {
if (node == &node_the_socket_belongs_to) {
/* Nodes after this one are underneath and can't occlude the socket. */
return false;
@ -1168,13 +1168,14 @@ bNodeSocket *node_find_indicated_socket(SpaceNode &snode,
bNodeTree &node_tree = *snode.edittree;
node_tree.ensure_topology_cache();
const Span<bNode *> nodes = node_tree.all_nodes();
if (nodes.is_empty()) {
const Array<bNode *> sorted_nodes = tree_draw_order_calc_nodes_reversed(*snode.edittree);
if (sorted_nodes.is_empty()) {
return nullptr;
}
for (int i = nodes.index_range().last(); i >= 0; i--) {
bNode &node = *nodes[i];
for (const int i : sorted_nodes.index_range()) {
bNode &node = *sorted_nodes[i];
BLI_rctf_init_pt_radius(&rect, cursor, size_sock_padded);
if (!(node.flag & NODE_HIDDEN)) {
@ -1195,13 +1196,13 @@ bNodeSocket *node_find_indicated_socket(SpaceNode &snode,
const float2 location = sock->runtime->location;
if (sock->flag & SOCK_MULTI_INPUT && !(node.flag & NODE_HIDDEN)) {
if (cursor_isect_multi_input_socket(cursor, *sock)) {
if (!socket_is_occluded(location, node, snode)) {
if (!socket_is_occluded(location, node, sorted_nodes)) {
return sock;
}
}
}
else if (BLI_rctf_isect_pt(&rect, location.x, location.y)) {
if (!socket_is_occluded(location, node, snode)) {
if (!socket_is_occluded(location, node, sorted_nodes)) {
return sock;
}
}
@ -1213,7 +1214,7 @@ bNodeSocket *node_find_indicated_socket(SpaceNode &snode,
if (node.is_socket_icon_drawn(*sock)) {
const float2 location = sock->runtime->location;
if (BLI_rctf_isect_pt(&rect, location.x, location.y)) {
if (!socket_is_occluded(location, node, snode)) {
if (!socket_is_occluded(location, node, sorted_nodes)) {
return sock;
}
}

8
source/blender/editors/space_node/node_intern.hh
View File

@ -174,10 +174,14 @@ void node_draw_space(const bContext &C, ARegion &region);
void node_socket_add_tooltip(const bNodeTree &ntree, const bNodeSocket &sock, uiLayout &layout);
/**
* Sort nodes by selection: unselected nodes first, then selected,
* Update node draw order nodes based on selection: unselected nodes first, then selected,
* then the active node at the very end. Relative order is kept intact.
*/
void node_sort(bNodeTree &ntree);
void tree_draw_order_update(bNodeTree &ntree);
/** Return the nodes in draw order, with the top nodes at the end. */
Array<bNode *> tree_draw_order_calc_nodes(bNodeTree &ntree);
/** Return the nodes in reverse draw order, with the top nodes at the start. */
Array<bNode *> tree_draw_order_calc_nodes_reversed(bNodeTree &ntree);
void node_set_cursor(wmWindow &win, SpaceNode &snode, const float2 &cursor);
/* DPI scaled coords */

18
source/blender/editors/space_node/node_relationships.cc
View File

@ -1782,7 +1782,7 @@ static int node_parent_set_exec(bContext *C, wmOperator * /*op*/)
}
}
node_sort(ntree);
tree_draw_order_update(ntree);
WM_event_add_notifier(C, NC_NODE | ND_DISPLAY, nullptr);
return OPERATOR_FINISHED;
@ -1869,7 +1869,7 @@ static int node_join_exec(bContext *C, wmOperator * /*op*/)
}
}
node_sort(ntree);
tree_draw_order_update(ntree);
ED_node_tree_propagate_change(C, &bmain, snode.edittree);
WM_event_add_notifier(C, NC_NODE | ND_DISPLAY, nullptr);
@ -1897,15 +1897,13 @@ void NODE_OT_join(wmOperatorType *ot)
/** \name Attach Operator
* \{ */
static bNode *node_find_frame_to_attach(ARegion &region,
const bNodeTree &ntree,
const int2 mouse_xy)
static bNode *node_find_frame_to_attach(ARegion &region, bNodeTree &ntree, const int2 mouse_xy)
{
/* convert mouse coordinates to v2d space */
float2 cursor;
UI_view2d_region_to_view(&region.v2d, mouse_xy.x, mouse_xy.y, &cursor.x, &cursor.y);
LISTBASE_FOREACH_BACKWARD (bNode *, frame, &ntree.nodes) {
for (bNode *frame : tree_draw_order_calc_nodes_reversed(ntree)) {
/* skip selected, those are the nodes we want to attach */
if ((frame->type != NODE_FRAME) || (frame->flag & NODE_SELECT)) {
continue;
@ -1929,7 +1927,7 @@ static int node_attach_invoke(bContext *C, wmOperator * /*op*/, const wmEvent *e
return OPERATOR_FINISHED;
}
LISTBASE_FOREACH_BACKWARD (bNode *, node, &ntree.nodes) {
for (bNode *node : tree_draw_order_calc_nodes_reversed(*snode.edittree)) {
if (!(node->flag & NODE_SELECT)) {
continue;
}
@ -1954,7 +1952,7 @@ static int node_attach_invoke(bContext *C, wmOperator * /*op*/, const wmEvent *e
nodeAttachNode(&ntree, node, frame);
}
node_sort(ntree);
tree_draw_order_update(ntree);
WM_event_add_notifier(C, NC_NODE | ND_DISPLAY, nullptr);
return OPERATOR_FINISHED;
@ -2029,7 +2027,7 @@ static int node_detach_exec(bContext *C, wmOperator * /*op*/)
}
}
node_sort(ntree);
tree_draw_order_update(ntree);
WM_event_add_notifier(C, NC_NODE | ND_DISPLAY, nullptr);
return OPERATOR_FINISHED;
@ -2461,7 +2459,7 @@ static void node_link_insert_offset_ntree(NodeInsertOfsData *iofsd,
rctf totr_frame;
/* check nodes front to back */
LISTBASE_FOREACH_BACKWARD (bNode *, frame, &ntree->nodes) {
for (bNode *frame : tree_draw_order_calc_nodes_reversed(*ntree)) {
/* skip selected, those are the nodes we want to attach */
if ((frame->type != NODE_FRAME) || (frame->flag & NODE_SELECT)) {
continue;

21
source/blender/editors/space_node/node_select.cc
View File

@ -8,6 +8,7 @@
#include <array>
#include <cstdlib>
#include <iostream>
#include "DNA_node_types.h"
#include "DNA_windowmanager_types.h"
@ -126,8 +127,7 @@ static bool node_frame_select_isect_mouse(const SpaceNode &snode,
static bNode *node_under_mouse_select(const SpaceNode &snode, const float2 mouse)
{
const bNodeTree &ntree = *snode.edittree;
LISTBASE_FOREACH_BACKWARD (bNode *, node, &ntree.nodes) {
for (bNode *node : tree_draw_order_calc_nodes_reversed(*snode.edittree)) {
switch (node->type) {
case NODE_FRAME: {
if (node_frame_select_isect_mouse(snode, *node, mouse)) {
@ -148,8 +148,7 @@ static bNode *node_under_mouse_select(const SpaceNode &snode, const float2 mouse
static bool node_under_mouse_tweak(const SpaceNode &snode, const float2 &mouse)
{
const bNodeTree &ntree = *snode.edittree;
LISTBASE_FOREACH_BACKWARD (const bNode *, node, &ntree.nodes) {
for (bNode *node : tree_draw_order_calc_nodes_reversed(*snode.edittree)) {
switch (node->type) {
case NODE_REROUTE: {
const float2 location = node_to_view(*node, {node->locx, node->locy});
@ -461,7 +460,7 @@ static int node_select_grouped_exec(bContext *C, wmOperator *op)
}
if (changed) {
node_sort(node_tree);
tree_draw_order_update(node_tree);
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
return OPERATOR_FINISHED;
}
@ -529,7 +528,7 @@ void node_select_single(bContext &C, bNode &node)
ED_node_set_active(bmain, &snode, &node_tree, &node, &active_texture_changed);
ED_node_set_active_viewer_key(&snode);
node_sort(node_tree);
tree_draw_order_update(node_tree);
if (active_texture_changed && has_workbench_in_texture_color(wm, scene, ob)) {
DEG_id_tag_update(&node_tree.id, ID_RECALC_COPY_ON_WRITE);
}
@ -687,7 +686,7 @@ static bool node_mouse_select(bContext *C,
viewer_path::activate_geometry_node(bmain, snode, *node);
}
ED_node_set_active_viewer_key(&snode);
node_sort(node_tree);
tree_draw_order_update(node_tree);
if ((active_texture_changed && has_workbench_in_texture_color(wm, scene, ob)) ||
viewer_node_changed)
{
@ -818,7 +817,7 @@ static int node_box_select_exec(bContext *C, wmOperator *op)
}
}
node_sort(node_tree);
tree_draw_order_update(node_tree);
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
@ -1128,7 +1127,7 @@ static int node_select_all_exec(bContext *C, wmOperator *op)
break;
}
node_sort(node_tree);
tree_draw_order_update(node_tree);
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
return OPERATOR_FINISHED;
@ -1180,7 +1179,7 @@ static int node_select_linked_to_exec(bContext *C, wmOperator * /*op*/)
}
}
node_sort(node_tree);
tree_draw_order_update(node_tree);
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
return OPERATOR_FINISHED;
@ -1230,7 +1229,7 @@ static int node_select_linked_from_exec(bContext *C, wmOperator * /*op*/)
}
}
node_sort(node_tree);
tree_draw_order_update(node_tree);
WM_event_add_notifier(C, NC_NODE | NA_SELECTED, nullptr);
return OPERATOR_FINISHED;

2
source/blender/editors/space_node/space_node.cc
View File

@ -1071,7 +1071,7 @@ static int /*eContextResult*/ node_context(const bContext *C,
}
if (CTX_data_equals(member, "selected_nodes")) {
if (snode->edittree) {
LISTBASE_FOREACH_BACKWARD (bNode *, node, &snode->edittree->nodes) {
for (bNode *node : snode->edittree->all_nodes()) {
if (node->flag & NODE_SELECT) {
CTX_data_list_add(result, &snode->edittree->id, &RNA_Node, node);
}

6
source/blender/makesdna/DNA_node_types.h
View File

@ -369,7 +369,11 @@ typedef struct bNode {
*/
int16_t type;
char _pad1[2];
/**
* Depth of the node in the node editor, used to keep recently selected nodes at the front, and
* to order frame nodes properly.
*/
int16_t ui_order;
/** Used for some builtin nodes that store properties but don't have a storage struct. */
int16_t custom1, custom2;