Page MenuHome

Geometry Nodes: Allow attribute nodes to use different domains
ClosedPublic

Authored by Hans Goudey (HooglyBoogly) on Feb 10 2021, 8:11 PM.

Details

Summary

Currently every attribute node assumes that the attribute exists on the
"points" domain, so it generally isn't possible to work with attributes
on other domains like edges, polygons, and corners.

This commit adds a heuristic to each attribute node to determine the
correct domain for the result attribute. In general, it works like this:

  • If the output attribute already exists, use that domain.
  • Otherwise use the highest priority domain of the input attributes.
  • If none of the inputs are attributes, use the default domain (points).

For the implementation I abstracted the check a bit, but in general each
node has a slightly different situation, so we end up with slightly
different get_result_domain functions in each node. I think this makes
sense, it keeps the code flexible and more easily understandable.

Note that we might eventually want to expose a domain drop-down to some
of the nodes. But I'd like to keep that a separate discussion from making
a more useful choice automatically, the goal of this patch.

Diff Detail

Repository
rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Feb 10 2021, 8:11 PM
Hans Goudey (HooglyBoogly) created this revision.
source/blender/nodes/geometry/node_geometry_util.cc
341–361

What's the reasoning for these priorities? Why is corner so high? I'd consider that to be one of the more "special" domains.

source/blender/nodes/geometry/nodes/node_geo_attribute_combine_xyz.cc
83

Two times "X"

source/blender/nodes/geometry/node_geometry_util.cc
341–361

It's not so much about how often different domains are used, but how much information they convey. E.g. if you have attribute "a" on points and attribute "b" on corners. Now when you add them, where should the new domain be (if not set explicitly). I think it should be on corners, because if it was on points, we would loose per-corner information.
Point attribute can be accessed without information loss on corners, but not the other way around.

source/blender/nodes/geometry/node_geometry_util.cc
341–361

Ok I see, makes a lot of sense! Thanks for explaining.

source/blender/nodes/geometry/nodes/node_geo_attribute_fill.cc
51

Why add this if we don't display it yet? Seems like the default should be something like "auto". If you set it to something explicitly, it should always output that, regardless of existing attributes.

source/blender/nodes/geometry/nodes/node_geo_attribute_randomize.cc
54

Same comment as for the Attribute Fill node.

source/blender/nodes/geometry/nodes/node_geo_attribute_separate_xyz.cc
80

We should probably add some utility method to GeometryComponent to get the domain of an attribute without actually allocating the read attribute etc. Shouldn't be too hard to do I think. That doesn't have to be part of this patch of course, just wanted to mention it.

source/blender/nodes/geometry/nodes/node_geo_point_separate.cc
93 ↗(On Diff #33839)

Maybe the asserts can be removed. But then the comment has to be updated.

Hans Goudey (HooglyBoogly) marked 8 inline comments as done.Feb 12 2021, 1:09 AM
Hans Goudey (HooglyBoogly) added inline comments.
source/blender/nodes/geometry/node_geometry_util.cc
341–361

I added a short comment here that help a bit when someone stumbles on this in the future.

source/blender/nodes/geometry/nodes/node_geo_attribute_combine_xyz.cc
83

Thanks

source/blender/nodes/geometry/nodes/node_geo_attribute_fill.cc
51

Okay, I'll leave this for when (if) we add a domain drop-down to these nodes.

source/blender/nodes/geometry/nodes/node_geo_attribute_separate_xyz.cc
80

Yes, that and data type would be helpful. I can look into that in another patch after this.

source/blender/nodes/geometry/nodes/node_geo_point_separate.cc
93 ↗(On Diff #33839)

Oops, these were temporary changes to fix an assert I've been getting with the point separate node. I'll work on fixing that when I update the node for instances.

Hans Goudey (HooglyBoogly) marked 5 inline comments as done.
  • Review feedback from Jacques and Victor-Louis, other cleanup

Looks good to me. Couldn't find issues yet, obviously there aren't too many ways to test this yet because almost all data is on points.

This revision is now accepted and ready to land.Feb 12 2021, 12:52 PM