From a2baf50242a315961fc7bc4d202a610442e813ec Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Tue, 26 May 2020 20:32:21 +0200 Subject: Cleanup/refactor: Workspace API, boilerplate code, early exit * Simplify workspace API a bit * Comment on behavior of workspace-layout relations where exposed in API * Remove annoying getters/setters * Avoid lookups if we can early exit * A NULL check is removed in `direct_link_workspace()` that I don't see a need for. Am not 100% sure though, fingers crossed. In general these changes should improve readability and make things easier to reason about. --- source/blender/blenkernel/BKE_workspace.h | 10 +-- source/blender/blenkernel/intern/workspace.c | 78 ++++++++++++---------- source/blender/blenloader/intern/readfile.c | 22 ++---- source/blender/blenloader/intern/versioning_280.c | 2 +- .../blenloader/intern/versioning_defaults.c | 9 ++- source/blender/blenloader/intern/writefile.c | 4 +- source/blender/editors/screen/workspace_edit.c | 9 ++- .../blender/editors/screen/workspace_layout_edit.c | 2 +- source/blender/makesrna/intern/rna_workspace.c | 2 +- source/blender/windowmanager/intern/wm.c | 2 +- source/blender/windowmanager/intern/wm_window.c | 4 +- 11 files changed, 65 insertions(+), 79 deletions(-) diff --git a/source/blender/blenkernel/BKE_workspace.h b/source/blender/blenkernel/BKE_workspace.h index 8582996108a..8a6afd8a753 100644 --- a/source/blender/blenkernel/BKE_workspace.h +++ b/source/blender/blenkernel/BKE_workspace.h @@ -84,6 +84,7 @@ void BKE_workspace_active_set(struct WorkSpaceInstanceHook *hook, struct WorkSpaceLayout *BKE_workspace_active_layout_get(const struct WorkSpaceInstanceHook *hook) GETTER_ATTRS; void BKE_workspace_active_layout_set(struct WorkSpaceInstanceHook *hook, + struct WorkSpace *workspace, struct WorkSpaceLayout *layout) SETTER_ATTRS; struct bScreen *BKE_workspace_active_screen_get(const struct WorkSpaceInstanceHook *hook) GETTER_ATTRS; @@ -91,21 +92,14 @@ void BKE_workspace_active_screen_set(struct WorkSpaceInstanceHook *hook, struct WorkSpace *workspace, struct bScreen *screen) SETTER_ATTRS; -struct ListBase *BKE_workspace_layouts_get(struct WorkSpace *workspace) GETTER_ATTRS; - const char *BKE_workspace_layout_name_get(const struct WorkSpaceLayout *layout) GETTER_ATTRS; void BKE_workspace_layout_name_set(struct WorkSpace *workspace, struct WorkSpaceLayout *layout, const char *new_name) ATTR_NONNULL(); struct bScreen *BKE_workspace_layout_screen_get(const struct WorkSpaceLayout *layout) GETTER_ATTRS; -void BKE_workspace_layout_screen_set(struct WorkSpaceLayout *layout, - struct bScreen *screen) SETTER_ATTRS; -struct WorkSpaceLayout *BKE_workspace_hook_layout_for_workspace_get( +struct WorkSpaceLayout *BKE_workspace_active_layout_for_workspace_get( const struct WorkSpaceInstanceHook *hook, const struct WorkSpace *workspace) GETTER_ATTRS; -void BKE_workspace_hook_layout_for_workspace_set(struct WorkSpaceInstanceHook *hook, - struct WorkSpace *workspace, - struct WorkSpaceLayout *layout) ATTR_NONNULL(); bool BKE_workspace_owner_id_check(const struct WorkSpace *workspace, const char *owner_id) ATTR_NONNULL(); diff --git a/source/blender/blenkernel/intern/workspace.c b/source/blender/blenkernel/intern/workspace.c index 3a69b95c114..4625fd76293 100644 --- a/source/blender/blenkernel/intern/workspace.c +++ b/source/blender/blenkernel/intern/workspace.c @@ -69,17 +69,9 @@ static void workspace_free_data(ID *id) static void workspace_foreach_id(ID *id, LibraryForeachIDData *data) { WorkSpace *workspace = (WorkSpace *)id; - ListBase *layouts = BKE_workspace_layouts_get(workspace); - LISTBASE_FOREACH (WorkSpaceLayout *, layout, layouts) { - bScreen *screen = BKE_workspace_layout_screen_get(layout); - - /* CALLBACK_INVOKE expects an actual pointer, not a variable holding the pointer. - * However we can't access layout->screen here - * since we are outside the workspace project. */ - BKE_LIB_FOREACHID_PROCESS(data, screen, IDWALK_CB_USER); - /* allow callback to set a different screen */ - BKE_workspace_layout_screen_set(layout, screen); + LISTBASE_FOREACH (WorkSpaceLayout *, layout, &workspace->layouts) { + BKE_LIB_FOREACHID_PROCESS(data, layout->screen, IDWALK_CB_USER); } } @@ -228,7 +220,7 @@ WorkSpaceInstanceHook *BKE_workspace_instance_hook_create(const Main *bmain) /* set an active screen-layout for each possible window/workspace combination */ for (WorkSpace *workspace = bmain->workspaces.first; workspace; workspace = workspace->id.next) { - BKE_workspace_hook_layout_for_workspace_set(hook, workspace, workspace->layouts.first); + BKE_workspace_active_layout_set(hook, workspace, workspace->layouts.first); } return hook; @@ -433,6 +425,10 @@ WorkSpace *BKE_workspace_active_get(WorkSpaceInstanceHook *hook) } void BKE_workspace_active_set(WorkSpaceInstanceHook *hook, WorkSpace *workspace) { + if (hook->active == workspace) { + return; + } + hook->active = workspace; if (workspace) { WorkSpaceLayout *layout = workspace_relation_get_data_matching_parent( @@ -443,13 +439,47 @@ void BKE_workspace_active_set(WorkSpaceInstanceHook *hook, WorkSpace *workspace) } } +/** + * Get the layout that is active for \a hook (which is the visible layout for the active workspace + * in \a hook). + */ WorkSpaceLayout *BKE_workspace_active_layout_get(const WorkSpaceInstanceHook *hook) { return hook->act_layout; } -void BKE_workspace_active_layout_set(WorkSpaceInstanceHook *hook, WorkSpaceLayout *layout) + +/** + * Get the layout to be activated should \a workspace become or be the active workspace in \a hook. + */ +WorkSpaceLayout *BKE_workspace_active_layout_for_workspace_get(const WorkSpaceInstanceHook *hook, + const WorkSpace *workspace) +{ + /* If the workspace is active, the active layout can be returned, no need for a lookup. */ + if (hook->active == workspace) { + return hook->act_layout; + } + + /* Inactive workspace */ + return workspace_relation_get_data_matching_parent(&workspace->hook_layout_relations, hook); +} + +/** + * \brief Activate a layout + * + * Sets \a layout as active for \a workspace when activated through or already active in \a hook. + * So when the active workspace of \a hook is \a workspace, \a layout becomes the active layout of + * \a hook too. See #BKE_workspace_active_set(). + * + * \a workspace does not need to be active for this. + * + * WorkSpaceInstanceHook.act_layout should only be modified directly to update the layout pointer. + */ +void BKE_workspace_active_layout_set(WorkSpaceInstanceHook *hook, + WorkSpace *workspace, + WorkSpaceLayout *layout) { hook->act_layout = layout; + workspace_relation_ensure_updated(&workspace->hook_layout_relations, hook, layout); } bScreen *BKE_workspace_active_screen_get(const WorkSpaceInstanceHook *hook) @@ -462,12 +492,7 @@ void BKE_workspace_active_screen_set(WorkSpaceInstanceHook *hook, { /* we need to find the WorkspaceLayout that wraps this screen */ WorkSpaceLayout *layout = BKE_workspace_layout_find(hook->active, screen); - BKE_workspace_hook_layout_for_workspace_set(hook, workspace, layout); -} - -ListBase *BKE_workspace_layouts_get(WorkSpace *workspace) -{ - return &workspace->layouts; + BKE_workspace_active_layout_set(hook, workspace, layout); } const char *BKE_workspace_layout_name_get(const WorkSpaceLayout *layout) @@ -485,22 +510,5 @@ bScreen *BKE_workspace_layout_screen_get(const WorkSpaceLayout *layout) { return layout->screen; } -void BKE_workspace_layout_screen_set(WorkSpaceLayout *layout, bScreen *screen) -{ - layout->screen = screen; -} - -WorkSpaceLayout *BKE_workspace_hook_layout_for_workspace_get(const WorkSpaceInstanceHook *hook, - const WorkSpace *workspace) -{ - return workspace_relation_get_data_matching_parent(&workspace->hook_layout_relations, hook); -} -void BKE_workspace_hook_layout_for_workspace_set(WorkSpaceInstanceHook *hook, - WorkSpace *workspace, - WorkSpaceLayout *layout) -{ - hook->act_layout = layout; - workspace_relation_ensure_updated(&workspace->hook_layout_relations, hook, layout); -} /** \} */ diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c index d90a730e8ed..e532ea17dca 100644 --- a/source/blender/blenloader/intern/readfile.c +++ b/source/blender/blenloader/intern/readfile.c @@ -3579,15 +3579,13 @@ static void direct_link_cachefile(FileData *fd, CacheFile *cache_file) static void lib_link_workspaces(FileData *fd, Main *bmain, WorkSpace *workspace) { - ListBase *layouts = BKE_workspace_layouts_get(workspace); ID *id = (ID *)workspace; id_us_ensure_real(id); - for (WorkSpaceLayout *layout = layouts->first, *layout_next; layout; layout = layout_next) { + LISTBASE_FOREACH_MUTABLE (WorkSpaceLayout *, layout, &workspace->layouts) { layout->screen = newlibadr(fd, id->lib, layout->screen); - layout_next = layout->next; if (layout->screen) { if (ID_IS_LINKED(id)) { layout->screen->winid = 0; @@ -3607,16 +3605,14 @@ static void lib_link_workspaces(FileData *fd, Main *bmain, WorkSpace *workspace) static void direct_link_workspace(FileData *fd, WorkSpace *workspace, const Main *main) { - link_list(fd, BKE_workspace_layouts_get(workspace)); + link_list(fd, &workspace->layouts); link_list(fd, &workspace->hook_layout_relations); link_list(fd, &workspace->owner_ids); link_list(fd, &workspace->tools); LISTBASE_FOREACH (WorkSpaceDataRelation *, relation, &workspace->hook_layout_relations) { - /* data from window - need to access through global oldnew-map */ relation->parent = newglobadr(fd, relation->parent); - relation->value = newdataadr(fd, relation->value); } @@ -3624,11 +3620,7 @@ static void direct_link_workspace(FileData *fd, WorkSpace *workspace, const Main * when reading windows, so have to update windows after/when reading workspaces. */ for (wmWindowManager *wm = main->wm.first; wm; wm = wm->id.next) { LISTBASE_FOREACH (wmWindow *, win, &wm->windows) { - WorkSpaceLayout *act_layout = newdataadr( - fd, BKE_workspace_active_layout_get(win->workspace_hook)); - if (act_layout) { - BKE_workspace_active_layout_set(win->workspace_hook, act_layout); - } + win->workspace_hook->act_layout = newdataadr(fd, win->workspace_hook->act_layout); } } @@ -8411,9 +8403,7 @@ void blo_lib_link_restore(Main *oldmain, for (WorkSpace *workspace = newmain->workspaces.first; workspace; workspace = workspace->id.next) { - ListBase *layouts = BKE_workspace_layouts_get(workspace); - - LISTBASE_FOREACH (WorkSpaceLayout *, layout, layouts) { + LISTBASE_FOREACH (WorkSpaceLayout *, layout, &workspace->layouts) { lib_link_workspace_layout_restore(id_map, newmain, layout); } } @@ -11625,9 +11615,7 @@ static void expand_gpencil(FileData *fd, Main *mainvar, bGPdata *gpd) static void expand_workspace(FileData *fd, Main *mainvar, WorkSpace *workspace) { - ListBase *layouts = BKE_workspace_layouts_get(workspace); - - LISTBASE_FOREACH (WorkSpaceLayout *, layout, layouts) { + LISTBASE_FOREACH (WorkSpaceLayout *, layout, &workspace->layouts) { expand_doit(fd, mainvar, BKE_workspace_layout_screen_get(layout)); } } diff --git a/source/blender/blenloader/intern/versioning_280.c b/source/blender/blenloader/intern/versioning_280.c index e4c102679a9..96e8981a944 100644 --- a/source/blender/blenloader/intern/versioning_280.c +++ b/source/blender/blenloader/intern/versioning_280.c @@ -257,7 +257,7 @@ static void do_version_workspaces_after_lib_link(Main *bmain) win->workspace_hook = BKE_workspace_instance_hook_create(bmain); BKE_workspace_active_set(win->workspace_hook, workspace); - BKE_workspace_hook_layout_for_workspace_set(win->workspace_hook, workspace, layout); + BKE_workspace_active_layout_set(win->workspace_hook, workspace, layout); /* Move scene and view layer to window. */ Scene *scene = screen->scene; diff --git a/source/blender/blenloader/intern/versioning_defaults.c b/source/blender/blenloader/intern/versioning_defaults.c index 665771cce1e..57c9ca16693 100644 --- a/source/blender/blenloader/intern/versioning_defaults.c +++ b/source/blender/blenloader/intern/versioning_defaults.c @@ -251,8 +251,7 @@ static void blo_update_defaults_screen(bScreen *screen, void BLO_update_defaults_workspace(WorkSpace *workspace, const char *app_template) { - ListBase *layouts = BKE_workspace_layouts_get(workspace); - LISTBASE_FOREACH (WorkSpaceLayout *, layout, layouts) { + LISTBASE_FOREACH (WorkSpaceLayout *, layout, &workspace->layouts) { if (layout->screen) { blo_update_defaults_screen(layout->screen, app_template, workspace->id.name + 2); } @@ -271,7 +270,7 @@ void BLO_update_defaults_workspace(WorkSpace *workspace, const char *app_templat /* For Sculpting template. */ if (STREQ(workspace->id.name + 2, "Sculpting")) { - LISTBASE_FOREACH (WorkSpaceLayout *, layout, layouts) { + LISTBASE_FOREACH (WorkSpaceLayout *, layout, &workspace->layouts) { bScreen *screen = layout->screen; if (screen) { LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) { @@ -489,8 +488,8 @@ void BLO_update_defaults_startup_blend(Main *bmain, const char *app_template) LISTBASE_FOREACH (wmWindowManager *, wm, &bmain->wm) { LISTBASE_FOREACH (wmWindow *, win, &wm->windows) { LISTBASE_FOREACH (WorkSpace *, workspace, &bmain->workspaces) { - WorkSpaceLayout *layout = BKE_workspace_hook_layout_for_workspace_get(win->workspace_hook, - workspace); + WorkSpaceLayout *layout = BKE_workspace_active_layout_for_workspace_get( + win->workspace_hook, workspace); /* Name all screens by their workspaces (avoids 'Default.###' names). */ /* Default only has one window. */ if (layout->screen) { diff --git a/source/blender/blenloader/intern/writefile.c b/source/blender/blenloader/intern/writefile.c index 5550c4ab6c6..33467317090 100644 --- a/source/blender/blenloader/intern/writefile.c +++ b/source/blender/blenloader/intern/writefile.c @@ -3789,11 +3789,9 @@ static void write_cachefile(WriteData *wd, CacheFile *cache_file, const void *id static void write_workspace(WriteData *wd, WorkSpace *workspace, const void *id_address) { - ListBase *layouts = BKE_workspace_layouts_get(workspace); - writestruct_at_address(wd, ID_WS, WorkSpace, 1, id_address, workspace); write_iddata(wd, &workspace->id); - writelist(wd, DATA, WorkSpaceLayout, layouts); + writelist(wd, DATA, WorkSpaceLayout, &workspace->layouts); writelist(wd, DATA, WorkSpaceDataRelation, &workspace->hook_layout_relations); writelist(wd, DATA, wmOwnerID, &workspace->owner_ids); writelist(wd, DATA, bToolRef, &workspace->tools); diff --git a/source/blender/editors/screen/workspace_edit.c b/source/blender/editors/screen/workspace_edit.c index 8feef0c675a..3edc51b038b 100644 --- a/source/blender/editors/screen/workspace_edit.c +++ b/source/blender/editors/screen/workspace_edit.c @@ -109,9 +109,9 @@ static WorkSpaceLayout *workspace_change_get_new_layout(Main *bmain, layout_new = win->workspace_hook->temp_layout_store; } else { - layout_new = BKE_workspace_hook_layout_for_workspace_get(win->workspace_hook, workspace_new); + layout_new = BKE_workspace_active_layout_for_workspace_get(win->workspace_hook, workspace_new); if (!layout_new) { - layout_new = BKE_workspace_layouts_get(workspace_new)->first; + layout_new = workspace_new->layouts.first; } } screen_new = BKE_workspace_layout_screen_get(layout_new); @@ -163,7 +163,7 @@ bool ED_workspace_change(WorkSpace *workspace_new, bContext *C, wmWindowManager return false; } - BKE_workspace_hook_layout_for_workspace_set(win->workspace_hook, workspace_new, layout_new); + BKE_workspace_active_layout_set(win->workspace_hook, workspace_new, layout_new); BKE_workspace_active_set(win->workspace_hook, workspace_new); /* update screen *after* changing workspace - which also causes the @@ -188,7 +188,6 @@ bool ED_workspace_change(WorkSpace *workspace_new, bContext *C, wmWindowManager WorkSpace *ED_workspace_duplicate(WorkSpace *workspace_old, Main *bmain, wmWindow *win) { WorkSpaceLayout *layout_active_old = BKE_workspace_active_layout_get(win->workspace_hook); - ListBase *layouts_old = BKE_workspace_layouts_get(workspace_old); WorkSpace *workspace_new = ED_workspace_add(bmain, workspace_old->id.name + 2); workspace_new->flags = workspace_old->flags; @@ -198,7 +197,7 @@ WorkSpace *ED_workspace_duplicate(WorkSpace *workspace_old, Main *bmain, wmWindo /* TODO(campbell): tools */ - LISTBASE_FOREACH (WorkSpaceLayout *, layout_old, layouts_old) { + LISTBASE_FOREACH (WorkSpaceLayout *, layout_old, &workspace_old->layouts) { WorkSpaceLayout *layout_new = ED_workspace_layout_duplicate( bmain, workspace_new, layout_old, win); diff --git a/source/blender/editors/screen/workspace_layout_edit.c b/source/blender/editors/screen/workspace_layout_edit.c index cf9637788a9..7ce92bc3e4d 100644 --- a/source/blender/editors/screen/workspace_layout_edit.c +++ b/source/blender/editors/screen/workspace_layout_edit.c @@ -140,7 +140,7 @@ bool ED_workspace_layout_delete(WorkSpace *workspace, WorkSpaceLayout *layout_ol const bScreen *screen_old = BKE_workspace_layout_screen_get(layout_old); WorkSpaceLayout *layout_new; - BLI_assert(BLI_findindex(BKE_workspace_layouts_get(workspace), layout_old) != -1); + BLI_assert(BLI_findindex(&workspace->layouts, layout_old) != -1); /* don't allow deleting temp fullscreens for now */ if (BKE_screen_is_fullscreen_area(screen_old)) { diff --git a/source/blender/makesrna/intern/rna_workspace.c b/source/blender/makesrna/intern/rna_workspace.c index 32e348ea72f..5f110189dd6 100644 --- a/source/blender/makesrna/intern/rna_workspace.c +++ b/source/blender/makesrna/intern/rna_workspace.c @@ -59,7 +59,7 @@ static void rna_window_update_all(Main *UNUSED(bmain), void rna_workspace_screens_begin(CollectionPropertyIterator *iter, PointerRNA *ptr) { WorkSpace *workspace = (WorkSpace *)ptr->owner_id; - rna_iterator_listbase_begin(iter, BKE_workspace_layouts_get(workspace), NULL); + rna_iterator_listbase_begin(iter, &workspace->layouts, NULL); } static PointerRNA rna_workspace_screens_item_get(CollectionPropertyIterator *iter) diff --git a/source/blender/windowmanager/intern/wm.c b/source/blender/windowmanager/intern/wm.c index 9aaaa0e2c81..0032a341610 100644 --- a/source/blender/windowmanager/intern/wm.c +++ b/source/blender/windowmanager/intern/wm.c @@ -388,7 +388,7 @@ void wm_add_default(Main *bmain, bContext *C) win->scene = CTX_data_scene(C); STRNCPY(win->view_layer_name, CTX_data_view_layer(C)->name); BKE_workspace_active_set(win->workspace_hook, workspace); - BKE_workspace_hook_layout_for_workspace_set(win->workspace_hook, workspace, layout); + BKE_workspace_active_layout_set(win->workspace_hook, workspace, layout); screen->winid = win->winid; wm->winactive = win; diff --git a/source/blender/windowmanager/intern/wm_window.c b/source/blender/windowmanager/intern/wm_window.c index fcb2be0a0bb..02b50af0ac3 100644 --- a/source/blender/windowmanager/intern/wm_window.c +++ b/source/blender/windowmanager/intern/wm_window.c @@ -326,7 +326,7 @@ wmWindow *wm_window_copy(Main *bmain, layout_new = duplicate_layout ? ED_workspace_layout_duplicate(bmain, workspace, layout_old, win_dst) : layout_old; - BKE_workspace_hook_layout_for_workspace_set(win_dst->workspace_hook, workspace, layout_new); + BKE_workspace_active_layout_set(win_dst->workspace_hook, workspace, layout_new); *win_dst->stereo3d_format = *win_src->stereo3d_format; @@ -2359,7 +2359,7 @@ WorkSpaceLayout *WM_window_get_active_layout(const wmWindow *win) } void WM_window_set_active_layout(wmWindow *win, WorkSpace *workspace, WorkSpaceLayout *layout) { - BKE_workspace_hook_layout_for_workspace_set(win->workspace_hook, workspace, layout); + BKE_workspace_active_layout_set(win->workspace_hook, workspace, layout); } /** -- cgit v1.2.3