Cleanup id->newid usage, initial work.
ClosedPublic
Actions

Authored by Bastien Montagne (mont29) on Nov 22 2016, 3:25 PM.

Details

Reviewers

Sergey Sharybin (sergey)
Dalai Felinto (dfelinto)

Summary

This aims at always ensuring that ID.newid (and relevant LIB_TAG_NEW)
stay in clean (i.e. cleared) state by default.

To achieve this, instead of clearing after all id copy call (would be
horribly noisy, and bad for performances), we try to completely remove
the setting of id->newid by default when copying a new ID.

This implies that areas actually needing that info (mainly, object editing
area (make single user...) and make local area) have to ensure they set
it themselves as needed.

This is far from simple change, many complex code paths to consider, so
will need some serious testing. :/

Diff Detail

Repository

rB Blender

Branch

id_newid_optional

Build Status

Buildable 300
Build 300: arc lint + arc unit

Event Timeline

Bastien Montagne (mont29) updated this revision to Diff 7871.Nov 22 2016, 3:25 PM

Bastien Montagne (mont29) retitled this revision from to Cleanup id->newid usage, initial work..

Bastien Montagne (mont29) added reviewers: Sergey Sharybin (sergey), Dalai Felinto (dfelinto).

Bastien Montagne (mont29) updated this object.

Note: put this patch here to at least get agreement about its principles (not set newid anymore for all ID copying, only code using it needs to set it explicitly).

That way, newid officially becomes kind of 'general usage temp pointer' in ID (acknowledging how it was already used by sequencer copy/paste, depsgraph, outliner, etc.).

Code itself seems to be working OK, but ensuring all cases are correctly handled (especially in the single user/copy-add object/etc. areas of Editor code) is rather hairy… :/

Approach seems good.

Didn't went through all the details of each individual changes tho.

This revision is now accepted and ready to land.Nov 22 2016, 4:58 PM

I still get a crash when using the new depsgraph with this patch:

=================================================================
==11092==ERROR: AddressSanitizer: SEGV on unknown address 0x7f320b1ca90c (pc 0x562d4b804714 bp 0x7ffeb2ff63b0 sp 0x7ffeb2ff63a0 T0)
    #0 0x562d4b804713 in BKE_id_clear_newpoin /home/guest/src/blender/blender/source/blender/blenkernel/intern/library.c:269
    #1 0x562d4bc2afe3 in rna_ID_make_local /home/guest/src/blender/blender/source/blender/makesrna/intern/rna_ID.c:362
    #2 0x562d4bc30054 in ID_make_local_call /home/guest/src/blender/release/master/source/blender/makesrna/intern/rna_ID_gen.c:757
    #3 0x562d4bc250aa in RNA_function_call /home/guest/src/blender/blender/source/blender/makesrna/intern/rna_access.c:6217
    #4 0x562d4a91912e in pyrna_func_call /home/guest/src/blender/blender/source/blender/python/intern/bpy_rna.c:5549
    #5 0x7f4b232fe446 in PyObject_Call (/usr/lib/x86_64-linux-gnu/libpython3.5m.so.1.0+0x1d2446)

Just for the records, the crash I get in master is:

ASAN:DEADLYSIGNAL
=================================================================
==21361==ERROR: AddressSanitizer: SEGV on unknown address 0x7f3259026578 (pc 0x5582598f669b bp 0x7fffd0a81df0 sp 0x7fffd0a81dd0 T0)
    #0 0x5582598f669a in rna_ID_refine /home/guest/src/blender/blender/source/blender/makesrna/intern/rna_ID.c:223
    #1 0x5582598ca486 in RNA_id_pointer_create /home/guest/src/blender/blender/source/blender/makesrna/intern/rna_access.c:121
    #2 0x5582585e4549 in pyrna_param_to_py /home/guest/src/blender/blender/source/blender/python/intern/bpy_rna.c:5242
    #3 0x5582585e591b in pyrna_func_call /home/guest/src/blender/blender/source/blender/python/intern/bp

Crash has been fixed (was missing clear of newid in read code), and patch applied to master as rBdf63195d2a7b.