Paths

Table of Contentst

Raycast geometry node.
ClosedPublic
Actions

Authored by Lukas Tönne (lukastoenne) on Jun 16 2021, 11:28 AM.

Details

Reviewers

Jacques Lucke (JacquesLucke)
Hans Goudey (HooglyBoogly)

Commits

rB4b673ebb9906: Raycast geometry node.

Summary

The Raycast node intersects rays from one geometry onto another.
It computes hit points on the target mesh and returns normals, distances
and any surface attribute specified by the user.

A ray starts on each point of the input Geometry. Rays continue
in the Ray Direction until they either hit the Target Geometry
or reach the Ray Length limit. If the target is hit, the value of the
Is Hit attribute in the output mesh will be true. Hit Position,
Hit Normal, Hit Distance and Hit Index are the properties of the
target mesh at the intersection point. In addition, a Target Attribute
can be specified that is interpolated at the hit point and the result
stored in Hit Attribute.

Docs: D11620

Diff Detail

Repository

rB Blender

Branch

geometry-nodes-raycast

Build Status

Buildable 15247
Build 15247: arc lint + arc unit

Event Timeline

Lukas Tönne (lukastoenne) requested review of this revision.Jun 16 2021, 11:28 AM

Lukas Tönne (lukastoenne) created this revision.

Harbormaster completed remote builds in B15247: Diff 38395.Jun 16 2021, 11:28 AM

Lukas Tönne (lukastoenne) added reviewers: Jacques Lucke (JacquesLucke), Hans Goudey (HooglyBoogly).Jun 16 2021, 11:30 AM

Lukas Tönne (lukastoenne) mentioned this in D11620: Documentation for Raycast geometry node.Jun 16 2021, 11:38 AM

Mindinsomnia (mindinsomnia) awarded a token.Jun 16 2021, 11:39 AM

Lukas Tönne (lukastoenne) edited the summary of this revision. (Show Details)Jun 16 2021, 11:39 AM

This looks quite good already! I really like how the "Is Hit" matches with our plans for selections.

Hit Index Output
I don't think we should include this in the node right now. I think mostly we should try to keep indices as an implementation detail while we can.
And also, inputting a list of attribute names will cover the case of fast transfer of multiple attributes when we implement it.

Naming and Category
Initially I was skeptical about putting this in the "Geometry" category, but I guess this node could work with more geometry types in the future.
The one I can think of is volume level sets when we add them in the future, I can't think of anything else though.

AttributeInterpolator Class
I like what you've done here, it looks quite useful, and simplifies interpolating multiple attributes. A few comments though:

Naming: This is really specific to meshes, so I would include that in the name.
Location: We already have mesh_sample.cc created recently for this purpose, I think it makes more sense to have this class there, since it's fairly low level code useful in many places.

source/blender/makesrna/intern/rna_nodetree.c
9899	`from corners` -> `from the corners`
source/blender/nodes/geometry/node_geometry_util_interp.cc
103–105	Do we ever actually expect the looptri indices to be empty? I think that would generally be checked before this. Maybe it's better to assert.
source/blender/nodes/geometry/nodes/node_geo_raycast.cc
33	You could add `PROP_DISTANCE` at the end so we get units on the button.
62	I think the default for the ray direction should be "Vector", since that's easier to understand and probably matches a large portion of use cases.
124	I don't think this should check if the hit positions span or the hit indices spans are empty, since you guarantee they aren't in the calling code.
228	This is a style thing, but I think you can use `{}` for the second case here, which is just a bit cleaner IMO.
258	`for (const int i : hit_attribute_names.index_range())`

This revision now requires changes to proceed.Jun 16 2021, 5:52 PM

Another thought I had is that this node could support the case of a single instance quite easily to avoid realizing the target geometry instances in some cases (like the output of the join geometry node)

if target only has instances && target.instances_amount == 1 && target.instances[0].type == mesh:
   transform source geometry coordinates and ray directions into the transform space of the target mesh instance

That's probably a valid optimization in plenty of cases, so it could just be done in the future if it was worth it.

Another option is making a reduced version of realizing instances that only duplicates the vertex positions in that 1-instance case (at some point in the future).

Merge branch 'geometry-nodes-raycast' of git.blender.org:blender into geometry-nodes-raycast
Merge branch 'master' into geometry-nodes-raycast
Removed the hit_index output attribute.
Move AttributeInterpolator util class to bke.
Fixed doc string.
Remove unnecessary noop check in interpolator.
Use distance subtype for the ray length input.
Show ray direction input value socket by default.
Modernized for loop style.

Harbormaster completed remote builds in B15278: Diff 38450.Jun 17 2021, 10:32 AM

Hit Index Output
I don't think we should include this in the node right now. I think mostly we should try to keep indices as an implementation detail while we can.
And also, inputting a list of attribute names will cover the case of fast transfer of multiple attributes when we implement it.

Yes, i wasn't sure about the index output socket, will remove it.

Naming and Category
Initially I was skeptical about putting this in the "Geometry" category, but I guess this node could work with more geometry types in the future.
The one I can think of is volume level sets when we add them in the future, I can't think of anything else though.

I agree, there doesn't seem to be a more fitting category atm. Easy to change though, and more future nodes will give us a better idea of how to organize them.

AttributeInterpolator Class
I like what you've done here, it looks quite useful, and simplifies interpolating multiple attributes. A few comments though:

Naming: This is really specific to meshes, so I would include that in the name.

Location: We already have mesh_sample.cc created recently for this purpose, I think it makes more sense to have this class there, since it's fairly low level code useful in many places.

Done, it lives in BKE_mesh_sample.hh as MeshAttributeInterpolator now.

source/blender/nodes/geometry/node_geometry_util_interp.cc
103–105	position/looptri arrays can be empty, but this is just a noop early exit. Isn't really necessary. Nothing expensive happens if there are no input points, so i removed the check.
source/blender/nodes/geometry/nodes/node_geo_raycast.cc
124	Not quite. They are only guaranteed when there is also a custom attribute for interpolation (because the interpolation needs pos + index). Otherwise you could use the node without position outputs and just read normals, for instance.
228	Compiler says "No" ... I've copied this from some other node. Will leave it as is.

kursad k (kursadk) awarded a token.Jun 17 2021, 5:50 PM

Looks great now, just one style thing mentioned inline.

I wonder if the mesh attribute interpolation class could be used in the point distribute node, that would be nice.

source/blender/nodes/geometry/nodes/node_geo_raycast.cc
287–289	Another style thing, only bringing this up because I like this idea and I'd like to use it for other nodes. What about this? for (const GeometryComponentType type : {GEO_COMPONENT_TYPE_MESH, GEO_COMPONENT_TYPE_POINT_CLOUD, GEO_COMPONENT_TYPE_CURVE}) { A bit more concise is nice IMO.

Lukas Tönne (lukastoenne) added inline comments.Jun 17 2021, 9:15 PM

source/blender/nodes/geometry/nodes/node_geo_raycast.cc
287–289	No strong opinion. I like making the variable first because the name is a bit of mini-documentation, and eventually the types might be defined somewhere else.

This revision was not accepted when it landed; it landed in state Needs Review.Jun 17 2021, 10:12 PM

Closed by commit rB4b673ebb9906: Raycast geometry node. (authored by Lukas Tönne (lukastoenne)). · Explain Why

This revision was automatically updated to reflect the committed changes.

Lukas Tönne (lukastoenne) added a commit: rB4b673ebb9906: Raycast geometry node..

Andres Stephens (Draise) awarded a token.Jun 18 2021, 4:11 AM

Lopo Isaac (lopoIsaac) awarded a token.Jun 18 2021, 8:06 AM

Damian (damian) awarded a token.Jun 18 2021, 11:44 AM

Charlie Jolly (charlie) mentioned this in T86749: Geometry Node Community Additions.Jun 18 2021, 12:52 PM