Details

Reviewers

Hans Goudey (HooglyBoogly)
Jacques Lucke (JacquesLucke)

Maniphest Tasks

T94714: Output signed angle from the edge angle node

Commits

rB4d5c08b9387c: Geometry Nodes: Add Signed Output to Edge Angle Node

Summary

This patch adds a second socket to output the signed angle from edge in addition to the unsigned angle.

When the mesh is concave the angle is -pi to 0, when it is convex, it is the angle from 0 to pi.

The second output was added as calculating a consistent reference angle was more computationally expensive than expected. So if unsigned works for a given task, it can still be used with an added speed benefit.

Diff Detail

Repository

rB Blender

Branch

new_edge (branched from master)

Build Status

Buildable 20010
Build 20010: arc lint + arc unit

Event Timeline

Johnny Matthews (guitargeek) requested review of this revision.Jan 11 2022, 3:59 AM

Johnny Matthews (guitargeek) created this revision.

Harbormaster completed remote builds in B19881: Diff 46845.Jan 11 2022, 3:59 AM

Johnny Matthews (guitargeek) edited the summary of this revision. (Show Details)Jan 11 2022, 4:05 AM

Multiple Outputs

Harbormaster completed remote builds in B19884: Diff 46848.Jan 11 2022, 6:40 AM

paul szajner (szap) added a subscriber: paul szajner (szap).Jan 11 2022, 2:08 PM

Johnny Matthews (guitargeek) added a task: T94714: Output signed angle from the edge angle node.Jan 11 2022, 3:39 PM

Johnny Matthews (guitargeek) retitled this revision from Geometry Nodes: Edge Angle Node - Return Signed Angle to Geometry Nodes: Edge Angle Node - Add Return Signed Angle Output.Jan 11 2022, 3:43 PM

Johnny Matthews (guitargeek) edited the summary of this revision. (Show Details)

Johnny Matthews (guitargeek) added a project: Geometry Nodes.

Johnny Matthews (guitargeek) edited the summary of this revision. (Show Details)Jan 11 2022, 3:46 PM

Johnny Matthews (guitargeek) added a reviewer: Jacques Lucke (JacquesLucke).Jan 14 2022, 2:59 AM

Merge branch 'master' into new_edge
Update to new math::normalize

Harbormaster completed remote builds in B19986: Diff 47058.Jan 14 2022, 4:03 AM

Johnny Matthews (guitargeek) edited the summary of this revision. (Show Details)Jan 14 2022, 4:08 PM

Better concavity checking logic from @Jacques Lucke (JacquesLucke)

Harbormaster completed remote builds in B20003: Diff 47082.Jan 14 2022, 4:46 PM

Johnny Matthews (guitargeek) edited the summary of this revision. (Show Details)Jan 14 2022, 5:42 PM

Diogo Valadares Reis dos Santos (DiogoX2) awarded a token.Jan 14 2022, 5:47 PM

Works well! This needs versioning to rename the Unsigned Angle output.

I did some performance testing: I found the unsigned output is ~~slower~~ faster than the signed output, though not by that much. (42 ms vs 47 ms in my test).

However, I think some things like using cached poly normals if they are available could help make this node faster.

source/blender/nodes/geometry/nodes/node_geo_input_mesh_edge_angle.cc
34	No need for the newline here, IMO
145–161	Share this edge map calculation as a static function
164	I'd suggest returning early here instead of indenting the rest of the lambda
174	`const`
174–175	Use `float3(...)` instead of the C-style casts
174–176	A bit arbitrary, but I'd stick with float math and use `* 0.5f`.
183–186	`const`

This revision now requires changes to proceed.Jan 14 2022, 10:53 PM

Updates from @Hans Goudey (HooglyBoogly)
Add versioning

Harbormaster completed remote builds in B20009: Diff 47091.Jan 14 2022, 11:30 PM

Hans Goudey (HooglyBoogly) accepted this revision.Jan 14 2022, 11:34 PM

Hans Goudey (HooglyBoogly) added 1 blocking reviewer(s): Jacques Lucke (JacquesLucke).

Hans Goudey (HooglyBoogly) added inline comments.

source/blender/blenloader/intern/versioning_300.c
2547	I think I'm going to add a node here too, so it's hopeless to list them all. Maybe this: `Rename sockets on multiple nodes.`

Comment wording

Harbormaster completed remote builds in B20010: Diff 47092.Jan 14 2022, 11:53 PM

Silas Opel (Schamph) awarded a token.Jan 17 2022, 10:58 AM

Henrik Dick (weasel) mentioned this in T94935: Edge Angle Node.Jan 17 2022, 2:20 PM

Hans Goudey (HooglyBoogly) retitled this revision from Geometry Nodes: Edge Angle Node - Add Return Signed Angle Output to Geometry Nodes: Edge Angle Node: Add Signed Angle Output.Jan 17 2022, 4:22 PM

Only got some small comments. Besides that, looks good.

source/blender/nodes/geometry/nodes/node_geo_input_mesh_edge_angle.cc
32	`Computing this value is faster [...]`
38	What does "fully concave" vs. "fully convex" even mean? Aren't they practically the same. Might be better to say "convex angles are positive".
48	Pass `Span` by value instead of by reference.