Page MenuHome
Differential D13640

Geometry Nodes: Curve Primitive Arc
ClosedPublic
Actions

Authored by Charlie Jolly (charlie) on Dec 21 2021, 3:09 AM.

Details

Reviewers
Hans Goudey (HooglyBoogly)
Commits
rBcc1a48e39514: Geometry Nodes: Curve Primitive Arc
Summary

Geometry Nodes: Curve Primitive Arc

Points mode: Generates a three point curve arc from Start to End via Middle with Angle Offset and option to use shortest arc.

Radius mode: Generates a fixed radius arc on XY with controls for Angle and Sweep.

Radius and Points mode, updated with code from D13100.

Note: I didn't check before submitting and there was a previous arc node in D11713 and D13100 from @Johnny Matthews (guitargeek). Doh!

Test file:

gn_curve.blend1 MBDownload

Diff Detail

Repository
rB Blender
Branch
master
Build Status
Buildable 19551
Build 19551: arc lint + arc unit

Event Timeline

Charlie Jolly (charlie) requested review of this revision.Dec 21 2021, 3:09 AM
Charlie Jolly (charlie) created this revision.
Harbormaster completed remote builds in B19551: Diff 46252.Dec 21 2021, 3:09 AM
Charlie Jolly (charlie) edited the summary of this revision. (Show Details)Dec 21 2021, 6:47 PM
Charlie Jolly (charlie) added a project: Geometry Nodes.
Charlie Jolly (charlie) updated this revision to Diff 46283.EditedDec 21 2021, 6:48 PM

Added Radius mode, updated with code from D13100.

gn_curve.blend1 MBDownload

Harbormaster completed remote builds in B19570: Diff 46283.Dec 21 2021, 6:48 PM
Charlie Jolly (charlie) edited the summary of this revision. (Show Details)Dec 21 2021, 6:56 PM
Charlie Jolly (charlie) updated this revision to Diff 46284.Dec 21 2021, 7:15 PM

Add some const and remove debug variable.

Harbormaster completed remote builds in B19571: Diff 46284.Dec 21 2021, 7:15 PM
Charlie Jolly (charlie) updated this revision to Diff 46291.Dec 21 2021, 10:50 PM

Rebase master.

Harbormaster completed remote builds in B19578: Diff 46291.Dec 21 2021, 10:50 PM
Charlie Jolly (charlie) updated this revision to Diff 46338.Dec 23 2021, 1:22 PM

Rebase to master.

Remove use of spline->add_point.

Harbormaster completed remote builds in B19603: Diff 46338.Dec 23 2021, 1:22 PM
Charlie Jolly (charlie) edited the summary of this revision. (Show Details)Dec 23 2021, 1:23 PM
Charlie Jolly (charlie) added a reviewer: Hans Goudey (HooglyBoogly).
Johnny Matthews (guitargeek) mentioned this in D13100: Geometry Nodes: Add Arc Mode to Circle Node.Dec 23 2021, 6:16 PM
Charlie Jolly (charlie) updated this revision to Diff 46356.Dec 24 2021, 10:49 AM

Tidy up.

Harbormaster completed remote builds in B19613: Diff 46356.Dec 24 2021, 10:49 AM
Charlie Jolly (charlie) updated this revision to Diff 46358.Dec 24 2021, 1:14 PM

Always draw longest line when colinear.

Harbormaster completed remote builds in B19614: Diff 46358.Dec 24 2021, 1:14 PM
Louis (Louis_v_a) added a subscriber: Louis (Louis_v_a).Dec 26 2021, 5:27 PM
Charlie Jolly (charlie) updated this revision to Diff 46628.Jan 5 2022, 12:04 PM

Rebase to master

Harbormaster completed remote builds in B19770: Diff 46628.Jan 5 2022, 12:04 PM
Hans Goudey (HooglyBoogly) added a comment.Jan 10 2022, 8:04 PM

We discussed this in the latest geometry nodes sub-module meeting. Here are the notes from that:

  • "Invert Arc" might be a more intuitive and more consistent than "Shortest Arc", and that would even work in radius mode
  • "Radius" can be the default mode, since it's a bit easier to understand and much smaller

Also, there seems to be a special case with weird behavior here:

Charlie Jolly (charlie) updated this revision to Diff 47017.EditedJan 13 2022, 2:57 AM

Address comments.

  • Replace Shortest Arc with Invert Arc.
  • Set Radius mode as default.
  • Fix bug in points mode. Remove code based on slerp and do simple rotate around center point.
  • Set Normal to always be relative to Z axis.

Harbormaster completed remote builds in B19959: Diff 47017.Jan 13 2022, 2:57 AM
Hans Goudey (HooglyBoogly) accepted this revision.Jan 20 2022, 6:39 PM

Testing this, it looks good to me now, I like the options that are exposed.

I'm curious, do you think the circle primitive's create_point_circle_curve function could be easily defined in terms of this one now? It might be nice to do some cleanup after this if possible.

source/blender/nodes/geometry/nodes/node_geo_curve_primitive_arc.cc
76–84

Candidates for descriptions:

  • The center of the circle described by the three points
  • The normal direction of the plane described by the three points, pointing towards the positive Z axis
  • The radius of the circle described by the three points
192–206

I found this a little hard to follow at first, and my instinct was to separate it into a separate function, but I don't see a great way to make that work. Maybe it deserves a comment like "Find the two points that are furthest away from each other"

329–330

Use node_storage function, call the variable storage for consistency

This revision is now accepted and ready to land.Jan 20 2022, 6:39 PM
Charlie Jolly (charlie) marked 3 inline comments as done.Jan 20 2022, 7:36 PM
Charlie Jolly (charlie) updated this revision to Diff 47291.Jan 20 2022, 7:43 PM

Address comments.

Harbormaster completed remote builds in B20097: Diff 47291.Jan 20 2022, 7:43 PM
Charlie Jolly (charlie) added a comment.Jan 20 2022, 7:46 PM
In D13640#368585, @Hans Goudey (HooglyBoogly) wrote:

I'm curious, do you think the circle primitive's create_point_circle_curve function could be easily defined in terms of this one now? It might be nice to do some cleanup after this if possible.

Yes, I can take a look.

Closed by commit rBcc1a48e39514: Geometry Nodes: Curve Primitive Arc (authored by Charlie Jolly (charlie)). · Explain WhyJan 20 2022, 8:04 PM
This revision was automatically updated to reflect the committed changes.
Charlie Jolly (charlie) added a commit: rBcc1a48e39514: Geometry Nodes: Curve Primitive Arc.
Duarte Farrajota Ramos (duarteframos) awarded a token.Jan 20 2022, 9:18 PM

Revision Contents
Changeset List

PathSize
release/
scripts/
startup/
1 line
source/
blender/
blenkernel/
1 line
intern/
1 line
nodes/
1 line
1 line
geometry/
1 line
nodes/
152 lines

Diff 46252

View Options

release/scripts/startup/nodeitems_builtins.py

Loading...
View Options

source/blender/blenkernel/BKE_node.h

Loading...
View Options

source/blender/blenkernel/intern/node.cc

Loading...
View Options

source/blender/nodes/NOD_geometry.h

Loading...
View Options

source/blender/nodes/NOD_static_types.h

Loading...
View Options

source/blender/nodes/geometry/CMakeLists.txt

Loading...
View Options

source/blender/nodes/geometry/nodes/node_geo_curve_primitive_arc.cc

Loading...