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

Holes in CSG meshes with coplanar faces and engine freeze when moving them #58637

Open
codecat opened this issue Feb 28, 2022 · 7 comments · Fixed by #74771
Open

Holes in CSG meshes with coplanar faces and engine freeze when moving them #58637

codecat opened this issue Feb 28, 2022 · 7 comments · Fixed by #74771

Comments

@codecat
Copy link
Contributor

codecat commented Feb 28, 2022

Godot version

4.0.alpha3

System information

Windows 11

Issue description

I am generating a number of CSGMesh3D objects and putting them in a CSGCombiner3D. All meshes are set to be a union. Sometimes, this generates random holes:

image

And, potentially related, when moving the mesh with the hole on the Z axis, the engine freezes.

Steps to reproduce

  1. Open csg_bug.tscn in the editor
  2. Look at the red CSG mesh
  3. Optionally, try to move the red CSG mesh on the Z axis

Minimal reproduction project

CSGBug.zip

@Calinou
Copy link
Member

Calinou commented Feb 28, 2022

Coplanar faces and CSG don't mix well. I suggest moving around your CSG nodes slightly (0.001 units will do) to avoid having coplanar faces. This is something that could be handled automatically internally, but I don't know how much complexity it would add to be done properly.

Accurate CSG algorithms (that handle coplanar faces correctly) are much slower, so I don't know if it's a viable tradeoff given many users are using those nodes at run-time.

@codecat
Copy link
Contributor Author

codecat commented Feb 28, 2022

Thanks! Moving around the nodes seems like a bit of a hack, and kinda breaks the use-case I need CSG for.

Accurate CSG algorithms (that handle coplanar faces correctly) are much slower

Kinda off-topic, but do you happen to know of any such algorithms that I can drop into C or C++?

@Calinou
Copy link
Member

Calinou commented Feb 28, 2022

Kinda off-topic, but do you happen to know of any such algorithms that I can drop into C or C++?

I don't have any resources for this (as most CSG libraries or implementations are GPL-licensed, such as Blender's or CGAL). reduz had to write a custom CSG implementation for Godot to get something permissively licensed…

@hoontee
Copy link
Contributor

hoontee commented Mar 9, 2022

Related to / duplicate of #41140.

@Calinou Calinou changed the title Holes in CSG meshes and engine freeze when moving them Holes in CSG meshes with coplanar faces and engine freeze when moving them Dec 6, 2022
@golddotasksquestions
Copy link

@Calinou

Accurate CSG algorithms (that handle coplanar faces correctly) are much slower, so I don't know if it's a viable tradeoff given many users are using those nodes at run-time.

There is no point in using CSG or CSG operations if the operation results in a mesh with holes. Not only does it look broken, it also results in bad prototyping functionality because to resulting collision shape is broken too.

I have had CSG blockout prototype levels were I failed to arrange the shapes in such a way there were no holes in the scene, despite knowing about he workaround fix of avoiding coplanar faces. In certain situations there is just always going to be coplanar faces. Also sometimes it is just not feasible to avoid them because non coplanar faces would create collision issues during gameplay.

I would at the very least offer this slower CSG algorithm as an option in the Project Settings. If is reduces this issue drastically it will be definitely worth the time it takes to compute the new mesh. Recalculating CSG shapes in game during runtime is strongly discouraged anyway already (because of performance) and does not really make much sense in most cases unless the user is building an editor, not a game. And if the user is creating an editor, the slower CSG solver will be no or much less of an issue, just like it is almost no issue in the Godot editor right now.

@fire
Copy link
Member

fire commented Mar 11, 2023

I have tested #74771 fixes this bug.

@Calinou Calinou mentioned this issue Apr 10, 2023
@akien-mga akien-mga added this to the 4.1 milestone Apr 27, 2023
@capnm
Copy link
Contributor

capnm commented Apr 27, 2023

eaa84bc fixes only the visual appearance.

  1. Translating some CSG Mesh on the Z axis still causes a deadlock inside the calculation of the shapes.
    (As already assumed in the PR, 9f12e7b doesn't fixes this case.)
* thread #1, name = 'godot.linuxbsd.', stop reason = step over
  * frame #0: 0x0000555556c1ebc4 godot.linuxbsd.editor.dev.x86_64`CowData<CSGBrushOperation::Build2DFaces::Face2D>::insert(this=0x000055556f642f70, p_pos=45, p_val=0x00007fffffffcd80) at cowdata.h:178:11
    frame #1: 0x0000555556c1acb6 godot.linuxbsd.editor.dev.x86_64`CSGBrushOperation::Build2DFaces::_find_edge_intersections(Vector2 const*, Vector<int>&) [inlined] Vector<CSGBrushOperation::Build2DFaces::Face2D>::insert(this=0x000055556f642f68, p_pos=45, p_val=Face2D @ 0x00007fffffffcd80) at vector.h:94:53
    frame #2: 0x0000555556c1acab godot.linuxbsd.editor.dev.x86_64`CSGBrushOperation::Build2DFaces::_find_edge_intersections(this=0x000055556f642f58, p_segment_points=0x00007fffffffceb0, r_segment_indices=0x00007fffffffcea0) at csg.cpp:1149:11
    frame #3: 0x0000555556c16a28 godot.linuxbsd.editor.dev.x86_64`CSGBrushOperation::Build2DFaces::insert(this=0x000055556f642f58, p_brush=0x0000555576e41750, p_face_idx=<unavailable>) at csg.cpp:1348:4
    frame #4: 0x0000555556c15190 godot.linuxbsd.editor.dev.x86_64`CSGBrushOperation::update_faces(this=<unavailable>, p_brush_a=0x0000555576e41750, p_face_idx_a=<unavailable>, p_brush_b=0x0000555576acf3a0, p_face_idx_b=4, p_collection=0x00007fffffffd1b0, p_vertex_snap=0) at csg.cpp:1529:43
    frame #5: 0x0000555556c1183f godot.linuxbsd.editor.dev.x86_64`CSGBrushOperation::merge_brushes(this=<unavailable>, p_operation=OPERATION_UNION, p_brush_a=0x0000555576e41750, p_brush_b=0x0000555576acf3a0, r_merged_brush=0x00005555704b1cf0, p_vertex_snap=0) at csg.cpp:289:5
    frame #6: 0x0000555556be667d godot.linuxbsd.editor.dev.x86_64`CSGShape3D::_get_brush(this=0x0000555575c88820) at csg_shape.cpp:0:5
    frame #7: 0x0000555556be6ca2 godot.linuxbsd.editor.dev.x86_64`CSGShape3D::_update_shape(this=0x0000555575c88820) at csg_shape.cpp:302:16
...

image

@akien-mga akien-mga reopened this Apr 27, 2023
@Calinou Calinou mentioned this issue Apr 27, 2023
JeffVenancius pushed a commit to JeffVenancius/godot that referenced this issue Apr 27, 2023
… 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>
justinwash pushed a commit to justinwash/godot that referenced this issue Apr 27, 2023
… 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>
fire added a commit to V-Sekai/godot that referenced this issue May 8, 2023
… 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 pushed a commit to akien-mga/godot that referenced this issue May 12, 2023
… 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>
(cherry picked from commit eaa84bc)
hakro pushed a commit to hakro/godot that referenced this issue May 21, 2023
… 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>
haunted-loaf pushed a commit to haunted-loaf/godot that referenced this issue Jun 19, 2023
… 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>
(cherry picked from commit eaa84bc)
@YuriSizov YuriSizov modified the milestones: 4.1, 4.2 Jun 23, 2023
TheSecondReal0 pushed a commit to TheSecondReal0/godot that referenced this issue Jul 20, 2023
… 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>
@YuriSizov YuriSizov modified the milestones: 4.2, 4.x Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants