Groups and collections
ClosedPublic

Authored by Dalai Felinto (dfelinto) on Oct 20 2017, 11:06 PM.

Details

Reviewers
Sergey Sharybin (sergey)
Bastien Montagne (mont29)
Commits
rB98b2d9fe9b5f: Remove assert from BKE_layer_collection_get_active_ensure
rB3478756a839b: Fixup for 1/3: dont write groupobjects
rBbe9e469ead22: Groups and collection: initial integration
rB7c0cafd95da2: Groups and collections 1/3
rB3874856cb67e: Fix adding new collections to group collections
rB589e760a34ce: Outliner: Support enable/disable group collections and fix select icon
rB13492442a0f1: Fixup for 2/3: rna_layer.c
rB019b3cf54ef5: Support deleting group collection individual collections
rBf2e6935f0af5: Outliner: Support drag & drop of GROUP_INTERNAL collections
rB3ddfbf0b2870: Fix build in release
rBfeb06b44182a: RNA: layer_collection.create_group()
rB25346c919aea: Move group sub-collection evaluation tp DEG
rB64db6d684f42: Show a different Outliner Tools menu option depending on collection type
rB13c1f29d88b9: Preserve the original overrides from collection to group
rB3ec6e2c126a1: Groups and collections 2/3
rBe95110c3da7a: RNA: fixup for allow for enable/disable group collections
rB3bede3ab8982: Fix pre-evaluation step
rB541ea2b7ad5e: Cleanup
rB641456cb3e02: Initial group unittests
rB213749f2f45d: Improve debug printing for layers evaluation
rB0eb400c35af5: Silence warning
rB11a916cf2bba: Fix crash for layer collection properties evaluation
rBc5235c2d96eb: Fix memleak when creating groups
rB7945419a30ed: Integrate group PRE processing to depsgraph
rB96490f082699: RNA: allow for enable/disable of group collections
rB2e3f662473f5: Unittesting (failing) for read/write
rB5498fb79530b: Fix collection groups crash after saving and loading file
rB2f6b146f9f43: Assert on IDProp merge routine to simplify debug process
rB16cf5647c8bf: Prevent crash on groups with no collections
rB76731a90925b: More work for 3/3 (aka 3.5/3)
rBf0a50823e6cb: Python RNA for group collections
rB77f4cb0416cd: Fixup UI for 1/3
rB67b18739ea0c: Groups and collections 3/3
rB40b18768b4df: Update unittests
Summary

Groups now use collections internally instead of GroupObject to handle group elements.
This way we can control object visibility via the collections and in the future, overrides.

What is working

  • Old 2.79 files (see todo for visibility), instancing, linking, appending, it all should work just like before.
  • Create group from collection (RMB in a Collection > Create Group).

What is not working

  • Create a group, then remove an object from the group using the outliner interface (Group > Collection > RMB > Remove Selected):

BLI_assert failed: /home/dfelinto/src/blender/blender/source/blender/blenkernel/intern/library.c:246, id_us_min(), at '0'

Todo

  • I think I'm not respecting the original group visibility flag, I will need to tackle that (I will create a different collection for the hidden objects).

Not included here

  • Collections of type Group. We have a design for this, but we want to handle it separately.

Diff Detail

Repository
rB Blender

Groups and collections 2/3

This allow a new collection of type "Group". To add it hold shift when creating a
new collection in a view layer.

Note: This introduces a big hack because I couldn't get Depsgraph to behave as I
wanted. Look for DEG_COLLECTION_GROUP.

Pending tasks:

  • Prevent drag and drop of collections that belong to a group unless:
    • The group is local
    • The source and destination are from the same group
  • Prevent changing any of a group nested collections values if the group is lib
  • Disable selectability for group collections

Groups and collections 3/3

This allows you to create a group from a collection.
It is i the Menu when you click in the collections in the Outliner

Also here:

  • Groups and collections 2/3
  • Groups and collections 3/3
  • More work for 3/3 (aka 3.5/3)
  • Python RNA for group collections
  • Fixup UI for 1/3
  • RNA: layer_collection.create_group()
  • Fixup for 1/3: dont write groupobjects
  • Initial group unittests
  • Fixup for 2/3: rna_layer.c

I'm getting non consistent crashes shown by the unittests. Since the crash is
coming from the override properties and I think we will need to change them for
other reasons I'm running short of motivation to tackle these crashes.

It will happen soon though.

  • Fix build in release
  • Fix memleak when creating groups
  • Move group sub-collection evaluation tp DEG
  • Integrate group PRE processing to depsgraph
  • Improve debug printing for layers evaluation
  • Assert on IDProp merge routine to simplify debug process
  • Remove assert from BKE_layer_collection_get_active_ensure
  • RNA: allow for enable/disable of group collections
  • Outliner: Support enable/disable group collections and fix select icon
  • Outliner: Support drag & drop of GROUP_INTERNAL collections
  • Update unittests
  • RNA: fixup for allow for enable/disable group collections
  • Fix pre-evaluation step
  • Fix crash for layer collection properties evaluation
  • Preserve the original overrides from collection to group

Now that's plenty. As long as you have a single render_layer in your scene, this is ready for tests. Otherwise you need to wait for Depsgraph to stick to a render layer per scene.
Note: If you can't mix an instance of a collection and a layer group at the same time for the time being. This is noted in the depsgraph, because we need to use a different tag for both cases, so we iterate over the same group twice in this case.

  • Unittesting for read/write
  • Fix adding new collections to group collections
  • Show a different Outliner Tools menu option depending on collection type
  • Support deleting group collection individual collections
  • Prevent crash on groups with no collections
  • Fix collection groups crash after saving and loading file

For the records, I just pushed this patch to temp-group-collections.

  • Remove "Group Collection" functionality
  • Outliner: Support Add Selected to group internal collections
  • Outliner: Support moving objects from/to group internal collections
Dalai Felinto (dfelinto) edited the summary of this revision. (Show Details)Nov 7 2017, 2:17 PM

Am really not happy with the same struct (SceneLayer) refcounting its objects in two different ways depending on who “owns” it (Scene or Group). 2.7 code is already a mess regarding how groups handle their objects (since they do not properly refcount them, but only use the shallow USER_ONE “owning” system).

But trying to use both real refcounting or USER_ONE one in same struct is imho a call for nightmares (means you need two ways of handling adding/removing/etc. objects to it, checking all over the code which type is the ID “owning” that scenelayer… yuck!). Why not just make groups actually refcount objects they use? That would be sooooooooo much simpler, cleaner and clearer…

source/blender/blenkernel/intern/collection.c
364–372

You cannot do that here, BKE_libblock_free_us() is only for real ID usages (those where *every* use increments the counter)!

If I believe code below, your SceneCollection can either be real ID user (in case “owner” is a scene), or 'USER_ONE' ID user (in case owner is a group). I’m already not happy with 2.7x code where groups are not really using their objects, this causes several nasty hacks and twist in ID management code. So am even less happy to have this in new 2.8 system, with two different behaviors mixed in the same struct!

But if you want to keep that mess, you can only call BKE_libblock_free_us() when id is a Scene.

  • Massive merge from 2.8
  • Lots of cleanup
  • Depsgraph: Refactor approach see upcoming note in D2892
  • Dupli groups overrides
  • Fix i/o for groups and update the unittest
  • From review: only call BKE_libblock_free_us for collections coming from scene
  • Layers: Use the same api for groups and scene as much as possible
  • More asserts

@Sergey Sharybin (sergey) I still need this patch (or some variation of it) P565.

As you said over IRC:

The whole depsgraph is expected to be up-to-date at the time you start iterating

However without this the group evaluation is never called (i.e., BKE_layer_eval_layer_collection_pre is never called for the group's view layer).

This is the depsgraph without the patch:

And this is the depsgraph with the patch:

source/blender/depsgraph/intern/builder/deg_builder_nodes.h
211–212

owner_id

  • [in blender2.8 already] Depsgraph: Tag all id_types on_visible_update
  • Depsgraph: From review: id > owner_id
  • Faster lookup for old group files
  • Fix change visibility of group in outliner updates depsgraph
  • Fix group.objects
  • Allow users to edit either the object group active collection or view layer one
  • Tag the correct id when updating the collection properties and overrides

Note: Everything is good but:

  • When we change a group collection setting we need to click on the viewport for the drawing to update.
  • Draw manager still struggles with duplis, so there are crashes from time to time.

All that said, I consider the patch final, and I want to commit it early Friday.

This revision was automatically updated to reflect the committed changes.