Print better manifold errors and avoid crash on non manifold csg input.

[fire]

Attempt to avoid crashing on #99888.

Needs also #99951 (has been integrated)

It does not resolve CSG (mesh boolean) spinning a non-manifold polygon, causing invalid input to elvish/manifold., it only turns an editor crash into an error.

Outdated photo

[Mickeon]

As I mentioned above, the corresponding setter and getter of snap should be surrounded with #ifndef DISABLE_DEPRECATED as well. Both their definitions, and their binding to the API.

[fire]

Is the #ifndef DISABLE_DEPRECATED condition correct?

[Mickeon]

Seems about right to me.

Edit, nope. By bindings to the API I meant these

ClassDB::bind_method(D_METHOD("set_snap", "snap"), &CSGShape3D::set_snap);
ClassDB::bind_method(D_METHOD("get_snap"), &CSGShape3D::get_snap);

Nope hold on.

[badsectoracula]

I tried this with the MRP i linked in the bug report and it didn't crash the editor (but it also showed an empty scene). Perhaps a message should be shown about the failures to the user because they could open some large map using CSGs here and there together with other stuff and wont notice the missing geometry.

I also tested resizing a new CSGBox3D to 0 size like i mentioned in the comment in the bug report. This PR doesn't crash the editor when resizing with the handles but it still crashes the editor with the following callstack (which looks to be similar like the spin crash) when i manually type 0 in the property editor for Y scale:

#0  0x00000000072c8e91 in (anonymous namespace)::DedupePropVerts (triProp=..., vert2vert=...) at thirdparty/manifold/src/impl.cpp:205
#1  0x00000000072c9f23 in manifold::Manifold::Impl::CreateFaces (this=0x21124590) at thirdparty/manifold/src/impl.cpp:358
#2  0x00000000072a0204 in manifold::Manifold::Impl::Impl<double, unsigned long> (this=0x21124590, meshGL=...)
    at thirdparty/manifold/src/./impl.h:215
#3  0x000000000729d1ba in std::_Construct<manifold::Manifold::Impl, manifold::MeshGLP<double, unsigned long> const&> (__p=0x21124590)
    at /usr/include/c++/14/bits/stl_construct.h:119
#4  0x000000000729af34 in std::allocator_traits<std::allocator<void> >::construct<manifold::Manifold::Impl, manifold::MeshGLP<double, unsigned long> const&> (__p=0x21124590) at /usr/include/c++/14/bits/alloc_traits.h:694
#5  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&> (this=0x21124580, __a=...) at /usr/include/c++/14/bits/shared_ptr_base.h:607
#6  0x0000000007299061 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count<manifold::Manifold::Impl, std::allocator<void>, manifold::MeshGLP<double, unsigned long> const&> (this=0x7fffffffc3d8, __p=@0x7fffffffc3d0: 0x0, __a=...)
    at /usr/include/c++/14/bits/shared_ptr_base.h:969
#7  0x00000000072966e0 in std::__shared_ptr<manifold::Manifold::Impl, (__gnu_cxx::_Lock_policy)2>::__shared_ptr<std::allocator<void>, manifold::MeshGLP<double, unsigned long> const&> (this=0x7fffffffc3d0, __tag=...) at /usr/include/c++/14/bits/shared_ptr_base.h:1713
#8  0x0000000007294689 in std::shared_ptr<manifold::Manifold::Impl>::shared_ptr<std::allocator<void>, manifold::MeshGLP<double, unsigned long> const&> (this=0x7fffffffc3d0, __tag=...) at /usr/include/c++/14/bits/shared_ptr.h:463
#9  0x00000000072921d5 in std::make_shared<manifold::Manifold::Impl, manifold::MeshGLP<double, unsigned long> const&> ()
    at /usr/include/c++/14/bits/shared_ptr.h:1008
#10 0x00000000072dd0e2 in manifold::Manifold::Manifold (this=0x7fffffffc5d0, meshGL64=...) at thirdparty/manifold/src/manifold.cpp:289
#11 0x0000000007348032 in _pack_manifold (p_mesh_merge=0x25788e40, r_manifold=..., p_mesh_materials=..., p_snap=0.00100000005, 
    p_csg_shape=0x291a5610) at modules/csg/csg_shape.cpp:328
#12 0x000000000734832a in CSGShape3D::_get_brush (this=0x291a5610) at modules/csg/csg_shape.cpp:371
#13 0x0000000007348bca in CSGShape3D::_update_shape (this=0x291a5610) at modules/csg/csg_shape.cpp:482
#14 0x00000000073811af in call_with_variant_args_helper<CSGShape3D>(CSGShape3D*, void (CSGShape3D::*)(), Variant const**, Callable::CallError&, IndexSequence<>) (p_instance=0x291a5610, p_method=(void (CSGShape3D::*)(CSGShape3D * const)) 0x7348b52 <CSGShape3D::_update_shape()>, 
    p_args=0x0, r_error=...) at ./core/variant/binder_common.h:304
#15 0x000000000737d745 in call_with_variant_args<CSGShape3D> (p_instance=0x291a5610, 
    p_method=(void (CSGShape3D::*)(CSGShape3D * const)) 0x7348b52 <CSGShape3D::_update_shape()>, p_args=0x0, p_argcount=0, r_error=...)
    at ./core/variant/binder_common.h:418
#16 0x0000000007379c88 in CallableCustomMethodPointer<CSGShape3D, void>::call (this=0x29544c80, p_arguments=0x0, p_argcount=0, 
    r_return_value=..., r_call_error=...) at ./core/object/callable_method_pointer.h:107
#17 0x000000000b8ae425 in Callable::callp (this=0xd9a06d0, p_arguments=0x0, p_argcount=0, r_return_value=..., r_call_error=...)
    at core/variant/callable.cpp:57
#18 0x000000000bc02303 in CallQueue::_call_function (this=0xc81f1d0, p_callable=..., p_args=0xd9a06e8, p_argcount=0, p_show_error=true)
    at core/object/message_queue.cpp:220
#19 0x000000000bc02698 in CallQueue::flush (this=0xc81f1d0) at core/object/message_queue.cpp:268
#20 0x00000000098075ce in SceneTree::physics_process (this=0xe5b3da0, p_time=0.016666666666666666) at scene/main/scene_tree.cpp:546
#21 0x0000000006854d09 in Main::iteration () at main/main.cpp:4383
#22 0x0000000006791bfe in OS_LinuxBSD::run (this=0x7fffffffcfb0) at platform/linuxbsd/os_linuxbsd.cpp:962
#23 0x000000000678a441 in main (argc=3, argv=0x7fffffffd608) at platform/linuxbsd/godot_linuxbsd.cpp:85

[fire]

Perhaps a message should be shown about the failures to the user because they could open some large map using CSGs here and there together with other stuff and wont notice the missing geometry.

I print a message about the invalid CSGNode and its node path.

[elalish]

Thank you - I can repro the crash with that MeshGL. Fix incoming.

[fire]

{"faceID":[],"halfedgeTangent":[],"mergeFromVert":[],"mergeToVert":[],"numProp":7,"runIndex":[0,144],"runOriginalID":[9],"runTransform":[],"tolerance":0.0,"triVerts":[0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125,126,127,128,129,130,131,132,133,134,135,136,137,138,139,140,141,142,143],"vertProperties":[0.0,1.0,0.0,0.0,0.0,0.0,0.0,0.716999232769012,1.01145005226135,-0.71699923276901,0.0,0.0,0.125,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.125,0.0,0.716999232769012,1.01145005226135,-0.71699923276901,0.0,0.0,0.125,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.0,0.0,1.01399004459381,1.01145005226135,0.0,0.0,0.0,0.0,0.16666667163372,1.01399004459381,1.01145005226135,0.0,0.0,0.0,0.0,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.125,0.333333343267441,0.716999232769012,1.01145005226135,-0.71699923276901,0.0,0.0,0.125,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.125,0.333333343267441,1.01399004459381,1.01145005226135,0.0,0.0,0.0,0.0,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.0,0.333333343267441,0.0,0.0,0.0,0.0,0.0,0.0,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.125,0.5,0.0,0.0,0.0,0.0,0.0,0.125,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.125,0.5,0.0,0.0,0.0,0.0,0.0,0.0,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.0,0.5,0.0,1.0,0.0,0.0,0.0,0.125,0.0,0.0,1.01145005226135,-1.01398992538452,0.0,0.0,0.25,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.25,0.0,0.0,1.01145005226135,-1.01398992538452,0.0,0.0,0.25,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.125,0.0,0.716999232769012,1.01145005226135,-0.71699923276901,0.0,0.0,0.125,0.16666667163372,0.716999232769012,1.01145005226135,-0.71699923276901,0.0,0.0,0.125,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.25,0.333333343267441,0.0,1.01145005226135,-1.01398992538452,0.0,0.0,0.25,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.25,0.333333343267441,0.716999232769012,1.01145005226135,-0.71699923276901,0.0,0.0,0.125,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.125,0.333333343267441,0.0,0.0,0.0,0.0,0.0,0.125,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.25,0.5,0.0,0.0,0.0,0.0,0.0,0.25,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.25,0.5,0.0,0.0,0.0,0.0,0.0,0.125,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.125,0.5,0.0,1.0,0.0,0.0,0.0,0.25,0.0,-0.71699917316437,1.01145005226135,-0.71699917316437,0.0,0.0,0.375,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.375,0.0,-0.71699917316437,1.01145005226135,-0.71699917316437,0.0,0.0,0.375,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.25,0.0,0.0,1.01145005226135,-1.01398992538452,0.0,0.0,0.25,0.16666667163372,0.0,1.01145005226135,-1.01398992538452,0.0,0.0,0.25,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.375,0.333333343267441,-0.71699917316437,1.01145005226135,-0.71699917316437,0.0,0.0,0.375,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.375,0.333333343267441,0.0,1.01145005226135,-1.01398992538452,0.0,0.0,0.25,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.25,0.333333343267441,0.0,0.0,0.0,0.0,0.0,0.25,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.375,0.5,0.0,0.0,0.0,0.0,0.0,0.375,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.375,0.5,0.0,0.0,0.0,0.0,0.0,0.25,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.25,0.5,0.0,1.0,0.0,0.0,0.0,0.375,0.0,-1.01398992538452,1.01145005226135,0.0,0.0,0.0,0.5,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.5,0.0,-1.01398992538452,1.01145005226135,0.0,0.0,0.0,0.5,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.375,0.0,-0.71699917316437,1.01145005226135,-0.71699917316437,0.0,0.0,0.375,0.16666667163372,-0.71699917316437,1.01145005226135,-0.71699917316437,0.0,0.0,0.375,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.5,0.333333343267441,-1.01398992538452,1.01145005226135,0.0,0.0,0.0,0.5,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.5,0.333333343267441,-0.71699917316437,1.01145005226135,-0.71699917316437,0.0,0.0,0.375,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.375,0.333333343267441,0.0,0.0,0.0,0.0,0.0,0.375,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.5,0.5,0.0,0.0,0.0,0.0,0.0,0.5,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.5,0.5,0.0,0.0,0.0,0.0,0.0,0.375,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.375,0.5,0.0,1.0,0.0,0.0,0.0,0.5,0.0,-0.71699917316437,1.01145005226135,0.716999173164368,0.0,0.0,0.625,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.625,0.0,-0.71699917316437,1.01145005226135,0.716999173164368,0.0,0.0,0.625,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.5,0.0,-1.01398992538452,1.01145005226135,0.0,0.0,0.0,0.5,0.16666667163372,-1.01398992538452,1.01145005226135,0.0,0.0,0.0,0.5,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.625,0.333333343267441,-0.71699917316437,1.01145005226135,0.716999173164368,0.0,0.0,0.625,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.625,0.333333343267441,-1.01398992538452,1.01145005226135,0.0,0.0,0.0,0.5,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.5,0.333333343267441,0.0,0.0,0.0,0.0,0.0,0.5,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.625,0.5,0.0,0.0,0.0,0.0,0.0,0.625,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.625,0.5,0.0,0.0,0.0,0.0,0.0,0.5,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.5,0.5,0.0,1.0,0.0,0.0,0.0,0.625,0.0,0.0,1.01145005226135,1.01398992538452,0.0,0.0,0.75,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.75,0.0,0.0,1.01145005226135,1.01398992538452,0.0,0.0,0.75,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.625,0.0,-0.71699917316437,1.01145005226135,0.716999173164368,0.0,0.0,0.625,0.16666667163372,-0.71699917316437,1.01145005226135,0.716999173164368,0.0,0.0,0.625,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.75,0.333333343267441,0.0,1.01145005226135,1.01398992538452,0.0,0.0,0.75,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.75,0.333333343267441,-0.71699917316437,1.01145005226135,0.716999173164368,0.0,0.0,0.625,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.625,0.333333343267441,0.0,0.0,0.0,0.0,0.0,0.625,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.75,0.5,0.0,0.0,0.0,0.0,0.0,0.75,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.75,0.5,0.0,0.0,0.0,0.0,0.0,0.625,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.625,0.5,0.0,1.0,0.0,0.0,0.0,0.75,0.0,0.716999173164368,1.01145005226135,0.716999173164368,0.0,0.0,0.875,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.875,0.0,0.716999173164368,1.01145005226135,0.716999173164368,0.0,0.0,0.875,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.75,0.0,0.0,1.01145005226135,1.01398992538452,0.0,0.0,0.75,0.16666667163372,0.0,1.01145005226135,1.01398992538452,0.0,0.0,0.75,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.875,0.333333343267441,0.716999173164368,1.01145005226135,0.716999173164368,0.0,0.0,0.875,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.875,0.333333343267441,0.0,1.01145005226135,1.01398992538452,0.0,0.0,0.75,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.75,0.333333343267441,0.0,0.0,0.0,0.0,0.0,0.75,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.875,0.5,0.0,0.0,0.0,0.0,0.0,0.875,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.875,0.5,0.0,0.0,0.0,0.0,0.0,0.75,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.75,0.5,0.0,1.0,0.0,0.0,0.0,0.875,0.0,1.01398992538452,1.01145005226135,0.0,0.0,0.0,1.0,0.16666667163372,0.0,1.0,0.0,0.0,0.0,1.0,0.0,1.01398992538452,1.01145005226135,0.0,0.0,0.0,1.0,0.16666667163372,0.0,1.0,0.0,0.0,0.0,0.875,0.0,0.716999173164368,1.01145005226135,0.716999173164368,0.0,0.0,0.875,0.16666667163372,0.716999173164368,1.01145005226135,0.716999173164368,0.0,0.0,0.875,0.16666667163372,0.0,0.0,0.0,0.0,0.0,1.0,0.333333343267441,1.01398992538452,1.01145005226135,0.0,0.0,0.0,1.0,0.16666667163372,0.0,0.0,0.0,0.0,0.0,1.0,0.333333343267441,0.716999173164368,1.01145005226135,0.716999173164368,0.0,0.0,0.875,0.16666667163372,0.0,0.0,0.0,0.0,0.0,0.875,0.333333343267441,0.0,0.0,0.0,0.0,0.0,0.875,0.333333343267441,0.0,1.0,0.0,0.0,0.0,1.0,0.5,0.0,0.0,0.0,0.0,0.0,1.0,0.333333343267441,0.0,1.0,0.0,0.0,0.0,1.0,0.5,0.0,0.0,0.0,0.0,0.0,0.875,0.333333343267441,0.0,1.0,0.0,0.0,0.0,0.875,0.5]}

Child CSGShape3D manifold creation from mesh failed at CSGPolygon3D: Not Manifold.

It's OK if it's non-manifoldable, but here's the "revolve" case.

[fire]

Couldn't we use the "warning" icon in the scene tree? That should stand out more.

@KoBeWi Do you know how to do this?

[elalish]

Okay, fixed. Now that you've got your MeshGL dumping function, feel free to send a PR that adds a test like this whenever you run into another problem with Manifold. We'll add a fix to any PR like that in very short order. E.g. - I'm not actually sure if the revolve example you give above is a bug or what the expected behavior is.

[fire]

Ready for testing.

[Calinou]

Tested locally, it works as expected. Code looks good to me.

An easy way to reproduce this error situation is to create a PlaneMesh, QuadMesh, RibbonTrailMesh or TextMesh with Depth set to 0.0 in a CSGMesh3D.

[fire]

Is anyone interested in adding a "warning" icon in the scene tree? It would be a good feature enhancement after this pull request is merged.

[KoBeWi]

Couldn't we use the "warning" icon in the scene tree? That should stand out more.
@KoBeWi Do you know how to do this?

get_configuration_warnings()

[Repiteo]

Thanks!