Page MenuHome

Geometry Nodes: Add side and fill segments to Cone/Cylinder mesh primitive nodes
ClosedPublic

Authored by Leon Schittek (lone_noel) on Sep 12 2021, 9:56 AM.

Details

Summary

This commit extends the 'Cone' and 'Cylinder' mesh primitive nodes, with
two inputs to control the segments along the side and in the fill. This
makes the nodes more flexible and brings them more in line with the
improved cube node.


Having control over the additional segments can be helpful for several
reasons:

  • You can have more geometry available for further deformation.
  • It allows creating cones/cylinders with more even geometry density even if the dimensions are very uneven/excentric.
  • I can help create better topology for subdividing.

Some of these points can already be achieved with the Curve to Mesh
node by sweeping a circular profile along a path and that kind of
setup can give you even more control in some regards but it is more
complex to and doesn’t result in a manifold mesh with a UV map.
So overall I think adding these controls is worth it.

Examples

The cone can even be used to create a polar grid. I don’t think that
this is necessarily the best solution for that specific use-case, but it
shows that the node becomes quite versatile:

Diff Detail

Repository
rB Blender
Branch
geometry-nodes-cone-primitive-segments (branched from master)
Build Status
Buildable 17514
Build 17514: arc lint + arc unit

Event Timeline

Leon Schittek (lone_noel) requested review of this revision.Sep 12 2021, 9:56 AM
Leon Schittek (lone_noel) created this revision.
Leon Schittek (lone_noel) retitled this revision from Geometry Nodes: Cone/Cylinder mesh primitives with rings/segments. to Geometry Nodes: Add side and fill segments to Cone/Cylinder mesh primitive nodes.Sep 12 2021, 10:10 AM
  • Simplify UV calculation.
  • Clean up: Adjust comments and variable names.

Curious about the progress here, is this work in progress or ready for review? I can't wait to play with it in 3.0 master, just asking because I am not sure whether this is one of those things that can still be committed to 3.0 after bcon2 (which apparently has been delayed to September 29 now)

  • Merge master and add tooltips for the cone primitive sockets.
  • Clean-up: improve some variable names and comments.

@Zijun Zhou (Eary) Thanks for the interest! From my side the patch is now ready for review. I'll mention it to the module devs, but the attribute/field conversion probably has priority right now.
Not sure if this kind of change would qualify for bcon2. We'll see!

Thanks for clarifying! I am not sure but if this is ready for review shouldn't you add Jacques and Hans as reviewers? I saw the majority of GN patches would have the reviwers specified, even some WIP ones, I am not sure how it works though.

Thanks. Didn't do a full review yet, but gave it a quick try. Didn't found any issues.
There is a clang-tidy warning: P2436.
I think this is the kind of change that we can easily do in bcon2.

  • Fix clang-tidy warning: mismatching parameter names in function declaration and definition.

Looks good to me and works perfectly in my tests. Leaving Hans as a reviewer since he implemented the original code.

source/blender/nodes/geometry/nodes/node_geo_mesh_primitive_cone.cc
77

This could have a more descriptive name now. Is it referring to the total number of vertices?

115

Use this->.

343

Use copy_v3_fl3. Same in a few other places.

Hans Goudey (HooglyBoogly) requested changes to this revision.Sep 30 2021, 6:47 PM

Great job, this is an awesome improvement, and works well!

It looks like the code is still calculating some edges manually and also calling BKE_mesh_calc_edges. We should probably pick one of the two.

I guess the new code you added doesn't necessarily deal with edge indices? How complicated did you find dealing with them? I'd be curious to see a performance comparison (if you still have the other code around, if not, I'm totally fine with automatic index calculation).

source/blender/nodes/geometry/nodes/node_geo_mesh_primitive_cone.cc
32–35

These max values are pretty high for a soft min, especially for the fill segments. It tends to increase pretty quickly. Maybe 512 would be better?

343

Nice, I didn't know about this function!

524–530

This could be a doxygen formatted comment above the function, like this:

/**
 * Text here.
 */
607

This can be written more simply like const float2 center_right(0.75f, 0.25f);

685

Same here, no need for = ConeConfig

This revision now requires changes to proceed.Sep 30 2021, 6:47 PM
Leon Schittek (lone_noel) marked 7 inline comments as done.

Changes based on review:

  • change verts_num to circle_segments to be more descriptive
  • use copy_v3_fl3 instead of copy_v3_v3
  • use doxygen format for the UV calculation comment
  • adjust soft maximum for "Vertices" and "Segments" to 512
  • use more compact initialization
  • remove code dealing with edge indices.

It looks like the code is still calculating some edges manually and also calling BKE_mesh_calc_edges. We should probably pick one of the two.

I missed that I don't even need the edge indices for the loops anymore, when using BKE_mesh_calc_edges. That makes sense. For now I removed the code dealing with edges in the update.

I guess the new code you added doesn't necessarily deal with edge indices? How complicated did you find dealing with them? I'd be curious to see a performance comparison (if you still have the other code around, if not, I'm totally fine with automatic index calculation).

Overall it wasn't too bad. I changed it mostly because it meant having less stuff to keep track of in the ConeConfig making things look less intimidating :) I do still have the code for the edge indices around and it would be interesting how it compares. I'll see if I can figure it out!

source/blender/nodes/geometry/nodes/node_geo_mesh_primitive_cone.cc
32–35

Yeah, that seems more reasonable!

77

It's referring to the number of vertices/segments around the circumference. I changed it to circle_segments, which seems in line with side_segmentsand fill_segments.

Ok, I compared the manual edge indices, with the mesh creation using BKE_mesh_calc_edges by putting a scoped timer at the beginning of create_cylinder_or_cone_mesh running it a couple (4-5) times and and averaging the results.

Hardware
I'm using a seven years old macbook. I'm not sure how much the numbers compare to todays hardware used in production.

Operating system: macOS-10.14.6-x86_64-i386-64bit 64 Bits
Graphics card: Intel Iris OpenGL Engine Intel Inc. 4.1 INTEL-12.10.31
Processor: 2,8 GHz Intel Core i5
RAM: 16 GB 1600 MHz DDR3

Results

n* verticesBKE_mesh_calc_edgesmanual edge indicesspeed up
5700.043 ms0.031 ms1.398
102900.105 ms0.051 ms2.054
507,4503.383 ms0.902 ms4.247
10029,90013.969 ms2.578 ms5.418
500749,500148.991 ms53.286 ms2.796
1,0002,999,000646.079 ms214.042 ms3.018
3,00026,997,0006.91 s1.93 s3.573
5,00074,995,000(106.86 s)5.55 s(19.24)

*n is the number of Vertices, Side Segments and Fill Segments used as input parameters in the node

Manual edge indices are clearly faster for me. I also noticed BKE_mesh_calc_edges causes memory to increase rapidly during the calculation. It doesn't really matter most of the time, but when creating a cone with 75 mil. vertices the process slowed down to a crawl when Blender needed more ram than I had (in the last result it peaked at 18.8 GB). Overall the calculation with manual indices seems to scale better.

Leon Schittek (lone_noel) edited the summary of this revision. (Show Details)
  • calculate edge indices manually
  • merge master
  • no functional changes (basically just updating because I fixed arcanist)
This revision is now accepted and ready to land.Oct 3 2021, 12:30 AM