Changeset View
Changeset View
Standalone View
Standalone View
source/blender/blenkernel/intern/collection.c
| Show All 15 Lines | |||||
| /** \file | /** \file | ||||
| * \ingroup bke | * \ingroup bke | ||||
| */ | */ | ||||
| #include <string.h> | #include <string.h> | ||||
| #include "BLI_blenlib.h" | #include "BLI_blenlib.h" | ||||
| #include "BLI_ghash.h" | |||||
| #include "BLI_iterator.h" | #include "BLI_iterator.h" | ||||
| #include "BLI_listbase.h" | #include "BLI_listbase.h" | ||||
| #include "BLI_math_base.h" | #include "BLI_math_base.h" | ||||
| #include "BLI_threads.h" | #include "BLI_threads.h" | ||||
| #include "BLT_translation.h" | #include "BLT_translation.h" | ||||
| #include "BKE_anim_data.h" | #include "BKE_anim_data.h" | ||||
| #include "BKE_collection.h" | #include "BKE_collection.h" | ||||
| ▲ Show 20 Lines • Show All 1,505 Lines • ▼ Show 20 Lines | |||||
| } | } | ||||
| /** \} */ | /** \} */ | ||||
| /* -------------------------------------------------------------------- */ | /* -------------------------------------------------------------------- */ | ||||
| /** \name Collection move (outliner drag & drop) | /** \name Collection move (outliner drag & drop) | ||||
| * \{ */ | * \{ */ | ||||
| typedef struct LayerCollectionFlag { | |||||
| struct LayerCollectionFlag *next, *prev; | |||||
| /** The view layer for the collections being moved, NULL for their children. */ | |||||
mont29: I would also store the `Collection *` pointer itself here, that way you can `BLI_assert` it… | |||||
Done Inline ActionsAlso, I would add here the ViewLayer * pointer (see other comments about the why). mont29: Also, I would add here the `ViewLayer *` pointer (see other comments about the why). | |||||
| ViewLayer *view_layer; | |||||
| /** The original #LayerCollection's collection field. */ | |||||
| Collection *collection; | |||||
| /** The original #LayerCollection's flag. */ | |||||
Done Inline Actionslayer_collection_flags_store_recursive mont29: `layer_collection_flags_store_recursive` | |||||
| int flag; | |||||
| /** Corresponds to #LayerCollection->layer_collections. */ | |||||
| ListBase children; | |||||
| } LayerCollectionFlag; | |||||
Done Inline ActionsAlso store here the Collection pointer from the layer_collection into the flag. mont29: Also store here the `Collection` pointer from the `layer_collection` into the `flag`. | |||||
| static void layer_collection_flags_store_recursive(const LayerCollection *layer_collection, | |||||
| LayerCollectionFlag *flag) | |||||
| { | |||||
| flag->collection = layer_collection->collection; | |||||
| flag->flag = layer_collection->flag; | |||||
| LISTBASE_FOREACH (const LayerCollection *, child, &layer_collection->layer_collections) { | |||||
| LayerCollectionFlag *child_flag = MEM_callocN(sizeof(LayerCollectionFlag), __func__); | |||||
| BLI_addtail(&flag->children, child_flag); | |||||
| layer_collection_flags_store_recursive(child, child_flag); | |||||
| } | |||||
| } | |||||
| /** | |||||
| * For every view layer, find the collection and save flags for its children, which will also be | |||||
| * moved. Store a temporary tree structure for the collection in each view layer since we want to | |||||
| * restore their flags as well. | |||||
| */ | |||||
| static void layer_collection_flags_store(Main *bmain, | |||||
Done Inline ActionsI'd rather have the listbase created in calling code and passed around as a pointer… mont29: I'd rather have the listbase created in calling code and passed around as a pointer… | |||||
| const Collection *collection, | |||||
| ListBase *r_layer_level_list) | |||||
| { | |||||
| BLI_listbase_clear(r_layer_level_list); | |||||
| LISTBASE_FOREACH (Scene *, scene, &bmain->scenes) { | |||||
| LISTBASE_FOREACH (ViewLayer *, view_layer, &scene->view_layers) { | |||||
| LayerCollection *layer_collection = BKE_layer_collection_first_from_scene_collection( | |||||
| view_layer, collection); | |||||
| if (layer_collection == NULL) { | |||||
| continue; | |||||
| } | |||||
Done Inline ActionsReplace this with inserting into a listbase of 'first level' LayerCollectionFlag items, that also store the ViewLayer pointer. mont29: Replace this with inserting into a listbase of 'first level' `LayerCollectionFlag` items, that… | |||||
| /* Store the flags for the collection and all of its children. */ | |||||
| LayerCollectionFlag *flag = MEM_callocN(sizeof(LayerCollectionFlag), __func__); | |||||
| flag->view_layer = view_layer; | |||||
Done Inline Actionslayer_collection_flags_restore_recursive mont29: `layer_collection_flags_restore_recursive` | |||||
| /* Recursively save flags from collection children. */ | |||||
| layer_collection_flags_store_recursive(layer_collection, flag); | |||||
Done Inline ActionsHere you can assert that layer_collection->collection == flag->collection. mont29: Here you can assert that `layer_collection->collection == flag->collection`. | |||||
| BLI_addtail(r_layer_level_list, flag); | |||||
| } | |||||
| } | |||||
| } | |||||
| static void layer_collection_flags_restore_recursive(LayerCollection *layer_collection, | |||||
Done Inline ActionsFree the listbase item properly: BLI_freelinkN(&flag->children, child_flag); mont29: Free the listbase item properly: `BLI_freelinkN(&flag->children, child_flag);` | |||||
Done Inline ActionsI thought it made a bit more sense to do it all in one call at the end of the loop, that way it doesn't need to be a mutable loop. I can still do it this way if you like, I think the way it is now is a bit cleaner though. HooglyBoogly: I thought it made a bit more sense to do it all in one call at the end of the loop, that way it… | |||||
Done Inline ActionsThe point of doing it item by item here is that you can then assert (once all children LayerCollection have been processed in upper level) that you also have processed all of the stored LayerCollectionFlag. This gives some basic check against potential discrepancy between the two lists… If you prefer, you can also assert that they have the same length (BLI_listbase_count()) before starting iterating on them, might actually be clearer (and we do not really care about some extra looping on those, especially if only for debug builds). mont29: The point of doing it item by item here is that you can then assert (once all children… | |||||
Done Inline ActionsOkay, yes, that seems clearer to me, I'll add that assert. HooglyBoogly: Okay, yes, that seems clearer to me, I'll add that assert. | |||||
| LayerCollectionFlag *flag) | |||||
| { | |||||
| /* Each child list should have the same count, as they were built together. */ | |||||
Done Inline ActionsNow here you can assert that BLI_listbase_is_empty(&flag->children). mont29: Now here you can assert that `BLI_listbase_is_empty(&flag->children)`. | |||||
| BLI_assert(BLI_listbase_count(&layer_collection->layer_collections) == | |||||
| BLI_listbase_count(&flag->children)); | |||||
| /* The flag and the the layer collection should actually correspond. */ | |||||
| BLI_assert(flag->collection == layer_collection->collection); | |||||
| LayerCollectionFlag *child_flag = flag->children.first; | |||||
| LISTBASE_FOREACH (LayerCollection *, child, &layer_collection->layer_collections) { | |||||
| layer_collection_flags_restore_recursive(child, child_flag); | |||||
| child_flag = child_flag->next; | |||||
| } | |||||
Done Inline ActionsThis is not needed anymore. mont29: This is not needed anymore. | |||||
Done Inline ActionsWould move that assert at beginning of the function, with the other one. mont29: Would move that assert at beginning of the function, with the other one. | |||||
| BLI_freelistN(&flag->children); | |||||
| /* We treat exclude as a special case. | |||||
| * | |||||
| * If in a different view layer the parent collection was disabled (e.g., background) | |||||
| * and now we moved a new collection to be part of the background this collection should | |||||
| * probably be disabled. | |||||
| * | |||||
| * Note: If we were to also keep the exclude flag we would need to re-sync the collections. | |||||
| */ | |||||
Done Inline ActionsTo be replaced by LISTBASE_FOREACH_MUTABLE over the top ListBase of LayerCollectionFlag, that now also store their ViewLayer pointer. mont29: To be replaced by `LISTBASE_FOREACH_MUTABLE` over the top `ListBase` of `LayerCollectionFlag`… | |||||
| layer_collection->flag = flag->flag | (layer_collection->flag & LAYER_COLLECTION_EXCLUDE); | |||||
| } | |||||
| /** | |||||
| * Restore a collection's (and its children's) flags for each view layer from the structure built | |||||
| * in #layer_collection_flags_store. | |||||
| */ | |||||
| static void layer_collection_flags_restore(ListBase *flags, const Collection *collection) | |||||
| { | |||||
| LISTBASE_FOREACH_MUTABLE (LayerCollectionFlag *, flag, flags) { | |||||
Done Inline ActionsDoesn't need to be mutable? You are not removing any item from that collection, but frees it all at once at the end after looping on it. mont29: Doesn't need to be mutable? You are not removing any item from that collection, but frees it… | |||||
Done Inline ActionsOops, no it does not! HooglyBoogly: Oops, no it does not! | |||||
| ViewLayer *view_layer = flag->view_layer; | |||||
| BLI_assert(view_layer != NULL); | |||||
| LayerCollection *layer_collection = BKE_layer_collection_first_from_scene_collection( | |||||
| view_layer, collection); | |||||
Done Inline Actionsisn't mont29: `isn't` | |||||
| /* The flags should only be added if the view layer isn't NULL in the first place. */ | |||||
| BLI_assert(layer_collection != NULL); | |||||
| layer_collection_flags_restore_recursive(layer_collection, flag); | |||||
Done Inline ActionsYou assert'ed on that, no need to check it anymore. mont29: You assert'ed on that, no need to check it anymore. | |||||
| } | |||||
| BLI_freelistN(&layer_flags); | |||||
| } | |||||
| bool BKE_collection_move(Main *bmain, | bool BKE_collection_move(Main *bmain, | ||||
| Collection *to_parent, | Collection *to_parent, | ||||
| Collection *from_parent, | Collection *from_parent, | ||||
| Collection *relative, | Collection *relative, | ||||
| bool relative_after, | bool relative_after, | ||||
| Collection *collection) | Collection *collection) | ||||
| { | { | ||||
| if (collection->flag & COLLECTION_IS_MASTER) { | if (collection->flag & COLLECTION_IS_MASTER) { | ||||
| Show All 25 Lines | if (relative_child) { | ||||
| BLI_insertlinkbefore(&to_parent->children, relative_child, child); | BLI_insertlinkbefore(&to_parent->children, relative_child, child); | ||||
| } | } | ||||
| BKE_collection_object_cache_free(to_parent); | BKE_collection_object_cache_free(to_parent); | ||||
| } | } | ||||
| } | } | ||||
| /* Make sure we store the flag of the layer collections before we remove and re-create them. | /* Make sure we store the flag of the layer collections before we remove and re-create them. | ||||
| * Otherwise they will get lost and everything will be copied from the new parent collection. */ | * Otherwise they will get lost and everything will be copied from the new parent collection. */ | ||||
| GHash *view_layer_hash = BLI_ghash_new(BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp, __func__); | ListBase layer_flags; | ||||
Done Inline ActionsReplace this by a ListBase of top-level LayerCollectionFlag storing a pointer to matching Viewlayer. mont29: Replace this by a `ListBase` of top-level `LayerCollectionFlag` storing a pointer to matching… | |||||
| layer_collection_flags_store(bmain, collection, &layer_flags); | |||||
| for (Scene *scene = bmain->scenes.first; scene; scene = scene->id.next) { | |||||
| LISTBASE_FOREACH (ViewLayer *, view_layer, &scene->view_layers) { | |||||
| LayerCollection *layer_collection = BKE_layer_collection_first_from_scene_collection( | |||||
| view_layer, collection); | |||||
| if (layer_collection == NULL) { | |||||
| continue; | |||||
| } | |||||
| BLI_ghash_insert(view_layer_hash, view_layer, POINTER_FROM_INT(layer_collection->flag)); | |||||
| } | |||||
| } | |||||
| /* Create and remove layer collections. */ | /* Create and remove layer collections. */ | ||||
| BKE_main_collection_sync(bmain); | BKE_main_collection_sync(bmain); | ||||
| /* Restore back the original layer collection flags. */ | /* Restore the original layer collection flags. */ | ||||
| GHashIterator gh_iter; | layer_collection_flags_restore(&layer_flags, collection); | ||||
Done Inline ActionsTo be replaced by BLI_freelistN call. mont29: To be replaced by `BLI_freelistN` call. | |||||
| GHASH_ITER (gh_iter, view_layer_hash) { | |||||
| ViewLayer *view_layer = BLI_ghashIterator_getKey(&gh_iter); | |||||
| LayerCollection *layer_collection = BKE_layer_collection_first_from_scene_collection( | |||||
| view_layer, collection); | |||||
| if (layer_collection) { | |||||
| /* We treat exclude as a special case. | |||||
| * | |||||
| * If in a different view layer the parent collection was disabled (e.g., background) | |||||
| * and now we moved a new collection to be part of the background this collection should | |||||
| * probably be disabled. | |||||
| * | |||||
| * Note: If we were to also keep the exclude flag we would need to re-sync the collections. | |||||
| */ | |||||
| layer_collection->flag = POINTER_AS_INT(BLI_ghashIterator_getValue(&gh_iter)) | | |||||
| (layer_collection->flag & LAYER_COLLECTION_EXCLUDE); | |||||
| } | |||||
| } | |||||
| BLI_ghash_free(view_layer_hash, NULL, NULL); | |||||
Done Inline Actionsshould go at the end of layer_collection_flags_restore mont29: should go at the end of `layer_collection_flags_restore` | |||||
Done Inline ActionsI don't think it can though, it still needs to be in the recursive function so that children of children would be freed. HooglyBoogly: I don't think it can though, it still needs to be in the recursive function so that children of… | |||||
Done Inline ActionsThose have already been freed inf the _restore_recursive function… Iḿ talking about that call freeing the main 'viewlayers´ list. mont29: Those have already been freed inf the `_restore_recursive` function… Iḿ talking about that call… | |||||
Done Inline ActionsOh sorry, I lost my place and didn't realize which part of this comment applied to. HooglyBoogly: Oh sorry, I lost my place and didn't realize which part of this comment applied to. | |||||
| /* We need to sync it again to pass the correct flags to the collections objects. */ | /* We need to sync it again to pass the correct flags to the collections objects. */ | ||||
| BKE_main_collection_sync(bmain); | BKE_main_collection_sync(bmain); | ||||
| return true; | return true; | ||||
| } | } | ||||
| /** \} */ | /** \} */ | ||||
| ▲ Show 20 Lines • Show All 190 Lines • Show Last 20 Lines | |||||
I would also store the Collection * pointer itself here, that way you can BLI_assert it matches the layer_collection->collection one in restore step.