WorkSpaces link to non-ID library data (ViewLayer) #54140

New Issue

Campbell Barton · 2018-02-23T04:03:59+01:00

Campbell Barton commented

2018-02-23 04:03:59 +01:00

Currently WorkSpaces.scene_viewlayer_relations link to ViewLayer via WorkSpaceDataRelation.value.

This works when the workspace and scene are stored in the same file, but crashes when a workspace & window is used to display a library linked scene.

In general this is something we don't support - referencing library pointers only works via ID's (I should have picked this up in initial review).

Checked on supporting this, but libraries don't have a datamap. This could be supported but is going outside the realm of what's reasonable regarding library loading. (snipped of kind of change that would be needed to support this - P622).

Suggest to use a string for the view layer as we do with bones.
An alternative solution would be to always ignore the workspace view layer with linked scenes, but this seems weak.

Currently `WorkSpaces.scene_viewlayer_relations` link to `ViewLayer` via `WorkSpaceDataRelation.value`. This works when the workspace and scene are stored in the same file, but crashes when a workspace & window is used to display a library linked scene. In general this is something we don't support - referencing library pointers only works via ID's *(I should have picked this up in initial review)*. Checked on supporting this, but libraries don't have a `datamap`. This could be supported but is going outside the realm of what's reasonable regarding library loading. (snipped of kind of change that would be needed to support this - [P622](https://archive.blender.org/developer/P622.txt)). ---- * Suggest to use a string for the view layer as we do with bones. * An alternative solution would be to always ignore the workspace view layer with linked scenes, but this seems weak.

Julian Eisel was assigned by Campbell Barton

2018-02-23 04:03:59 +01:00

Campbell Barton commented

2018-02-23 04:03:59 +01:00

Added subscribers: @ideasman42, @JulianEisel

blender-admin commented

2018-02-23 04:03:59 +01:00

#54375 was marked as duplicate of this issue

Bastien Montagne commented

2018-02-23 08:13:08 +01:00

Added subscriber: @mont29

Bastien Montagne commented

2018-02-23 08:13:08 +01:00

That should be known by now, indeed, never ever store pointers to non-ID data that do not belongs to same datablock. This is not supported, and we absolutely do not want to support it. At all.

Even the few cases where this is done currently (runtime-only, like e.g. object's pose storing pointers to armature's bones) are already a pain to handle in libdata management code.

So yes, using names is the correct solution here. And if you absolutely need to have runtime-only pointers (for performance reasons), then add functions to easily invalidate and rebuild those. And NULLify those pointers in readfile.c. But again, even that is a pain to handle correctly everywhere, so please only do it if absolutely necessary.

That should be known by now, indeed, **never ever store pointers to non-ID data that do not belongs to same datablock**. This is not supported, and we absolutely do not want to support it. At all. Even the few cases where this is done currently (runtime-only, like e.g. object's pose storing pointers to armature's bones) are already a pain to handle in libdata management code. So yes, using names is the correct solution here. And if you absolutely need to have **runtime-only** pointers (for performance reasons), then add functions to easily invalidate and rebuild those. And NULLify those pointers in readfile.c. But again, even that is a pain to handle correctly everywhere, so please only do it if absolutely necessary.

Julian Eisel commented

2018-02-23 10:45:20 +01:00

Yes, seems like we can use names/identifiers. I can do this if you're not on it already?

The whole handling of the workspace view-layer has to be done rather carefully, easy to mess things up. That especially matters for the whole workspaces.blend idea. Appending a workspace from there should also somehow append settings for the viewport from this specific workspace.
I think that's exactly why I used the layer pointer in the first place: Back then my understanding was that we'd manage viewport render settings through view-layers, meaning we had to read them when appending somehow anyway. Now, what we do is storing the viewport settings directly in the workspace (which I still dislike a lot for various reasons), so indeed, now we can/should use identifiers.

I also have to admit that I was sure scene data-blocks couldn't be linked when coding this (thought only individual objects or groups could).... no idea where that came from, and I obviously also didn't double check that...

Yes, seems like we can use names/identifiers. I can do this if you're not on it already? The whole handling of the workspace view-layer has to be done rather carefully, easy to mess things up. That especially matters for the whole workspaces.blend idea. Appending a workspace from there should also somehow append settings for the viewport from this specific workspace. I think that's exactly why I used the layer pointer in the first place: Back then my understanding was that we'd manage viewport render settings through view-layers, meaning we had to read them when appending somehow anyway. Now, what we do is storing the viewport settings directly in the workspace (which I still dislike a lot for various reasons), so indeed, now we can/should use identifiers. I also have to admit that I was sure scene data-blocks couldn't be linked when coding this (thought only individual objects or groups could).... no idea where that came from, and I obviously also didn't double check that...

Dalai Felinto commented

2018-03-20 15:14:06 +01:00

Added subscriber: @dfelinto