Page MenuHome
Differential D4954

Fix T58251: Cycles ignores linked meshes when rendering
ClosedPublic
Actions

Authored by Sergey Sharybin (sergey) on May 27 2019, 11:49 AM.

Details

Reviewers
Brecht Van Lommel (brecht)
Bastien Montagne (mont29)
Commits
rBS2f7711962a69: Fix T58251: Cycles ignores linked meshes when rendering
rB2f7711962a69: Fix T58251: Cycles ignores linked meshes when rendering
Summary

The idea is to share a mesh data-block as a result across all objects which are sharing same original mesh and have
no effective modifiers. This mesh is owned by an original copy-on-written version of object data.

Tricky part is to make sure it is only initialized once, and currently a silly mutex lock is used. In practice it only locks if the mesh is not already there.




As an extra bonus, even viewport memory is also lower after this change.
Diff Detail
Repository 
rB Blender 
Event Timeline
Sergey Sharybin (sergey) created this revision.May 27 2019, 11:49 AM
Harbormaster completed remote builds in B3740: Diff 15624.May 27 2019, 11:49 AM
Brecht Van Lommel (brecht) added a comment.May 27 2019, 2:35 PM
The orco case is fine.



For all the stuff that happens in mesh_calc_modifier_final_normals() I doubt it's reliable though. Cycles will probably recompute any normals or split normals as needed. Viewport display or modifiers using this as a target object seems likely to have issues?
Sergey Sharybin (sergey) planned changes to this revision.May 27 2019, 2:59 PM
@Brecht Van Lommel (brecht) yeah, after reading that function closer i don't know why my quick tests in viewport even worked.

Need to find a better way. Or just limit the fix to just Cycles side by adjusting the mesh key, but that feels somewhat weak..
Bastien Montagne (mont29) added a comment.EditedMay 27 2019, 3:17 PM
hrrrmmmm… I would expect meshes with custom normals to fail completely in that case?



Wouldn’t it be possible to have e.g.  a flag (that could be set atomically) in the mesh struct, to be checked at the end of the process by eval code to perform those normal computations (and possibly other needed data in the future)?



[EDIT] Meeeh… am always late! :(
Sergey Sharybin (sergey) updated this revision to Diff 15652.May 28 2019, 2:29 PM
Make sure the normals are properly evaluated.
Harbormaster completed remote builds in B3753: Diff 15652.May 28 2019, 2:29 PM
Sergey Sharybin (sergey) edited the summary of this revision. (Show Details)May 28 2019, 2:30 PM
Sergey Sharybin (sergey) mentioned this in T65238: Latest depsgraph changes, now Object.to_mesh(preserve_all_data_layers=True, depsgraph=depsgraph) results in mesh with no materials.May 28 2019, 4:45 PM
Bastien Montagne (mont29) added a comment.May 28 2019, 5:07 PM
Besides points noted below, LGTM.
source/blender/blenkernel/intern/DerivedMesh.c


1598–1600
This can be put above (in block defined by lines 1579-1581) I think? ;)


source/blender/blenkernel/intern/mesh_runtime.c


79–84
Pretty sure pointers should be set to NULL here?


80
Aren’t you supposed to call BLI_mutex_end() first?
Sergey Sharybin (sergey) added inline comments.May 28 2019, 5:15 PM
source/blender/blenkernel/intern/DerivedMesh.c


1598–1600
Probably, but then it's not "Compute normals" anymore. However, the block inside of mutex is already not really just-a-normals =\


source/blender/blenkernel/intern/mesh_runtime.c


79–84
Not strictly speaking necessary i guess: the function is more like BKE_mesh_runtime_free. But think it's indeed better to zero stuff out.



Not sure whether i should bother with clearing rest of the pointers though.


80
That is a very good point!
Sergey Sharybin (sergey) updated this revision to Diff 15660.May 28 2019, 5:16 PM
Sergey Sharybin (sergey) edited the summary of this revision. (Show Details)
Address the mutex point from Bastien.



The finalization i am on a split. Kind of like the idea of

having more dedicated steps of evaluation, but this shared

ownership is already adding a mess in the general code flow =\
Harbormaster completed remote builds in B3758: Diff 15660.May 28 2019, 5:16 PM
Brecht Van Lommel (brecht) requested changes to this revision.May 28 2019, 5:24 PM
Crashes for me when transforming linked duplicates in edit mode.


duplicate_linked_edit.blend667 KBDownload



P986 (An Untitled Masterwork)
1=================================================================

2==31605==ERROR: AddressSanitizer: heap-use-after-free on address 0x60e0000073c0 at pc 0x55556849e29b bp 0x7fff870073f0 sp 0x7fff870073e0

3READ of size 8 at 0x60e0000073c0 thread T119

4    #0 0x55556849e29a in MEM_lockfree_allocN_len /home/brecht/dev/worktree/intern/guardedalloc/intern/mallocn_lockfree_impl.c:113

5    #1 0x55556849e2e0 in MEM_lockfree_freeN /home/brecht/dev/worktree/intern/guardedalloc/intern/mallocn_lockfree_impl.c:123

6    #2 0x555567a0e9f9 in BKE_mesh_free /home/brecht/dev/worktree/source/blender/blenkernel/intern/mesh.c:494

7    #3 0x5555679a39ad in BKE_libblock_free_datablock /home/brecht/dev/worktree/source/blender/blenkernel/intern/library_remap.c:781

8    #4 0x5555679a4119 in BKE_id_free_ex /home/brecht/dev/worktree/source/blender/blenkernel/intern/library_remap.c:951

9    #5 0x5555679a4276 in BKE_id_free /home/brecht/dev/worktree/source/blender/blenkernel/intern/library_remap.c:995

10    #6 0x555567a5ca59 in BKE_mesh_runtime_clear_cache /home/brecht/dev/worktree/source/blender/blenkernel/intern/mesh_runtime.c:85

11    #7 0x555567a0e45d in BKE_mesh_free /home/brecht/dev/worktree/source/blender/blenkernel/intern/mesh.c:483

12    #8 0x5555679a39ad in BKE_libblock_free_datablock /home/brecht/dev/worktree/source/blender/blenkernel/intern/library_remap.c:781

13    #9 0x555568365d8e in DEG::deg_free_copy_on_write_datablock(ID*) /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1277

14    #10 0x55556836547c in DEG::deg_update_copy_on_write_datablock(DEG::Depsgraph const*, DEG::IDNode const*) /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1156

15    #11 0x555568365f6e in DEG::deg_evaluate_copy_on_write(Depsgraph*, DEG::IDNode const*) /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1292

16    #12 0x55556831a58f 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*&) (/home/brecht/dev/worktree_debug/bin/blender+0x12dc658f)

17    #13 0x5555683160ad 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*&) (/home/brecht/dev/worktree_debug/bin/blender+0x12dc20ad)

18    #14 0x5555683113ec 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>) (/home/brecht/dev/worktree_debug/bin/blender+0x12dbd3ec)

19    #15 0x5555683090b2 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::operator()<Depsgraph*, void>(Depsgraph*&&) (/home/brecht/dev/worktree_debug/bin/blender+0x12db50b2)

20    #16 0x5555682fe0c7 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*&&) (/home/brecht/dev/worktree_debug/bin/blender+0x12daa0c7)

21    #17 0x55556835c1da in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const (/home/brecht/dev/worktree_debug/bin/blender+0x12e081da)

22    #18 0x555568359d1d in deg_task_run_func /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval.cc:86

23    #19 0x555568293dfc in task_scheduler_thread_run /home/brecht/dev/worktree/source/blender/blenlib/intern/task.c:450

24    #20 0x7ffff6c026da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)

25    #21 0x7ffff573a88e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0x12188e)

26

270x60e0000073c0 is located 0 bytes inside of 152-byte region [0x60e0000073c0,0x60e000007458)

28freed by thread T119 here:

29    #0 0x7ffff6ef87b8 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xde7b8)

30    #1 0x55556849e6ec in MEM_lockfree_freeN /home/brecht/dev/worktree/intern/guardedalloc/intern/mallocn_lockfree_impl.c:157

31    #2 0x5555683659c5 in discard_mesh_edit_mode_pointers /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1204

32    #3 0x555568365b8a in discard_edit_mode_pointers /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1225

33    #4 0x555568365d6a in DEG::deg_free_copy_on_write_datablock(ID*) /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1276

34    #5 0x55556836547c in DEG::deg_update_copy_on_write_datablock(DEG::Depsgraph const*, DEG::IDNode const*) /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1156

35    #6 0x555568365f6e in DEG::deg_evaluate_copy_on_write(Depsgraph*, DEG::IDNode const*) /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1292

36    #7 0x55556831a58f 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*&) (/home/brecht/dev/worktree_debug/bin/blender+0x12dc658f)

37    #8 0x5555683160ad 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*&) (/home/brecht/dev/worktree_debug/bin/blender+0x12dc20ad)

38    #9 0x5555683113ec 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>) (/home/brecht/dev/worktree_debug/bin/blender+0x12dbd3ec)

39    #10 0x5555683090b2 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::operator()<Depsgraph*, void>(Depsgraph*&&) (/home/brecht/dev/worktree_debug/bin/blender+0x12db50b2)

40    #11 0x5555682fe0c7 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*&&) (/home/brecht/dev/worktree_debug/bin/blender+0x12daa0c7)

41    #12 0x55556835c1da in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const (/home/brecht/dev/worktree_debug/bin/blender+0x12e081da)

42    #13 0x555568359d1d in deg_task_run_func /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval.cc:86

43    #14 0x555568293dfc in task_scheduler_thread_run /home/brecht/dev/worktree/source/blender/blenlib/intern/task.c:450

44    #15 0x7ffff6c026da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)

45

46previously allocated by thread T116 here:

47    #0 0x7ffff6ef8b50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50)

48    #1 0x55556849f066 in MEM_lockfree_mallocN /home/brecht/dev/worktree/intern/guardedalloc/intern/mallocn_lockfree_impl.c:308

49    #2 0x55556849e87d in MEM_lockfree_dupallocN /home/brecht/dev/worktree/intern/guardedalloc/intern/mallocn_lockfree_impl.c:177

50    #3 0x55556836023e in update_mesh_edit_mode_pointers /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:599

51    #4 0x555568360572 in update_edit_mode_pointers /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:615

52    #5 0x555568361351 in update_id_after_copy /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:759

53    #6 0x555568361a6a in DEG::deg_expand_copy_on_write_datablock(DEG::Depsgraph const*, DEG::IDNode const*, DEG::DepsgraphNodeBuilder*, bool) /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:866

54    #7 0x55556836549c in DEG::deg_update_copy_on_write_datablock(DEG::Depsgraph const*, DEG::IDNode const*) /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1157

55    #8 0x555568365f6e in DEG::deg_evaluate_copy_on_write(Depsgraph*, DEG::IDNode const*) /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc:1292

56    #9 0x55556831a58f 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*&) (/home/brecht/dev/worktree_debug/bin/blender+0x12dc658f)

57    #10 0x5555683160ad 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*&) (/home/brecht/dev/worktree_debug/bin/blender+0x12dc20ad)

58    #11 0x5555683113ec 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>) (/home/brecht/dev/worktree_debug/bin/blender+0x12dbd3ec)

59    #12 0x5555683090b2 in void std::_Bind<void (*(std::_Placeholder<1>, DEG::IDNode*))(Depsgraph*, DEG::IDNode const*)>::operator()<Depsgraph*, void>(Depsgraph*&&) (/home/brecht/dev/worktree_debug/bin/blender+0x12db50b2)

60    #13 0x5555682fe0c7 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*&&) (/home/brecht/dev/worktree_debug/bin/blender+0x12daa0c7)

61    #14 0x55556835c1da in std::function<void (Depsgraph*)>::operator()(Depsgraph*) const (/home/brecht/dev/worktree_debug/bin/blender+0x12e081da)

62    #15 0x555568359d1d in deg_task_run_func /home/brecht/dev/worktree/source/blender/depsgraph/intern/eval/deg_eval.cc:86

63    #16 0x555568293dfc in task_scheduler_thread_run /home/brecht/dev/worktree/source/blender/blenlib/intern/task.c:450

64    #17 0x7ffff6c026da in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76da)

This revision now requires changes to proceed.May 28 2019, 5:24 PM
Sergey Sharybin (sergey) updated this revision to Diff 15661.May 28 2019, 5:30 PM
Brecht, wow, totally missed that.



Should be addressed now.
Harbormaster completed remote builds in B3759: Diff 15661.May 28 2019, 5:30 PM
Brecht Van Lommel (brecht) accepted this revision.May 28 2019, 5:39 PM
This revision is now accepted and ready to land.May 28 2019, 5:39 PM
Bastien Montagne (mont29) accepted this revision.May 28 2019, 9:04 PM
Closed by commit rB2f7711962a69: Fix T58251: Cycles ignores linked meshes when rendering (authored by Sergey Sharybin (sergey)).  ·  Explain WhyMay 29 2019, 10:51 AM
This revision was automatically updated to reflect the committed changes.
Revision Contents
Changeset List
PathSize
source/
blender/
blenkernel/
intern/
117 lines
2 lines
17 lines
18 lines
makesdna/
6 lines
4 lines
Diff 15672
View Options
source/blender/blenkernel/intern/DerivedMesh.c
Loading...
View Options
source/blender/blenkernel/intern/mesh.c
Loading...
View Options
source/blender/blenkernel/intern/mesh_runtime.c
Loading...
View Options
source/blender/blenkernel/intern/object.c
Loading...
View Options
source/blender/makesdna/DNA_mesh_types.h
Loading...
View Options
source/blender/makesdna/DNA_object_types.h
Loading...