Fix T38340 and T38473: fixed Scene pointers in Composite and Defocus nodes don't get updated based on context.
ClosedPublic
Actions

Authored by Lukas Tönne (lukastoenne) on Feb 5 2014, 11:43 AM.

Details

Reviewers

Monique Dewanchand (mdewanchand)
Brecht Van Lommel (brecht)
Jeroen Bakker (jbakker)

Maniphest Tasks

T38473: "Composite" node copied from another scene don't work
T38340: When used inside a node group in multiple scenes, the defocus node does not refresh active camera data.

Commits

rBS1687023776a3: Fix T38340 and T38473: fixed Scene pointers in Composite and Defocus nodes…
rBAC1687023776a3: Fix T38340 and T38473: fixed Scene pointers in Composite and Defocus nodes…
rB1687023776a3: Fix T38340 and T38473: fixed Scene pointers in Composite and Defocus nodes…

Summary

As discussed in T38340 the solution is to use the current scene from
context whenever feasible.

Composite does not use node->id at all now, the scene which owns the
compositing node tree is retrieved from context instead.

Defocus node->id is made editable by the user. By default it is not set,
which also will make it use the contextual scene and camera info.
The node->id pointer in Defocus is not cleared in older blend files.
This is done for backward compatibility: the node will then behave as
before in untouched scenes.

File Output nodes also don't store scene in node->id. This is only needed
when creating a new node for initializing the file format.

Diff Detail

Branch: node_scene_pointers

Event Timeline

What happens when you open files with NULL node->id in older Blender versions, does it crash? We may need to bump the minversion or find some way around it.

Otherwise LGTM.

@Brecht Van Lommel (brecht): Should have mentioned: this is already taken care of by an earlier lib-link fix:
https://developer.blender.org/diffusion/B/browse/master/source/blender/blenloader/intern/readfile.c;fc9626b1df5f9a05673199e82f4aafd1f4d42d08$5059

This will run in any case (not version dependent) and set all missing node->id scene pointers. So the patch is back- and forward compatible.

That composite_patch function goes back to 2009 btw, so i don't think we need to worry about minversion: rB1b27cd7

Great, that's fine then.

Lukas Tönne (lukastoenne) closed this revision.Feb 5 2014, 1:56 PM

Hi Lukas,

not sure about the defaults, perhaps it should be made empty, otherwise it could be difficult to reuse in a node group with linking. (scene not loaded)

We have not tested it so you should really test this case. For the rest, this is the good direction to solve this issue.

@Jeroen Bakker (jbakker) Not quite sure what you mean by "defaults". If you link a node group containing a Defocus node with an explicit scene pointer that doesn't exist (iirc RenderLayers are not allowed inside groups) the pointer will be reset to NULL and therefore use the context scene. Is this the case you are refering to?

This behavior may not be ideal, perhaps we should add an "expand" entry for such scene pointers so the scene gets loaded as well - but then we might get huge unwanted scene imports just for a Defocus node, which is probably not desirable either ...