Paths

Table of Contentst

Differential D12005

Geometry Nodes: Expose Bezier handle positions as an attribute
ClosedPublic
Actions

Authored by Hans Goudey (HooglyBoogly) on Jul 22 2021, 11:43 PM.

Details

Reviewers

Jacques Lucke (JacquesLucke)
Erindale (Erindale)
Dalai Felinto (dfelinto)

Commits

rB81f552e9ad1a: Geometry Nodes: Expose Bezier handle positions as an attribute

Summary

This commit exposes left and right bezier handles as an attribute.
Interaction basically works like edit mode, where if you move an
aligned handle, it also moves the opposite handle of the control point.
The difference is that you can't edit "Auto" or "Vector" handles,
you have to first use the "Set Handle Type" node. I think that is
more intuitive, since it gives the handle types a bit more meaning
in the node tree-- changing them in edit mode is more like a "UI override".

The attributes are named handle_start and handle_end,
which is the same name used in the curve RNA API.

As is tradition at this point, there is a new virtual array implementation,
which handles the case of splines that don't have these attributes,
but it also calls two new functions on BezierSpline to set the
handle position and account for aligned handles, etc.

The virtual arrays and attribute providers will be refactored
(probably templated) in the future, as a next step after the last built-in
curve attribute provider has landed.

Diff Detail

Repository

rB Blender

Branch

geometry-nodes-curve-handles-attribute (branched from master)

Build Status

Buildable 16369
Build 16369: arc lint + arc unit

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Jul 22 2021, 11:43 PM

Hans Goudey (HooglyBoogly) created this revision.

Harbormaster completed remote builds in B15971: Diff 39837.Jul 22 2021, 11:43 PM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Jul 22 2021, 11:46 PM

Hans Goudey (HooglyBoogly) added reviewers: Jacques Lucke (JacquesLucke), Erindale (Erindale), Dalai Felinto (dfelinto).

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

Hans Goudey (HooglyBoogly) added a parent revision: D12004: Geometry Nodes: Expose NURBS weight as an attribute.

I wonder if it is possible to deduplicate some parts of all the virtual array implementation stuff. I'm thinking about something along those lines:

template<typename T,
         typename SplineData,
         T GetF(const Spline &spline, const SplineData &data, int index)>
class VArray_For_Splines {
 private:
  Span<SplinePtr> splines_;
  Array<int> offsets_;
  Array<SplineData> spline_data_;

 public:
  VArray_For_Splines(Span<SplinePtr> splines, Array<int> offsets, Array<SplineData> spline_data)
      : splines_(splines), offsets_(offsets), spline_data_(std::move(spline_data))
  {
  }

  T get_impl(const int64_t index) const final
  {
    const PointIndices indices = lookup_point_indices(offsets_, index);
    return GetF(
        *splines_[indices.spline_index], spline_data_[indices.spline_index], indices.point_index);
  }
};

Note sure if that actually works out, but if it does, it should help quite well with separation of concerns.

source/blender/blenkernel/intern/geometry_component_curve.cc
771	This could probably a template parameter, but seems fine this way as well, the overhead should be negligible here.
882	Revert the condition and return early.

This revision now requires changes to proceed.Jul 23 2021, 10:42 AM

Charlie Jolly (charlie) mentioned this in T86749: Geometry Node Community Additions.Jul 23 2021, 10:43 AM

Jacques Lucke (JacquesLucke) mentioned this in D12004: Geometry Nodes: Expose NURBS weight as an attribute.Jul 23 2021, 10:53 AM

From the daily meeting regarding naming:

"Left" and "Right" are arbitrary, it should use "start" and "end" instead.
"handle" should come first in the name so that alphabetical sorting puts them in the same spot.
These names will be less important in the future anyway, when built-in attributes have special sockets.

Also, I mentioned to Jacques that I'd like to refactor the virtual array and attribute provider implementations after committing these two patches.

Cleanup
Rename to "handle_start" and "handle_end" based on feedback

Harbormaster completed remote builds in B16369: Diff 40609.Aug 11 2021, 7:53 PM

Hans Goudey (HooglyBoogly) removed a parent revision: D12004: Geometry Nodes: Expose NURBS weight as an attribute.Aug 11 2021, 7:53 PM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Aug 11 2021, 8:13 PM

Johnny Matthews (guitargeek) awarded a token.Aug 11 2021, 8:40 PM

I wonder if there is any value in being consistent with the python api (https://docs.blender.org/api/current/bpy.types.BezierSplinePoint.html#bpy.types.BezierSplinePoint).
I understand where the start and end comes from, I'm just much more used to left and right in this context. Maybe I just need to get used to it. Could you maybe ask one/two other devs working on curves to see if they are fine with start/end?
Other than that, the patch seems fine.

Not that the answer belongs in this patch, but this raises a question about a specific use case. If I wanted each segment of this wire to have a random droop and made a random attribute to drive it, it would effect control 1&2 , 3&4, 5&6 in pairs which would not give the desired results since in this case the paired values should be 2&3, 4&5.

You can randomize the droop on entire splines with no trouble, but this end handle matched with next point start handle is not really possible that I can tell.

Merge branch 'master' into geometry-nodes-curve-handles-attribute
Switch back to "handle_left" and "handle_right" names, for consistency with RNA API

That's a great point Johnny, we may need to think about that. Maybe having some way to "shift forward"
in the spline when evaluating a field. Though I'm not sure how that would work technically.

Harbormaster completed remote builds in B16814: Diff 41439.Sep 4 2021, 12:27 AM

Hans Goudey (HooglyBoogly) mentioned this in T91780: Create nodes to get/set builtin attributes.Sep 28 2021, 10:38 PM

Andy Cuccaro (AndyCuccaro) added a subscriber: Andy Cuccaro (AndyCuccaro).Sep 29 2021, 1:19 AM

Johnny Matthews (guitargeek) mentioned this in D12687: Geometry Nodes: Add Nodes to Get/Set Built-in Attributes.Sep 29 2021, 4:43 AM

Jacques Lucke (JacquesLucke) accepted this revision.Sep 29 2021, 2:55 PM

Jacques Lucke (JacquesLucke) added inline comments.

source/blender/blenkernel/intern/geometry_component_curve.cc
517	Why is this change necessary now?

This revision is now accepted and ready to land.Sep 29 2021, 2:55 PM

Hans Goudey (HooglyBoogly) added inline comments.Sep 29 2021, 3:05 PM

source/blender/blenkernel/intern/geometry_component_curve.cc
517	It's a way to avoid writing a separate materialize function for attributes that might not exist on some spline types. The caller can add an empty span in `data` for spline types besides Bezier splines. I'll add that in a comment.