Render API/Cycles: Allow Render Engines to add passes, add ID Properties to Render Layers, bring back Cycles Debug Passes
ClosedPublic
Actions

Authored by Lukas Stockner (lukasstockner97) on Jan 4 2017, 1:14 AM.

Details

Reviewers

Brecht Van Lommel (brecht)
Thomas Dinges (dingto)
Sergey Sharybin (sergey)
Martijn Berger (juicyfruit)

Summary

The title says it all - these changes add RE_engine_add_pass to the Engine API, allow engine plugins to add own properties to SceneRenderLayers and implement the Cycles Debug passes using the new system.

Credit for the ID property part goes to @Julian Eisel (Severin).

Depends on D2443.

Diff Detail

Repository

rB Blender

Branch

arcpatch-D2444

Build Status

Buildable 398
Build 398: arc lint + arc unit

Event Timeline

Lukas Stockner (lukasstockner97) updated this revision to Diff 8072.Jan 4 2017, 1:14 AM

Lukas Stockner (lukasstockner97) retitled this revision from to Render API/Cycles: Allow Render Engines to add passes, add ID Properties to Render Layers, bring back Cycles Debug Passes.

Lukas Stockner (lukasstockner97) added reviewers: Sergey Sharybin (sergey), Brecht Van Lommel (brecht), Thomas Dinges (dingto), Martijn Berger (juicyfruit).

Lukas Stockner (lukasstockner97) updated this object.

Lukas Stockner (lukasstockner97) added a parent revision: D2443: Render API/Cycles: Identify Render Passes by their Name instead of a type flag.

Lukas Stockner (lukasstockner97) added a subscriber: Julian Eisel (Severin).

Some preliminary feedback.

Not sure we should keep adding things to existing RenderLayer from master branch. This is a subject of reworking in the render-layers branch. Doing changes in this area in master will likely lead to a duplicated work of porting this over to the new system.

Even if we want to do such change in master now, this change should be split into several smaller ones.

intern/cycles/blender/addon/ui.py
512–516	Dons't seems correct, debug passes are still are inside of a compile-time option. Meaning, for official releases we expose something to the interface which will never work.
intern/cycles/blender/blender_sync.cpp
479	Not really fond of such c++-ism. It's not necessarily easy to read and not strictly speaking needed here. Also, don't use capitals for argument names.

Okay, the two issues you mentioned are fixed.

As for the render_layers branch - I'm not really up-to-date on the changes there, but these two patches don't really change anything regarding layers, so I'd expect that merging isn't too hard.

Anyways, render_layers is 2.8, and I don't see a reason why this patch would need to wait for 2.8 - it's fully forwards and backwards compatible and allows to add some long-awaited functionality - I already have locally:

A patch for light groups - passes only including some lights
An experimental patch for an AOV Output node that would allow the user to create as many passes as they want and plug any shader noodle into it

And could easily use the work from D2106 to implement a better ID matte system that supports DoF/MoBlur/transparency.
In addition, it allows external render plugin writers to implement passes in their own renderer.

Edit: Regarding splitting the changes, I already split them into 5 commits. Finer splitting isn't really possible without getting broken intermediate versions.

Some minor comments, overall seems fine.

Perhaps adding these kinds of features in master is ok if the person adding them then also does the merge into the 2.8 branch?

source/blender/makesrna/intern/rna_render.c
480–487	The order of these parameters seems a bit odd, particularly to have the number of channels and channel names separate. Perhaps use: add_pass(name, num_channels, channel_names, view, layer); Do we need to support adding a pass only to specific views? I'm not sure if there are practical use cases, and it goes beyond what users can do in the UI and what the compositor expects I think.
source/blender/render/intern/source/render_result.c
471–490	The code in this function could be simplified with `BLI_findstring`.

This revision now requires changes to proceed.Jan 21 2017, 4:35 PM

In D2444#57596, @Brecht Van Lommel (brecht) wrote:

Perhaps adding these kinds of features in master is ok if the person adding them then also does the merge into the 2.8 branch?

I guess someone who's familiar with the 2.8 changes should do a master->blender2.8 merge before merging this. Not only for this patch, but as a general practice now that blender2.8 and master started to differ quite a bit. Just to ensure merge conflicts in other areas are handled by people who are at least a bit familiar with them.

Stefan Werner (swerner) added a subscriber: Stefan Werner (swerner).Mar 21 2017, 6:51 PM

Steffen Dünner (SteffenD) added a subscriber: Steffen Dünner (SteffenD).Mar 25 2017, 8:19 PM

Lukas Stockner (lukasstockner97) mentioned this in rBd9bfce36b70b: Cycles experimental build v5.Mar 28 2017, 5:20 AM

Lukas Stockner (lukasstockner97) mentioned this in rB82bcfb9b342a: Cycles experimental build v5, fixed version.Mar 28 2017, 4:41 PM

Stefan Werner (swerner) mentioned this in D2443: Render API/Cycles: Identify Render Passes by their Name instead of a type flag.Apr 5 2017, 2:54 PM