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

Fix multiple issues with CSG module. #36422

Merged
merged 1 commit into from
Feb 29, 2020
Merged

Conversation

madmiraal
Copy link
Contributor

Replaces BuildPoly with Build2DFaces, which creates faces as each pair of face intersections are processed, instead of trying to create them after all the intersections are processed. Ensures that faces are merged when possible, and removes degenerate triangles.

Treats the child as inside the parent when faces are coplanar.

Includes a general clean up of csg.h and csg.cpp.

Fixes #21125
Fixes #33117
Fixes #33121
Fixes #33665

modules/csg/csg.h Outdated Show resolved Hide resolved
@akien-mga akien-mga added this to the 4.0 milestone Feb 21, 2020
@reduz
Copy link
Member

reduz commented Feb 21, 2020

that is impressive

- Replaces BuildPoly with Build2DFaces, which creates faces as each
  pair of face intersections are processed, instead of trying to create
  them after all the intersections are processed. Ensures that faces are
  merged when possible, and removes degenerate triangles.

- Treats the child as inside the parent when faces are coplanar.

- General clean up of csg.h and csg.cpp.
@Calinou
Copy link
Member

Calinou commented Feb 21, 2020

Thanks a lot for working on this 🙂

Correctness is important to have, but is performance as good or better than the current implementation?

@madmiraal
Copy link
Contributor Author

@Calinou Do you have a benchmark we can test it against?

@Calinou
Copy link
Member

Calinou commented Feb 21, 2020

@madmiraal I made one: test_csg.zip

It may not be representative of a real-world use case, but it's a start. Try moving the CSGSphere node called "MoveMe" and see how fast the editor reacts (compare the current master branch and your PR).

@madmiraal
Copy link
Contributor Author

@Calinou I've tested it against your scene (and many of my own scenes during debugging) and the subjective experience is the same.

@madmiraal
Copy link
Contributor Author

In the absence of an existing benchmark, I've created the attached scene to push the CSG calculations beyond what they're designed for:
CSG Benchmark.zip

It consists of a CSGTorus, which I've increased to 16 sides and 16 ring sides and a script that creates 5 (but this is configurable) default CSGSpheres (12 segments and 6 rings) in subtraction mode, which are positioned evenly around the torus and configured to revolve along the torus every frame creating around 350,000 triangle intersections per frame.

On my laptop I get the following frame times:
Current CSG: 127ms ±3ms
Fixed CSG: 141ms ±3ms

So the fixed version is marginally slower, but I wouldn't say significantly so.

@Calinou
Copy link
Member

Calinou commented Feb 22, 2020

@madmiraal Sounds good to me 🙂

In the future, we could add a debouncing delay to CSG mesh rebuilds to make the editor more responsive.

@akien-mga akien-mga merged commit 8c3ec8c into godotengine:master Feb 29, 2020
@akien-mga
Copy link
Member

Thanks!

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