Render API/Cycles: Identify Render Passes by their Name instead of a type flag
ClosedPublic

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

Details

Summary

Previously, every RenderPass would have a bitfield that specified its type. That limits the number of passes to 32, which was reached a while ago.
However, most of the code already supported arbitrary RenderPasses since they were also used to store Multilayer EXR images.
Therefore, this commit completely removes the passflag from RenderPass and changes all code to use the unique pass name for identification.
Since Blender Internal relies on hardcoded passes and to preserve compatibility, 32 pass names are reserved for the old hardcoded passes.

To support these arbitrary passes, the Render Result compositor node now adds dynamic sockets. For compatibility, the old hardcoded sockets are always stored and just hidden when the corresponding pass isn't available.

From a user perspective, not much changes with this commit - the only difference is that now the Render Result node will only update its sockets after rendering (but before running the compositor), not as soon as the render layer options are changed.

The actual RNA function that allows render engines to create own passes and a new integration of the Cycles Debug passes will follow later, I guess it's easier to review the changes one at a time. This one is already big enough :)

Also, included as a separate commit, a visual improvement:

To provide better visual indication in the compositor, sockets for passes that aren't availiable are still drawn, but all their links are dashed and they behave as if SOCK_UNAVAIL was set. This is handled by the new socket flag SOCK_VIRTUAL.
This is to avoid confusion since, when the user hasn't rendered yet after loading a file, only the Image and Alpha sockets are available. Therefore, without this change, the compositor setup would appear broken until the first render.
Since this comes down to UI preference, I'll just go ahead and poke @Julian Eisel (Severin) as well :)

Diff Detail

Repository
rB Blender
Lukas Stockner (lukasstockner97) retitled this revision from to Render API/Cycles: Identify Render Passes by their Name instead of a type flag.Jan 4 2017, 1:05 AM
Brecht Van Lommel (brecht) requested changes to this revision.Jan 21 2017, 6:27 PM

It's great to see this finally tackled, and nice to see so much red in the diff.

From a user perspective, not much changes with this commit - the only difference is that now the Render Result node will only update its sockets after rendering (but before running the compositor), not as soon as the render layer options are changed.

Can we fix this? I can see how postponing creating the sockets until after rendering simplifies the code and gives some flexibility to the renderer to dynamically decide which passes to output at render time. But still, being able to set up the compositing before you press render seems important to me. Maybe also for scripting/automation use cases.

Not sure what the best solution would be. A utility function could be added on SceneRenderLayer, that lets renderers update the list of available passes? And this would then typically be run from an update callback on custom render layer properties as defined by the renderer.

source/blender/blenkernel/intern/image.c
3754

Is the addition of rpass->view_id == actview a bugfix for something that's broken now?

source/blender/blenloader/intern/versioning_270.c
1582

Is CMP_NODE_COMPOSITE supposed to be here? It doesn't have any output sockets so this code would do nothing?

source/blender/compositor/nodes/COM_RenderLayersNode.cpp
69–73

Could use BLI_findstring here.

81–83

Why is AO a special exception? Shouldn't it be a color?

source/blender/compositor/operations/COM_RenderLayersProg.h
68

Is it strictly safe for this to be a pointer instead of an array? Could the memory get freed when still in use here?

source/blender/editors/space_image/image_buttons.c
463–466

Use BLI_findstring?

source/blender/makesrna/intern/rna_render.c
824–825

Can we avoid breaking API compatibility and keep find_by_type, at least in master? In 2.8 it could be removed.

892–893

Same comments for these properties, would prefer to avoid breaking python API compatibility in master.

source/blender/render/intern/source/render_result.c
657

With latest master on macOS I get a build error here:

error: use of undeclared identifier 'uint'; did you mean 'rint'?

Guess this can just be changed to match passtype_a.

This revision now requires changes to proceed.Jan 21 2017, 6:27 PM

Can confirm the compile error with uint on windows as well, however if i manually fix the error, I'm also greeted by a linker error

bf_rna.lib(rna_render_gen.obj) : error LNK2019: unresolved external symbol RE_pass_find_by_name referenced in function rna_RenderPass_find_by_name

What will it take to get this and D2444 into master? I'd love to be able to have the foundation for many other passes (light groups, Cryptomatte, eventually even LPEs) to be added to Blender and Cycles.

@Stefan Werner (swerner), not sure if you already discussed this further with other Cycles devs, but I guess talk to Lukas about taking over this patch if it's important to you?

I'm guessing most of the comments I added are not a lot of work to address, except for the UI update issue for which we need to decide on a design.

I'm working on updating this patch right now, so there's no need to take it over currently.

For the node update issue, I added a function to the Renderer API that's called from the node, and the renderer then calls a function in the RNA of the node that registers passes that will be generated.

Lukas Stockner (lukasstockner97) marked 8 inline comments as done.

Addressed all review points.

source/blender/blenkernel/intern/image.c
3754

To be honest, I don't really remember - while I was working on this patch I got a lot of duplicated passes, which was fixed by adding this.
However, what I did to cause the error was fairly basic, so I'm pretty sure that it would have been noticed already if this was an existing bug.

source/blender/compositor/nodes/COM_RenderLayersNode.cpp
81–83

Hm, I though that AO was a special case somewhere, but I can't find it and COM_DT_VECTOR really doesn't make any sense.

Forgot to commit the AO pass change.

Forgot to remove some changes while cherrypicking. Sorry for the noise :S

Brecht Van Lommel (brecht) requested changes to this revision.Apr 20 2017, 9:35 PM

This seems to be working well in my testing.

For the pass registration callback, could we make it not work on a node but instead a get_passes(self, srl) method where the API user calls engine.register_pass(srl, 'UV', ..)? There may be other cases in the future where we want to query the passes. Internally it could just register them on the node, but would be nice if the public API did not make that assumption.

There is currently also no distinction between RGB and vectors, maybe add an is_color boolean parameter, or even an enum with options FLOAT/RGB/RGBA/VECTOR2/VECTOR3/VECTOR4/INT? To show the appropriate socket type in the compositor, decision to use color management, how to save it in EXR files, etc.

intern/cycles/blender/addon/engine.py
214

This line must be removed, it's part of the patch to bring back debug passes I think.

This revision now requires changes to proceed.Apr 20 2017, 9:35 PM

Yes, that's a lot better, thanks for the suggestion!

This revision is now accepted and ready to land.Apr 25 2017, 12:35 PM
This revision was automatically updated to reflect the committed changes.