Page MenuHome

Curves: Port realize instances node to the new data-block
ClosedPublic

Authored by Hans Goudey (HooglyBoogly) on Mar 5 2022, 5:13 AM.

Details

Summary

This commit replaces the temporary conversion to CurveEval with use
of the new curves data-block. The end result is that the process looks
more like the other components-- somewhere in between meshes and
point clouds in terms of complexity.

The final result is that the logic between meshes and curves is very similar.
There are a few different strategies to reduce duplication here, so I'll
investigate that separately.

There is some special behavior for the radius and handle position attributes.
I used the attribute API to store spans of these attributes temporarily.
Using access methods on CurvesGeometry would be reasonable to,
storing spans separately feels a bit more predictable for now though.

There should be significant performance improvements in some cases,
I haven't tested that specifically though.

Diff Detail

Repository
rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Mar 5 2022, 5:13 AM
Hans Goudey (HooglyBoogly) created this revision.
Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)
Hans Goudey (HooglyBoogly) set the repository for this revision to rB Blender.

Cleanup

Jacques Lucke (JacquesLucke) added inline comments.
source/blender/geometry/intern/realize_instances.cc
1244

add break

1253

Any reason why you changed construct to assign? Same below. It obviously doesn't matter for trivial types, it's just a bit inconsistent. Also we don't really want that the memory is initialized already, right? Because that would mean that we are writing to it twice.

This revision is now accepted and ready to land.Mar 9 2022, 10:30 AM
source/blender/geometry/intern/realize_instances.cc
1253

I only did that because I was copying from the point cloud and mesh implementations. I agree that construct makes more sense.
Maybe I'll keep it like this when committing this patch, and change it in a follow-up?