Change the Curve Parameter Node to Spline Parameter and add an Index output that contains the index of each control point on the given spline.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- indexonspline (branched from master)
- Build Status
Buildable 18947 Build 18947: arc lint + arc unit
Event Timeline
Comment Actions
I like this idea! Even though it doesn't exactly fit with the idea of parameterization, the concept is similar enough in a way that makes it fit well enough, at least compared to adding a separate node for it.
Small thing, personally I'd go with "Index In Spline", it just feels more correct that way somehow.
| source/blender/nodes/geometry/nodes/node_geo_curve_parameter.cc | ||
|---|---|---|
| 40 | it's -> its Could be simplified a bit maybe, since this only has a definition for points. Something like this maybe? Each control point's index on its spline | |
| 197–205 | I'd suggest using the offsets from control_point_offsets to our advantage and structure the loop as a loop over all splines: for (const int i : IndexRange(curve.splines.size())) {. That should simplify it a lot. | |
| 209–211 | This function has no return when the domain is not either of these values. You could just skip the if statement. | |
Comment Actions
- Merge branch 'master' into indexonspline
- Use total_control_point_size()
- small cleanup
Comment Actions
Looks good to me. I haven't tested it but the concept is simple and I'm trusting that Johnny has :)
Want to make sure Jacques sees this though.
Comment Actions
This is missing a change in nodeitems_builtins.py.
We have a new policy regarding versioning that would be easy to test with this patch (more info about that still needs to be written down (T93409)). The new socket should be added in versioning code as well. Otherwise future versioning code will have to deal with both versions of the code (which will become very messy over time).
Comment Actions
Like @Jacques Lucke (JacquesLucke) mentioned above, the versioning code should still add the new socket. Other than that this looks good to me.
| source/blender/nodes/geometry/nodes/node_geo_curve_spline_parameter.cc | ||
|---|---|---|
| 23 | I think I'd go with node_geo_spline_parameter_cc (without the "curve_", just because it's shorter and matches up exactly with the name we use in the UI). I don't feel that strongly though. | |
| source/blender/blenloader/intern/versioning_300.c | ||
|---|---|---|
| 2415 | Either increase the subversion or check if the socket exists already (making the versioning code idempotent). I prefer the second option. Feel free to add an utility method. You can use nodeAddStaticSocket instead of calling nodeStaticSocketType manually. | |
| source/blender/nodes/geometry/nodes/node_geo_curve_spline_parameter.cc | ||
| 194 | Can just use output directly. No need to create a span. | |
| 195 | Rename index variables to output_index, spline_index and point_index. That should make it more clear I think. | |
Comment Actions
- Merge branch 'master' into indexonspline
- Move versioning function to versioning_common
Comment Actions
I was just wondering, how does one access the spline index when processing individual points currently?