Details

Reviewers

Hans Goudey (HooglyBoogly)
Dalai Felinto (dfelinto)

Commits

rBfd477e738dfd: Geometry Nodes: remove reference to anonymous attributes in tooltips

Summary

This changes the socket inspection for fields according to T91881.

This could be made a bit smarter in the future when node groups are involved. We could e.g. show the names of node group outputs instead of the names of nodes inside the node group. That's a bit out of scope for now unfortunately.

Diff Detail

Repository

rB Blender

Branch

field-tooltips-update (branched from master)

Build Status

Buildable 18235
Build 18235: arc lint + arc unit

Event Timeline

Jacques Lucke (JacquesLucke) requested review of this revision.Oct 26 2021, 1:50 PM

Jacques Lucke (JacquesLucke) created this revision.

Harbormaster completed remote builds in B18229: Diff 43963.Oct 26 2021, 1:50 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Oct 26 2021, 1:52 PM

sort by length instead of alphabetically

Harbormaster completed remote builds in B18230: Diff 43964.Oct 26 2021, 1:53 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Oct 26 2021, 1:53 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Oct 26 2021, 1:56 PM

Jacques Lucke (JacquesLucke) added reviewers: Hans Goudey (HooglyBoogly), Dalai Felinto (dfelinto).Oct 26 2021, 2:12 PM

The results look good to me, and the implementation seems fine. I mentioned another option inline though, that might help to separate the UI code a bit, curious what you think about that.

source/blender/nodes/geometry/nodes/node_geo_distribute_points_on_faces.cc
572	Another option would probably be storing a reference to the node and calculating the name when building the inspection strings from the fields. That's just a bit more general, and may be useful for other things, like adding a node warning message from the field input's `get_varray_for_context`. It also means that the UI code and actual logic are a little less mixed. What do you think about that?
source/blender/nodes/intern/node_geometry_exec.cc
188	Did you think about using the label here, if it's set?

use node label

Harbormaster completed remote builds in B18234: Diff 43968.Oct 26 2021, 2:35 PM

Jacques Lucke (JacquesLucke) added inline comments.Oct 26 2021, 2:38 PM

source/blender/nodes/geometry/nodes/node_geo_distribute_points_on_faces.cc
572	I'm not generally opposed to storing a pointer to the node, but I'd rather only do this when we actually need this. Then we can answer questions like "Do we need the bNode or DNode?" better. Don't think this is a design question I want to get into for this patch.

Hans Goudey (HooglyBoogly) accepted this revision.Oct 26 2021, 2:43 PM

Hans Goudey (HooglyBoogly) added inline comments.

source/blender/nodes/geometry/nodes/node_geo_distribute_points_on_faces.cc
572	Okay, fair enough.

This revision is now accepted and ready to land.Oct 26 2021, 2:43 PM

Hans Goudey (HooglyBoogly) added inline comments.Oct 26 2021, 2:44 PM

source/blender/nodes/intern/node_geometry_exec.cc
188	Oops, looks like this if statement is empty to me.

cleanup

Harbormaster completed remote builds in B18235: Diff 43969.Oct 26 2021, 2:48 PM

Right now it seems to be using the node name, while I think we should use its Label when the label exists.

Other than that +1

Nevermind, I tested an early version of the patch. Patch is good to go.

Closed by commit rBfd477e738dfd: Geometry Nodes: remove reference to anonymous attributes in tooltips (authored by Jacques Lucke (JacquesLucke)). · Explain WhyOct 26 2021, 3:33 PM

This revision was automatically updated to reflect the committed changes.

Jacques Lucke (JacquesLucke) added a commit: rBfd477e738dfd: Geometry Nodes: remove reference to anonymous attributes in tooltips.

Geometry Nodes: Remove reference to anonymous attributes in socket inspection.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 43969

source/blender/blenkernel/BKE_geometry_set.hh

source/blender/blenkernel/intern/attribute_access.cc

source/blender/editors/space_node/node_draw.cc

source/blender/functions/FN_field.hh

source/blender/functions/intern/field.cc

source/blender/nodes/NOD_geometry_exec.hh

source/blender/nodes/NOD_node_tree_ref.hh

source/blender/nodes/geometry/nodes/node_geo_attribute_capture.cc

source/blender/nodes/geometry/nodes/node_geo_curve_endpoint_selection.cc

source/blender/nodes/geometry/nodes/node_geo_curve_handle_type_selection.cc

source/blender/nodes/geometry/nodes/node_geo_curve_parameter.cc

source/blender/nodes/geometry/nodes/node_geo_curve_to_points.cc

source/blender/nodes/geometry/nodes/node_geo_distribute_points_on_faces.cc

source/blender/nodes/geometry/nodes/node_geo_input_normal.cc

source/blender/nodes/geometry/nodes/node_geo_input_spline_length.cc

source/blender/nodes/geometry/nodes/node_geo_input_tangent.cc

source/blender/nodes/geometry/nodes/node_geo_material_selection.cc

source/blender/nodes/geometry/nodes/node_geo_transfer_attribute.cc

source/blender/nodes/intern/geometry_nodes_eval_log.cc

source/blender/nodes/intern/node_geometry_exec.cc

Geometry Nodes: Remove reference to anonymous attributes in socket inspection.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 43969

source/blender/blenkernel/BKE_geometry_set.hh

source/blender/blenkernel/intern/attribute_access.cc

source/blender/editors/space_node/node_draw.cc

source/blender/functions/FN_field.hh

source/blender/functions/intern/field.cc

source/blender/nodes/NOD_geometry_exec.hh

source/blender/nodes/NOD_node_tree_ref.hh

source/blender/nodes/geometry/nodes/node_geo_attribute_capture.cc

source/blender/nodes/geometry/nodes/node_geo_curve_endpoint_selection.cc

source/blender/nodes/geometry/nodes/node_geo_curve_handle_type_selection.cc

source/blender/nodes/geometry/nodes/node_geo_curve_parameter.cc

source/blender/nodes/geometry/nodes/node_geo_curve_to_points.cc

source/blender/nodes/geometry/nodes/node_geo_distribute_points_on_faces.cc

source/blender/nodes/geometry/nodes/node_geo_input_normal.cc

source/blender/nodes/geometry/nodes/node_geo_input_spline_length.cc

source/blender/nodes/geometry/nodes/node_geo_input_tangent.cc

source/blender/nodes/geometry/nodes/node_geo_material_selection.cc

source/blender/nodes/geometry/nodes/node_geo_transfer_attribute.cc

source/blender/nodes/intern/geometry_nodes_eval_log.cc

source/blender/nodes/intern/node_geometry_exec.cc

Geometry Nodes: Remove reference to anonymous attributes in socket inspection.
ClosedPublic
Actions

Revision Contents
Changeset List