Details

Reviewers

Maniphest Tasks

Commits

rB85421c4fab02: Geometry Nodes: Attribute search drop-down

Summary

This patch adds a search for existing attributes when you click
on an attribute field. It also adds the necessary changes to allow
any string to be entered from the search button.

This is only an initial implementation. It should be enough of an
improvement to add to master, but later versions will expose the
data type and domain of the attributes.

Ref T85658

Diff Detail

Repository

rB Blender

Branch

temp-geometry-nodes-attribute-search

Build Status

Buildable 13114
Build 13114: arc lint + arc unit

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Feb 24 2021, 12:34 AM

Hans Goudey (HooglyBoogly) created this revision.

Harbormaster completed remote builds in B13085: Diff 34351.Feb 24 2021, 12:35 AM

Hans Goudey (HooglyBoogly) added a parent revision: D10462: Geometry Nodes: Store available attribute names in node ui storage..Feb 24 2021, 12:35 AM

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

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

Also, I can split the change to the search button into a separate patch.

Florian Kamenz (TimBrown) awarded a token.Feb 24 2021, 9:48 AM

Overall this looks very nice!

Right now this is a bit tricky to test in master, because D10462 is not in yet. I suggest we merge that first and then you update this patch on top of that.
I'd prefer to split the attribute search specific parts from the internal ui code changes.
One thing that does not seem to work is to click on one of the suggestions to insert it into the text box.
In the long run, we should probably generalize "attribute search" on string sockets a bit. E.g. a string socket shouldn't know that it has an attribute search. It should just know that there is a hints for possible values. I'm not sure how to do that exactly yet.

source/blender/editors/interface/interface_handlers.c
174 ↗	(On Diff #34351)	Clang format
3411 ↗	(On Diff #34351)	Seems like it would make sense to put the assert before the type cast.
source/blender/editors/space_node/drawnode.c
3394	It would probably be good to only do this change for geometry node trees for now. Otherwise we might accidentally change addons using this socket.
source/blender/editors/space_node/node_geometry_attribute_search.cc
111	It's not important right now (and not worth the complexity), but I wonder if there is the possibility that this allocation is only done when the button becomes active.
source/blender/editors/space_node/node_intern.h
321	Do we have to pass the block separately or can it be derived from `uiBut`?

i did a mocap for built-in attributes:

Some User (someuser) awarded a token.Feb 24 2021, 1:20 PM

Arian (14AUDDIN) awarded a token.Feb 24 2021, 1:31 PM

Hans Goudey (HooglyBoogly) mentioned this in D10527: UI: Add an option to search buttons to allow any string input.Feb 24 2021, 9:14 PM

Hans Goudey (HooglyBoogly) added a parent revision: D10527: UI: Add an option to search buttons to allow any string input.Feb 24 2021, 9:15 PM

Merge branch 'master' into temp-geometry-nodes-attribute-search
Merge branch 'search-button-allow-all' into temp-geometry-nodes-attribute-search
Cleanup, check for node tree type
Cleanup: Remove unused variable
Attribute Search: Always display search string with + icon
Cleanup: Add comment
Add todo comment

Harbormaster completed remote builds in B13106: Diff 34397.Feb 25 2021, 12:32 AM

Hans Goudey (HooglyBoogly) added a parent revision: D10528: UI: Expose an "is first search" boolean to search button callbacks.Feb 25 2021, 12:50 AM

Use exposed "is first search" boolean
Merge branch 'search-button-first-search' into temp-geometry-nodes-attribute-search

Harbormaster completed remote builds in B13108: Diff 34399.Feb 25 2021, 12:52 AM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Feb 25 2021, 3:55 AM

Hans Goudey (HooglyBoogly) marked 2 inline comments as done.Feb 25 2021, 4:00 AM

Hans Goudey (HooglyBoogly) added inline comments.

source/blender/editors/space_node/drawnode.c
3393	Accessing the context in here is not great. This function could get a node tree argument if you agreed @Jacques Lucke (JacquesLucke)
source/blender/editors/space_node/node_geometry_attribute_search.cc
111	Yeah, the UI code is full of this unnecessary allocation. I don't see any way to avoid it in the context of the current code though, all of the interaction happens after the new layout is built. I did look though : P