diff options
author | Bastien Montagne <bastien@blender.org> | 2020-09-30 15:49:42 +0300 |
---|---|---|
committer | Bastien Montagne <bastien@blender.org> | 2020-10-02 12:47:34 +0300 |
commit | 6219d0d145d3f2e6bd30be1c91e952e18db44e74 (patch) | |
tree | bef402396e0076094364277403922709f8fc60ce /source/blender/blenkernel/intern/workspace.c | |
parent | d74d35e39eaf97a63178c84a6c4e9ab4e0bc384c (diff) |
Fix (unreported) design flow in how workspace's relation data are read from .blend file.
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
Diffstat (limited to 'source/blender/blenkernel/intern/workspace.c')
-rw-r--r-- | source/blender/blenkernel/intern/workspace.c | 28 |
1 files changed, 19 insertions, 9 deletions
diff --git a/source/blender/blenkernel/intern/workspace.c b/source/blender/blenkernel/intern/workspace.c index 324c8db0fe9..775d83278e9 100644 --- a/source/blender/blenkernel/intern/workspace.c +++ b/source/blender/blenkernel/intern/workspace.c @@ -125,10 +125,14 @@ static WorkSpaceLayout *workspace_layout_find_exec(const WorkSpace *workspace, return BLI_findptr(&workspace->layouts, screen, offsetof(WorkSpaceLayout, screen)); } -static void workspace_relation_add(ListBase *relation_list, void *parent, void *data) +static void workspace_relation_add(ListBase *relation_list, + void *parent, + const int parentid, + void *data) { WorkSpaceDataRelation *relation = MEM_callocN(sizeof(*relation), __func__); relation->parent = parent; + relation->parentid = parentid; relation->value = data; /* add to head, if we switch back to it soon we find it faster. */ BLI_addhead(relation_list, relation); @@ -139,11 +143,15 @@ static void workspace_relation_remove(ListBase *relation_list, WorkSpaceDataRela MEM_freeN(relation); } -static void workspace_relation_ensure_updated(ListBase *relation_list, void *parent, void *data) +static void workspace_relation_ensure_updated(ListBase *relation_list, + void *parent, + const int parentid, + void *data) { - WorkSpaceDataRelation *relation = BLI_findptr( - relation_list, parent, offsetof(WorkSpaceDataRelation, parent)); + WorkSpaceDataRelation *relation = BLI_listbase_bytes_find( + relation_list, &parentid, sizeof(parentid), offsetof(WorkSpaceDataRelation, parentid)); if (relation != NULL) { + relation->parent = parent; relation->value = data; /* reinsert at the head of the list, so that more commonly used relations are found faster. */ BLI_remlink(relation_list, relation); @@ -151,7 +159,7 @@ static void workspace_relation_ensure_updated(ListBase *relation_list, void *par } else { /* no matching relation found, add new one */ - workspace_relation_add(relation_list, parent, data); + workspace_relation_add(relation_list, parent, parentid, data); } } @@ -219,13 +227,13 @@ void BKE_workspace_remove(Main *bmain, WorkSpace *workspace) BKE_id_free(bmain, workspace); } -WorkSpaceInstanceHook *BKE_workspace_instance_hook_create(const Main *bmain) +WorkSpaceInstanceHook *BKE_workspace_instance_hook_create(const Main *bmain, const int winid) { WorkSpaceInstanceHook *hook = MEM_callocN(sizeof(WorkSpaceInstanceHook), __func__); /* set an active screen-layout for each possible window/workspace combination */ for (WorkSpace *workspace = bmain->workspaces.first; workspace; workspace = workspace->id.next) { - BKE_workspace_active_layout_set(hook, workspace, workspace->layouts.first); + BKE_workspace_active_layout_set(hook, winid, workspace, workspace->layouts.first); } return hook; @@ -483,11 +491,12 @@ WorkSpaceLayout *BKE_workspace_active_layout_for_workspace_get(const WorkSpaceIn * WorkSpaceInstanceHook.act_layout should only be modified directly to update the layout pointer. */ void BKE_workspace_active_layout_set(WorkSpaceInstanceHook *hook, + const int winid, WorkSpace *workspace, WorkSpaceLayout *layout) { hook->act_layout = layout; - workspace_relation_ensure_updated(&workspace->hook_layout_relations, hook, layout); + workspace_relation_ensure_updated(&workspace->hook_layout_relations, hook, winid, layout); } bScreen *BKE_workspace_active_screen_get(const WorkSpaceInstanceHook *hook) @@ -495,12 +504,13 @@ bScreen *BKE_workspace_active_screen_get(const WorkSpaceInstanceHook *hook) return hook->act_layout->screen; } void BKE_workspace_active_screen_set(WorkSpaceInstanceHook *hook, + const int winid, WorkSpace *workspace, bScreen *screen) { /* we need to find the WorkspaceLayout that wraps this screen */ WorkSpaceLayout *layout = BKE_workspace_layout_find(hook->active, screen); - BKE_workspace_active_layout_set(hook, workspace, layout); + BKE_workspace_active_layout_set(hook, winid, workspace, layout); } const char *BKE_workspace_layout_name_get(const WorkSpaceLayout *layout) |