griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Nodes: show link errors directly on link

Browse Source 
Previously, we had shown link errors on the target node as part of the node
warning system. While better than not showing any information about invalid
links (as was the state before that), it's still not ideal because it's easy to
miss when just looking at the link.

This patch adds an error icon in the middle of the invalid link. When hovering
over it, it shows the error text. When the middle of the link is not in view but
part of the link is, then the error icon will also stay visible.

Pull Request: https://projects.blender.org/blender/blender/pulls/138529
This commit is contained in:
Jacques Lucke
2025-05-09 04:06:00 +02:00
parent fb86bf0367
commit 4a760c1a70
8 changed files with 326 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
source/blender/blenkernel/BKE_node_runtime.hh
View File

@@ -15,6 +15,7 @@
#include "BLI_multi_value_map.hh"
#include "BLI_mutex.hh"
#include "BLI_set.hh"
#include "BLI_struct_equality_utils.hh"
#include "BLI_utility_mixins.hh"
#include "BLI_vector.hh"
#include "BLI_vector_set.hh"
@@ -69,6 +70,34 @@ struct NodeLinkError {
std::string tooltip;
};
/**
* Utility to weakly reference a link. Weak references are safer because they avoid dangling
* references which can easily happen temporarily when editing the node tree.
*/
struct NodeLinkKey {
private:
int to_node_id_;
int input_socket_index_;
int input_link_index_;
public:
/** Assumes that the topology cache is up to date. */
explicit NodeLinkKey(const bNodeLink &link);
bNodeLink *try_find(bNodeTree &ntree) const;
const bNodeLink *try_find(const bNodeTree &ntree) const;
uint64_t hash() const
{
return get_default_hash(this->to_node_id_, this->input_socket_index_, this->input_link_index_);
}
BLI_STRUCT_EQUALITY_OPERATORS_3(NodeLinkKey,
to_node_id_,
input_socket_index_,
input_link_index_);
};
struct LoggedZoneGraphs {
Mutex mutex;
/**
@@ -163,11 +192,10 @@ class bNodeTreeRuntime : NonCopyable, NonMovable {
geometry_nodes_lazy_function_graph_info;
/**
* Stores information about invalid links. This information is then displayed to the user. The
* key of the map is the node identifier. The data is stored per target-node because we want to
* display the error information there.
* Stores information about invalid links. This information is then displayed to the user. This
* is updated in #update_link_validation and is valid during drawing code.
*/
MultiValueMap<int, NodeLinkError> link_errors_by_target_node;
MultiValueMap<NodeLinkKey, NodeLinkError> link_errors;
/**
* Protects access to all topology cache variables below. This is necessary so that the cache can
@@ -421,6 +449,7 @@ inline bool topology_cache_is_available(const bNodeSocket &socket)
namespace node_field_inferencing {
bool update_field_inferencing(const bNodeTree &tree);
}
} // namespace blender::bke
/* -------------------------------------------------------------------- */

33
source/blender/blenkernel/intern/node_runtime.cc
View File

@@ -576,6 +576,39 @@ static void ensure_topology_cache(const bNodeTree &ntree)
} // namespace blender::bke::node_tree_runtime
namespace blender::bke {
NodeLinkKey::NodeLinkKey(const bNodeLink &link)
{
to_node_id_ = link.tonode->identifier;
input_socket_index_ = link.tosock->index();
input_link_index_ =
const_cast<const bNodeSocket *>(link.tosock)->directly_linked_links().first_index(&link);
}
bNodeLink *NodeLinkKey::try_find(bNodeTree &ntree) const
{
return const_cast<bNodeLink *>(this->try_find(const_cast<const bNodeTree &>(ntree)));
}
const bNodeLink *NodeLinkKey::try_find(const bNodeTree &ntree) const
{
const bNode *to_node = ntree.node_by_id(to_node_id_);
if (!to_node) {
return nullptr;
}
if (input_socket_index_ >= to_node->input_sockets().size()) {
return nullptr;
}
const bNodeSocket &input_socket = to_node->input_socket(input_socket_index_);
if (input_link_index_ >= input_socket.directly_linked_links().size()) {
return nullptr;
}
return input_socket.directly_linked_links()[input_link_index_];
}
} // namespace blender::bke
void bNodeTree::ensure_topology_cache() const
{
blender::bke::node_tree_runtime::ensure_topology_cache(*this);

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

@@ -488,7 +488,7 @@ class NodeTreeMainUpdater {
{
TreeUpdateResult result;
ntree.runtime->link_errors_by_target_node.clear();
ntree.runtime->link_errors.clear();
if (this->update_panel_toggle_names(ntree)) {
result.interface_changed = true;
@@ -1205,8 +1205,8 @@ class NodeTreeMainUpdater {
}
if (is_invalid_enum_ref(*link->fromsock) || is_invalid_enum_ref(*link->tosock)) {
link->flag &= ~NODE_LINK_VALID;
ntree.runtime->link_errors_by_target_node.add(
link->tonode->identifier,
ntree.runtime->link_errors.add(
NodeLinkKey{*link},
NodeLinkError{TIP_("Use node groups to reuse the same menu multiple times")});
continue;
}
@@ -1216,9 +1216,8 @@ class NodeTreeMainUpdater {
field_states[link->tosock->index_in_tree()] != FieldSocketState::IsField)
{
link->flag &= ~NODE_LINK_VALID;
ntree.runtime->link_errors_by_target_node.add(
link->tonode->identifier,
NodeLinkError{TIP_("The node input does not support fields")});
ntree.runtime->link_errors.add(
NodeLinkKey{*link}, NodeLinkError{TIP_("The node input does not support fields")});
continue;
}
}
@@ -1228,8 +1227,8 @@ class NodeTreeMainUpdater {
to_node.runtime->toposort_left_to_right_index)
{
link->flag &= ~NODE_LINK_VALID;
ntree.runtime->link_errors_by_target_node.add(
link->tonode->identifier,
ntree.runtime->link_errors.add(
NodeLinkKey{*link},
NodeLinkError{TIP_("The links form a cycle which is not supported")});
continue;
}
@@ -1238,8 +1237,8 @@ class NodeTreeMainUpdater {
const eNodeSocketDatatype to_type = eNodeSocketDatatype(link->tosock->type);
if (!ntree.typeinfo->validate_link(from_type, to_type)) {
link->flag &= ~NODE_LINK_VALID;
ntree.runtime->link_errors_by_target_node.add(
link->tonode->identifier,
ntree.runtime->link_errors.add(
NodeLinkKey{*link},
NodeLinkError{fmt::format("{}: {} " BLI_STR_UTF8_BLACK_RIGHT_POINTING_SMALL_TRIANGLE
" {}",
TIP_("Conversion is not supported"),

8
source/blender/blenlib/BLI_rect.h
View File

@@ -147,6 +147,14 @@ void BLI_rctf_rcti_copy(struct rctf *dst, const struct rcti *src);
void BLI_rcti_rctf_copy_floor(struct rcti *dst, const struct rctf *src);
void BLI_rcti_rctf_copy_round(struct rcti *dst, const struct rctf *src);
/**
* Clamps the given segment to be within the rectangle.
*
* \return False when no part of the segment is within the rectangle, in which case the s1 and s2
* values should be ignored.
*/
bool BLI_rctf_clamp_segment(const struct rctf *rect, float s1[2], float s2[2]);
/**
* Expand the rectangle to fit a rotated \a src.
*/

64
source/blender/blenlib/intern/rct.cc
View File

@@ -16,7 +16,9 @@
#include <cstdlib>
#include "BLI_math_base.h"
#include "BLI_math_geom.h"
#include "BLI_math_matrix.h"
#include "BLI_math_vector.hh"
#include "BLI_rect.h"
#include "BLI_utildefines.h"
@@ -1126,4 +1128,66 @@ void BLI_rctf_rotate_expand(rctf *dst, const rctf *src, const float angle)
#undef ROTATE_SINCOS
bool BLI_rctf_clamp_segment(const struct rctf *rect, float s1[2], float s2[2])
{
using namespace blender;
const bool p1_inside = BLI_rctf_isect_pt_v(rect, s1);
const bool p2_inside = BLI_rctf_isect_pt_v(rect, s2);
if (p1_inside && p2_inside) {
return true;
}
const std::array<float2, 2> top_line = {float2{rect->xmin, rect->ymax},
float2{rect->xmax, rect->ymax}};
const std::array<float2, 2> bottom_line = {float2{rect->xmin, rect->ymin},
float2{rect->xmax, rect->ymin}};
const std::array<float2, 2> left_line = {float2{rect->xmin, rect->ymin},
float2{rect->xmin, rect->ymax}};
const std::array<float2, 2> right_line = {float2{rect->xmax, rect->ymin},
float2{rect->xmax, rect->ymax}};
const std::array<std::array<float2, 2>, 4> lines = {
top_line, bottom_line, left_line, right_line};
if (p1_inside && !p2_inside) {
for (const std::array<float2, 2> &line : lines) {
float2 intersection;
if (isect_seg_seg_v2_point(s1, s2, line[0], line[1], intersection) == 1) {
copy_v2_v2(s2, intersection);
}
}
return true;
}
if (!p1_inside && p2_inside) {
for (const std::array<float2, 2> &line : lines) {
float2 intersection;
if (isect_seg_seg_v2_point(s1, s2, line[0], line[1], intersection) == 1) {
copy_v2_v2(s1, intersection);
}
}
return true;
}
for (const std::array<float2, 2> &line : lines) {
float2 intersection;
if (isect_seg_seg_v2_point(s1, s2, line[0], line[1], intersection) == 1) {
copy_v2_v2(s1, intersection);
}
else {
return false;
}
}
for (const std::array<float2, 2> &line : lines) {
float2 intersection;
if (isect_seg_seg_v2_point(s2, s1, line[0], line[1], intersection) == 1) {
copy_v2_v2(s2, intersection);
}
else {
return false;
}
}
return true;
}
/** \} */

6
source/blender/editors/space_node/drawnode.cc
View File

@@ -1674,9 +1674,9 @@ void draw_nodespace_back_pix(const bContext &C,
GPU_matrix_pop();
}
static float2 socket_link_connection_location(const bNode &node,
const bNodeSocket &socket,
const bNodeLink &link)
float2 socket_link_connection_location(const bNode &node,
const bNodeSocket &socket,
const bNodeLink &link)
{
const float2 socket_location = socket.runtime->location;
if (socket.is_multi_input() && socket.is_input() && !(node.flag & NODE_HIDDEN)) {

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

@@ -3068,41 +3068,6 @@ static void node_get_compositor_extra_info(TreeDrawContext &tree_draw_ctx,
}
}
static void node_get_invalid_links_extra_info(const SpaceNode &snode,
const bNode &node,
Vector<NodeExtraInfoRow> &rows)
{
const bNodeTree &tree = *snode.edittree;
const Span<bke::NodeLinkError> link_errors = tree.runtime->link_errors_by_target_node.lookup(
node.identifier);
if (link_errors.is_empty()) {
return;
}
NodeExtraInfoRow row;
row.text = IFACE_("Invalid Link");
row.tooltip_fn = [](bContext *C, void *arg, const StringRef /*tip*/) {
const bNodeTree &tree = *CTX_wm_space_node(C)->edittree;
const bNode &node = *static_cast<const bNode *>(arg);
const Span<bke::NodeLinkError> link_errors = tree.runtime->link_errors_by_target_node.lookup(
node.identifier);
std::stringstream ss;
Set<StringRef> already_added_errors;
for (const int i : link_errors.index_range()) {
const StringRefNull tooltip = link_errors[i].tooltip;
if (already_added_errors.add_as(tooltip)) {
ss << "\u2022 " << tooltip << "\n";
}
}
ss << "\n";
ss << "Any invalid links are highlighted";
return ss.str();
};
row.tooltip_fn_arg = const_cast<bNode *>(&node);
row.icon = ICON_ERROR;
rows.append(std::move(row));
}
static Vector<NodeExtraInfoRow> node_get_extra_info(const bContext &C,
TreeDrawContext &tree_draw_ctx,
const SpaceNode &snode,
@@ -3123,8 +3088,6 @@ static Vector<NodeExtraInfoRow> node_get_extra_info(const bContext &C,
rows.append(std::move(row));
}
node_get_invalid_links_extra_info(snode, node, rows);
if (snode.edittree->type == NTREE_COMPOSIT) {
node_get_compositor_extra_info(tree_draw_ctx, snode, node, rows);
return rows;
@@ -4855,6 +4818,168 @@ static void draw_frame_overlays(const bContext &C,
}
}
/**
* Tries to find a position on the link where we can draw link information like an error icon. If
* the link center is not visible, it finds the closest point to the link center that's still
* visible with some padding if possible. If none such point is found, nullopt is returned.
*/
static std::optional<float2> find_visible_center_of_link(const View2D &v2d,
const bNodeLink &link,
const float radius,
const float region_padding)
{
/* Compute center of the link because that's used as "ideal" position. */
const float2 start = socket_link_connection_location(*link.fromnode, *link.fromsock, link);
const float2 end = socket_link_connection_location(*link.tonode, *link.tosock, link);
const float2 center = math::midpoint(start, end);
/* The rectangle that we would like to stay within if possible. */
rctf inner_rect = v2d.cur;
BLI_rctf_pad(&inner_rect, -(region_padding + radius), -(region_padding + radius));
if (BLI_rctf_isect_pt_v(&inner_rect, center)) {
/* The center is visible. */
return center;
}
/* The rectangle containing all points which are valid result positions. */
rctf outer_rect = v2d.cur;
BLI_rctf_pad(&outer_rect, radius, radius);
/* Get the straight individual link segments. */
std::array<float2, NODE_LINK_RESOL + 1> link_points;
node_link_bezier_points_evaluated(link, link_points);
const float required_socket_distance = UI_UNIT_X;
/* Define a cost function that returns a value that is larger the worse the given position is.
* The point on the link with the lowest cost will be picked. */
const auto cost_function = [&](const float2 &p) -> float {
const float distance_to_inner_rect = std::max(BLI_rctf_length_x(&inner_rect, p.x),
BLI_rctf_length_y(&inner_rect, p.y));
const float distance_to_center = math::distance(p, center);
/* Set a high cost when the point is close to a socket. The distance to the center still has to
* be taken account though. Otherwise there is bad behavior when both sockets are close to the
* point. */
const float distance_to_socket = std::min(math::distance(p, start), math::distance(p, end));
if (distance_to_socket < required_socket_distance) {
return 1e5f + distance_to_center;
}
return
/* The larger the distance to the link center, the higher the cost. The importance of this
distance decreases the further the center is away. */
std::sqrt(distance_to_center)
/* The larger the distance to the inner rectangle, the higher the cost. Apply an additional
* factor because it's more important that the position stays visible than that it is at
* the center. */
+ 10.0f * distance_to_inner_rect;
};
/* Iterate over visible points on the link, compute the cost of each and pick the best one. A
* more direct algorithm to find a good position would be nice. However, that seems to be
* surprisingly tricky to achieve without resulting in very "jumpy" positions, especially when
* the link is colinear to the region border. */
float best_cost;
std::optional<float2> best_position;
for (const int i : IndexRange(link_points.size() - 1)) {
float2 p0 = link_points[i];
float2 p1 = link_points[i + 1];
if (!BLI_rctf_clamp_segment(&outer_rect, p0, p1)) {
continue;
}
const float length = math::distance(p0, p1);
const float point_distance = 1.0f;
/* Might be possible to do a smarter scan of the cost function using some sort of binary sort,
* but it's not entirely straight forward because the cost function is not monotonic. */
const int points_to_check = std::max(2, 1 + int(length / point_distance));
for (const int j : IndexRange(points_to_check)) {
const float t = float(j) / (points_to_check - 1);
const float2 p = math::interpolate(p0, p1, t);
const float cost = cost_function(p);
if (!best_position.has_value() || cost < best_cost) {
best_cost = cost;
best_position = p;
}
}
}
return best_position;
}
static void draw_link_errors(const bContext &C,
SpaceNode &snode,
const bNodeLink &link,
const Span<bke::NodeLinkError> errors,
uiBlock &invalid_links_block)
{
const ARegion &region = *CTX_wm_region(&C);
if (errors.is_empty()) {
return;
}
if (!link.fromsock || !link.tosock || !link.fromnode || !link.tonode) {
/* Likely because the link is being dragged. */
return;
}
/* Generate full tooltip from potentially multiple errors. */
std::string error_tooltip;
if (errors.size() == 1) {
error_tooltip = errors[0].tooltip;
}
else {
for (const bke::NodeLinkError &error : errors) {
error_tooltip += fmt::format("\u2022 {}\n", error.tooltip);
}
}
const float bg_radius = UI_UNIT_X * 0.5f;
const float bg_corner_radius = UI_UNIT_X * 0.2f;
const float icon_size = UI_UNIT_X;
const float region_padding = UI_UNIT_X * 0.5f;
/* Compute error icon location. */
std::optional<float2> draw_position_opt = find_visible_center_of_link(
region.v2d, link, bg_radius, region_padding);
if (!draw_position_opt.has_value()) {
return;
}
const int2 draw_position = int2(draw_position_opt.value());
/* Draw a background for the error icon. */
rctf bg_rect;
BLI_rctf_init_pt_radius(&bg_rect, float2(draw_position), bg_radius);
ColorTheme4f bg_color;
UI_GetThemeColor4fv(TH_REDALERT, bg_color);
UI_draw_roundbox_corner_set(UI_CNR_ALL);
ui_draw_dropshadow(&bg_rect, bg_corner_radius, UI_UNIT_X * 0.2f, snode.runtime->aspect, 0.5f);
UI_draw_roundbox_4fv(&bg_rect, true, bg_corner_radius, bg_color);
/* Draw the icon itself with a tooltip. */
UI_block_emboss_set(&invalid_links_block, ui::EmbossType::None);
uiBut *but = uiDefIconBut(&invalid_links_block,
UI_BTYPE_BUT,
0,
ICON_ERROR,
draw_position.x - icon_size / 2,
draw_position.y - icon_size / 2,
icon_size,
icon_size,
nullptr,
0,
0,
std::nullopt);
UI_but_func_tooltip_label_set(
but, [tooltip = std::move(error_tooltip)](const uiBut * /*but*/) { return tooltip; });
}
static uiBlock &invalid_links_uiblock_init(const bContext &C)
{
Scene *scene = CTX_data_scene(&C);
wmWindow *window = CTX_wm_window(&C);
ARegion *region = CTX_wm_region(&C);
return *UI_block_begin(
&C, scene, window, region, "invalid_links", blender::ui::EmbossType::None);
}
#define USE_DRAW_TOT_UPDATE
static void node_draw_nodetree(const bContext &C,
@@ -4896,6 +5021,7 @@ static void node_draw_nodetree(const bContext &C,
}
nodelink_batch_end(snode);
GPU_blend(GPU_BLEND_NONE);
draw_frame_overlays(C, tree_draw_ctx, region, snode, ntree, blocks);
@@ -4911,6 +5037,15 @@ static void node_draw_nodetree(const bContext &C,
const bNodeInstanceKey key = bke::node_instance_key(parent_key, &ntree, &node);
node_draw(C, tree_draw_ctx, region, snode, ntree, node, *blocks[node.index()], key);
}
uiBlock &invalid_links_block = invalid_links_uiblock_init(C);
for (auto &&item : ntree.runtime->link_errors.items()) {
if (const bNodeLink *link = item.key.try_find(ntree)) {
draw_link_errors(C, snode, *link, item.value, invalid_links_block);
}
}
UI_block_end(&C, &invalid_links_block);
UI_block_draw(&C, &invalid_links_block);
}
/* Draw the breadcrumb on the top of the editor. */

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

@@ -229,6 +229,10 @@ void NODE_OT_backimage_sample(wmOperatorType *ot);
/* `drawnode.cc` */
float2 socket_link_connection_location(const bNode &node,
const bNodeSocket &socket,
const bNodeLink &link);
NodeResizeDirection node_get_resize_direction(const SpaceNode &snode,
const bNode *node,
int x,