Geometry Nodes: Attribute search in the modifier
ClosedPublic
Actions

Authored by Hans Goudey (HooglyBoogly) on Oct 8 2021, 6:47 AM.

Details

Reviewers

Jacques Lucke (JacquesLucke)
Julian Eisel (Severin)

Maniphest Tasks

T91485: Input Attribute Toggle on left side in modifier
T91486: Attribute Search in modifier

Summary

This adds attribute search to the geometry nodes modifier
for the input and output attributes. The "New" search item
is only shown for the output attributes.

This required duplicating a lot of the attribute search code,
although it is slightly changed. Theoretically it's possible to
de-duplicate this more, but I don't think it's great to share
more code between the editors and modifiers modules, and
there is no great API for search boxes in the UI to simplify this
sort of thing.

The UI code required two fixes so that the search would work
for dynamic length strings (IDProperties do not have a fixed size).

Since this does changes to the UI layout of the modifier, I also
addressed T91485 here.

Diff Detail

Repository

rB Blender

Branch

attribute-search-modifier (branched from master)

Build Status

Buildable 18085
Build 18085: arc lint + arc unit

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Oct 8 2021, 6:47 AM

Hans Goudey (HooglyBoogly) created this revision.

Harbormaster completed remote builds in B17646: Diff 43035.Oct 8 2021, 6:47 AM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Oct 8 2021, 6:49 AM

Hans Goudey (HooglyBoogly) added a project: Geometry Nodes.

Hans Goudey (HooglyBoogly) added a task: T91486: Attribute Search in modifier.

Make patch work
Cleanup

Harbormaster completed remote builds in B17770: Diff 43213.Oct 12 2021, 9:20 PM

Hans Goudey (HooglyBoogly) added a task: T91485: Input Attribute Toggle on left side in modifier.Oct 12 2021, 9:24 PM

Hans Goudey (HooglyBoogly) retitled this revision from Geometry Nodes: Attribute search in the modifier (WIP) to Geometry Nodes: Attribute search in the modifier.

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)

Hans Goudey (HooglyBoogly) added a reviewer: Jacques Lucke (JacquesLucke).

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Oct 12 2021, 9:27 PM

Jacques Lucke (JacquesLucke) requested changes to this revision.Oct 13 2021, 3:35 PM

Jacques Lucke (JacquesLucke) added inline comments.

source/blender/editors/interface/interface_region_search.c
320	typo (`there`)
source/blender/modifiers/intern/MOD_nodes.cc
1120	It's a bit sad that there is so much code duplication with `node_geometry_attribute_search.cc`. Can you think about ways to reduce the duplication?
1121	It's fine here, but just wanted to mention that I'm usually careful with using references in these trivial structs. That is because with references the implicit copy/move assignment operators are not generated. See https://godbolt.org/z/8Mbe9E3or.
source/blender/nodes/NOD_geometry_nodes_eval_log.hh
193	Think this should just be a `unique_ptr` instead of `optional`. Same in `ModifierLog`. We shouldn't add assumptions for having exactly one geometry input/output if not necessary.

This revision now requires changes to proceed.Oct 13 2021, 3:35 PM

Hans Goudey (HooglyBoogly) marked 2 inline comments as done.Oct 21 2021, 12:01 AM

Hans Goudey (HooglyBoogly) added inline comments.

source/blender/modifiers/intern/MOD_nodes.cc
1121	If I can get away with it, I like using references a little more, it just looks nicer. Thanks for the warning though.
source/blender/nodes/NOD_geometry_nodes_eval_log.hh
193	A `unique_ptr` is fine I suppose, but how does that remove the assumption that there is a single input or output?

Share some attribute search code in the interface module
Don't show the legacy read-only normal attribute
Fix typo

Harbormaster completed remote builds in B18062: Diff 43670.Oct 21 2021, 12:01 AM

Hans Goudey (HooglyBoogly) added a reviewer: Julian Eisel (Severin).Oct 21 2021, 5:21 PM

Jacques Lucke (JacquesLucke) added inline comments.Oct 21 2021, 5:30 PM

source/blender/nodes/NOD_geometry_nodes_eval_log.hh
193	It doesn't remove the assumption here, but below in `ModifierLog` where it should be a `unique_ptr` as well.

Merge branch 'master' into attribute-search-modifier

Harbormaster completed remote builds in B18085: Diff 43704.Oct 21 2021, 5:36 PM

Works well in my tests and couldn't find any issue.

This revision is now accepted and ready to land.Oct 21 2021, 6:01 PM

MENU_SEP is used in multiple places throughout the UI code now. I'd suggest putting that in UI_interface.h as UI_MENU_ARROW_SEP now, but in its own cleanup.