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

Crash with CSGPolygon3D in Spin mode #99888

Closed
badsectoracula opened this issue Nov 30, 2024 · 24 comments
Closed

Crash with CSGPolygon3D in Spin mode #99888

badsectoracula opened this issue Nov 30, 2024 · 24 comments

Comments

@badsectoracula
Copy link
Contributor

Tested versions

Reproducible in: 893bbdf
Not reproducible in: d09d82d

I bisected the crash and it seems to be introduced in fda444b from #94321.

System information

Godot v4.4.dev (893bbdf) - openSUSE Tumbleweed 20241022 on X11 - X11 display driver, Multi-window, 1 monitor - OpenGL 3 (Compatibility) - AMD Radeon RX 5700 XT (radeonsi, navi10, LLVM 18.1.8, DRM 3.59, 6.11.3-2-default) - AMD Ryzen 7 3700X 8-Core Processor (16 threads)

Issue description

A CSGPolygon3D in the scene with the mode set to Spin will crash the editor.

Steps to reproduce

  1. Create a new project
  2. Create a 3D scene
  3. Add a new node CSGPolygon3D to the scene
  4. Change the Mode property to Spin

The editor will crash at step 4.

Alternatively try to open the MRP.

Minimal reproduction project (MRP)

MRP: this-csg-crashes-the-editor.zip

@smix8
Copy link
Contributor

smix8 commented Dec 1, 2024

Crashes due to error in manifold::UnionFind.

Likely regression from #94321 @fire

@smix8 smix8 added the topic:3d label Dec 1, 2024
@fire
Copy link
Member

fire commented Dec 1, 2024

Investigating

@fire
Copy link
Member

fire commented Dec 1, 2024

Can you help me get a full stack trace for a manifold upstream report? @elalish @smix8

@fire
Copy link
Member

fire commented Dec 1, 2024

Image

[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!manifold::UnionFind<int,unsigned char>::find(int x) Line 146
	at /__w/world-godot/world-godot/godot/thirdparty/manifold/src/././././utils.h(146)
[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!manifold::UnionFind<int,unsigned char>::unionXY(int x, int y) Line 158
	at /__w/world-godot/world-godot/godot/thirdparty/manifold/src/././././utils.h(158)
godot.windows.editor.double.x86_64.llvm.exe!`anonymous namespace'::GetLabels(std::__1::vector<int,std::__1::allocator<int>> & components, const manifold::Vec<std::__1::pair<int,int>> & edges, int numNodes) Line 190
	at /__w/world-godot/world-godot/godot/thirdparty/manifold/src/impl.cpp(190)
[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!`anonymous namespace'::DedupePropVerts(manifold::Vec<linalg::vec<int,3>> & triProp, const manifold::Vec<std::__1::pair<int,int>> & vert2vert) Line 200
	at /__w/world-godot/world-godot/godot/thirdparty/manifold/src/impl.cpp(200)
godot.windows.editor.double.x86_64.llvm.exe!manifold::Manifold::Impl::CreateFaces() Line 358
	at /__w/world-godot/world-godot/godot/thirdparty/manifold/src/impl.cpp(358)
godot.windows.editor.double.x86_64.llvm.exe!manifold::Manifold::Impl::Impl<double,unsigned long long>(const manifold::MeshGLP<double,unsigned long long> & meshGL) Line 217
	at /__w/world-godot/world-godot/godot/thirdparty/manifold/src/././impl.h(217)
[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!std::__1::allocator<manifold::Manifold::Impl>::construct(manifold::Manifold::Impl * __p, const manifold::MeshGLP<double,unsigned long long> & __args) Line 165
	at /__w/world-godot/world-godot/mingw/x86_64-w64-mingw32/include/c++/v1/__memory/allocator.h(165)
[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!std::__1::allocator_traits<std::__1::allocator<manifold::Manifold::Impl>>::construct(std::__1::allocator<manifold::Manifold::Impl> & __p, manifold::Manifold::Impl * __args, const manifold::MeshGLP<double,unsigned long long> &) Line 319
	at /__w/world-godot/world-godot/mingw/x86_64-w64-mingw32/include/c++/v1/__memory/allocator_traits.h(319)
[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!std::__1::__shared_ptr_emplace<manifold::Manifold::Impl,std::__1::allocator<manifold::Manifold::Impl>>::__shared_ptr_emplace(std::__1::allocator<manifold::Manifold::Impl> __args, const manifold::MeshGLP<double,unsigned long long> &) Line 264
	at /__w/world-godot/world-godot/mingw/x86_64-w64-mingw32/include/c++/v1/__memory/shared_ptr.h(264)
[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!std::__1::allocate_shared(const std::__1::allocator<manifold::Manifold::Impl> & __args, const manifold::MeshGLP<double,unsigned long long> &) Line 843
	at /__w/world-godot/world-godot/mingw/x86_64-w64-mingw32/include/c++/v1/__memory/shared_ptr.h(843)
[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!std::__1::make_shared(const manifold::MeshGLP<double,unsigned long long> & __args) Line 851
	at /__w/world-godot/world-godot/mingw/x86_64-w64-mingw32/include/c++/v1/__memory/shared_ptr.h(851)
godot.windows.editor.double.x86_64.llvm.exe!manifold::Manifold::Manifold(const manifold::MeshGLP<double,unsigned long long> & meshGL64) Line 289
	at /__w/world-godot/world-godot/godot/thirdparty/manifold/src/manifold.cpp(289)
godot.windows.editor.double.x86_64.llvm.exe!_pack_manifold(const CSGBrush * const p_mesh_merge, manifold::Manifold & r_manifold, HashMap<int,Ref<Material>,HashMapHasherDefault,HashMapComparatorDefault<int>,DefaultTypedAllocator<HashMapElement<int,Ref<Material>>>> & p_mesh_materials, float p_snap) Line 295
	at /__w/world-godot/world-godot/godot/modules/csg/csg_shape.cpp(295)
godot.windows.editor.double.x86_64.llvm.exe!CSGShape3D::_get_brush() Line 333
	at /__w/world-godot/world-godot/godot/modules/csg/csg_shape.cpp(333)
godot.windows.editor.double.x86_64.llvm.exe!CSGShape3D::_update_shape() Line 444
	at /__w/world-godot/world-godot/godot/modules/csg/csg_shape.cpp(444)
godot.windows.editor.double.x86_64.llvm.exe!CallQueue::_call_function(const Callable & p_callable, const Variant * p_args, int p_argcount, bool p_show_error) Line 220
	at /__w/world-godot/world-godot/godot/core/object/message_queue.cpp(220)
godot.windows.editor.double.x86_64.llvm.exe!CallQueue::flush() Line 268
	at /__w/world-godot/world-godot/godot/core/object/message_queue.cpp(268)
godot.windows.editor.double.x86_64.llvm.exe!SceneTree::physics_process(double p_time) Line 548
	at /__w/world-godot/world-godot/godot/scene/main/scene_tree.cpp(548)
godot.windows.editor.double.x86_64.llvm.exe!Main::iteration() Line 4383
	at /__w/world-godot/world-godot/godot/main/main.cpp(4383)
[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!ProgressDialog::_update_ui() Line 133
	at /__w/world-godot/world-godot/godot/editor/progress_dialog.cpp(133)
godot.windows.editor.double.x86_64.llvm.exe!ProgressDialog::task_step(const String & p_task, const String & p_state, int p_step, bool p_force_redraw) Line 223
	at /__w/world-godot/world-godot/godot/editor/progress_dialog.cpp(223)
godot.windows.editor.double.x86_64.llvm.exe!EditorNode::progress_task_step(const String & p_task, const String & p_state, int p_step, bool p_force_refresh) Line 4917
	at /__w/world-godot/world-godot/godot/editor/editor_node.cpp(4917)
[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!EditorProgress::step(const String & p_state, int p_step, bool p_force_refresh) Line 184
	at /__w/world-godot/world-godot/godot/editor/editor_node.cpp(184)
godot.windows.editor.double.x86_64.llvm.exe!EditorNode::_load_editor_layout() Line 5290
	at /__w/world-godot/world-godot/godot/editor/editor_node.cpp(5290)
godot.windows.editor.double.x86_64.llvm.exe!EditorNode::_sources_changed(bool) Line 1139
	at /__w/world-godot/world-godot/godot/editor/editor_node.cpp(1139)
godot.windows.editor.double.x86_64.llvm.exe!Object::emit_signalp(const StringName & p_name, const Variant * * p_args, int p_argcount) Line 1200
	at /__w/world-godot/world-godot/godot/core/object/object.cpp(1200)
godot.windows.editor.double.x86_64.llvm.exe!Object::emit_signal<bool>(const StringName & p_name, bool p_args) Line 922
	at /__w/world-godot/world-godot/godot/./core/object/object.h(922)
godot.windows.editor.double.x86_64.llvm.exe!EditorFileSystem::_notification(int p_what) Line 1701
	at /__w/world-godot/world-godot/godot/editor/editor_file_system.cpp(1701)
godot.windows.editor.double.x86_64.llvm.exe!Object::notification(int p_notification, bool p_reversed) Line 878
	at /__w/world-godot/world-godot/godot/core/object/object.cpp(878)
godot.windows.editor.double.x86_64.llvm.exe!SceneTree::_process_group(SceneTree::ProcessGroup * p_group, bool p_physics) Line 1061
	at /__w/world-godot/world-godot/godot/scene/main/scene_tree.cpp(1061)
godot.windows.editor.double.x86_64.llvm.exe!SceneTree::_process(bool p_physics) Line 1127
	at /__w/world-godot/world-godot/godot/scene/main/scene_tree.cpp(1127)
godot.windows.editor.double.x86_64.llvm.exe!SceneTree::process(double p_time) Line 590
	at /__w/world-godot/world-godot/godot/scene/main/scene_tree.cpp(590)
godot.windows.editor.double.x86_64.llvm.exe!Main::iteration() Line 4427
	at /__w/world-godot/world-godot/godot/main/main.cpp(4427)
godot.windows.editor.double.x86_64.llvm.exe!OS_Windows::run() Line 1991
	at /__w/world-godot/world-godot/godot/platform/windows/os_windows.cpp(1991)
godot.windows.editor.double.x86_64.llvm.exe!widechar_main(int argc, wchar_t * * argv) Line 184
	at /__w/world-godot/world-godot/godot/platform/windows/godot_windows.cpp(184)
[Inline Frame] godot.windows.editor.double.x86_64.llvm.exe!_main() Line 206
	at /__w/world-godot/world-godot/godot/platform/windows/godot_windows.cpp(206)
godot.windows.editor.double.x86_64.llvm.exe!main(int argc, char * * argv) Line 225
	at /__w/world-godot/world-godot/godot/platform/windows/godot_windows.cpp(225)
[External Code]

https://github.com/V-Sekai/world-godot/tree/main/godot

@badsectoracula
Copy link
Contributor Author

Mine is almost the same:

[1] /lib64/libc.so.6(+0x41580) [0x7f9758441580] (??:0)
[2] manifold::UnionFind<int, unsigned char>::find(int) (/mnt/haus/badsector/Code/godot/thirdparty/manifold/src/././utils.h:147 (discriminator 2))
[3] manifold::UnionFind<int, unsigned char>::unionXY(int, int) (/mnt/haus/badsector/Code/godot/thirdparty/manifold/src/././utils.h:158 (discriminator 1))
[4] /mnt/haus/badsector/Code/godot/bin/godot.linuxbsd.editor.dev.x86_64() [0x72c233f] (/mnt/haus/badsector/Code/godot/thirdparty/manifold/src/impl.cpp:190)
[5] /mnt/haus/badsector/Code/godot/bin/godot.linuxbsd.editor.dev.x86_64() [0x72c23ce] (/mnt/haus/badsector/Code/godot/thirdparty/manifold/src/impl.cpp:200 (discriminator 1))
[6] manifold::Manifold::Impl::CreateFaces() (/mnt/haus/badsector/Code/godot/thirdparty/manifold/src/impl.cpp:362)
[7] manifold::Manifold::Impl::Impl<double, unsigned long>(manifold::MeshGLP<double, unsigned long> const&) (/mnt/haus/badsector/Code/godot/thirdparty/manifold/src/./impl.h:217)
[8] void std::_Construct<manifold::Manifold::Impl, manifold::MeshGLP<double, unsigned long> const&>(manifold::Manifold::Impl*, manifold::MeshGLP<double, unsigned long> const&) (/usr/include/c++/14/bits/stl_construct.h:120)
[9] std::_Sp_counted_ptr_inplace<manifold::Manifold::Impl, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_Sp_counted_ptr_inplace<manifold::MeshGLP<double, unsigned long> const&>(std::allocator<void>, manifold::MeshGLP<double, unsigned long> const&) (/usr/include/c++/14/bits/alloc_traits.h:694)
[10] std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<manifold::Manifold::Impl, std::allocator<void>, manifold::MeshGLP<double, unsigned long> const&>(manifold::Manifold::Impl*&, std::_Sp_alloc_shared_tag<std::allocator<void> >, manifold::MeshGLP<double, unsigned long> const&) (/usr/include/c++/14/bits/shared_ptr_base.h:969 (discriminator 3))
[11] std::__shared_ptr<manifold::Manifold::Impl, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<void>, manifold::MeshGLP<double, unsigned long> const&>(std::_Sp_alloc_shared_tag<std::allocator<void> >, manifold::MeshGLP<double, unsigned long> const&) (/usr/include/c++/14/bits/shared_ptr_base.h:1714)
[12] std::shared_ptr<manifold::Manifold::Impl>::shared_ptr<std::allocator<void>, manifold::MeshGLP<double, unsigned long> const&>(std::_Sp_alloc_shared_tag<std::allocator<void> >, manifold::MeshGLP<double, unsigned long> const&) (/usr/include/c++/14/bits/shared_ptr.h:464)
[13] std::shared_ptr<std::enable_if<!std::is_array<manifold::Manifold::Impl>::value, manifold::Manifold::Impl>::type> std::make_shared<manifold::Manifold::Impl, manifold::MeshGLP<double, unsigned long> const&>(manifold::MeshGLP<double, unsigned long> const&) (/usr/include/c++/14/bits/shared_ptr.h:1009)
[14] manifold::Manifold::Manifold(manifold::MeshGLP<double, unsigned long> const&) (/mnt/haus/badsector/Code/godot/thirdparty/manifold/src/manifold.cpp:289 (discriminator 1))
[15] /mnt/haus/badsector/Code/godot/bin/godot.linuxbsd.editor.dev.x86_64() [0x734156a] (/mnt/haus/badsector/Code/godot/modules/csg/csg_shape.cpp:296 (discriminator 1))
[16] CSGShape3D::_get_brush() (/mnt/haus/badsector/Code/godot/modules/csg/csg_shape.cpp:334)
[17] CSGShape3D::_update_shape() (/mnt/haus/badsector/Code/godot/modules/csg/csg_shape.cpp:444)
[18] void call_with_variant_args_helper<CSGShape3D>(CSGShape3D*, void (CSGShape3D::*)(), Variant const**, Callable::CallError&, IndexSequence<>) (/mnt/haus/badsector/Code/godot/./core/variant/binder_common.h:309)
[19] void call_with_variant_args<CSGShape3D>(CSGShape3D*, void (CSGShape3D::*)(), Variant const**, int, Callable::CallError&) (/mnt/haus/badsector/Code/godot/./core/variant/binder_common.h:419)
[20] CallableCustomMethodPointer<CSGShape3D, void>::call(Variant const**, int, Variant&, Callable::CallError&) const (/mnt/haus/badsector/Code/godot/./core/object/callable_method_pointer.h:111)
[21] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/mnt/haus/badsector/Code/godot/core/variant/callable.cpp:57)
[22] CallQueue::_call_function(Callable const&, Variant const*, int, bool) (/mnt/haus/badsector/Code/godot/core/object/message_queue.cpp:221)
[23] CallQueue::flush() (/mnt/haus/badsector/Code/godot/core/object/message_queue.cpp:270)
[24] SceneTree::physics_process(double) (/mnt/haus/badsector/Code/godot/scene/main/scene_tree.cpp:548)
[25] Main::iteration() (/mnt/haus/badsector/Code/godot/main/main.cpp:4383 (discriminator 3))
[26] OS_LinuxBSD::run() (/mnt/haus/badsector/Code/godot/platform/linuxbsd/os_linuxbsd.cpp:962 (discriminator 1))
[27] /mnt/haus/badsector/Code/godot/bin/godot.linuxbsd.editor.dev.x86_64(main+0x14b) [0x67859c1] (/mnt/haus/badsector/Code/godot/platform/linuxbsd/godot_linuxbsd.cpp:85)
[28] /lib64/libc.so.6(+0x2a2ae) [0x7f975842a2ae] (??:0)
[29] /lib64/libc.so.6(__libc_start_main+0x8b) [0x7f975842a379] (??:0)
[30] /mnt/haus/badsector/Code/godot/bin/godot.linuxbsd.editor.dev.x86_64(_start+0x25) [0x67857a5] (/home/abuild/rpmbuild/BUILD/glibc-2.40/csu/../sysdeps/x86_64/start.S:117)

@fire
Copy link
Member

fire commented Dec 2, 2024

Waiting for elalish/manifold#1040 to be completed, but the cause of the crash was identified as:

I'm replacing our CreateFaces algorithm. Our previous was based on connected components, but in the case of e.g. a high-res sphere, it would connect everything into one big face. So then we added an algorithm to check global tolerance of each face after the fact, and if that failed, it would just throw out all the faces and say each triangle was individual.

@elalish
Copy link

elalish commented Dec 2, 2024

To be clear, I have not at all identified (or even reproduced) a crash yet.

@smix8
Copy link
Contributor

smix8 commented Dec 2, 2024

Maybe unrelated but I also noticed crashes in master when dragging the finicky CSG editor handles e.g. CSGBox3D size handles in the editor.

Just spawn a single CSGBox3D node and try to drag the editor handles for its size adjustment.
If one axis lag-jumps to e.g. a zero size it crashes the editor with a manifold crash.

I dont know what data we are feeding to the poor manifold but it does not seem to like such non-geometry input so such input should likely be already skipped on the editor side.

@fire
Copy link
Member

fire commented Dec 2, 2024

I want to do it at least if the mesh is not manifold, avoid crashing, and return the empty mesh.

@fire
Copy link
Member

fire commented Dec 3, 2024

I dont know what data we are feeding to the poor manifold but it does not seem to like such non-geometry input so such input should likely be already skipped on the editor side.

Is there a easy to code way to verify non manifold without constructing a Manifold:manifold?

@elalish
Copy link

elalish commented Dec 3, 2024

Manifold should already be robust to this: if you try to construct a manifold with non-manifold input, it returns an empty manifold with the status set to the appropriate error condition. If you do further operations using that empty result, that error will propagate through those results as well. It certainly shouldn't crash. You can check the error status after each construction to find the input mesh that has a problem.

@fire
Copy link
Member

fire commented Dec 3, 2024

The current pull request code merely patches over the crash.

Image

We will need an investigation to see why this polygon cannot be spun.

@elalish
Copy link

elalish commented Dec 3, 2024

Does Spin mean "create an object of revolution"? If so, have you tried Manifold's Revolve function?

@fire
Copy link
Member

fire commented Dec 3, 2024

I'm starting to get some results with Manifold's Revolve. Image

#99977

@badsectoracula
Copy link
Contributor Author

Spinning is most likely a red herring as a crash inside manifold can happen by creating a CSGBox3D and dragging one of its handles to make it zero sized (e.g. i grabbed the topmost handle and dragged it down and the editor crashed).

I'll also check the PR to see if i can get it to crash.

@TCROC
Copy link
Contributor

TCROC commented Dec 4, 2024

It looks like this issue is occurring for quads and planes as well. Manifold errors and then not rendering them. I'll see if I can create a simple MRP.

@fire
Copy link
Member

fire commented Dec 4, 2024

Can you help test #99959 and let me know if it's relevant?

@TCROC
Copy link
Contributor

TCROC commented Dec 4, 2024

Yep. I'll test that now

@TCROC
Copy link
Contributor

TCROC commented Dec 4, 2024

@fire The error messages are definitely improved. Instead of generic "manifold error" I get exactly which mesh is the problem:

  CSGShape3D manifold creation from mesh failed at UsernameContainer/TeamColor: Not Manifold.
  CSGShape3D manifold creation from mesh failed at CSGMesh3D: Not Manifold.

And the CSGMesh3D is currently selected as a QuadMesh. Is there an issue tracking meshes not being rendered and these manifold errors? Is this the issue? Should I add an MRP? Where can I be of service to help next? :) Thanks for quickly tackling this error btw! ♥️

I tested and it just seems to error with Quad and Plane. Meshes with depth such as cubes and cylinders render just fine.

@fire
Copy link
Member

fire commented Dec 4, 2024

Can you post a small example for each of those failing meshes?

I want to see if we can do something to make the CSG code more manifold and not result in an empty mesh.

@TCROC
Copy link
Contributor

TCROC commented Dec 4, 2024

Yep I can do that! :) I'll open a separate issue with additional details such as my system info and such + link it to this issue. It sounds like its different enough from this one that it can be its own separate thing.

@TCROC
Copy link
Contributor

TCROC commented Dec 4, 2024

@fire I opened a new issue here: #100014

@fire
Copy link
Member

fire commented Dec 5, 2024

Does the crash happen in any case of the spin mode?

@fire
Copy link
Member

fire commented Dec 8, 2024

Image

Was fixed by #99959

@fire fire closed this as completed Dec 8, 2024
@fire fire mentioned this issue Dec 8, 2024
12 tasks
@akien-mga akien-mga moved this from Unassessed to Very Bad in 4.x Release Blockers Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Very Bad
Development

No branches or pull requests

6 participants