Geometry Nodes: Add Active Element tool node #121333

Merged
Hans Goudey merged 10 commits from cmbasnett/bdk-blender:cmbasnett/active-element-node into main 2024-05-20 21:01:42 +02:00
12 changed files with 161 additions and 2 deletions

View File

@ -187,6 +187,7 @@ class NODE_MT_geometry_node_GEO_GEOMETRY_READ(Menu):
node_add_menu.add_node_type(layout, "GeometryNodeInputRadius")
if context.space_data.geometry_nodes_type == 'TOOL':
node_add_menu.add_node_type(layout, "GeometryNodeToolSelection")
node_add_menu.add_node_type(layout, "GeometryNodeToolActiveElement")
node_add_menu.draw_assets_for_catalog(layout, "Geometry/Read")

View File

@ -1327,6 +1327,7 @@ void BKE_nodetree_remove_layer_n(bNodeTree *ntree, Scene *scene, int layer_index
#define GEO_NODE_TOOL_VIEWPORT_TRANSFORM 2132
#define GEO_NODE_TOOL_MOUSE_POSITION 2133
#define GEO_NODE_SAMPLE_GRID_INDEX 2134
#define GEO_NODE_TOOL_ACTIVE_ELEMENT 2135
/** \} */

View File

@ -838,6 +838,30 @@ void BM_mesh_active_face_set(BMesh *bm, BMFace *f)
bm->act_face = f;
}
int BM_mesh_active_face_index_get(BMesh *bm, bool is_sloppy, bool is_selected)
{
const BMFace *f = BM_mesh_active_face_get(bm, is_sloppy, is_selected);
return f ? BM_elem_index_get(f) : -1;
}
int BM_mesh_active_edge_index_get(BMesh *bm)
{
const BMEdge *e = BM_mesh_active_edge_get(bm);
return e ? BM_elem_index_get(e) : -1;
}
int BM_mesh_active_vert_index_get(BMesh *bm)
{
const BMVert *v = BM_mesh_active_vert_get(bm);
return v ? BM_elem_index_get(v) : -1;
}
int BM_mesh_active_elem_index_get(BMesh *bm)
{
const BMElem *e = BM_mesh_active_elem_get(bm);
return e ? BM_elem_index_get(e) : -1;
}
BMFace *BM_mesh_active_face_get(BMesh *bm, const bool is_sloppy, const bool is_selected)
{
if (bm->act_face && (!is_selected || BM_elem_flag_test(bm->act_face, BM_ELEM_SELECT))) {

View File

@ -120,6 +120,10 @@ int BM_mesh_elem_hflag_count_disabled(BMesh *bm, char htype, char hflag, bool re
/* Edit selection stuff. */
void BM_mesh_active_face_set(BMesh *bm, BMFace *f);
int BM_mesh_active_face_index_get(BMesh *bm, bool is_sloppy, bool is_selected);
int BM_mesh_active_edge_index_get(BMesh *bm);
int BM_mesh_active_vert_index_get(BMesh *bm);
int BM_mesh_active_elem_index_get(BMesh *bm);
BMFace *BM_mesh_active_face_get(BMesh *bm, bool is_sloppy, bool is_selected);
BMEdge *BM_mesh_active_edge_get(BMesh *bm);
BMVert *BM_mesh_active_vert_get(BMesh *bm);

View File

@ -170,7 +170,8 @@ static void find_socket_log_contexts(const Main &bmain,
* we need to create evaluated copies of geometry before passing it to geometry nodes. Implicit
* sharing lets us avoid copying attribute data though.
*/
static bke::GeometrySet get_original_geometry_eval_copy(Object &object)
static bke::GeometrySet get_original_geometry_eval_copy(Object &object,
nodes::GeoNodesOperatorData &operator_data)
{
switch (object.type) {
case OB_CURVES: {
@ -185,6 +186,9 @@ static bke::GeometrySet get_original_geometry_eval_copy(Object &object)
case OB_MESH: {
const Mesh *mesh = static_cast<const Mesh *>(object.data);
if (std::shared_ptr<BMEditMesh> &em = mesh->runtime->edit_mesh) {
operator_data.active_point_index = BM_mesh_active_vert_index_get(em->bm);
operator_data.active_edge_index = BM_mesh_active_edge_index_get(em->bm);
operator_data.active_face_index = BM_mesh_active_face_index_get(em->bm, false, true);
cmbasnett marked this conversation as resolved Outdated

I think this should pass true for `is_selected. Otherwise this gives a value when there is no face selected, just based on whatever was active last or something. That's consistent with that's visualized in the viewport too.

I think this should pass `true` for `is_selected. Otherwise this gives a value when there is no face selected, just based on whatever was active last or something. That's consistent with that's visualized in the viewport too.
Mesh *mesh_copy = BKE_mesh_wrapper_from_editmesh(em, nullptr, mesh);
BKE_mesh_wrapper_ensure_mdata(mesh_copy);
Mesh *final_copy = BKE_mesh_copy_for_eval(mesh_copy);
@ -556,7 +560,7 @@ static int run_node_group_exec(bContext *C, wmOperator *op)
call_data.socket_log_contexts = &socket_log_contexts;
}
bke::GeometrySet geometry_orig = get_original_geometry_eval_copy(*object);
bke::GeometrySet geometry_orig = get_original_geometry_eval_copy(*object, operator_eval_data);
bke::GeometrySet new_geometry = nodes::execute_geometry_nodes_on_geometry(
*node_tree, properties, compute_context, call_data, std::move(geometry_orig));

View File

@ -1986,6 +1986,11 @@ typedef struct NodeGeometryBake {
char _pad[4];
} NodeGeometryBake;
typedef struct NodeGeometryToolActiveElement {
/** #AttrDomain. */
int8_t domain;
} NodeGeometryToolActiveElement;
/* script node mode */
enum {
NODE_SCRIPT_INTERNAL = 0,

View File

@ -226,6 +226,7 @@ DEF_ENUM(rna_enum_attribute_domain_items)
DEF_ENUM(rna_enum_attribute_domain_edge_face_items)
DEF_ENUM(rna_enum_attribute_domain_only_mesh_items)
DEF_ENUM(rna_enum_attribute_domain_only_mesh_no_edge_items)
DEF_ENUM(rna_enum_attribute_domain_only_mesh_no_corner_items)
DEF_ENUM(rna_enum_attribute_domain_point_face_curve_items)
DEF_ENUM(rna_enum_attribute_domain_point_edge_face_curve_items)
DEF_ENUM(rna_enum_attribute_curves_domain_items)

View File

@ -111,6 +111,13 @@ const EnumPropertyItem rna_enum_attribute_domain_only_mesh_no_edge_items[] = {
{0, nullptr, 0, nullptr, nullptr},
};
const EnumPropertyItem rna_enum_attribute_domain_only_mesh_no_corner_items[] = {
{int(AttrDomain::Point), "POINT", 0, "Point", "Attribute on point"},
{int(AttrDomain::Edge), "EDGE", 0, "Edge", "Attribute on mesh edge"},
{int(AttrDomain::Face), "FACE", 0, "Face", "Attribute on mesh faces"},
{0, nullptr, 0, nullptr, nullptr},
};
const EnumPropertyItem rna_enum_attribute_domain_point_face_curve_items[] = {
{int(AttrDomain::Point), "POINT", 0, "Point", "Attribute on point"},
{int(AttrDomain::Face), "FACE", 0, "Face", "Attribute on mesh faces"},

View File

@ -193,6 +193,9 @@ struct GeoNodesOperatorData {
int2 mouse_position;
int2 region_size;
const RegionView3D *rv3d = nullptr;
int active_point_index = -1;
int active_edge_index = -1;
int active_face_index = -1;
};
struct GeoNodesCallData {

View File

@ -475,6 +475,7 @@ DefNode(GeometryNode, GEO_NODE_TOOL_3D_CURSOR, 0, "TOOL_3D_CURSOR", Tool3DCursor
DefNode(GeometryNode, GEO_NODE_TOOL_FACE_SET, 0, "TOOL_FACE_SET", ToolFaceSet, "Face Set", "Each face's sculpt face set value")
DefNode(GeometryNode, GEO_NODE_TOOL_MOUSE_POSITION, 0, "TOOL_MOUSE_POSITION", ToolMousePosition, "Mouse Position", "Retrieve the position of the mouse cursor")
DefNode(GeometryNode, GEO_NODE_TOOL_SELECTION, 0, "TOOL_SELECTION", ToolSelection, "Selection", "User selection of the edited geometry, for tool execution")
DefNode(GeometryNode, GEO_NODE_TOOL_ACTIVE_ELEMENT, 0, "TOOL_ACTIVE_ELEMENT", ToolActiveElement, "Active Element", "Active element indices of the edited geometry, for tool execution")
DefNode(GeometryNode, GEO_NODE_TOOL_SET_FACE_SET, 0, "TOOL_SET_FACE_SET", ToolSetFaceSet, "Set Face Set", "Set sculpt face set values for faces")
DefNode(GeometryNode, GEO_NODE_TOOL_SET_SELECTION, 0, "TOOL_SELECTION_SET", ToolSetSelection, "Set Selection", "Set selection of the edited geometry, for tool execution")
DefNode(GeometryNode, GEO_NODE_TOOL_VIEWPORT_TRANSFORM, 0, "VIEWPORT_TRANFORM", ViewportTransform, "Viewport Transform", "Retrieve the view direction and location of the 3D viewport")

View File

@ -197,6 +197,7 @@ set(SRC
nodes/node_geo_subdivision_surface.cc
nodes/node_geo_switch.cc
nodes/node_geo_tool_3d_cursor.cc
nodes/node_geo_tool_active_element.cc
nodes/node_geo_tool_face_set.cc
nodes/node_geo_tool_selection.cc
nodes/node_geo_tool_set_face_set.cc

View File

@ -0,0 +1,107 @@
/* SPDX-FileCopyrightText: 2024 Blender Authors
*
* SPDX-License-Identifier: GPL-2.0-or-later */
#include "UI_interface.hh"
cmbasnett marked this conversation as resolved Outdated

Unnecessary include?

Unnecessary include?
#include "RNA_enum_types.hh"
#include "BKE_mesh.hh"
cmbasnett marked this conversation as resolved Outdated

Unnecessary include?

Unnecessary include?

This one is needed for ME_VSEL etc.

This one is needed for `ME_VSEL` etc.

Ah right of course, thanks

Ah right of course, thanks
#include "BKE_node.hh"
#include "DNA_meshdata_types.h"
#include "NOD_rna_define.hh"
#include "node_geometry_util.hh"

No need to specify a default value on output sockets.

No need to specify a default value on output sockets.

Is that true? I'm not that familiar with the system, but if someone tries to use one of the sockets in an unsupported scenario (like object mode) wouldn't I want to output the sockets as -1?

Is that true? I'm not that familiar with the system, but if someone tries to use one of the sockets in an unsupported scenario (like object mode) wouldn't I want to output the sockets as `-1`?

Yep, it's true :P This does nothing for outputs.

Yep, it's true :P This does nothing for outputs.
cmbasnett marked this conversation as resolved Outdated

I'd use "Point" so this can be used for curves and point clouds in the future

I'd use "Point" so this can be used for curves and point clouds in the future

Think it could be a bit simpler with no point -> none, no edge -> none, etc.

Think it could be a bit simpler with `no point` -> `none`, `no edge` -> `none`, etc.
namespace blender::nodes::node_geo_tool_active_element_cc {
cmbasnett marked this conversation as resolved Outdated

The descriptions should mention that -1 is the output if there is no active element on that domain

The descriptions should mention that -1 is the output if there is no active element on that domain
NODE_STORAGE_FUNCS(NodeGeometryToolActiveElement)
static void node_declare(NodeDeclarationBuilder &b)
{
b.add_output<decl::Int>("Index").description(
"Index of the active element in the specified domain");
b.add_output<decl::Bool>("Exists").description(
"True if an active element exists in the mesh, false otherwise");
}
cmbasnett marked this conversation as resolved Outdated

Switch the condition and return early (with params.set_default_remaining_outputs();)

Switch the condition and return early (with `params.set_default_remaining_outputs();`)
cmbasnett marked this conversation as resolved Outdated

Comment style

Comment style
static void node_init(bNodeTree * /*tree*/, bNode *node)
cmbasnett marked this conversation as resolved Outdated

Make format

Make format
{
NodeGeometryToolActiveElement *data = MEM_cnew<NodeGeometryToolActiveElement>(__func__);
HooglyBoogly marked this conversation as resolved Outdated

I do think the retrieval of the self object and the mesh are unnecessary now. The mesh variable is unused now too

I do think the retrieval of the self object and the mesh are unnecessary now. The `mesh` variable is unused now too

I have removed those object and data fetches, and also made the point, edge and face index initialize to -1 so that the hair curve case is properly handled and the values are always -1 without needing to explicitly check the object type.

I have removed those object and data fetches, and also made the point, edge and face index initialize to `-1` so that the hair curve case is properly handled and the values are always `-1` without needing to explicitly check the object type.
data->domain = int16_t(AttrDomain::Point);
node->storage = data;
}
static void node_rna(StructRNA *srna)
cmbasnett marked this conversation as resolved Outdated

I think the const cast can be avoided by just using static_cast<const Mesh *>(object.data); The BKE_mesh_from_object function isn't really helping here since we already checked the object type.

I think the const cast can be avoided by just using `static_cast<const Mesh *>(object.data);` The `BKE_mesh_from_object` function isn't really helping here since we already checked the object type.

Can this also be moved to operator call function?

Can this also be moved to operator call function?
{
RNA_def_node_enum(srna,
"domain",
"Domain",
"",
rna_enum_attribute_domain_only_mesh_no_corner_items,
cmbasnett marked this conversation as resolved Outdated

All the outputs are set, best to remove this

All the outputs are set, best to remove this
NOD_storage_enum_accessors(domain),
int(AttrDomain::Point));
}
static void node_layout(uiLayout *layout, bContext * /*C*/, PointerRNA *ptr)
{
uiLayoutSetPropSep(layout, true);
uiLayoutSetPropDecorate(layout, false);
uiItemR(layout, ptr, "domain", UI_ITEM_NONE, "", ICON_NONE);
}
static void node_exec(GeoNodeExecParams params)
{
if (!check_tool_context_and_error(params)) {
return;
}
if (params.user_data()->call_data->operator_data->mode != OB_MODE_EDIT) {
params.set_default_remaining_outputs();
return;
}
const GeoNodesOperatorData *operator_data = params.user_data()->call_data->operator_data;
switch (static_cast<AttrDomain>(node_storage(params.node()).domain)) {
case AttrDomain::Point:
params.set_output("Exists", operator_data->active_point_index >= 0);
params.set_output("Index", std::max(0, operator_data->active_point_index));
HooglyBoogly marked this conversation as resolved Outdated

I think we should output 0 instead of -1 in the error case.

I think we should output 0 instead of -1 in the error case.
break;
case AttrDomain::Edge:
params.set_output("Exists", operator_data->active_edge_index >= 0);
params.set_output("Index", std::max(0, operator_data->active_edge_index));
break;
case AttrDomain::Face:
params.set_output("Exists", operator_data->active_face_index >= 0);
params.set_output("Index", std::max(0, operator_data->active_face_index));
break;
default:
params.set_default_remaining_outputs();
BLI_assert_unreachable();
break;
}
}
static void node_register()
{
static blender::bke::bNodeType ntype;
geo_node_type_base(&ntype, GEO_NODE_TOOL_ACTIVE_ELEMENT, "Active Element", NODE_CLASS_INPUT);
node_type_storage(&ntype,
"NodeGeometryToolActiveElement",
node_free_standard_storage,
node_copy_standard_storage);
ntype.initfunc = node_init;
ntype.geometry_node_execute = node_exec;
ntype.declare = node_declare;
ntype.gather_link_search_ops = search_link_ops_for_tool_node;
ntype.draw_buttons = node_layout;
blender::bke::nodeRegisterType(&ntype);
node_rna(ntype.rna_ext.srna);
}
NOD_REGISTER_NODE(node_register)
} // namespace blender::nodes::node_geo_tool_active_element_cc