I think the proper reviewers for this should be B&B, not S&S (Brecht and Bastien, not Sergey and Sybren).

@Bastien Montagne (mont29), as we can see in a comment on BKE_collection_master_add the master_collection is not a datablock, it is a member that uses the struct of Collection, but everything in it is evaluated along with the scene.
Therefore, as far as I can understand, the master_collection cannot be considered an embedded ID/datablock, but the IDs inside it are the embedded ones.

master collections are embedded ID just like root nodetrees, as tagged with LIB_EMBEDDED_DATA

The title of the patch needs some adjustment.

The direction seems to be good: more consistent separation of evaluation and original ID. However, from my understanding to make it fully correct an old standing TODO is to be implemented. See the comment in the scene_setup_view_layers_after_remap. If a base from master collection is not in any animated collections it will not be a part of the dependency graph, hence remapping will not work reliably.

Much of what is done in view_layer_remove_disabled_objects_in_collections_recursive(...) was originally based on view_layer_remove_disabled_bases(...) which checks whether the object is_object_enabled by checking if the original version pointed to by the Base is in the depsgraph.

In my opinion, the is_object_enabled description itself is misleading, as "disabled" objects can remain in view_layer->object_bases if it is in depsgraph.

The description of view_layer_remove_disabled_bases says:

"We are using original base since the object which evaluated base points to is not yet copied...".

But the function where it's called is named "scene_setup_view_layers_after_remap", if it's after remap, then why aren't the objects in view_layer->object_bases the "copied" ones? Or are they?

The real purpose of view_layer_remove_disabled_objects_in_collections_recursive is to remove objects that no longer have Base in view_layer->object_bases (because they were removed in view_layer_remove_disabled_bases).

For that we have some solutions:

• Do the same test on view_layer_remove_disabled_bases to check what was removed or
• Once a Base is removed in view_layer_remove_disabled_bases look for all references to the base object in the view_layer collections. Or
• Use BLI_ghash_lookup(view_layer->object_bases_hash, cob->ob) == nullptr or BKE_view_layer_base_find or
• Somehow tag the objects that are removed.

For the first solution, it would be more efficient and safe to change how tests are done in view_layer_remove_disabled_bases. Using DEG_is_evaluated_object alone would be nice.

The second solution appears to be of O(n*m) complexity in the worst case.

For the third solution, we would have to initialize the lazy object_bases_hash. But, except with patch D12469, object_bases_hash is never initialized to an evaluated view_layer.
Creating this hash map seems to remove the purpose of the lazy initialization.

For the fourth solution, as far as I've seen, there doesn't seem to be a support for temporarily tagging an object.

(There must be other solutions as well).

The remap operates on pointers. The pointer values (aka memory addresses) are known after building a depsgeraph. The issue is: even if you know pointer it doesn't mean you can dereference it in a meaningful manner. When the copy-on-write operation is run on a view later it is not guaranteed that copy-on-write operation was run on all objects which the view layer uses (in fact, I am not sure that is even possible, since intuitively object depends on view layer, not vice versa). Hence reading from evaluated object's fields from view layer cop-on-write is is malformed: the result will depend on exact threading order, could result reading zero-ed memory from the evaluated ID memory allocation, or could result accessing stale data from the previous dependency graph evaluation. Accessing original ID from the copy-on-write operation is OK since the original IDs are expected to be available at that time, and it does allow to overcome the evaluation order described above without going into complicated relations which will be hard to maintain and will have negative effect on performance.

The remap only remaps IDs which are "known" by the dependency graph, and leaves "unknown" at their original values. On the one hand this is because not all IDs are covered with copy-on-write, and on another hand this is because the remapping function does not know how to properly remove ID which is not needed by the dependency graph from user of that ID. The copy-on-write is copying an entire datablock, similar to what would happen if you duplicate the ID from the UI.

So in order to have a copy-on-written ID datablock which doesn't reference original IDs when the ID is expected to be covered with copy-on-write the following steps are needed:

1. Duplicate datablock into a pre-allocated memory.
2. Run generic function to remap known evaluated pointers.
3. Remove parts of the datablock which are not relevant to the current dependency graph.

The order is such due to convenience of having first two steps happening together as they are needed by all ID types, and after that a per-ID code can be run.
This should also explain why in the middle of the process you can have view layer referencing objects which are not covered by the depednecy graph.

If you have some concrete suggestions about how to make it more clear and obvious in the code please let us know.

For the first solution, it would be more efficient and safe to change how tests are done in view_layer_remove_disabled_bases. Using DEG_is_evaluated_object alone would be nice.

The tag of the object being evaluated is not guaranteed to be assigned yet. Similarly to the reason why you can not access fields of evaluated object described above.
Using deg_check_id_in_depsgraphon the original's object's ID is currently the way to do it. The tricky part is to get an original ID from evaluated one without dereferencing the evaluated. Iterating over both original and evaluated collections (similar to how it is done in the view_layer_update_orig_base_pointers for bases) is a bit verbose but easy-to-fit into the current design solution.

The complexity will be O(N * log(N)) which isn't too horrible and is not worse that it is currently (due to complexity of view_layer_remove_disabled_bases). If optimizing this step shows up user measurable performance improvement the new can look into complicating the code to gain this performance. Until then just live within similar ballpark.