archive/blender-archive
Archived
1
1
Fork 0
Code Pull Requests Packages Activity

Fix (unreported) design flow in how workspace's relation data are read from .blend file.

Browse Source 
Relying on pointer addresses across different data-blocks is extremely
not recommended (and should be strictly forbidden ideally), in
particular in direct_link step of blend file reading.
- It assumes a specific order in reading of data, which is not ensured
  in future, and is in any case a very bad, non explicit, hidden
  dependency on behaviors of other parts of the codebase.
- It is intrinsically unsafe (as in, it makes writing bad code and making
  mistakes easy, see e.g. fix in rB84b3f6e049b35f9).
- It makes advanced handling of data-blocks harder (thinking about
  partial undo code e.g., even though in this specific case it was not
  an issue as we do not re-read neither windowmanagers nor worspaces
  during undo).

New code uses windows' `winid` instead as 'anchor' to find again proper
workspace hook in windows at read time.

As a bonus, it will also cleanup the list of relations from any invalid
ones (afaict it was never done previously).

Differential Revision: https://developer.blender.org/D9073
This commit is contained in:
Bastien Montagne
2020-09-30 14:49:42 +02:00
parent d74d35e39e
commit 6219d0d145
10 changed files with 91 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
source/blender/blenloader/intern/readfile.c
View File

@@ -1794,7 +1794,7 @@ static void *newdataadr_no_us(FileData *fd, const void *adr)
}
/* direct datablocks with global linking */
static void *newglobadr(FileData *fd, const void *adr)
void *blo_read_get_new_globaldata_address(FileData *fd, const void *adr)
{
return oldnewmap_lookup_and_inc(fd->globmap, adr, true);
}
@@ -2553,6 +2553,21 @@ static void lib_link_workspaces(BlendLibReader *reader, WorkSpace *workspace)
{
ID *id = (ID *)workspace;
/* Restore proper 'parent' pointers to relevant data, and clean up unused/invalid entries. */
LISTBASE_FOREACH_MUTABLE (WorkSpaceDataRelation *, relation, &workspace->hook_layout_relations) {
relation->parent = NULL;
LISTBASE_FOREACH (wmWindowManager *, wm, &reader->main->wm) {
LISTBASE_FOREACH (wmWindow *, win, &wm->windows) {
if (win->winid == relation->parentid) {
relation->parent = win->workspace_hook;
}
}
}
if (relation->parent == NULL) {
BLI_freelinkN(&workspace->hook_layout_relations, relation);
}
}
LISTBASE_FOREACH_MUTABLE (WorkSpaceLayout *, layout, &workspace->layouts) {
BLO_read_id_address(reader, id->lib, &layout->screen);
@@ -2581,12 +2596,8 @@ static void direct_link_workspace(BlendDataReader *reader, WorkSpace *workspace)
BLO_read_list(reader, &workspace->tools);
LISTBASE_FOREACH (WorkSpaceDataRelation *, relation, &workspace->hook_layout_relations) {
/* data from window - need to access through global oldnew-map */
/* XXX This is absolutely not acceptable. There is no acceptable reasons to mess with other
* ID's data in read code, and certainly never, ever in `direct_link_` functions.
* Kept for now because it seems to work, but it should be refactored. Probably store and use
* window's `winid`, just like it was already done for screens? */
relation->parent = newglobadr(reader->fd, relation->parent);
/* parent pointer does not belong to workspace data and is therefore restored in lib_link step
* of window manager.*/
BLO_read_data_address(reader, &relation->value);
}
@@ -5472,8 +5483,9 @@ static void direct_link_windowmanager(BlendDataReader *reader, wmWindowManager *
WorkSpaceInstanceHook *hook = win->workspace_hook;
BLO_read_data_address(reader, &win->workspace_hook);
/* we need to restore a pointer to this later when reading workspaces,
* so store in global oldnew-map. */
/* We need to restore a pointer to this later when reading workspaces,
* so store in global oldnew-map.
* Note that this is only needed for versionning of older .blend files now.. */
oldnewmap_insert(reader->fd->globmap, hook, win->workspace_hook, 0);
direct_link_area_map(reader, &win->global_areas);
@@ -6847,7 +6859,7 @@ static BHead *read_global(BlendFileData *bfd, FileData *fd, BHead *bhead)
/* note, this has to be kept for reading older files... */
static void link_global(FileData *fd, BlendFileData *bfd)
{
bfd->cur_view_layer = newglobadr(fd, bfd->cur_view_layer);
bfd->cur_view_layer = blo_read_get_new_globaldata_address(fd, bfd->cur_view_layer);
bfd->curscreen = newlibadr(fd, NULL, bfd->curscreen);
bfd->curscene = newlibadr(fd, NULL, bfd->curscene);
// this happens in files older than 2.35