Paths

Table of Contentst

Differential D5766

Solidify Modifier Extension
ClosedPublic
Actions

Authored by Henrik Dick (weasel) on Sep 11 2019, 9:17 PM.

Details

Reviewers

Campbell Barton (campbellbarton)

Group Reviewers

Modifiers

Summary

I added new features to the solidify modifier.

Clamp with respect to angles
New Manifold Solidify mode to solidify non manifold geometry into manifold geometry

The C code is roughly based on my temporary addon script at:
https://github.com/redweasel/manifold-solidify
It is rewritten though and a lot of bugs from the script are fixed.
Also the fix intersections feature isn't implemented yet, but I will
likely implement it sometime and add a new revision then.

Diff Detail

Repository

rB Blender

Branch

arcpatch-D5766 (branched from master)

Build Status

Buildable 5225
Build 5225: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

improved naming further, used a few more makros, fixed constraints calculation for offset != 0. Need to fix even and simple method for offset != 0 as well but had no time for that yet. The error in solidify_crash.blend is still unresolved.

fixed all sort of linear algebra mistakes and cleaned up the angle calculation.

As I was fixing the even and simple method I noticed, that they will always give worse results than the default constraints method. I think we should still include them, because they are faster to compute (for animation) and they can in some cases look more like what the artist wants, but thats open for discussion here: https://devtalk.blender.org/t/new-solidify-modifier/9192/21

Kenn Nyström (Frozen_Death_Knight) awarded a token.Sep 19 2019, 4:03 AM

Cleanup: use 'uint' instead of 'unsigned int', wrap struct declarations using trailing comma

Harbormaster completed remote builds in B5025: Diff 18354.Sep 19 2019, 5:31 AM

Minor cleanup:

Use 'uint' instead of 'unsigned int'
Wrap struct declarations (adding trailing comma)

Harbormaster completed remote builds in B5026: Diff 18355.Sep 19 2019, 5:39 AM

Declare iterators in for loops where possible

Harbormaster completed remote builds in B5027: Diff 18356.Sep 19 2019, 5:51 AM

Think this is close to completion.

I'm still getting an assert in
solidify_crash.blend547 KBDownload
(frame 10).

BLI_assert failed: /src/blender/source/blender/modifiers/intern/MOD_solidify.c:1977, manifold_solidify(), at 'new_edge->old_edge == orig_mloop[loopstart + j].e'

As you pointed out Only Rim does something different in Manifold Mode than in the default Extrude Mode. That is so because in Manifold Mode no original geometry is kept anyway. I think it is correct to leave it like this, because it also does exactly what it says. In cases where you want the other behaviour you will most certainly not have non manifold geometry and you would use Extrude Mode anyway.

While technically this is true, from a user perspective we have the use case of not needing to create an entire duplicate shell, yet still warning rims the the appearance of being solid the way this works now isn't very useful (Only making rims with no inner/outer surface).

Campbell Barton (campbellbarton) added inline comments.Sep 19 2019, 6:02 AM

source/blender/modifiers/intern/MOD_solidify.c
1618–1619	The clamping could be checked before calling `cosf`.

Metin Seven (MetinSeven) awarded a token.Sep 19 2019, 2:57 PM

michael knubben (michaelknubben) awarded a token.Sep 19 2019, 4:33 PM

B (bnzs) awarded a token.Sep 19 2019, 4:38 PM

fixed assertion error (had an outdated condition)
fixed size of vert_adj_edges (was numEdges instead of numVerts) and fix for isolated verts
added clamping for cosf everywhere where I used it

Harbormaster completed remote builds in B5041: Diff 18376.Sep 19 2019, 9:03 PM

removed clamps for cosf in confusion why I ever added them (cosf is defined for all finite values of float)

Harbormaster completed remote builds in B5042: Diff 18377.Sep 19 2019, 9:59 PM

cleaned up code a little
fixed isolated verts in result for only rim

Harbormaster completed remote builds in B5043: Diff 18378.Sep 19 2019, 11:21 PM

check for offset == 0 for extrude mode
there is a segmentation fault that occurs sometimes when using extrude, even thickness, angle clamp. It seems to appear at no specific point, just after using it a while (like for animation)

Harbormaster completed remote builds in B5049: Diff 18389.Sep 20 2019, 4:23 PM

Metin Seven (MetinSeven) added a subscriber: Metin Seven (MetinSeven).Sep 21 2019, 11:43 AM

clamped mat_nr (could go out of range before)
fixed clamp angle segmentation fault

I think the revision is finished now. I didn't find any more bugs.

Harbormaster completed remote builds in B5054: Diff 18397.Sep 21 2019, 12:23 PM

Running into a crash with the current patch:

solidify_crash_2.blend559 KBDownload

Open this file and play it, frame 1 crashes (other frames crash too, jump to any frame and play).

This is the asan output.

P1112 D5766-Crash

2 READ

23 freed

44 previously

66 Thread

==24947==ERROR: AddressSanitizer: heap-use-after-free on address 0x61100012b288 at pc 0x5561cda00e42 bp 0x7f4ab63e5a00 sp 0x7f4ab63e59f0 of size 8 at 0x61100012b288 thread T15 #0 0x5561cda00e41 in manifold_solidify /src/blender/source/blender/modifiers/intern/MOD_solidify.c:1137 #1 0x5561cda300b8 in applyModifier /src/blender/source/blender/modifiers/intern/MOD_solidify.c:3044 #2 0x5561cb1a5ad4 in modwrap_applyModifier /src/blender/source/blender/blenkernel/intern/modifier.c:945 #3 0x5561cb7f22b8 in mesh_calc_modifiers /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1208 #4 0x5561cb7faa40 in mesh_build_data /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1844 #5 0x5561cb7fc379 in makeDerivedMesh /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1973 #6 0x5561cb2da70a in BKE_object_handle_data_update /src/blender/source/blender/blenkernel/intern/object_update.c:187 #7 0x5561cb2dd852 in BKE_object_eval_uber_data /src/blender/source/blender/blenkernel/intern/object_update.c:358 #8 0x5561cbd33b31 in void std::__invoke_impl<void, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(std::__invoke_other, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/9.1.0/bits/invoke.h:60 #9 0x5561cbd2db59 in std::__invoke_result<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>::type std::__invoke<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/9.1.0/bits/invoke.h:95 #10 0x5561cbd2538c in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::__call<void, Depsgraph*&&, 0ul, 1ul, 2ul>(std::tuple<Depsgraph*&&>&&, std::_Index_tuple<0ul, 1ul, 2ul>) /usr/include/c++/9.1.0/functional:400 #11 0x5561cbd1aa06 in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::operator()<Depsgraph*, void>(Depsgraph*&&) /usr/include/c++/9.1.0/functional:484 #12 0x5561cbd0f956 in std::_Function_handler<void (Depsgraph*), std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)> >::_M_invoke(std::_Any_data const&, Depsgraph*&&) /usr/include/c++/9.1.0/bits/std_function.h:300 #13 0x5561cbd8cba2 in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const /usr/include/c++/9.1.0/bits/std_function.h:690 #14 0x5561cbd88e4d in deg_task_run_func /src/blender/source/blender/depsgraph/intern/eval/deg_eval.cc:86 #15 0x5561cbc80918 in task_scheduler_thread_run /src/blender/source/blender/blenlib/intern/task.c:450 #16 0x7f4ada5b157e in start_thread (/usr/lib/libpthread.so.0+0x957e) #17 0x7f4ad99c40e2 in __clone (/usr/lib/libc.so.6+0xfc0e2) 0x61100012b288 is located 8 bytes inside of 208-byte region [0x61100012b280,0x61100012b350) by thread T15 here: #0 0x7f4adb6386c0 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:122 #1 0x5561cbf1795e in MEM_lockfree_freeN /src/blender/intern/guardedalloc/intern/mallocn_lockfree_impl.c:157 #2 0x5561cda00c1c in manifold_solidify /src/blender/source/blender/modifiers/intern/MOD_solidify.c:1117 #3 0x5561cda300b8 in applyModifier /src/blender/source/blender/modifiers/intern/MOD_solidify.c:3044 #4 0x5561cb1a5ad4 in modwrap_applyModifier /src/blender/source/blender/blenkernel/intern/modifier.c:945 #5 0x5561cb7f22b8 in mesh_calc_modifiers /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1208 #6 0x5561cb7faa40 in mesh_build_data /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1844 #7 0x5561cb7fc379 in makeDerivedMesh /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1973 #8 0x5561cb2da70a in BKE_object_handle_data_update /src/blender/source/blender/blenkernel/intern/object_update.c:187 #9 0x5561cb2dd852 in BKE_object_eval_uber_data /src/blender/source/blender/blenkernel/intern/object_update.c:358 #10 0x5561cbd33b31 in void std::__invoke_impl<void, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(std::__invoke_other, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/9.1.0/bits/invoke.h:60 #11 0x5561cbd2db59 in std::__invoke_result<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>::type std::__invoke<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/9.1.0/bits/invoke.h:95 #12 0x5561cbd2538c in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::__call<void, Depsgraph*&&, 0ul, 1ul, 2ul>(std::tuple<Depsgraph*&&>&&, std::_Index_tuple<0ul, 1ul, 2ul>) /usr/include/c++/9.1.0/functional:400 #13 0x5561cbd1aa06 in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::operator()<Depsgraph*, void>(Depsgraph*&&) /usr/include/c++/9.1.0/functional:484 #14 0x5561cbd0f956 in std::_Function_handler<void (Depsgraph*), std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)> >::_M_invoke(std::_Any_data const&, Depsgraph*&&) /usr/include/c++/9.1.0/bits/std_function.h:300 #15 0x5561cbd8cba2 in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const /usr/include/c++/9.1.0/bits/std_function.h:690 #16 0x5561cbd88e4d in deg_task_run_func /src/blender/source/blender/depsgraph/intern/eval/deg_eval.cc:86 #17 0x5561cbc80918 in task_scheduler_thread_run /src/blender/source/blender/blenlib/intern/task.c:450 #18 0x7f4ada5b157e in start_thread (/usr/lib/libpthread.so.0+0x957e) allocated by thread T15 here: #0 0x7f4adb638ce8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153 #1 0x5561cbf18108 in MEM_lockfree_callocN /src/blender/intern/guardedalloc/intern/mallocn_lockfree_impl.c:267 #2 0x5561cbf183f3 in MEM_lockfree_calloc_arrayN /src/blender/intern/guardedalloc/intern/mallocn_lockfree_impl.c:299 #3 0x5561cd9fbe81 in manifold_solidify /src/blender/source/blender/modifiers/intern/MOD_solidify.c:892 #4 0x5561cda300b8 in applyModifier /src/blender/source/blender/modifiers/intern/MOD_solidify.c:3044 #5 0x5561cb1a5ad4 in modwrap_applyModifier /src/blender/source/blender/blenkernel/intern/modifier.c:945 #6 0x5561cb7f22b8 in mesh_calc_modifiers /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1208 #7 0x5561cb7faa40 in mesh_build_data /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1844 #8 0x5561cb7fc379 in makeDerivedMesh /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1973 #9 0x5561cb2da70a in BKE_object_handle_data_update /src/blender/source/blender/blenkernel/intern/object_update.c:187 #10 0x5561cb2dd852 in BKE_object_eval_uber_data /src/blender/source/blender/blenkernel/intern/object_update.c:358 #11 0x5561cbd33b31 in void std::__invoke_impl<void, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(std::__invoke_other, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/9.1.0/bits/invoke.h:60 #12 0x5561cbd2db59 in std::__invoke_result<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>::type std::__invoke<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/9.1.0/bits/invoke.h:95 #13 0x5561cbd2538c in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::__call<void, Depsgraph*&&, 0ul, 1ul, 2ul>(std::tuple<Depsgraph*&&>&&, std::_Index_tuple<0ul, 1ul, 2ul>) /usr/include/c++/9.1.0/functional:400 #14 0x5561cbd1aa06 in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::operator()<Depsgraph*, void>(Depsgraph*&&) /usr/include/c++/9.1.0/functional:484 #15 0x5561cbd0f956 in std::_Function_handler<void (Depsgraph*), std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)> >::_M_invoke(std::_Any_data const&, Depsgraph*&&) /usr/include/c++/9.1.0/bits/std_function.h:300 #16 0x5561cbd8cba2 in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const /usr/include/c++/9.1.0/bits/std_function.h:690 #17 0x5561cbd88e4d in deg_task_run_func /src/blender/source/blender/depsgraph/intern/eval/deg_eval.cc:86 #18 0x5561cbc80918 in task_scheduler_thread_run /src/blender/source/blender/blenlib/intern/task.c:450 #19 0x7f4ada5b157e in start_thread (/usr/lib/libpthread.so.0+0x957e) T15 created by T0 here: #0 0x7f4adb562367 in __interceptor_pthread_create /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cc:208 #1 0x5561cbc81dd0 in BLI_task_scheduler_create /src/blender/source/blender/blenlib/intern/task.c:517 #2 0x5561cbc8fbb9 in BLI_task_scheduler_get /src/blender/source/blender/blenlib/intern/threads.c:177 #3 0x5561cbc8d41c in BLI_task_parallel_range /src/blender/source/blender/blenlib/intern/task.c:1198 #4 0x5561cbdae206 in flush_prepare /src/blender/source/blender/depsgraph/intern/eval/deg_eval_flush.cc:118 #5 0x5561cbdae206 in DEG::deg_graph_flush_updates(Main*, DEG::Depsgraph*) /src/blender/source/blender/depsgraph/intern/eval/deg_eval_flush.cc:365 #6 0x5561cbcb4c50 in DEG_evaluate_on_refresh /src/blender/source/blender/depsgraph/intern/depsgraph_eval.cc:63 #7 0x5561cb4938f0 in scene_graph_update_tagged /src/blender/source/blender/blenkernel/intern/scene.c:1334 #8 0x5561cb4939f1 in BKE_scene_graph_update_tagged /src/blender/source/blender/blenkernel/intern/scene.c:1360 #9 0x5561cca2be96 in wm_event_do_depsgraph /src/blender/source/blender/windowmanager/intern/wm_event_system.c:370 #10 0x5561cca6bfba in wm_file_read_post /src/blender/source/blender/windowmanager/intern/wm_files.c:555 #11 0x5561cca6ee69 in wm_homefile_read /src/blender/source/blender/windowmanager/intern/wm_files.c:1059 #12 0x5561cca9d2d2 in WM_init /src/blender/source/blender/windowmanager/intern/wm_init_exit.c:293 #13 0x5561caaa6548 in main /src/blender/source/creator/creator.c:414 #14 0x7f4ad98eeee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2)

Campbell Barton (campbellbarton) requested changes to this revision.Sep 23 2019, 3:37 AM

This revision now requires changes to proceed.Sep 23 2019, 3:37 AM

finally fixed everything (a lot and I forgot about most things already)

fixes include:

handling of singularities (as I call verts which have a one-sided faceloop around them)
fix duplicate edge and poly handling (still not recommended to use but the modifier wont fail you)
various other smaller bugs

Harbormaster completed remote builds in B5217: Diff 18744.Oct 2 2019, 11:53 PM

Unfortunately still getting crashes:

solidify_crash_3.blend753 KBDownload

- open and play animation, crashes around frame 1.

==7488==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200016bf88 at pc 0x55e4749df19f bp 0x7f7cf0325930 sp 0x7f7cf0325920
WRITE of size 4 at 0x60200016bf88 thread T12
    #0 0x55e4749df19e in manifold_solidify /src/blender/source/blender/modifiers/intern/MOD_solidify.c:1791
    #1 0x55e474a05c14 in applyModifier /src/blender/source/blender/modifiers/intern/MOD_solidify.c:3249
    #2 0x55e47212f3b2 in modwrap_applyModifier /src/blender/source/blender/blenkernel/intern/modifier.c:945
    #3 0x55e4727813e2 in mesh_calc_modifiers /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1190
    #4 0x55e472789ad6 in mesh_build_data /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1819
    #5 0x55e47278b2a9 in makeDerivedMesh /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1943
    #6 0x55e47226454a in BKE_object_handle_data_update /src/blender/source/blender/blenkernel/intern/object_update.c:187
    #7 0x55e472267692 in BKE_object_eval_uber_data /src/blender/source/blender/blenkernel/intern/object_update.c:358
    #8 0x55e472ce173b in void std::__invoke_impl<void, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(std::__invoke_other, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/9.1.0/bits/invoke.h:60
    #9 0x55e472cdb763 in std::__invoke_result<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>::type std::__invoke<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/9.1.0/bits/invoke.h:95
    #10 0x55e472cd2f96 in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::__call<void, Depsgraph*&&, 0ul, 1ul, 2ul>(std::tuple<Depsgraph*&&>&&, std::_Index_tuple<0ul, 1ul, 2ul>) /usr/include/c++/9.1.0/functional:400
    #11 0x55e472cc8610 in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::operator()<Depsgraph*, void>(Depsgraph*&&) /usr/include/c++/9.1.0/functional:484
    #12 0x55e472cbd560 in std::_Function_handler<void (Depsgraph*), std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)> >::_M_invoke(std::_Any_data const&, Depsgraph*&&) /usr/include/c++/9.1.0/bits/std_function.h:300
    #13 0x55e472d62444 in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const /usr/include/c++/9.1.0/bits/std_function.h:690
    #14 0x55e472d5e6ef in deg_task_run_func /src/blender/source/blender/depsgraph/intern/eval/deg_eval.cc:86
    #15 0x55e472c0f7ad in task_scheduler_thread_run /src/blender/source/blender/blenlib/intern/task.c:450
    #16 0x7f7d12ad157e in start_thread (/usr/lib/libpthread.so.0+0x957e)
    #17 0x7f7d11ee40e2 in __clone (/usr/lib/libc.so.6+0xfc0e2)

0x60200016bf88 is located 12 bytes to the right of 12-byte region [0x60200016bf70,0x60200016bf7c)
allocated by thread T12 here:
    #0 0x7f7d13b58ada in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x55e472eee9dc in MEM_lockfree_mallocN /src/blender/intern/guardedalloc/intern/mallocn_lockfree_impl.c:308
    #2 0x55e472eee438 in MEM_lockfree_recallocN_id /src/blender/intern/guardedalloc/intern/mallocn_lockfree_impl.c:229
    #3 0x55e4749df0da in manifold_solidify /src/blender/source/blender/modifiers/intern/MOD_solidify.c:1788
    #4 0x55e474a05c14 in applyModifier /src/blender/source/blender/modifiers/intern/MOD_solidify.c:3249
    #5 0x55e47212f3b2 in modwrap_applyModifier /src/blender/source/blender/blenkernel/intern/modifier.c:945
    #6 0x55e4727813e2 in mesh_calc_modifiers /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1190
    #7 0x55e472789ad6 in mesh_build_data /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1819
    #8 0x55e47278b2a9 in makeDerivedMesh /src/blender/source/blender/blenkernel/intern/DerivedMesh.c:1943
    #9 0x55e47226454a in BKE_object_handle_data_update /src/blender/source/blender/blenkernel/intern/object_update.c:187
    #10 0x55e472267692 in BKE_object_eval_uber_data /src/blender/source/blender/blenkernel/intern/object_update.c:358
    #11 0x55e472ce173b in void std::__invoke_impl<void, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(std::__invoke_other, void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/9.1.0/bits/invoke.h:60
    #12 0x55e472cdb763 in std::__invoke_result<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>::type std::__invoke<void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*, Scene*&, Object*&>(void (*&)(Depsgraph*, Scene*, Object*), Depsgraph*&&, Scene*&, Object*&) /usr/include/c++/9.1.0/bits/invoke.h:95
    #13 0x55e472cd2f96 in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::__call<void, Depsgraph*&&, 0ul, 1ul, 2ul>(std::tuple<Depsgraph*&&>&&, std::_Index_tuple<0ul, 1ul, 2ul>) /usr/include/c++/9.1.0/functional:400
    #14 0x55e472cc8610 in void std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)>::operator()<Depsgraph*, void>(Depsgraph*&&) /usr/include/c++/9.1.0/functional:484
    #15 0x55e472cbd560 in std::_Function_handler<void (Depsgraph*), std::_Bind<void (*(std::_Placeholder<1>, Scene*, Object*))(Depsgraph*, Scene*, Object*)> >::_M_invoke(std::_Any_data const&, Depsgraph*&&) /usr/include/c++/9.1.0/bits/std_function.h:300
    #16 0x55e472d62444 in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const /usr/include/c++/9.1.0/bits/std_function.h:690
    #17 0x55e472d5e6ef in deg_task_run_func /src/blender/source/blender/depsgraph/intern/eval/deg_eval.cc:86
    #18 0x55e472c0f7ad in task_scheduler_thread_run /src/blender/source/blender/blenlib/intern/task.c:450
    #19 0x7f7d12ad157e in start_thread (/usr/lib/libpthread.so.0+0x957e)

Thread T12 created by T0 here:
    #0 0x7f7d13a82367 in __interceptor_pthread_create /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cc:208
    #1 0x55e472c10c65 in BLI_task_scheduler_create /src/blender/source/blender/blenlib/intern/task.c:517
    #2 0x55e472c1ea4e in BLI_task_scheduler_get /src/blender/source/blender/blenlib/intern/threads.c:177
    #3 0x55e472c1c2b1 in BLI_task_parallel_range /src/blender/source/blender/blenlib/intern/task.c:1198
    #4 0x55e472d83b66 in flush_prepare /src/blender/source/blender/depsgraph/intern/eval/deg_eval_flush.cc:118
    #5 0x55e472d83b66 in DEG::deg_graph_flush_updates(Main*, DEG::Depsgraph*) /src/blender/source/blender/depsgraph/intern/eval/deg_eval_flush.cc:365
    #6 0x55e472c4d2ea in DEG_evaluate_on_refresh /src/blender/source/blender/depsgraph/intern/depsgraph_eval.cc:63
    #7 0x55e4724228fe in scene_graph_update_tagged /src/blender/source/blender/blenkernel/intern/scene.c:1334
    #8 0x55e4724229ff in BKE_scene_graph_update_tagged /src/blender/source/blender/blenkernel/intern/scene.c:1360
    #9 0x55e473a04842 in wm_event_do_depsgraph /src/blender/source/blender/windowmanager/intern/wm_event_system.c:370
    #10 0x55e473a452f3 in wm_file_read_post /src/blender/source/blender/windowmanager/intern/wm_files.c:555
    #11 0x55e473a481a2 in wm_homefile_read /src/blender/source/blender/windowmanager/intern/wm_files.c:1059
    #12 0x55e473a7660b in WM_init /src/blender/source/blender/windowmanager/intern/wm_init_exit.c:293
    #13 0x55e471a2c6c8 in main /src/blender/source/creator/creator.c:414
    #14 0x7f7d11e0eee2 in __libc_start_main (/usr/lib/libc.so.6+0x26ee2)

SUMMARY: AddressSanitizer: heap-buffer-overflow /src/blender/source/blender/modifiers/intern/MOD_solidify.c:1791 in manifold_solidify

Metin Seven (MetinSeven) removed a subscriber: Metin Seven (MetinSeven).Oct 3 2019, 3:15 PM

fixed segmentation fault

ok I will stop saying "fixed all bugs" just fixed some bugs

Harbormaster completed remote builds in B5225: Diff 18764.Oct 3 2019, 5:43 PM

solidify_crash_5.blend607 KBDownload

BLI_assert failed: /src/blender/source/blender/modifiers/intern/MOD_solidify.c:2252, manifold_solidify(), at 'loop_index == numNewLoops'

In a release build, enable the modifier and play animation crashes after a few frames.

Please try and stress test further updates to this patch.

This revision now requires changes to proceed.Oct 5 2019, 4:27 AM

Yiwei Ye (yebyte) added a subscriber: Yiwei Ye (yebyte).Oct 6 2019, 12:24 PM

fixed previously unhandled open singularities
fixed miscounting of unused loops for fill faces

Harbormaster completed remote builds in B5342: Diff 18981.Oct 12 2019, 8:59 PM

I still found a bug, but your file is fixed now. The new one comes in when you extrude you mesh without translating afterwards, creating duplicate geometry.

source/blender/modifiers/intern/MOD_solidify.c
273	The old solidify modifier has the same check, thats why I added it here as well. If you think it should be removed in the old one as well thats fine, but keep it consistent.

Bartosz Moniewski (monio) awarded a token.Oct 13 2019, 1:51 AM

Dspazio (Dspazio) awarded a token.Oct 14 2019, 8:51 AM

Still crashing here woth ASAN enabled, open this file and set method to manifold:

solidify_crash_6.blend990 KBDownload

Campbell Barton (campbellbarton) requested changes to this revision.Oct 14 2019, 9:20 AM

fixed previously unhandled open singularities
fixed miscounting of unused loops for fill faces
fixed problems with singularities at rims
Fixed miscalculation of loops for closed splits
removed printf's

Should be ready for review now again.
EDIT: (forgot to apply clang formatting, will fix that with the next revision or before it lands)

Harbormaster completed remote builds in B5415: Diff 19112.Oct 18 2019, 8:39 PM

Great, tested an works well.
Will do final review and commit next week.

I've been doing some user-level testing and wasn't able to make an example that works well with this new solidify method, but doesn't work well with the existing one, could you show some example Blend file where this is the case?

Also, before committing this I'd also like to use a more descriptive name, "manifold" is a generic term. suggest: Simple / Intersect.

Here is an example file with all the objects from development and fixing.
The name Manifold comes from it being able to make non Manifold geometry into completly manifold shells.
I don't know how "Intersect" would fit as a description, because that is exactly what it is missing.

SolidifyTest.blend974 KBDownload

Thanks for the test cases. I was mainly checking the code and that it wasn't crashing, the name manifold makes more sense now.

How about calling this "Simple / Complex" - where the tool-tip for "complex" notes that it can make manifold shells from complex non-manifold input?

Otherwise it might seem as if Extrude wont give manifold output.

changed the names of the modes for the user (left the internal names the same to avoid collision of the word SIMPLE for method and mode)
applied clang formatting

I think Complex and Simple is good with the right description.
The point with Extrude and Manifold was, that Extrude will not
always give you a manifold output, whereas Manifold does.

Harbormaster completed remote builds in B5530: Diff 19340.Nov 2 2019, 1:43 PM

Split solidify implementations in two files
Use static instead of BLI_INLINE (let the compiler handle inlining)

Harbormaster completed remote builds in B5534: Diff 19350.Nov 3 2019, 5:47 AM

Rename variables

Use '_len' suffix instead of 'tot' prefix.
Use 'is_' prefix for booleans that aren't obviously booleans.
Use full sentences.

Harbormaster completed remote builds in B5535: Diff 19352.Nov 3 2019, 6:12 AM

Use const declarations.
Use uint for indices instead of char.
Use braces.

Edit: will finish some renaming updates & commit tomorrow.

Harbormaster completed remote builds in B5536: Diff 19353.Nov 3 2019, 6:49 AM

Rename 'invalid' variable.

Harbormaster completed remote builds in B5537: Diff 19356.Nov 3 2019, 6:58 AM

Rename solidify modes:

Extrude: existing method.
Non-manifold: new method which handles non-manifold input.

The UI still uses simple/complex since this difference
doesn't fit well into a single term.

Harbormaster completed remote builds in B5539: Diff 19369.Nov 3 2019, 11:00 PM

Rename files to match new names, remove unused headers.
Remove unused MOD_solidify_util.c file.

Harbormaster completed remote builds in B5542: Diff 19372.Nov 3 2019, 11:12 PM

Committed rBe45cfb574ee7: Solidify Modifier: support non-manifold input

Summary of changes:

Split solidify into two files to isolate logic for improved readability.

Existing test files made with this patch will need to be manually updated because they used flag values when they arent needed as we don't allow multiple modes at once.

Moved the thickness and boundary options under the mode drop-down.

Where they were located didn't have room to show the labels, this looks a bit awkward, so we may want to re-arrange that.