Add automatic smoothing for CSG nodes#116749
Conversation
Jesusemora
force-pushed
the
testing-changes
branch
from
e96285e to
1951fb5
Compare
|
As a contributor to Godot CSG. I like the proposal. As you know there's a per |
I didn't know, and I don't see how that's relevant. CSG constructs an ArrayMesh, using a SurfaceTool would probably have a performance cost. It's also not very practical I think. For smoothing groups I would use 0 to disable rather than -1, because it's more intuitive. |
Jesusemora
force-pushed
the
testing-changes
branch
from
30a5a18 to
e6008c3
Compare
|
I mean the internals of the CSG implementation allow per vertex smooth toggles, but it's painful to expose this to the user layer. |
Calinou
left a comment
Calinou
left a comment
There was a problem hiding this comment.
Tested locally, it works as expected. Code looks good to me.
I think this is pretty good as it is from an UX perspective. I think 50.0 is a reasonable default value so that CSGCylinder3D is smoothed out of the box. We should have increased its default number of sides a while ago, but unfortunately, we missed this for 4.0.
Testing project: test_pr_116749.zip
|
Could you squash your commits? See our pull request guidelines for more information |
Jesusemora
force-pushed
the
testing-changes
branch
4 times, most recently
from
169e1e7 to
9d77894
Compare
added properties update inspector UX improvements and minor optimizations deg_to_rad moved methods move variables performance and cleanup Faster Compare the angles/smooth of faces instead of vertices, skipping for loop. Using a LocalVector instead of Vector, this simplifies the code while making it faster, and I don't think we need COW. all done and working Optimizations and cleanup Clarifying and expanding the description of autosmooth and smoothing_angle properties. Optimizations to skip steps in the loops: Split loops to be able to skip vertices better and replace division with multiplication. Suggestions for future changes. Skip loop if smoothing_angle is lower than 0.1, as a more performant alternative to default smoothing. Do not compare vertices that belong to the same triangle. Remove has_smooth. Use a check at the start of the loop instead. This allows us to skip faces. Perform normalize() at the end of the loop as we don't need the normal of the vertex afterwards, as we use the normal of the face for smoothing. This way we skip normalizing flat faces. If one vertex of a triangle is connected to the current vertex, the other 2 can never physically connect, so we skip them (break;). This isn't as reliable but it could add a slight performance increase. I calculate a decrease of at least 80% of calculations in the for loops compared to the previous code. Co-Authored-By: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Jesusemora
force-pushed
the
testing-changes
branch
from
6ca067c to
6114c6a
Compare
|
Thanks! |
|
CSG Autosmooth behavior is affected by non-uniform scaling #117190 |
5 participants
Closes godotengine/godot-proposals#1862
Prepares for godotengine/godot-proposals#14319 in order to implement smoothing groups, I need this and #115926 merged first, so that would be it's own PR.
Added automatic smoothing to all CSG nodes through CSGShape3D
It works on individual meshes and also on children of
CSGCombiner3D.CSGCombinerCSGBox,CSGCylinder,CSGTorus,CSGSphere,CSGPolygon3DandCSGMesh3D.autosmoothenables automatic smoothing. This overrides any smooth faces on the mesh, meaning it works on flat faced meshes like CSGBoxsmoothing_angle: when comparing two faces of a mesh, if the angle is higher than this, the vertices are smoothed, otherwise they remain sharp.Setting this to a value lower than 0.1 skips the smoothing, which could lead to a performance increase.