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

Fixes to CSG robustness #74771

Merged
merged 1 commit into from
Apr 27, 2023
Merged

Fixes to CSG robustness #74771

merged 1 commit into from
Apr 27, 2023

Conversation

fire
Copy link
Member

@fire fire commented Mar 11, 2023

…iangles

Collaboration with OldBelge

Previously, the code assumed that if the intersection line hit the common edge of 2 triangles, it meant that the intersection entered or exited a shape. However, there is also a case where the intersection just "skirts" the other shape on the outside. To handle this case, we added a check for the direction of the normals of the faces. If the inner product of the normals and the intersection is greater than 0, then the intersection is inside the shape.

Fixes: #58637

Still happens: #41140

@fire

This comment was marked as outdated.

@fire fire force-pushed the csg-fixes branch 4 times, most recently from 1ca83d0 to db156fc Compare March 11, 2023 14:20
@fire fire added this to the 4.1 milestone Mar 11, 2023
@fire fire force-pushed the csg-fixes branch 2 times, most recently from aa46ddc to 54cf963 Compare March 11, 2023 14:39
@fire

This comment was marked as outdated.

@fire
Copy link
Member Author

fire commented Mar 11, 2023

Before:

image

After:

image

@fire

This comment was marked as resolved.

@fire

This comment was marked as outdated.

@fire fire marked this pull request as ready for review March 11, 2023 16:14
@fire fire requested a review from a team as a code owner March 11, 2023 16:14
@fire
Copy link
Member Author

fire commented Mar 11, 2023

Before:

image

After:

image

@fire

This comment was marked as outdated.

@fire fire force-pushed the csg-fixes branch 2 times, most recently from 67bb3af to 04baa3c Compare March 11, 2023 23:01
@fire
Copy link
Member Author

fire commented Mar 11, 2023

Latest test cases CSGBug_2023-03-11.zip

@fire fire force-pushed the csg-fixes branch 2 times, most recently from 21d9106 to 6c3af47 Compare March 11, 2023 23:34
@fire fire marked this pull request as draft March 11, 2023 23:39
@fire fire force-pushed the csg-fixes branch 4 times, most recently from 04baa3c to 4c3f7a8 Compare March 12, 2023 00:31
@fire fire changed the title Handle CSG edge case where intersection line hits common edge of 2 tr… Fixes to CSG robustness Mar 12, 2023
@fire fire force-pushed the csg-fixes branch 9 times, most recently from e1930b2 to 826fa4b Compare March 13, 2023 14:16
@fire fire marked this pull request as ready for review March 14, 2023 00:23
@akien-mga
Copy link
Member

I started testing this PR, as I have a project using CSG which suffers from glitches. This PR seems to fix a lot of the problems!

It's a fairly pathological case, we stitched 40 Path3Ds together, generating CSGPolygon3D from each Path3D and using them to subtract from a big CSGBox3D.

Here's how it looks like in 4.0.2-stable:
image
(same in 4.1 master as of 6bf94cf)
The glitches can move around, forcing a regeneration of the CSGCombiner can typically make things better or worse, so it's clearly a problem of precision issues.

And here's how it looks like with this PR (rebased on latest master):
image

So... it's definitely better :D

I'll soon make that project open source so it can be used by others for testing too. It's this game: https://akien.itch.io/godog

The above screenshots is from a level we didn't use in the end due to those glitches. But even the playable build on itch has CSG glitches (weird hard lines going through the level where it's supposed to be subtracting geometry), and I confirmed that this PR fixes them.

modules/csg/csg.cpp Outdated Show resolved Hide resolved
modules/csg/csg.h Outdated Show resolved Hide resolved
modules/csg/csg_shape.cpp Outdated Show resolved Hide resolved
modules/csg/csg_shape.cpp Outdated Show resolved Hide resolved
@YuriSizov
Copy link
Contributor

Does this also fix #73927? (Which may be a duplicate of the linked issue.)

… 2 triangles.

The previous implementation assumed that the intersection entered or exited a
shape when it hit right on the common edge of 2 triangles. However, there is
also a case where it just "skirts" the other shape on the outside.

To fix this, we added code to check the intersection distance and if the
normals of the faces are pointed in the same direction as the intersection or
not (e.g. inner product > 0). This handles the case where the intersection
line hits the common edge of 2 triangles and skirts the other shape on the
outside.

Extended code to cover a third case.

Fixes godotengine#58637.

Co-authored-by: OldBelge <StevenGeens@users.noreply.github.com>
@akien-mga
Copy link
Member

I pushed some changes to avoid breaking compatibility. It's still possible to reduce the snap value down to 1e-6 m, i.e. 1 µm, which should help solve situations with missing triangles on small CSG meshes as seen by @goatchurchprime in #41140.

It's not perfect, but hopefully that makes things more usable.

@akien-mga
Copy link
Member

akien-mga commented Apr 27, 2023

Tested some more projects from previously reported issues / duplicates.

This PR does fix the MRPs from those issues:

But doesn't fix the MRPs from those issues:

So it doesn't solve everything, we should probably reopen one of the issues closed as duplicate of #58637, if we're going to close that one. Or open a new one after this is merged, linking the old and new MRPs which still have issues.

@akien-mga akien-mga merged commit 190f158 into godotengine:master Apr 27, 2023
@akien-mga
Copy link
Member

Thanks!

@justinwash
Copy link
Contributor

Does this also fix #73927? (Which may be a duplicate of the linked issue.)

Testing with the repro project here from #74343 it does not appear so. I have a branch on my fork that fixes this issue by keeping track of which edges have already been processed by the loop. Will post over on issue #73927 to continue the conversation there.

@akien-mga
Copy link
Member

Cherry-picked for 4.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Holes in CSG meshes with coplanar faces and engine freeze when moving them
4 participants