Commit: 07332dd2bbe62506bef89ffad22ad01766922c8e Author: Bastien Montagne Date: Mon Oct 12 12:15:05 2015 +0200 Branches: master https://developer.blender.org/rB07332dd2bbe62506bef89ffad22ad01766922c8e
Fix T34446: Make Local on linked mesh object: object gets removed if redo function is used. Root of the issue is that we do not re-read lib data blocks and ID placholders (ID_ID bheads) in undo context (in `blo_read_file_internal`), because `BLO_read_from_memfile` copies lib datablocks and Main data directly from oldmain. The idea being, linked data do not change from undo/redo. This is valid as long as linked data was not changed by the undo step - but if some was deleted or localized, it will be missing from oldmain, leading to data loss (note that does not only concern objects, all linkable data types can be affected, at least in theory). This commit addresses that issue by carefully mixing reuse of needed data from oldmain, and "normal" re-reading of missing one. Makes us swimming in some rather dark waters, and gives a rather non-easy-to-follow code, but it seems to work quite well, and only other solution would be to get rid of that optimization (not re-reading all libs on undo/redo), which is not acceptable. Also, thanks to @carlosdp for initial investigation of the issue. Differential Revision: https://developer.blender.org/D1485 =================================================================== M source/blender/blenloader/intern/readblenentry.c M source/blender/blenloader/intern/readfile.c M source/blender/blenloader/intern/readfile.h =================================================================== diff --git a/source/blender/blenloader/intern/readblenentry.c b/source/blender/blenloader/intern/readblenentry.c index e8d7a46..6132a09 100644 --- a/source/blender/blenloader/intern/readblenentry.c +++ b/source/blender/blenloader/intern/readblenentry.c @@ -303,7 +303,7 @@ BlendFileData *BLO_read_from_memfile(Main *oldmain, const char *filename, MemFil { BlendFileData *bfd = NULL; FileData *fd; - ListBase mainlist; + ListBase old_mainlist; fd = blo_openblendermemfile(memfile, reports); if (fd) { @@ -314,9 +314,9 @@ BlendFileData *BLO_read_from_memfile(Main *oldmain, const char *filename, MemFil blo_clear_proxy_pointers_from_lib(oldmain); /* separate libraries from old main */ - blo_split_main(&mainlist, oldmain); + blo_split_main(&old_mainlist, oldmain); /* add the library pointers in oldmap lookup */ - blo_add_library_pointer_map(&mainlist, fd); + blo_add_library_pointer_map(&old_mainlist, fd); /* makes lookup of existing images in old main */ blo_make_image_pointer_map(fd, oldmain); @@ -340,19 +340,44 @@ BlendFileData *BLO_read_from_memfile(Main *oldmain, const char *filename, MemFil /* ensures relinked sounds are not freed */ blo_end_sound_pointer_map(fd, oldmain); - /* move libraries from old main to new main */ - if (bfd && mainlist.first != mainlist.last) { - - /* Library structs themselves */ - bfd->main->library = oldmain->library; - BLI_listbase_clear(&oldmain->library); - - /* add the Library mainlist to the new main */ - BLI_remlink(&mainlist, oldmain); - BLI_addhead(&mainlist, bfd->main); + /* Still in-use libraries have already been moved from oldmain to new mainlist, + * but oldmain itself shall *never* be 'transferred' to new mainlist! */ + BLI_assert(old_mainlist.first == oldmain); + + if (bfd && old_mainlist.first != old_mainlist.last) { + /* Even though directly used libs have been already moved to new main, indirect ones have not. + * This is a bit annoying, but we have no choice but to keep them all for now - means some now unused + * data may remain in memory, but think we'll have to live with it. */ + Main *libmain; + Main *newmain = bfd->main; + ListBase new_mainlist = {newmain, newmain}; + + for (libmain = oldmain->next; libmain; libmain = libmain->next) { + /* Note that LIB_INDIRECT does not work with libraries themselves, so we use non-NULL parent + * to detect indirect-linked ones... */ + if (libmain->curlib && (libmain->curlib->parent != NULL)) { + BLI_remlink(&old_mainlist, libmain); + BLI_addtail(&new_mainlist, libmain); + } +#if 0 + else { + printf("Dropped Main for lib: %s\n", libmain->curlib->id.name); + } +#endif + } + /* In any case, we need to move all lib datablocks themselves - those are 'first level data', + * getting rid of them would imply updating spaces & co to prevent invalid pointers access. */ + BLI_movelisttolist(&newmain->library, &oldmain->library); + + blo_join_main(&new_mainlist); } - blo_join_main(&mainlist); - + + /* printf("Remaining mains/libs in oldmain: %d\n", BLI_listbase_count(&fd->old_mainlist) - 1); */ + + /* That way, libs (aka mains) we did not reuse in new undone/redone state + * will be cleared together with oldmain... */ + blo_join_main(&old_mainlist); + blo_freefiledata(fd); } diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c index 2ce4a93..9760f21 100644 --- a/source/blender/blenloader/intern/readfile.c +++ b/source/blender/blenloader/intern/readfile.c @@ -854,6 +854,12 @@ BHead *blo_nextbhead(FileData *fd, BHead *thisblock) return(bhead); } +/* Warning! Caller's responsability to ensure given bhead **is** and ID one! */ +const char *bhead_id_name(const FileData *fd, const BHead *bhead) +{ + return (const char *)POINTER_OFFSET(bhead, sizeof(*bhead) + fd->id_name_offs); +} + static void decode_blender_header(FileData *fd) { char header[SIZEOFBLENDERHEADER], num[4]; @@ -1772,9 +1778,9 @@ void blo_end_packed_pointer_map(FileData *fd, Main *oldmain) /* undo file support: add all library pointers in lookup */ -void blo_add_library_pointer_map(ListBase *mainlist, FileData *fd) +void blo_add_library_pointer_map(ListBase *old_mainlist, FileData *fd) { - Main *ptr = mainlist->first; + Main *ptr = old_mainlist->first; ListBase *lbarray[MAX_LIBARRAY]; for (ptr = ptr->next; ptr; ptr = ptr->next) { @@ -1785,6 +1791,8 @@ void blo_add_library_pointer_map(ListBase *mainlist, FileData *fd) oldnewmap_insert(fd->libmap, id, id, GS(id->name)); } } + + fd->old_mainlist = old_mainlist; } @@ -7822,14 +7830,66 @@ static BHead *read_data_into_oldnewmap(FileData *fd, BHead *bhead, const char *a static BHead *read_libblock(FileData *fd, Main *main, BHead *bhead, int flag, ID **r_id) { - /* this routine reads a libblock and its direct data. Use link functions - * to connect it all + /* this routine reads a libblock and its direct data. Use link functions to connect it all */ ID *id; ListBase *lb; const char *allocname; bool wrong_id = false; - + + /* In undo case, most libs and linked data should be kept as is from previous state (see BLO_read_from_memfile). + * However, some needed by the snapshot being read may have been removed in previous one, and would go missing. + * This leads e.g. to desappearing objects in some undo/redo case, see T34446. + * That means we have to carefully check whether current lib or libdata already exits in old main, if it does + * we merely copy it over into new main area, otherwise we have to do a full read of that bhead... */ + if (fd->memfile && ELEM(bhead->code, ID_LI, ID_ID)) { + const char *idname = bhead_id_name(fd, bhead); + + /* printf("Checking %s...\n", idname); */ + + if (bhead->code == ID_LI) { + Main *libmain = fd->old_mainlist->first; + /* Skip oldmain itself... */ + for (libmain = libmain->next; libmain; libmain = libmain->next) { + /* printf("... against %s: ", libmain->curlib ? libmain->curlib->id.name : "<NULL>"); */ + if (libmain->curlib && STREQ(idname, libmain->curlib->id.name)) { + Main *oldmain = fd->old_mainlist->first; + /* printf("FOUND!\n"); */ + /* In case of a library, we need to re-add its main to fd->mainlist, because if we have later + * a missing ID_ID, we need to get the correct lib it is linked to! + * Order is crucial, we cannot bulk-add it in BLO_read_from_memfile() like it used to be... */ + BLI_remlink(fd->old_mainlist, libmain); + BLI_remlink_safe(&oldmain->library, libmain->curlib); + BLI_addtail(fd->mainlist, libmain); + BLI_addtail(&main->library, libmain->curlib); + + if (r_id) { + *r_id = NULL; /* Just in case... */ + } + return blo_nextbhead(fd, bhead); + } + /* printf("nothing...\n"); */ + } + } + else { + /* printf("... in %s (%s): ", main->curlib ? main->curlib->id.name : "<NULL>", main->curlib ? main->curlib->name : "<NULL>"); */ + if ((id = BKE_libblock_find_name_ex(main, GS(idname), idname + 2))) { + /* printf("FOUND!\n"); */ + /* Even though we found our linked ID, there is no guarantee its address is still the same... */ + if (id != bhead->old) { + oldnewmap_insert(fd->libmap, bhead->old, id, GS(id->name)); + } + + /* No need to do anything else for ID_ID, it's assumed already present in its lib's main... */ + if (r_id) { + *r_id = NULL; /* Just in case... */ + } + return blo_nextbhead(fd, bhead); + } + /* printf("nothing...\n"); */ + } + } + /* read libblock */ id = read_struct(fd, bhead, "lib block"); if (r_id) @@ -8300,26 +8360,10 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath) bhead = NULL; break; - case ID_LI: - /* skip library datablocks in undo, this works together with - * BLO_read_from_memfile, where the old main->library is restored - * overwriting the libraries from the memory file. previously - * it did not save ID_LI/ID_ID blocks in this case, but they are - * needed to make quit.blend recover them correctly. */ - if (fd->memfile) - bhead = blo_nextbhead(fd, bhead); - else - bhead = read_libblock(fd, bfd->main, bhead, LIB_LOCAL, NULL); - break; case ID_ID: - /* same as above */ - if (fd->memfile) - bhead = blo_nextbhead(fd, bhead); - else - /* always adds to the most recently loaded - * ID_LI block, see direct_link_library. - * this is part of the file format definition. */ - bhead = read_libblock(fd, mainlist.last, bhead, LIB_READ+LIB_EXTERN, NULL); + /* Always adds to the most recently loaded ID_LI block, see direct_link_library. + * This is part of the file format definition. */ + bhead = read_libblock(fd, mainlist.last, bhead, LIB_READ | LIB_EXTERN, NULL); break; /* in 2.50+ files, the file identifier for screens is patched, forward compatibility */ @@ -8473,11 +8517,6 @@ static BHead *find_bhead_from_idname(FileData *fd, const char *idname) #endif } -const char *bhead_id_name(const FileData *fd, const BHead *bhead) -{ - return (const char *)POINTER_OFFSET(bhead, sizeof(*bhead) + fd->id_name_offs); -} - static ID *is_yet_read(FileData *fd, Main *mainvar, BHead *bhead) { const char *idname= bhead_id_name(fd, bhead); diff --git a/source/blender/blenloader/intern/readfile.h b/source/blender/blenloader/intern/readfile.h index ed22dae..f6c3b69 100644 --- a/source/blender/blenloader/intern/readfile.h +++ b/source/blender/blenloader/intern/readfile.h @@ -96,7 +96,8 @@ typedef struct FileData { struct GHash *bhead_idname_hash; ListBase *mainlist; - + ListBase *old_mainlist; /* Used for undo. */ + /* ick ick, used to return * data through streamglue. */ @@ -139,7 +140,7 @@ void blo_make_sound_pointer_map(FileData *fd, Main *oldmain); void blo_end_sound_pointer_map(FileData *fd, Main *oldmain); void blo_make_packed_pointer_map(FileData *fd, Main *oldmain); void blo_end_packed_pointer_map(FileData *fd, Main *oldmain); -void blo_add_library_pointer_map(ListBase *mainlist, FileData *fd); +void blo_add_library_pointer_map(ListBase *old_mainlist, FileData *fd); void blo_freefiledata(FileData * @@ Diff output truncated at 10240 characters. @@ _______________________________________________ Bf-blender-cvs mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-blender-cvs
