This node sets the selected (or all) splines in curve to a chosen target spline type. Polyspline, Bezier, and NURBS can be converted to any of the 3.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
Looks pretty good! It's nice how well the Bezier to NURBS conversion works.
I'm getting some sharp corners with the opposite conversion though:
| release/scripts/startup/nodeitems_builtins.py | ||
|---|---|---|
| 565 | Extra newline here | |
| source/blender/makesrna/intern/rna_nodetree.c | ||
| 9447–9448 | Periods are added automatically to tooltips. | |
| source/blender/nodes/geometry/nodes/node_geo_curve_spline_type.cc | ||
| 78–81 | Split out this part to a separate function that takes a span and a mutable span, call that for the built-in attributes below as well. | |
| 82 | I don't think returning is necessary here. | |
| 92 | Extra newline | |
| 111–116 | Split out this part to a separate function that takes a span and a mutable span, call that for the built-in attributes below as well. | |
| 117 | I'm not sure returning is necessary here. | |
| 130 | I made "copy_base_settings" public, so it can be used to copy the normal mode and cyclic values for all of the conversions. | |
| 139 | Use output.resize() to avoid reallocation when growing the vectors. | |
| 147–148 | reallocate is unecessary when the value is assigned next. | |
| 149 | Is marking the cache invalid explicitly necessary? It should start out dirty when you create a new spline. | |
| 187 | You can avoid the for loop with stuff like output->radii().copy_from(input.radii()) and output->handle_types_left().fill(BezierSpline::HandleType::Vector); | |
| 236–269 | /home/hans/Blender-Git/blender/source/blender/nodes/geometry/nodes/node_geo_curve_spline_type.cc:256:1: error: control reaches end of non-void function [-Werror=return-type]
256 | }
| ^
/home/hans/Blender-Git/blender/source/blender/nodes/geometry/nodes/node_geo_curve_spline_type.cc: In function ‘SplinePtr blender::nodes::convert_to_nurbs(const SplinePtr&)’:
/home/hans/Blender-Git/blender/source/blender/nodes/geometry/nodes/node_geo_curve_spline_type.cc:268:1: error: control reaches end of non-void function [-Werror=return-type]
268 | }
| ^The right way to fix this is probably BLI_assert_unreachable();
return {}; | |
| 285 | This comment is incorrect here. | |
Comment Actions
Updated with changes from @Hans Goudey (HooglyBoogly)
I left the old bezier to nurbs code commented out. This code made the converted curve look like the source (or try to), while the new code returns the same result as the "Set Spline Type To Nurbs" operator in curve edit mode. See comparison image in summary.
Comment Actions
Is there any reason to leave it commented out? Generally there should be a really good reason to do that, since commented code tends to rot very quickly.
If the conversion that keeps the shape only sometimes, probably best to just keep things simple for now.
BTW, if you want to feel satisfied, compare what we have here to BKE_nurb_type_convert. The difference is striking.
| source/blender/nodes/geometry/nodes/node_geo_curve_spline_type.cc | ||
|---|---|---|
| 229–241 | Is there a reason to average the radii and tilt values but not the generic attributes? Maybe we could reuse scale_input_assign? Maybe I'm missing something. | |
| 243–244 | Can use fill here too. | |
| 248 | This works fine for me: Spline::copy_base_settings(*input, *output); Basically just using the * operator on the unique_ptr directly instead of the raw pointer from the .get() function. | |
| source/blender/nodes/geometry/nodes/node_geo_curve_spline_type.cc | ||
|---|---|---|
| 229–241 | problably just overthinking it. Just doing a straight scaled copy instead | |
Comment Actions
When cleaning this up for committing I made a few cleanup changes:
- Use references instead of pointers.
- Use static_cast instead of dynamic cast when we already know the spline type (somehow that allowed casting away const BTW).
- Fixed alphabetical sorting
| source/blender/nodes/geometry/nodes/node_geo_curve_spline_type.cc | ||
|---|---|---|
| 59 | Span is small (just a pointer and a size), so it should be passed by value rather than by reference. | |
| 104 | This was unused, same with scale_output_assign above. I added the more specific bezier to nurbs handling though, since it works perfectly with bezier knots mode and non-cyclic splines (which can be improve separately). I templated the function to avoid copying the attribute iteration boilerplate code though. | |