Page MenuHome

Geometry Nodes: New Interpolate Curves node.
ClosedPublic

Authored by Jacques Lucke (JacquesLucke) on Nov 29 2022, 1:52 PM.
Tags
None
Tokens
"Love" token, awarded by zhruith."Love" token, awarded by hitrpr."Like" token, awarded by duarteframos."Like" token, awarded by erickblender."Love" token, awarded by dbystedt."Burninate" token, awarded by Draise."Love" token, awarded by Schamph."Love" token, awarded by galenb."Love" token, awarded by andruxa696."Love" token, awarded by zNight."Love" token, awarded by Yuro."100" token, awarded by Frozen_Death_Knight."Love" token, awarded by turbocheke."Like" token, awarded by RedMser."Love" token, awarded by Dangry."Party Time" token, awarded by Bit."Burninate" token, awarded by dulrich."100" token, awarded by strangerman."Like" token, awarded by Moder.

Details

Summary

This adds a new Interpolate Curves node. It allows generating new curves between a set of existing guide curves. This is essential for procedural hair.

Usage:

  • One has to provide a set of guide curves and a set of root positions for the generated curves. New curves are created starting from these root positions. The N closest guide curves are used for the interpolation.
  • An additional up vector can be provided for every guide curve and root position. This is typically a surface normal or nothing. This allows generating child curves that are properly oriented based on the surface orientation.
  • Sometimes a point should only be interpolated using a subset of the guides. This can be achieved using the Guide Group ID and Point Group ID inputs. The curve generated at a specific point will only take the guides with the same id into account. This allows e.g. for hair parting.
  • The Max Neighbors input limits how many guide curves are taken into account for every interpolated curve.

Below is a set of regressions tests that cover 96.4% of the new node code.

Related todos:

Diff Detail

Repository
rB Blender

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Yuro (Yuro) added a subscriber: Yuro (Yuro).
  • Merge branch 'master' into curve-interpolation
  • fix
  • Merge branch 'master' into curve-interpolation
  • add socket descriptions
  • cleanup
  • support interpolating curves with arbitrary length
  • support faster interpolation if guides has the same number of control points
  • Merge branch 'master' into curve-interpolation
  • fix naming
  • Merge branch 'master' into curve-interpolation
  • prefer flat data structure over nested vector
  • cleanup
  • reorder functions
  • support meshes as point source
  • add index and weight outputs

I find the code a bit dense. There's probably room for abstracting some concepts and making some of it more generic or splitting into smaller function.
On the other hand what I've read so far is intuitive, so I'm sure it's all understandable given some more time.

Just adding a few things I've noticed so far as inline comments.

source/blender/nodes/geometry/nodes/node_geo_interpolate_curves.cc
260

For the up vectors, I'd like to see the normalization happen as part of the field evaluation, since it will be faster for single values
and more easily target-able with future optimizations-- removing redundant normalization, SIMD, etc. In some other nodes we
insert operation nodes in the field network before evaluation. I think that would make sense here.

It would also just separate things conceptually. "The node only does what it needs to do"

285

Is it possible to use the new matrix API here yet?

487

Newline isn't so helpful here IMO

548–551

Why do you need to specify NoInitialization() manually? I thought we could count on trivial types not being initialized at all in new arrays and vectors.

576–582

If I'm recognizing the pattern correctly, it's accumulate_counts_to_offsets. Better to use that directly then IMO, to make it more recognizable.
The way to do that is allocate the curve domain and the offsets, fill the offsets array with the sizes in parallel, and then accumulate the offsets directly with that function. Then the number of points is known and the point domain can be created.
Eventually hopefully that pattern can be simplified further with a bke::CurvesGeometry(Array<int> offsets) constructor. At least it's a standard thing though.

586–588

IMO a large contiguous array and an array of offsets (similar to curves offsets) would be simpler and more standard here. Is there a reason you used a bunch of smaller allocations here? (I know they're amortized by LinearAllocator, but still

Jacques Lucke (JacquesLucke) marked 6 inline comments as done.
  • Merge branch 'master' into curve-interpolation
  • cleanup
  • normalize as part of field evaluation
  • use accumulate_counts_to_offsets
  • use offsets array for parameterization
  • extract function to store output attribute
source/blender/nodes/geometry/nodes/node_geo_interpolate_curves.cc
285

Almost. Can use the API, but it still uses the old implementation. We'll have to rip out the old matrix types.

548–551

You're right, that's a left over from when I didn't use a flat array, but vectors.

586–588

Yeah, that's fine too. It's a bit more verbose currently, but that could be solved with a better abstraction for offset arrays.

Jacques Lucke (JacquesLucke) retitled this revision from Geometry Nodes: New Interpolate Curves nodes (WIP). to Geometry Nodes: New Interpolate Curves nodes..Tue, Jan 17, 7:03 PM
Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)

I left some suggestions inline. None are correctness related, but I think they could be improvements. Other than that I think this is ready, so I'll accept.

One thing-- I thought we discussed using hide_value for the group ID inputs, favoring consistency with other similar sockets over the use case where only one is a single value in this node.

I noticed in testing that it could be interesting if it would be possible to use geodesic distances instead of 3D distances. Just a reminder that splitting up this node would be a nice eventual goal when we have lists.

source/blender/nodes/geometry/nodes/node_geo_interpolate_curves.cc
77
96–115

What about balancing the KD trees in parallel?

227

compute_points_per_child -> compute_point_counts_per_child

No strong opinion, but isn't that a bit clearer?

522–534

It's fine how it is, but what about this version oriented around simpler hot loops. Seems more oriented to future generalizations and optimizations.
Not a big deal if you have reasons to prefer the current version though.

761–768

Nicer to save some space and text here?

This revision is now accepted and ready to land.Tue, Jan 17, 11:43 PM
Hans Goudey (HooglyBoogly) retitled this revision from Geometry Nodes: New Interpolate Curves nodes. to Geometry Nodes: New Interpolate Curves node..Tue, Jan 17, 11:45 PM

I'm still in the process of testing the node. One thing I'd like to address is on the topic of how exactly the shape interpolation works to preserve the general flow.

There is an issue that the old system used to have, which is also an issue with the node currently. Here I'm trying to address it with a node setup after the main interpoolation node. but the issue is that then I can only take a single guide into account and the effect falls apart when multiple guides are considered. So this should be something inside of the node.

Let's talk about this in more detail in today's meeting, as this requires some more discussion. But I wanted to bring it up already.

Jacques Lucke (JacquesLucke) marked 2 inline comments as done.
  • Merge branch 'master' into curve-interpolation
  • use offset indices
  • balance kdtrees in parallel
  • cleanup
  • use smaller loops
  • fix
  • Merge branch 'master' into curve-interpolation
  • hide group id values
  • fix crazy space

Is there a way to handle interpolation for thin geometry with guides on both sides?

Is there a way to handle interpolation for thin geometry with guides on both sides?

Not natively right now. You could potentially use separate interpolation groups for that, but that's not really ideal. We could also add the option to lower the weight of certain neighbors based on the angle of the up directions. Has to be investigated. The most correct solution is probably to use some kind of geodesic distances, but that's quite tricky and expensive. Doesn't mean that this will never be possible though.

I'd be fine with getting this patch committed without the curve tangent space functionality and splitting that into a separate patch, as it's already usable without that.
Could you set that up and add the toggle so we can test and tweak that behavior separately?