Bind in Laplacian Deform Modifier with new vertex group in Edit Mode crashes Blender #67385

New Issue

takaaki takeda · 2019-07-21T14:55:32+02:00

takaaki takeda commented

2019-07-21 14:55:32 +02:00

System Information
Operating system: Windows-10-10.0.17134 64 Bits
Graphics card: GeForce GTX 970/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 430.86

Blender Version
Broken: version: 2.80 (sub 74), branch: master, commit date: 2019-07-19 00:44, hash: 5f140e61c2
Worked: (optional)

Short description of error
Bind in Laplacian Deform Modifier with newly made vertex group in Edit mode sometimes crashes Blender.
If blender is not crashed, an error message comes in console window.
[Please fill out a short description of the error here]

Exact steps for others to reproduce the error
Select the defaul cube and set mode to Edit Mode.
Make new vertex group and Add Laplacian Deform Modifier, set the vertex group and Bind.
Sometimes, the blender crashes, or, if is not crashed, error message that says vertex group is not valid comes to the console window.
I have captured two cases to animation gif images below.

If swithed to the object mode, I found nothing wrong.

**System Information** Operating system: Windows-10-10.0.17134 64 Bits Graphics card: GeForce GTX 970/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 430.86 **Blender Version** Broken: version: 2.80 (sub 74), branch: master, commit date: 2019-07-19 00:44, hash: `5f140e61c2` Worked: (optional) **Short description of error** Bind in Laplacian Deform Modifier with newly made vertex group in Edit mode sometimes crashes Blender. If blender is not crashed, an error message comes in console window. [Please fill out a short description of the error here] **Exact steps for others to reproduce the error** Select the defaul cube and set mode to Edit Mode. Make new vertex group and Add Laplacian Deform Modifier, set the vertex group and Bind. Sometimes, the blender crashes, or, if is not crashed, error message that says vertex group is not valid comes to the console window. I have captured two cases to animation gif images below. ![LaplacianDeform.gif](https://archive.blender.org/developer/F7621783/LaplacianDeform.gif) ![LaplacianDeform2.gif](https://archive.blender.org/developer/F7621784/LaplacianDeform2.gif) If swithed to the object mode, I found nothing wrong.

takaaki takeda commented

2019-07-21 14:55:32 +02:00

Added subscriber: @popqjp

Erik Fernandes commented

2019-07-21 18:34:51 +02:00

Added subscriber: @ErikFernandes

Erik Fernandes commented

2019-07-21 18:34:51 +02:00

I can reproduce this error in windows 10 with 'GeForce GT 1030'

Germano Cavalcante commented

2019-07-23 15:24:23 +02:00

Added subscribers: @mont29, @mano-wii

Bastien Montagne was assigned by Germano Cavalcante

2019-07-23 15:24:23 +02:00

Germano Cavalcante commented

2019-07-23 15:24:23 +02:00

@mont29, can you take a look?

Bastien Montagne commented

2019-07-26 14:51:30 +02:00

Added subscriber: @Sergey

Bastien Montagne commented

2019-07-26 14:51:30 +02:00

OK, so there are several issues here actually.

Main one (crashing one) is related to temp mesh freeing, I'd need @Sergey to have a look at one-liner patch below and tell what he thinks about it ( fixes crash with backtrace also attached, double-free of editmesh data...).

P1052: (An Untitled Masterwork)

diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h
index 54fbda1fa31..d6b4fa72281 100644
--- a/source/blender/blenkernel/BKE_mesh.h
+++ b/source/blender/blenkernel/BKE_mesh.h
@@ -123,6 +123,8 @@ struct Mesh *BKE_mesh_new_nomain_from_template(const struct Mesh *me_src,
                                                int loops_len,
                                                int polys_len);
 
+void BKE_mesh_eval_delete(struct Mesh *me_eval);
+
 /* Performs copy for use during evaluation,
  * optional referencing original arrays to reduce memory. */
 struct Mesh *BKE_mesh_copy_for_eval(struct Mesh *source, bool reference);
diff --git a/source/blender/blenkernel/intern/mesh.c b/source/blender/blenkernel/intern/mesh.c
index f5e93dcf9b7..9e01bfe62d6 100644
--- a/source/blender/blenkernel/intern/mesh.c
+++ b/source/blender/blenkernel/intern/mesh.c
@@ -697,6 +697,15 @@ Mesh *BKE_mesh_new_nomain_from_template(const Mesh *me_src,
       me_src, verts_len, edges_len, tessface_len, loops_len, polys_len, CD_MASK_EVERYTHING);
 }
 
+void BKE_mesh_eval_delete(struct Mesh *mesh_eval)
+{
+  /* Evaluated mesh may point to edit mesh, but never owns it. */
+  mesh_eval->edit_mesh = NULL;
+  BKE_mesh_free(mesh_eval);
+  BKE_libblock_free_data(&mesh_eval->id, false);
+  MEM_freeN(mesh_eval);
+}
+
 Mesh *BKE_mesh_copy_for_eval(struct Mesh *source, bool reference)
 {
   int flags = LIB_ID_COPY_LOCALIZE;
diff --git a/source/blender/blenkernel/intern/object.c b/source/blender/blenkernel/intern/object.c
index d7256cc9604..69ba7ccab8e 100644
--- a/source/blender/blenkernel/intern/object.c
+++ b/source/blender/blenkernel/intern/object.c
@@ -442,19 +442,13 @@ void BKE_object_free_derived_caches(Object *ob)
   if (ob->runtime.mesh_eval != NULL) {
     if (ob->runtime.is_mesh_eval_owned) {
       Mesh *mesh_eval = ob->runtime.mesh_eval;
-      /* Evaluated mesh points to edit mesh, but does not own it. */
-      mesh_eval->edit_mesh = NULL;
-      BKE_mesh_free(mesh_eval);
-      BKE_libblock_free_data(&mesh_eval->id, false);
-      MEM_freeN(mesh_eval);
+      BKE_mesh_eval_delete(mesh_eval);
     }
     ob->runtime.mesh_eval = NULL;
   }
   if (ob->runtime.mesh_deform_eval != NULL) {
     Mesh *mesh_deform_eval = ob->runtime.mesh_deform_eval;
-    BKE_mesh_free(mesh_deform_eval);
-    BKE_libblock_free_data(&mesh_deform_eval->id, false);
-    MEM_freeN(mesh_deform_eval);
+    BKE_mesh_eval_delete(mesh_deform_eval);
     ob->runtime.mesh_deform_eval = NULL;
   }
 
diff --git a/source/blender/editors/object/object_modifier.c b/source/blender/editors/object/object_modifier.c
index 08012842c37..e72bf1ea10a 100644
--- a/source/blender/editors/object/object_modifier.c
+++ b/source/blender/editors/object/object_modifier.c
@@ -99,7 +99,7 @@ static void object_force_modifier_update_for_bind(Depsgraph *depsgraph, Object *
   BKE_object_eval_reset(ob_eval);
   if (ob->type == OB_MESH) {
     Mesh *me_eval = mesh_create_eval_final_view(depsgraph, scene_eval, ob_eval, &CD_MASK_BAREMESH);
-    BKE_id_free(NULL, me_eval);
+    BKE_mesh_eval_delete(me_eval);
   }
   else if (ob->type == OB_LATTICE) {
     BKE_lattice_modifiers_calc(depsgraph, scene_eval, ob_eval);

=================================================================
==18076==ERROR: AddressSanitizer: heap-use-after-free on address 0x60e00006e2b8 at pc 0x5646174bebb9 bp 0x7ffc2be66a40 sp 0x7ffc2be66a38
READ of size 8 at 0x60e00006e2b8 thread T0
    - 0 0x5646174bebb8 in BKE_editmesh_free_derivedmesh /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/editmesh.c:148
    - 1 0x5646182cad5f in discard_mesh_edit_mode_pointers /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1524
    - 2 0x5646182cb070 in discard_edit_mode_pointers /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1546
    - 3 0x5646182cb2f9 in DEG::deg_free_copy_on_write_datablock(ID*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1597
    - 4 0x5646182ca7bc in DEG::deg_update_copy_on_write_datablock(DEG::Depsgraph const*, DEG::IDNode const*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1477
    - 5 0x5646182cb5fc in DEG::deg_evaluate_copy_on_write(Depsgraph*, DEG::IDNode const*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1613
    - 6 0x564618268307 in void std::__invoke_impl<void, void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>(std::__invoke_other, void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*&&, DEG::IDNode*&) /usr/include/c++/8/bits/invoke.h:60
    - 7 0x564618263139 in std::__invoke_result<void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>::type std::__invoke<void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>(void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*&&, DEG::IDNode*&) /usr/include/c++/8/bits/invoke.h:95
    - 8 0x56461825ce22 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::__call<void, Depsgraph*&&, 0ul, 1ul>(std::tuple<Depsgraph*&&>&&, std::_Index_tuple<0ul, 1ul>) /usr/include/c++/8/functional:400
    - 9 0x564618254161 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::operator()<Depsgraph*, void>(Depsgraph*&&) /usr/include/c++/8/functional:484
    - 10 0x564618247ead in std::_Function_handler<void (Depsgraph*), std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)> >::_M_invoke(std::_Any_data const&, Depsgraph*&&) /usr/include/c++/8/bits/std_function.h:297
    - 11 0x5646182baf11 in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const /usr/include/c++/8/bits/std_function.h:687
    - 12 0x5646182b81a2 in deg_task_run_func /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval.cc:86
    - 13 0x5646181cc1ee in BLI_task_pool_work_and_wait /home/i74700deb64/blender/__work__/src/source/blender/blenlib/intern/task.c:939
    - 14 0x5646181cce34 in BLI_task_pool_work_wait_and_reset /home/i74700deb64/blender/__work__/src/source/blender/blenlib/intern/task.c:969
    - 15 0x5646182ba9b2 in DEG::deg_evaluate_on_refresh(DEG::Depsgraph*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval.cc:297
    - 16 0x5646181ff86d in DEG_evaluate_on_refresh /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/depsgraph_eval.cc:63
    - 17 0x564617a21548 in scene_graph_update_tagged /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/scene.c:1567
    - 18 0x564617a215ef in BKE_scene_graph_update_tagged /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/scene.c:1582
    - 19 0x564618e7ac7c in wm_event_do_depsgraph /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:369
    - 20 0x564618e7afd1 in wm_event_do_refresh_wm_and_depsgraph /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:395
    - 21 0x564618e7cf2c in wm_event_do_notifiers /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:560
    - 22 0x564618e6388b in WM_main /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm.c:420
    - 23 0x5646171071a9 in main /home/i74700deb64/blender/__work__/src/source/creator/creator.c:500
    - 24 0x7f22c068809a in __libc_start_main ../csu/libc-start.c:308
    - 25 0x564617106339 in _start (/home/i74700deb64/blender/__work__/build_master_debug/bin/blender+0x1b714339)

0x60e00006e2b8 is located 56 bytes inside of 152-byte region [0x60e00006e280,0x60e00006e318)
freed by thread T0 here:
    - 0 0x7f22c66c01d7 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x1071d7)
    - 1 0x5646184605ed in MEM_lockfree_freeN /home/i74700deb64/blender/__work__/src/intern/guardedalloc/intern/mallocn_lockfree_impl.c:157
    - 2 0x5646176b75cb in BKE_mesh_free /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/mesh.c:495
    - 3 0x564617629d1e in BKE_libblock_free_datablock /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/library_remap.c:781
    - 4 0x56461762a554 in BKE_id_free_ex /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/library_remap.c:951
    - 5 0x56461762a6b1 in BKE_id_free /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/library_remap.c:995
    - 6 0x56461b19a569 in object_force_modifier_update_for_bind /home/i74700deb64/blender/__work__/src/source/blender/editors/object/object_modifier.c:102
    - 7 0x56461b19a93c in object_force_modifier_bind_simple_options /home/i74700deb64/blender/__work__/src/source/blender/editors/object/object_modifier.c:124
    - 8 0x56461b1bac41 in laplaciandeform_bind_exec /home/i74700deb64/blender/__work__/src/source/blender/editors/object/object_modifier.c:2421
    - 9 0x56461b1bb082 in laplaciandeform_bind_invoke /home/i74700deb64/blender/__work__/src/source/blender/editors/object/object_modifier.c:2441
    - 10 0x564618e87671 in wm_operator_invoke /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:1438
    - 11 0x564618e89867 in wm_operator_call_internal /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:1685
    - 12 0x564618e899db in WM_operator_name_call_ptr /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:1699
    - 13 0x56461a7af5f3 in ui_apply_but_funcs_after /home/i74700deb64/blender/__work__/src/source/blender/editors/interface/interface_handlers.c:835
    - 14 0x56461a82e8be in ui_handler_region_menu /home/i74700deb64/blender/__work__/src/source/blender/editors/interface/interface_handlers.c:10182
    - 15 0x564618e7d9fb in wm_handler_ui_call /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:623
    - 16 0x564618e93d18 in wm_handlers_do_intern /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:2699
    - 17 0x564618e9699b in wm_handlers_do /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:2945
    - 18 0x564618e9b99e in wm_event_do_handlers /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:3323
    - 19 0x564618e6387f in WM_main /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm.c:417
    - 20 0x5646171071a9 in main /home/i74700deb64/blender/__work__/src/source/creator/creator.c:500
    - 21 0x7f22c068809a in __libc_start_main ../csu/libc-start.c:308

previously allocated by thread T0 here:
    - 0 0x7f22c66c0588 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107588)
    - 1 0x564618461094 in MEM_lockfree_mallocN /home/i74700deb64/blender/__work__/src/intern/guardedalloc/intern/mallocn_lockfree_impl.c:308
    - 2 0x5646184607c3 in MEM_lockfree_dupallocN /home/i74700deb64/blender/__work__/src/intern/guardedalloc/intern/mallocn_lockfree_impl.c:177
    - 3 0x5646182c07be in update_mesh_edit_mode_pointers /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:625
    - 4 0x5646182c0c81 in update_edit_mode_pointers /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:641
    - 5 0x5646182c2415 in update_id_after_copy /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:805
    - 6 0x5646182c2c1f in DEG::deg_expand_copy_on_write_datablock(DEG::Depsgraph const*, DEG::IDNode const*, DEG::DepsgraphNodeBuilder*, bool) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:912
    - 7 0x5646182ca7dc in DEG::deg_update_copy_on_write_datablock(DEG::Depsgraph const*, DEG::IDNode const*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1478
    - 8 0x5646182cb5fc in DEG::deg_evaluate_copy_on_write(Depsgraph*, DEG::IDNode const*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1613
    - 9 0x564618268307 in void std::__invoke_impl<void, void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>(std::__invoke_other, void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*&&, DEG::IDNode*&) /usr/include/c++/8/bits/invoke.h:60
    - 10 0x564618263139 in std::__invoke_result<void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>::type std::__invoke<void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>(void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*&&, DEG::IDNode*&) /usr/include/c++/8/bits/invoke.h:95
    - 11 0x56461825ce22 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::__call<void, Depsgraph*&&, 0ul, 1ul>(std::tuple<Depsgraph*&&>&&, std::_Index_tuple<0ul, 1ul>) /usr/include/c++/8/functional:400
    - 12 0x564618254161 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::operator()<Depsgraph*, void>(Depsgraph*&&) /usr/include/c++/8/functional:484
    - 13 0x564618247ead in std::_Function_handler<void (Depsgraph*), std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)> >::_M_invoke(std::_Any_data const&, Depsgraph*&&) /usr/include/c++/8/bits/std_function.h:297
    - 14 0x5646182baf11 in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const /usr/include/c++/8/bits/std_function.h:687
    - 15 0x5646182b81a2 in deg_task_run_func /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval.cc:86
    - 16 0x5646181cc1ee in BLI_task_pool_work_and_wait /home/i74700deb64/blender/__work__/src/source/blender/blenlib/intern/task.c:939
    - 17 0x5646181cce34 in BLI_task_pool_work_wait_and_reset /home/i74700deb64/blender/__work__/src/source/blender/blenlib/intern/task.c:969
    - 18 0x5646182ba9b2 in DEG::deg_evaluate_on_refresh(DEG::Depsgraph*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval.cc:297
    - 19 0x5646181ff86d in DEG_evaluate_on_refresh /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/depsgraph_eval.cc:63
    - 20 0x564617a21548 in scene_graph_update_tagged /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/scene.c:1567
    - 21 0x564617a215ef in BKE_scene_graph_update_tagged /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/scene.c:1582
    - 22 0x564618e7ac7c in wm_event_do_depsgraph /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:369
    - 23 0x564618e7afd1 in wm_event_do_refresh_wm_and_depsgraph /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:395
    - 24 0x564618e7cf2c in wm_event_do_notifiers /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:560
    - 25 0x564618e6388b in WM_main /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm.c:420
    - 26 0x5646171071a9 in main /home/i74700deb64/blender/__work__/src/source/creator/creator.c:500
    - 27 0x7f22c068809a in __libc_start_main ../csu/libc-start.c:308

OK, so there are several issues here actually. Main one (crashing one) is related to temp mesh freeing, I'd need @Sergey to have a look at one-liner patch below and tell what he thinks about it ( fixes crash with backtrace also attached, double-free of editmesh data...). [P1052: (An Untitled Masterwork)](https://archive.blender.org/developer/P1052.txt) ``` diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h index 54fbda1fa31..d6b4fa72281 100644 --- a/source/blender/blenkernel/BKE_mesh.h +++ b/source/blender/blenkernel/BKE_mesh.h @@ -123,6 +123,8 @@ struct Mesh *BKE_mesh_new_nomain_from_template(const struct Mesh *me_src, int loops_len, int polys_len); +void BKE_mesh_eval_delete(struct Mesh *me_eval); + /* Performs copy for use during evaluation, * optional referencing original arrays to reduce memory. */ struct Mesh *BKE_mesh_copy_for_eval(struct Mesh *source, bool reference); diff --git a/source/blender/blenkernel/intern/mesh.c b/source/blender/blenkernel/intern/mesh.c index f5e93dcf9b7..9e01bfe62d6 100644 --- a/source/blender/blenkernel/intern/mesh.c +++ b/source/blender/blenkernel/intern/mesh.c @@ -697,6 +697,15 @@ Mesh *BKE_mesh_new_nomain_from_template(const Mesh *me_src, me_src, verts_len, edges_len, tessface_len, loops_len, polys_len, CD_MASK_EVERYTHING); } +void BKE_mesh_eval_delete(struct Mesh *mesh_eval) +{ + /* Evaluated mesh may point to edit mesh, but never owns it. */ + mesh_eval->edit_mesh = NULL; + BKE_mesh_free(mesh_eval); + BKE_libblock_free_data(&mesh_eval->id, false); + MEM_freeN(mesh_eval); +} + Mesh *BKE_mesh_copy_for_eval(struct Mesh *source, bool reference) { int flags = LIB_ID_COPY_LOCALIZE; diff --git a/source/blender/blenkernel/intern/object.c b/source/blender/blenkernel/intern/object.c index d7256cc9604..69ba7ccab8e 100644 --- a/source/blender/blenkernel/intern/object.c +++ b/source/blender/blenkernel/intern/object.c @@ -442,19 +442,13 @@ void BKE_object_free_derived_caches(Object *ob) if (ob->runtime.mesh_eval != NULL) { if (ob->runtime.is_mesh_eval_owned) { Mesh *mesh_eval = ob->runtime.mesh_eval; - /* Evaluated mesh points to edit mesh, but does not own it. */ - mesh_eval->edit_mesh = NULL; - BKE_mesh_free(mesh_eval); - BKE_libblock_free_data(&mesh_eval->id, false); - MEM_freeN(mesh_eval); + BKE_mesh_eval_delete(mesh_eval); } ob->runtime.mesh_eval = NULL; } if (ob->runtime.mesh_deform_eval != NULL) { Mesh *mesh_deform_eval = ob->runtime.mesh_deform_eval; - BKE_mesh_free(mesh_deform_eval); - BKE_libblock_free_data(&mesh_deform_eval->id, false); - MEM_freeN(mesh_deform_eval); + BKE_mesh_eval_delete(mesh_deform_eval); ob->runtime.mesh_deform_eval = NULL; } diff --git a/source/blender/editors/object/object_modifier.c b/source/blender/editors/object/object_modifier.c index 08012842c37..e72bf1ea10a 100644 --- a/source/blender/editors/object/object_modifier.c +++ b/source/blender/editors/object/object_modifier.c @@ -99,7 +99,7 @@ static void object_force_modifier_update_for_bind(Depsgraph *depsgraph, Object * BKE_object_eval_reset(ob_eval); if (ob->type == OB_MESH) { Mesh *me_eval = mesh_create_eval_final_view(depsgraph, scene_eval, ob_eval, &CD_MASK_BAREMESH); - BKE_id_free(NULL, me_eval); + BKE_mesh_eval_delete(me_eval); } else if (ob->type == OB_LATTICE) { BKE_lattice_modifiers_calc(depsgraph, scene_eval, ob_eval); ``` ```lines=10 ================================================================= ==18076==ERROR: AddressSanitizer: heap-use-after-free on address 0x60e00006e2b8 at pc 0x5646174bebb9 bp 0x7ffc2be66a40 sp 0x7ffc2be66a38 READ of size 8 at 0x60e00006e2b8 thread T0 - 0 0x5646174bebb8 in BKE_editmesh_free_derivedmesh /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/editmesh.c:148 - 1 0x5646182cad5f in discard_mesh_edit_mode_pointers /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1524 - 2 0x5646182cb070 in discard_edit_mode_pointers /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1546 - 3 0x5646182cb2f9 in DEG::deg_free_copy_on_write_datablock(ID*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1597 - 4 0x5646182ca7bc in DEG::deg_update_copy_on_write_datablock(DEG::Depsgraph const*, DEG::IDNode const*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1477 - 5 0x5646182cb5fc in DEG::deg_evaluate_copy_on_write(Depsgraph*, DEG::IDNode const*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1613 - 6 0x564618268307 in void std::__invoke_impl<void, void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>(std::__invoke_other, void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*&&, DEG::IDNode*&) /usr/include/c++/8/bits/invoke.h:60 - 7 0x564618263139 in std::__invoke_result<void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>::type std::__invoke<void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>(void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*&&, DEG::IDNode*&) /usr/include/c++/8/bits/invoke.h:95 - 8 0x56461825ce22 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::__call<void, Depsgraph*&&, 0ul, 1ul>(std::tuple<Depsgraph*&&>&&, std::_Index_tuple<0ul, 1ul>) /usr/include/c++/8/functional:400 - 9 0x564618254161 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::operator()<Depsgraph*, void>(Depsgraph*&&) /usr/include/c++/8/functional:484 - 10 0x564618247ead in std::_Function_handler<void (Depsgraph*), std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)> >::_M_invoke(std::_Any_data const&, Depsgraph*&&) /usr/include/c++/8/bits/std_function.h:297 - 11 0x5646182baf11 in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const /usr/include/c++/8/bits/std_function.h:687 - 12 0x5646182b81a2 in deg_task_run_func /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval.cc:86 - 13 0x5646181cc1ee in BLI_task_pool_work_and_wait /home/i74700deb64/blender/__work__/src/source/blender/blenlib/intern/task.c:939 - 14 0x5646181cce34 in BLI_task_pool_work_wait_and_reset /home/i74700deb64/blender/__work__/src/source/blender/blenlib/intern/task.c:969 - 15 0x5646182ba9b2 in DEG::deg_evaluate_on_refresh(DEG::Depsgraph*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval.cc:297 - 16 0x5646181ff86d in DEG_evaluate_on_refresh /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/depsgraph_eval.cc:63 - 17 0x564617a21548 in scene_graph_update_tagged /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/scene.c:1567 - 18 0x564617a215ef in BKE_scene_graph_update_tagged /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/scene.c:1582 - 19 0x564618e7ac7c in wm_event_do_depsgraph /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:369 - 20 0x564618e7afd1 in wm_event_do_refresh_wm_and_depsgraph /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:395 - 21 0x564618e7cf2c in wm_event_do_notifiers /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:560 - 22 0x564618e6388b in WM_main /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm.c:420 - 23 0x5646171071a9 in main /home/i74700deb64/blender/__work__/src/source/creator/creator.c:500 - 24 0x7f22c068809a in __libc_start_main ../csu/libc-start.c:308 - 25 0x564617106339 in _start (/home/i74700deb64/blender/__work__/build_master_debug/bin/blender+0x1b714339) 0x60e00006e2b8 is located 56 bytes inside of 152-byte region [0x60e00006e280,0x60e00006e318) freed by thread T0 here: - 0 0x7f22c66c01d7 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x1071d7) - 1 0x5646184605ed in MEM_lockfree_freeN /home/i74700deb64/blender/__work__/src/intern/guardedalloc/intern/mallocn_lockfree_impl.c:157 - 2 0x5646176b75cb in BKE_mesh_free /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/mesh.c:495 - 3 0x564617629d1e in BKE_libblock_free_datablock /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/library_remap.c:781 - 4 0x56461762a554 in BKE_id_free_ex /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/library_remap.c:951 - 5 0x56461762a6b1 in BKE_id_free /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/library_remap.c:995 - 6 0x56461b19a569 in object_force_modifier_update_for_bind /home/i74700deb64/blender/__work__/src/source/blender/editors/object/object_modifier.c:102 - 7 0x56461b19a93c in object_force_modifier_bind_simple_options /home/i74700deb64/blender/__work__/src/source/blender/editors/object/object_modifier.c:124 - 8 0x56461b1bac41 in laplaciandeform_bind_exec /home/i74700deb64/blender/__work__/src/source/blender/editors/object/object_modifier.c:2421 - 9 0x56461b1bb082 in laplaciandeform_bind_invoke /home/i74700deb64/blender/__work__/src/source/blender/editors/object/object_modifier.c:2441 - 10 0x564618e87671 in wm_operator_invoke /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:1438 - 11 0x564618e89867 in wm_operator_call_internal /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:1685 - 12 0x564618e899db in WM_operator_name_call_ptr /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:1699 - 13 0x56461a7af5f3 in ui_apply_but_funcs_after /home/i74700deb64/blender/__work__/src/source/blender/editors/interface/interface_handlers.c:835 - 14 0x56461a82e8be in ui_handler_region_menu /home/i74700deb64/blender/__work__/src/source/blender/editors/interface/interface_handlers.c:10182 - 15 0x564618e7d9fb in wm_handler_ui_call /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:623 - 16 0x564618e93d18 in wm_handlers_do_intern /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:2699 - 17 0x564618e9699b in wm_handlers_do /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:2945 - 18 0x564618e9b99e in wm_event_do_handlers /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:3323 - 19 0x564618e6387f in WM_main /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm.c:417 - 20 0x5646171071a9 in main /home/i74700deb64/blender/__work__/src/source/creator/creator.c:500 - 21 0x7f22c068809a in __libc_start_main ../csu/libc-start.c:308 previously allocated by thread T0 here: - 0 0x7f22c66c0588 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107588) - 1 0x564618461094 in MEM_lockfree_mallocN /home/i74700deb64/blender/__work__/src/intern/guardedalloc/intern/mallocn_lockfree_impl.c:308 - 2 0x5646184607c3 in MEM_lockfree_dupallocN /home/i74700deb64/blender/__work__/src/intern/guardedalloc/intern/mallocn_lockfree_impl.c:177 - 3 0x5646182c07be in update_mesh_edit_mode_pointers /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:625 - 4 0x5646182c0c81 in update_edit_mode_pointers /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:641 - 5 0x5646182c2415 in update_id_after_copy /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:805 - 6 0x5646182c2c1f in DEG::deg_expand_copy_on_write_datablock(DEG::Depsgraph const*, DEG::IDNode const*, DEG::DepsgraphNodeBuilder*, bool) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:912 - 7 0x5646182ca7dc in DEG::deg_update_copy_on_write_datablock(DEG::Depsgraph const*, DEG::IDNode const*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1478 - 8 0x5646182cb5fc in DEG::deg_evaluate_copy_on_write(Depsgraph*, DEG::IDNode const*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1613 - 9 0x564618268307 in void std::__invoke_impl<void, void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>(std::__invoke_other, void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*&&, DEG::IDNode*&) /usr/include/c++/8/bits/invoke.h:60 - 10 0x564618263139 in std::__invoke_result<void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>::type std::__invoke<void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*, DEG::IDNode*&>(void (*&)(Depsgraph*, DEG::IDNode const*), Depsgraph*&&, DEG::IDNode*&) /usr/include/c++/8/bits/invoke.h:95 - 11 0x56461825ce22 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::__call<void, Depsgraph*&&, 0ul, 1ul>(std::tuple<Depsgraph*&&>&&, std::_Index_tuple<0ul, 1ul>) /usr/include/c++/8/functional:400 - 12 0x564618254161 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::operator()<Depsgraph*, void>(Depsgraph*&&) /usr/include/c++/8/functional:484 - 13 0x564618247ead in std::_Function_handler<void (Depsgraph*), std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)> >::_M_invoke(std::_Any_data const&, Depsgraph*&&) /usr/include/c++/8/bits/std_function.h:297 - 14 0x5646182baf11 in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const /usr/include/c++/8/bits/std_function.h:687 - 15 0x5646182b81a2 in deg_task_run_func /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval.cc:86 - 16 0x5646181cc1ee in BLI_task_pool_work_and_wait /home/i74700deb64/blender/__work__/src/source/blender/blenlib/intern/task.c:939 - 17 0x5646181cce34 in BLI_task_pool_work_wait_and_reset /home/i74700deb64/blender/__work__/src/source/blender/blenlib/intern/task.c:969 - 18 0x5646182ba9b2 in DEG::deg_evaluate_on_refresh(DEG::Depsgraph*) /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/eval/deg_eval.cc:297 - 19 0x5646181ff86d in DEG_evaluate_on_refresh /home/i74700deb64/blender/__work__/src/source/blender/depsgraph/intern/depsgraph_eval.cc:63 - 20 0x564617a21548 in scene_graph_update_tagged /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/scene.c:1567 - 21 0x564617a215ef in BKE_scene_graph_update_tagged /home/i74700deb64/blender/__work__/src/source/blender/blenkernel/intern/scene.c:1582 - 22 0x564618e7ac7c in wm_event_do_depsgraph /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:369 - 23 0x564618e7afd1 in wm_event_do_refresh_wm_and_depsgraph /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:395 - 24 0x564618e7cf2c in wm_event_do_notifiers /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm_event_system.c:560 - 25 0x564618e6388b in WM_main /home/i74700deb64/blender/__work__/src/source/blender/windowmanager/intern/wm.c:420 - 26 0x5646171071a9 in main /home/i74700deb64/blender/__work__/src/source/creator/creator.c:500 - 27 0x7f22c068809a in __libc_start_main ../csu/libc-start.c:308 ```

Bastien Montagne commented

2019-07-26 14:54:05 +02:00

Added subscribers: @ideasman42, @brecht

Bastien Montagne commented

2019-07-26 14:54:05 +02:00

Remaining issue is that those binding modifier operators are allowed to run from edit mode, which is not sync with non-edit 'general' mesh data until we leave that edit mode, hence mesh won't have up-to-date data (like vgroups) while in Edit mode, and binding operation will fail or miss-behave. Do we really want to support binding in modifiers from Edit mode? @brecht, @ideasman42?

Sergey Sharybin commented

2019-07-26 15:01:27 +02:00

I don't think it is possible to reliably bind from edit mode.

As for the patch, from logic point of view it's fine. Ideally, need to introduce something like BKE_object_free_eval_mesh() which will set edit_mesh to NULL. Use this function here and from BKE_object_free_derived_caches().

I don't think it is possible to reliably bind from edit mode. As for the patch, from logic point of view it's fine. Ideally, need to introduce something like `BKE_object_free_eval_mesh()` which will set `edit_mesh` to `NULL`. Use this function here and from `BKE_object_free_derived_caches()`.

Brecht Van Lommel commented

2019-07-26 15:28:51 +02:00

It's fine to not support binding in edit mode.

Bastien Montagne commented

2019-07-26 16:02:24 +02:00

OK, so edited first patch to be more generic:

P1052: (An Untitled Masterwork)

diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h
index 54fbda1fa31..d6b4fa72281 100644
--- a/source/blender/blenkernel/BKE_mesh.h
+++ b/source/blender/blenkernel/BKE_mesh.h
@@ -123,6 +123,8 @@ struct Mesh *BKE_mesh_new_nomain_from_template(const struct Mesh *me_src,
                                                int loops_len,
                                                int polys_len);
 
+void BKE_mesh_eval_delete(struct Mesh *me_eval);
+
 /* Performs copy for use during evaluation,
  * optional referencing original arrays to reduce memory. */
 struct Mesh *BKE_mesh_copy_for_eval(struct Mesh *source, bool reference);
diff --git a/source/blender/blenkernel/intern/mesh.c b/source/blender/blenkernel/intern/mesh.c
index f5e93dcf9b7..9e01bfe62d6 100644
--- a/source/blender/blenkernel/intern/mesh.c
+++ b/source/blender/blenkernel/intern/mesh.c
@@ -697,6 +697,15 @@ Mesh *BKE_mesh_new_nomain_from_template(const Mesh *me_src,
       me_src, verts_len, edges_len, tessface_len, loops_len, polys_len, CD_MASK_EVERYTHING);
 }
 
+void BKE_mesh_eval_delete(struct Mesh *mesh_eval)
+{
+  /* Evaluated mesh may point to edit mesh, but never owns it. */
+  mesh_eval->edit_mesh = NULL;
+  BKE_mesh_free(mesh_eval);
+  BKE_libblock_free_data(&mesh_eval->id, false);
+  MEM_freeN(mesh_eval);
+}
+
 Mesh *BKE_mesh_copy_for_eval(struct Mesh *source, bool reference)
 {
   int flags = LIB_ID_COPY_LOCALIZE;
diff --git a/source/blender/blenkernel/intern/object.c b/source/blender/blenkernel/intern/object.c
index d7256cc9604..69ba7ccab8e 100644
--- a/source/blender/blenkernel/intern/object.c
+++ b/source/blender/blenkernel/intern/object.c
@@ -442,19 +442,13 @@ void BKE_object_free_derived_caches(Object *ob)
   if (ob->runtime.mesh_eval != NULL) {
     if (ob->runtime.is_mesh_eval_owned) {
       Mesh *mesh_eval = ob->runtime.mesh_eval;
-      /* Evaluated mesh points to edit mesh, but does not own it. */
-      mesh_eval->edit_mesh = NULL;
-      BKE_mesh_free(mesh_eval);
-      BKE_libblock_free_data(&mesh_eval->id, false);
-      MEM_freeN(mesh_eval);
+      BKE_mesh_eval_delete(mesh_eval);
     }
     ob->runtime.mesh_eval = NULL;
   }
   if (ob->runtime.mesh_deform_eval != NULL) {
     Mesh *mesh_deform_eval = ob->runtime.mesh_deform_eval;
-    BKE_mesh_free(mesh_deform_eval);
-    BKE_libblock_free_data(&mesh_deform_eval->id, false);
-    MEM_freeN(mesh_deform_eval);
+    BKE_mesh_eval_delete(mesh_deform_eval);
     ob->runtime.mesh_deform_eval = NULL;
   }
 
diff --git a/source/blender/editors/object/object_modifier.c b/source/blender/editors/object/object_modifier.c
index 08012842c37..e72bf1ea10a 100644
--- a/source/blender/editors/object/object_modifier.c
+++ b/source/blender/editors/object/object_modifier.c
@@ -99,7 +99,7 @@ static void object_force_modifier_update_for_bind(Depsgraph *depsgraph, Object *
   BKE_object_eval_reset(ob_eval);
   if (ob->type == OB_MESH) {
     Mesh *me_eval = mesh_create_eval_final_view(depsgraph, scene_eval, ob_eval, &CD_MASK_BAREMESH);
-    BKE_id_free(NULL, me_eval);
+    BKE_mesh_eval_delete(me_eval);
   }
   else if (ob->type == OB_LATTICE) {
     BKE_lattice_modifiers_calc(depsgraph, scene_eval, ob_eval);

And added some option to modifiers generic poll function to forbid Edit mode (some are fine with it, some were already forbidding it in their own way... Tired to be conservative here, only changing behaviors when expected one was clear and obvious):

P1054: (An Untitled Masterwork)

diff --git a/source/blender/editors/object/object_data_transfer.c b/source/blender/editors/object/object_data_transfer.c
index 690bc270605..465a60ba75c 100644
--- a/source/blender/editors/object/object_data_transfer.c
+++ b/source/blender/editors/object/object_data_transfer.c
@@ -760,7 +760,7 @@ void OBJECT_OT_data_transfer(wmOperatorType *ot)
 
 static bool datalayout_transfer_poll(bContext *C)
 {
-  return (edit_modifier_poll_generic(C, &RNA_DataTransferModifier, (1 << OB_MESH)) ||
+  return (edit_modifier_poll_generic(C, &RNA_DataTransferModifier, (1 << OB_MESH), true) ||
           data_transfer_poll(C));
 }
 
diff --git a/source/blender/editors/object/object_intern.h b/source/blender/editors/object/object_intern.h
index b9350052093..e697c25b37f 100644
--- a/source/blender/editors/object/object_intern.h
+++ b/source/blender/editors/object/object_intern.h
@@ -137,7 +137,10 @@ void COLLECTION_OT_objects_add_active(struct wmOperatorType *ot);
 void COLLECTION_OT_objects_remove_active(struct wmOperatorType *ot);
 
 /* object_modifier.c */
-bool edit_modifier_poll_generic(struct bContext *C, struct StructRNA *rna_type, int obtype_flag);
+bool edit_modifier_poll_generic(struct bContext *C,
+                                struct StructRNA *rna_type,
+                                int obtype_flag,
+                                const bool is_editmode_allowed);
 bool edit_modifier_poll(struct bContext *C);
 void edit_modifier_properties(struct wmOperatorType *ot);
 int edit_modifier_invoke_properties(struct bContext *C, struct wmOperator *op);
diff --git a/source/blender/editors/object/object_modifier.c b/source/blender/editors/object/object_modifier.c
index 08012842c37..2a535dbac6a 100644
--- a/source/blender/editors/object/object_modifier.c
+++ b/source/blender/editors/object/object_modifier.c
@@ -912,7 +912,10 @@ void OBJECT_OT_modifier_add(wmOperatorType *ot)
 
 /********** generic functions for operators using mod names and data context *********************/
 
-bool edit_modifier_poll_generic(bContext *C, StructRNA *rna_type, int obtype_flag)
+bool edit_modifier_poll_generic(bContext *C,
+                                StructRNA *rna_type,
+                                int obtype_flag,
+                                const bool is_editmode_allowed)
 {
   PointerRNA ptr = CTX_data_pointer_get_type(C, "modifier", rna_type);
   Object *ob = (ptr.id.data) ? ptr.id.data : ED_object_active_context(C);
@@ -932,12 +935,17 @@ bool edit_modifier_poll_generic(bContext *C, StructRNA *rna_type, int obtype_fla
     return (((ModifierData *)ptr.data)->flag & eModifierFlag_OverrideLibrary_Local) != 0;
   }
 
+  if (!is_editmode_allowed && CTX_data_edit_object(C) != NULL) {
+    CTX_wm_operator_poll_msg_set(C, "This modifier operation is not allowed from Edit mode");
+    return 0;
+  }
+
   return 1;
 }
 
 bool edit_modifier_poll(bContext *C)
 {
-  return edit_modifier_poll_generic(C, &RNA_Modifier, 0);
+  return edit_modifier_poll_generic(C, &RNA_Modifier, 0, true);
 }
 
 void edit_modifier_properties(wmOperatorType *ot)
@@ -1274,7 +1282,7 @@ void OBJECT_OT_modifier_copy(wmOperatorType *ot)
 
 static bool multires_poll(bContext *C)
 {
-  return edit_modifier_poll_generic(C, &RNA_MultiresModifier, (1 << OB_MESH));
+  return edit_modifier_poll_generic(C, &RNA_MultiresModifier, (1 << OB_MESH), true);
 }
 
 static int multires_higher_levels_delete_exec(bContext *C, wmOperator *op)
@@ -1627,14 +1635,13 @@ static void modifier_skin_customdata_delete(Object *ob)
 
 static bool skin_poll(bContext *C)
 {
-  return (!CTX_data_edit_object(C) &&
-          edit_modifier_poll_generic(C, &RNA_SkinModifier, (1 << OB_MESH)));
+  return (edit_modifier_poll_generic(C, &RNA_SkinModifier, (1 << OB_MESH), false));
 }
 
 static bool skin_edit_poll(bContext *C)
 {
   return (CTX_data_edit_object(C) &&
-          edit_modifier_poll_generic(C, &RNA_SkinModifier, (1 << OB_MESH)));
+          edit_modifier_poll_generic(C, &RNA_SkinModifier, (1 << OB_MESH), true));
 }
 
 static void skin_root_clear(BMVert *bm_vert, GSet *visited, const int cd_vert_skin_offset)
@@ -1987,7 +1994,7 @@ void OBJECT_OT_skin_armature_create(wmOperatorType *ot)
 
 static bool correctivesmooth_poll(bContext *C)
 {
-  return edit_modifier_poll_generic(C, &RNA_CorrectiveSmoothModifier, 0);
+  return edit_modifier_poll_generic(C, &RNA_CorrectiveSmoothModifier, 0, true);
 }
 
 static int correctivesmooth_bind_exec(bContext *C, wmOperator *op)
@@ -2065,7 +2072,7 @@ void OBJECT_OT_correctivesmooth_bind(wmOperatorType *ot)
 
 static bool meshdeform_poll(bContext *C)
 {
-  return edit_modifier_poll_generic(C, &RNA_MeshDeformModifier, 0);
+  return edit_modifier_poll_generic(C, &RNA_MeshDeformModifier, 0, true);
 }
 
 static int meshdeform_bind_exec(bContext *C, wmOperator *op)
@@ -2138,7 +2145,7 @@ void OBJECT_OT_meshdeform_bind(wmOperatorType *ot)
 
 static bool explode_poll(bContext *C)
 {
-  return edit_modifier_poll_generic(C, &RNA_ExplodeModifier, 0);
+  return edit_modifier_poll_generic(C, &RNA_ExplodeModifier, 0, true);
 }
 
 static int explode_refresh_exec(bContext *C, wmOperator *op)
@@ -2188,7 +2195,7 @@ void OBJECT_OT_explode_refresh(wmOperatorType *ot)
 
 static bool ocean_bake_poll(bContext *C)
 {
-  return edit_modifier_poll_generic(C, &RNA_OceanModifier, 0);
+  return edit_modifier_poll_generic(C, &RNA_OceanModifier, 0, true);
 }
 
 typedef struct OceanBakeJob {
@@ -2389,7 +2396,7 @@ void OBJECT_OT_ocean_bake(wmOperatorType *ot)
 
 static bool laplaciandeform_poll(bContext *C)
 {
-  return edit_modifier_poll_generic(C, &RNA_LaplacianDeformModifier, 0);
+  return edit_modifier_poll_generic(C, &RNA_LaplacianDeformModifier, 0, false);
 }
 
 static int laplaciandeform_bind_exec(bContext *C, wmOperator *op)
@@ -2464,7 +2471,7 @@ void OBJECT_OT_laplaciandeform_bind(wmOperatorType *ot)
 
 static bool surfacedeform_bind_poll(bContext *C)
 {
-  return edit_modifier_poll_generic(C, &RNA_SurfaceDeformModifier, 0);
+  return edit_modifier_poll_generic(C, &RNA_SurfaceDeformModifier, 0, true);
 }
 
 static int surfacedeform_bind_exec(bContext *C, wmOperator *op)

OK, so edited first patch to be more generic: [P1052: (An Untitled Masterwork)](https://archive.blender.org/developer/P1052.txt) ``` diff --git a/source/blender/blenkernel/BKE_mesh.h b/source/blender/blenkernel/BKE_mesh.h index 54fbda1fa31..d6b4fa72281 100644 --- a/source/blender/blenkernel/BKE_mesh.h +++ b/source/blender/blenkernel/BKE_mesh.h @@ -123,6 +123,8 @@ struct Mesh *BKE_mesh_new_nomain_from_template(const struct Mesh *me_src, int loops_len, int polys_len); +void BKE_mesh_eval_delete(struct Mesh *me_eval); + /* Performs copy for use during evaluation, * optional referencing original arrays to reduce memory. */ struct Mesh *BKE_mesh_copy_for_eval(struct Mesh *source, bool reference); diff --git a/source/blender/blenkernel/intern/mesh.c b/source/blender/blenkernel/intern/mesh.c index f5e93dcf9b7..9e01bfe62d6 100644 --- a/source/blender/blenkernel/intern/mesh.c +++ b/source/blender/blenkernel/intern/mesh.c @@ -697,6 +697,15 @@ Mesh *BKE_mesh_new_nomain_from_template(const Mesh *me_src, me_src, verts_len, edges_len, tessface_len, loops_len, polys_len, CD_MASK_EVERYTHING); } +void BKE_mesh_eval_delete(struct Mesh *mesh_eval) +{ + /* Evaluated mesh may point to edit mesh, but never owns it. */ + mesh_eval->edit_mesh = NULL; + BKE_mesh_free(mesh_eval); + BKE_libblock_free_data(&mesh_eval->id, false); + MEM_freeN(mesh_eval); +} + Mesh *BKE_mesh_copy_for_eval(struct Mesh *source, bool reference) { int flags = LIB_ID_COPY_LOCALIZE; diff --git a/source/blender/blenkernel/intern/object.c b/source/blender/blenkernel/intern/object.c index d7256cc9604..69ba7ccab8e 100644 --- a/source/blender/blenkernel/intern/object.c +++ b/source/blender/blenkernel/intern/object.c @@ -442,19 +442,13 @@ void BKE_object_free_derived_caches(Object *ob) if (ob->runtime.mesh_eval != NULL) { if (ob->runtime.is_mesh_eval_owned) { Mesh *mesh_eval = ob->runtime.mesh_eval; - /* Evaluated mesh points to edit mesh, but does not own it. */ - mesh_eval->edit_mesh = NULL; - BKE_mesh_free(mesh_eval); - BKE_libblock_free_data(&mesh_eval->id, false); - MEM_freeN(mesh_eval); + BKE_mesh_eval_delete(mesh_eval); } ob->runtime.mesh_eval = NULL; } if (ob->runtime.mesh_deform_eval != NULL) { Mesh *mesh_deform_eval = ob->runtime.mesh_deform_eval; - BKE_mesh_free(mesh_deform_eval); - BKE_libblock_free_data(&mesh_deform_eval->id, false); - MEM_freeN(mesh_deform_eval); + BKE_mesh_eval_delete(mesh_deform_eval); ob->runtime.mesh_deform_eval = NULL; } diff --git a/source/blender/editors/object/object_modifier.c b/source/blender/editors/object/object_modifier.c index 08012842c37..e72bf1ea10a 100644 --- a/source/blender/editors/object/object_modifier.c +++ b/source/blender/editors/object/object_modifier.c @@ -99,7 +99,7 @@ static void object_force_modifier_update_for_bind(Depsgraph *depsgraph, Object * BKE_object_eval_reset(ob_eval); if (ob->type == OB_MESH) { Mesh *me_eval = mesh_create_eval_final_view(depsgraph, scene_eval, ob_eval, &CD_MASK_BAREMESH); - BKE_id_free(NULL, me_eval); + BKE_mesh_eval_delete(me_eval); } else if (ob->type == OB_LATTICE) { BKE_lattice_modifiers_calc(depsgraph, scene_eval, ob_eval); ``` And added some option to modifiers generic poll function to forbid Edit mode (some are fine with it, some were already forbidding it in their own way... Tired to be conservative here, only changing behaviors when expected one was clear and obvious): [P1054: (An Untitled Masterwork)](https://archive.blender.org/developer/P1054.txt) ``` diff --git a/source/blender/editors/object/object_data_transfer.c b/source/blender/editors/object/object_data_transfer.c index 690bc270605..465a60ba75c 100644 --- a/source/blender/editors/object/object_data_transfer.c +++ b/source/blender/editors/object/object_data_transfer.c @@ -760,7 +760,7 @@ void OBJECT_OT_data_transfer(wmOperatorType *ot) static bool datalayout_transfer_poll(bContext *C) { - return (edit_modifier_poll_generic(C, &RNA_DataTransferModifier, (1 << OB_MESH)) || + return (edit_modifier_poll_generic(C, &RNA_DataTransferModifier, (1 << OB_MESH), true) || data_transfer_poll(C)); } diff --git a/source/blender/editors/object/object_intern.h b/source/blender/editors/object/object_intern.h index b9350052093..e697c25b37f 100644 --- a/source/blender/editors/object/object_intern.h +++ b/source/blender/editors/object/object_intern.h @@ -137,7 +137,10 @@ void COLLECTION_OT_objects_add_active(struct wmOperatorType *ot); void COLLECTION_OT_objects_remove_active(struct wmOperatorType *ot); /* object_modifier.c */ -bool edit_modifier_poll_generic(struct bContext *C, struct StructRNA *rna_type, int obtype_flag); +bool edit_modifier_poll_generic(struct bContext *C, + struct StructRNA *rna_type, + int obtype_flag, + const bool is_editmode_allowed); bool edit_modifier_poll(struct bContext *C); void edit_modifier_properties(struct wmOperatorType *ot); int edit_modifier_invoke_properties(struct bContext *C, struct wmOperator *op); diff --git a/source/blender/editors/object/object_modifier.c b/source/blender/editors/object/object_modifier.c index 08012842c37..2a535dbac6a 100644 --- a/source/blender/editors/object/object_modifier.c +++ b/source/blender/editors/object/object_modifier.c @@ -912,7 +912,10 @@ void OBJECT_OT_modifier_add(wmOperatorType *ot) /********** generic functions for operators using mod names and data context *********************/ -bool edit_modifier_poll_generic(bContext *C, StructRNA *rna_type, int obtype_flag) +bool edit_modifier_poll_generic(bContext *C, + StructRNA *rna_type, + int obtype_flag, + const bool is_editmode_allowed) { PointerRNA ptr = CTX_data_pointer_get_type(C, "modifier", rna_type); Object *ob = (ptr.id.data) ? ptr.id.data : ED_object_active_context(C); @@ -932,12 +935,17 @@ bool edit_modifier_poll_generic(bContext *C, StructRNA *rna_type, int obtype_fla return (((ModifierData *)ptr.data)->flag & eModifierFlag_OverrideLibrary_Local) != 0; } + if (!is_editmode_allowed && CTX_data_edit_object(C) != NULL) { + CTX_wm_operator_poll_msg_set(C, "This modifier operation is not allowed from Edit mode"); + return 0; + } + return 1; } bool edit_modifier_poll(bContext *C) { - return edit_modifier_poll_generic(C, &RNA_Modifier, 0); + return edit_modifier_poll_generic(C, &RNA_Modifier, 0, true); } void edit_modifier_properties(wmOperatorType *ot) @@ -1274,7 +1282,7 @@ void OBJECT_OT_modifier_copy(wmOperatorType *ot) static bool multires_poll(bContext *C) { - return edit_modifier_poll_generic(C, &RNA_MultiresModifier, (1 << OB_MESH)); + return edit_modifier_poll_generic(C, &RNA_MultiresModifier, (1 << OB_MESH), true); } static int multires_higher_levels_delete_exec(bContext *C, wmOperator *op) @@ -1627,14 +1635,13 @@ static void modifier_skin_customdata_delete(Object *ob) static bool skin_poll(bContext *C) { - return (!CTX_data_edit_object(C) && - edit_modifier_poll_generic(C, &RNA_SkinModifier, (1 << OB_MESH))); + return (edit_modifier_poll_generic(C, &RNA_SkinModifier, (1 << OB_MESH), false)); } static bool skin_edit_poll(bContext *C) { return (CTX_data_edit_object(C) && - edit_modifier_poll_generic(C, &RNA_SkinModifier, (1 << OB_MESH))); + edit_modifier_poll_generic(C, &RNA_SkinModifier, (1 << OB_MESH), true)); } static void skin_root_clear(BMVert *bm_vert, GSet *visited, const int cd_vert_skin_offset) @@ -1987,7 +1994,7 @@ void OBJECT_OT_skin_armature_create(wmOperatorType *ot) static bool correctivesmooth_poll(bContext *C) { - return edit_modifier_poll_generic(C, &RNA_CorrectiveSmoothModifier, 0); + return edit_modifier_poll_generic(C, &RNA_CorrectiveSmoothModifier, 0, true); } static int correctivesmooth_bind_exec(bContext *C, wmOperator *op) @@ -2065,7 +2072,7 @@ void OBJECT_OT_correctivesmooth_bind(wmOperatorType *ot) static bool meshdeform_poll(bContext *C) { - return edit_modifier_poll_generic(C, &RNA_MeshDeformModifier, 0); + return edit_modifier_poll_generic(C, &RNA_MeshDeformModifier, 0, true); } static int meshdeform_bind_exec(bContext *C, wmOperator *op) @@ -2138,7 +2145,7 @@ void OBJECT_OT_meshdeform_bind(wmOperatorType *ot) static bool explode_poll(bContext *C) { - return edit_modifier_poll_generic(C, &RNA_ExplodeModifier, 0); + return edit_modifier_poll_generic(C, &RNA_ExplodeModifier, 0, true); } static int explode_refresh_exec(bContext *C, wmOperator *op) @@ -2188,7 +2195,7 @@ void OBJECT_OT_explode_refresh(wmOperatorType *ot) static bool ocean_bake_poll(bContext *C) { - return edit_modifier_poll_generic(C, &RNA_OceanModifier, 0); + return edit_modifier_poll_generic(C, &RNA_OceanModifier, 0, true); } typedef struct OceanBakeJob { @@ -2389,7 +2396,7 @@ void OBJECT_OT_ocean_bake(wmOperatorType *ot) static bool laplaciandeform_poll(bContext *C) { - return edit_modifier_poll_generic(C, &RNA_LaplacianDeformModifier, 0); + return edit_modifier_poll_generic(C, &RNA_LaplacianDeformModifier, 0, false); } static int laplaciandeform_bind_exec(bContext *C, wmOperator *op) @@ -2464,7 +2471,7 @@ void OBJECT_OT_laplaciandeform_bind(wmOperatorType *ot) static bool surfacedeform_bind_poll(bContext *C) { - return edit_modifier_poll_generic(C, &RNA_SurfaceDeformModifier, 0); + return edit_modifier_poll_generic(C, &RNA_SurfaceDeformModifier, 0, true); } static int surfacedeform_bind_exec(bContext *C, wmOperator *op) ```

blender-admin commented

2019-07-28 15:12:38 +02:00

This issue was referenced by 3e45f01494

This issue was referenced by 3e45f01494bcadc0f4d644f3c7d4b936b6553c9b

Bastien Montagne commented

2019-07-28 16:12:03 +02:00

Changed status from 'Open' to: 'Resolved'

Bastien Montagne closed this issue

2019-07-28 16:12:03 +02:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Bind in Laplacian Deform Modifier with new vertex group in Edit Mode crashes Blender #67385