Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Smooth quads #793

Merged
merged 17 commits into from
Apr 25, 2024
Merged

Smooth quads #793

merged 17 commits into from
Apr 25, 2024

Conversation

elalish
Copy link
Owner

@elalish elalish commented Apr 24, 2024

Adding quad smoothing support, for proper cylinders, etc.

@elalish elalish self-assigned this Apr 24, 2024
Copy link
Owner Author

@elalish elalish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pca006132 The CI is reproducing what I see locally - these smooth/refine tests are now throwing Vec out of range, but when I run each test in isolation, they pass just fine. I tried address sanitizer, and it finds an integer overflow, but a check in that function finds nothing. Any ideas?

@@ -91,7 +91,7 @@ else()
else()
set(WARNING_FLAGS -Werror -Wall -Wno-sign-compare -Wno-unused)
endif()
set(MANIFOLD_FLAGS ${MANIFOLD_FLAGS} ${WARNING_FLAGS} -O3)
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this lets release and debug mode work by default - debug mode actually works great now that we're not on CUDA.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like something caused by -O3? Perhaps we got some undefined behavior... I should look into this later.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the undefined behavior is independent of the amount of optimization.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so removing this line does not make it work, just allows you to compile in debug mode?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly - and actually I just solved the problem. Turns out I had some extra unused verts that weren't being used, but were being processed and contained undefined stuff. Address sanitizer is great, but even though it doesn't slow down running a debug build that much, it makes the actual debugger unusably slow. Oh well, went back to print statements and figured it out.

@elalish
Copy link
Owner Author

elalish commented Apr 24, 2024

@hrgdavor, since you were interested, smoothing is working better now. From our Manifold.SmoothFlat test, we start with this low-res truncated cone:
image

We can use CalculateNormals to create vertex normals, designed to make everything smooth except for edges beyond a sharpness threshold, in this case only the lower circle. The renderer can make the reflections look smooth, sort of, but it doesn't match the geometry:
image

With refined geometry and flat facets (based either on vertex normals or the geometry directly - same results in this case):
image
Note the quads on the sides are refined as quads - the edge between the two triangles is ignored, allowing a symmetric solution. Note also that the polygonal face on top remains perfectly flat, and the sides adopt its tangents to interpolate smoothly. Also note that the lower edge is sharp (as requested) but still smoothly circular.

With refined geometry and interpolated normals:
image

Now the normals match the geometry better and the shape looks real.

@elalish
Copy link
Owner Author

elalish commented Apr 24, 2024

However, our interpolation is not perfectly circular (it still has slight ridges where the original mesh's edges were, though it is at least G1 continuous). It never will be perfect, but I may try to improve it a bit still:

Screen.Recording.2024-04-24.at.2.26.45.PM.mov

@elalish
Copy link
Owner Author

elalish commented Apr 24, 2024

Cylinders are perfect at least.

@hrgdavor
Copy link

@elalish thanks for sharing the visuals. I really admire you work on manifold. I have finally started to play a bit with manifold, so looking forward to trying out these things after I spend some time with basics.

@elalish elalish merged commit c738f1e into master Apr 25, 2024
16 checks passed
@elalish elalish deleted the smoothQuads branch April 25, 2024 15:23
@elalish elalish mentioned this pull request Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants