Page MenuHome

Render: Use dependency graph for compositor/sequencer
ClosedPublic

Authored by Sergey Sharybin (sergey) on May 21 2019, 1:53 PM.

Details

Summary

This change makes it so a minimal dependency graph which only includes
compositor and sequencer is built for the render pipeline purposes.

Tricky part here is that it's only compositor itself and sequencer who
to use this dependency graph and IDs from it. Render engines are still
to be provided original IDs because:

  • They will create dependency graph for the given scene, and currently it is not possible to create dependency graph from CoW scene.
  • IDs from the compositor/sequencer dependency graph are "stripped", as in, they wouldn't have all view layers, collections or objects

required for proper final render.

This creates annoying mess of mixing evaluated and original scene
access in various parts of the pipeline.

Fixes T63927: Compositing nodes - drivers don't really work

Diff Detail

Repository
rB Blender
Branch
depsgraph_render_pipeline (branched from master)
Build Status
Buildable 3687
Build 3687: arc lint + arc unit

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.May 22 2019, 11:56 AM

I'm wondering if scene->camera should be included for the defocus node to work with animated cameras. I guess that risks pulling in a lot of other stuff, but probably ok in practice.

Looks ok, I couldn't break it in testing. Mainly comments about naming, which can be debated.

source/blender/depsgraph/intern/builder/deg_builder_map.h
36

Could we use another word than "Tag"? It already has a different meaning, or at least the link to existing tagging is not clear to me.

Maybe just "USE_ANIMATION" or "BUILD_ANIMATION".

source/blender/depsgraph/intern/depsgraph.h
201

Is it just for postprocessing, or would it be used for the pipeline as a whole to animate render parameters that do not affect the compositor and sequencer?

source/blender/render/intern/include/render_types.h
123–124

Can we name these pipeline_depsgraph and pipeline_scene_eval? Just to make the distinction more clear with other depsgraphs.

This revision now requires changes to proceed.May 22 2019, 11:56 AM
source/blender/depsgraph/intern/builder/deg_builder_map.h
36

USE or BUILD is misleading. You are not building here and not using anything here.

Can just strip the prefix, you always access to it via BuilderMap anyway.

But i can't think of another name for the internal variables. But also not sure why there is such an urge to avoid it? There are other types of tagging in a lot of other areas (library/id, bmesh, ...) and didn't seem to be causing issues? It's just what it is.

source/blender/depsgraph/intern/depsgraph.h
201

Ehm. Probably better to rename to bool keep_all_view_layers_but_remove_bases;

All the IDs which are pulled into dependency graph will have animation components. But depsgraph itself doesn't care how the result is used.

As for original question: depsgraph will evaluate scene's animation, which might or might not affect compositor and sequencer.

Use better name for pipeline's depsgraph/scene

Brecht Van Lommel (brecht) added inline comments.
source/blender/depsgraph/intern/depsgraph.h
201

Or it could be is_render_pipeline_depsgraph, easier if the same terminology is used in the render and depsgraph module.

This revision is now accepted and ready to land.May 22 2019, 12:57 PM
  • Use better naming for depsgraph variable
  • Make sure defocus node has camera to work with

Not sure what's better: pull in camera all the time, or have node type
exception in the depsgraph.

This revision was automatically updated to reflect the committed changes.