Paths

Table of Contentst

Differential D10415

Geometry Nodes: add Location output to Attribute Proximity node
ClosedPublic
Actions

Authored by Victor-Louis De Gusseme (victorlouis) on Feb 14 2021, 2:51 PM.

Details

Reviewers

Hans Goudey (HooglyBoogly)

Commits

rBf7933d0744df: Geometry Nodes: Add "Location" output to Attribute Proximity node

Summary

Description

This patch adds an output field to the Attribute Proximity node and renames the existing one from "Result" to "Distance".

The "Distance" output contains distance to the closest position on the Target geometry.
New: The "Location" output contains the coordinates of the closest position on the Target geometry.

UI before:

UI after:

Use Cases

Shrinkwrap:

Also once normals on meshes are available, you'll be able to use those for a kind of signed distance.

Diff Detail

Repository

rB Blender

Branch

geometry-nodes-attribute-proximity-coordinate (branched from master)

Build Status

Buildable 12884
Build 12884: arc lint + arc unit

Event Timeline

Victor-Louis De Gusseme (victorlouis) requested review of this revision.Feb 14 2021, 2:51 PM

Victor-Louis De Gusseme (victorlouis) created this revision.

Harbormaster completed remote builds in B12884: Diff 33976.Feb 14 2021, 2:51 PM

Victor-Louis De Gusseme (victorlouis) edited the summary of this revision. (Show Details)Feb 14 2021, 3:02 PM

Victor-Louis De Gusseme (victorlouis) added a project: Geometry Nodes.

Victor-Louis De Gusseme (victorlouis) edited the summary of this revision. (Show Details)Feb 14 2021, 3:06 PM

Kenzie (kenziemac130) awarded a token.Feb 14 2021, 4:03 PM

Kenzie (kenziemac130) added a subscriber: Kenzie (kenziemac130).

Leon Schittek (lone_noel) awarded a token.Feb 14 2021, 5:33 PM

Konstantin (Ko) awarded a token.Feb 14 2021, 9:50 PM

Charlie Jolly (charlie) awarded a token.Feb 15 2021, 12:05 AM

Paul Larson (GeorgiaPacific) awarded a token.Feb 15 2021, 1:27 AM

Colin Marmond (Kdaf) awarded a token.Feb 15 2021, 11:51 AM

As we talked about earlier, this is a cool idea. I know we talked about only exposing one of the attribute fields at a time, but there are definitely situations where you want to use both, and the UI is nice and simple the way you have it here.

The only issue with the naming is that it's harder to tell these are result attributes that the node fills with values. Maybe it's obvious though, not sure.

source/blender/nodes/geometry/nodes/node_geo_attribute_proximity.cc
35	I think "Location" is usually the preferred word for this.
214	Since you know the size already and the size won't change, use `Array`
214	Paying the memory cost for the coordinate array even if it's used is not acceptable IMO. The first other option I can think of is templating the proximity calculation with a function that could either add to just the distance or both, or just the coordinates. That probably needs some further thought and experimentation though.
215	Vector can automatically convert to span, so just pass the vector (or array) directly to the other function and let it convert implicitly.

This revision now requires changes to proceed.Feb 17 2021, 4:59 AM

Victor-Louis De Gusseme (victorlouis) retitled this revision from Geometry Nodes: add coordinate output to Attribute Proximity node to Geometry Nodes: add Location output to Attribute Proximity node.Feb 20 2021, 12:35 PM

Victor-Louis De Gusseme (victorlouis) edited the summary of this revision. (Show Details)

Merge branch 'master' into geometry-nodes-attribute-proximity-coordinate
Rename coordinate to location

Harbormaster completed remote builds in B13026: Diff 34211.Feb 20 2021, 12:36 PM

Victor-Louis De Gusseme (victorlouis) marked 3 inline comments as done.Feb 20 2021, 12:40 PM

Victor-Louis De Gusseme (victorlouis) added inline comments.Feb 20 2021, 1:32 PM

source/blender/nodes/geometry/nodes/node_geo_attribute_proximity.cc
214	I'll think about how we should best organize the code here.

Merge branch 'master' into geometry-nodes-attribute-proximity-coordinate
If one of the outputs is not requested by the user, no span is allocated for it anymore.

Harbormaster completed remote builds in B13039: Diff 34233.Feb 21 2021, 4:36 PM

@Hans Goudey (HooglyBoogly) for now I've updated to code such that it doesn't allocate the temporary spans anymore, and instead I've added if statements in the main for loop with keep_distance and keep_location. I know it's not ideal to have these if statements inside the loop, but honestly, I doubt they have a significant impact on performance. keep_distance and keep_location are const bools so I would just trust the compiler in this case.

I tested the performance in a couple situations, and I actually didn't even observe a difference between when the location attribute is used or not. So the boolean check here likely has a negligible performance impact, at most. There are other ways I think this code could be restructured, but there's no pressing need for it.

Since the team has discussed the design of this idea specifically, I'm going to go ahead and commit this. In the commit I will change keep_distance to store_distance since I think it's a bit more clear.

This revision is now accepted and ready to land.Feb 26 2021, 10:10 PM

Closed by commit rBf7933d0744df: Geometry Nodes: Add "Location" output to Attribute Proximity node (authored by Victor-Louis De Gusseme (victorlouis), committed by Hans Goudey (HooglyBoogly)). · Explain WhyFeb 26 2021, 10:23 PM

This revision was automatically updated to reflect the committed changes.

Hans Goudey (HooglyBoogly) added a commit: rBf7933d0744df: Geometry Nodes: Add "Location" output to Attribute Proximity node.

I also realized after I accepted this that it needed versioning. It was pretty trivial so I just added it myself.

Charlie Jolly (charlie) mentioned this in T86749: Geometry Node Community Additions.Mar 19 2021, 10:20 PM