Details

Reviewers

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

Commits

rC40e64c1f6b40: Render API/Cycles: Identify Render Passes by their name instead of a type flag
rBS4cf7fc3b3a4d: Render API/Cycles: Identify Render Passes by their name instead of a type flag
rB4cf7fc3b3a4d: Render API/Cycles: Identify Render Passes by their name instead of a type flag

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

Event Timeline

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

Lukas Stockner (lukasstockner97) retitled this revision from to Render API/Cycles: Identify Render Passes by their Name instead of a type flag.

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 subscriber: Julian Eisel (Severin).

Lukas Stockner (lukasstockner97) added a child revision: D2444: Render API/Cycles: Allow Render Engines to add passes, add ID Properties to Render Layers, bring back Cycles Debug Passes.Jan 4 2017, 1:14 AM

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

Stefan Werner (swerner) added a subscriber: Stefan Werner (swerner).Mar 21 2017, 6:51 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

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.

Addressed all review points.

Lukas Stockner (lukasstockner97) added inline comments.Apr 15 2017, 2:18 AM

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

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!

Looks good to me!

This revision is now accepted and ready to land.Apr 25 2017, 12:35 PM

Closed by commit rB4cf7fc3b3a4d: Render API/Cycles: Identify Render Passes by their name instead of a type flag (authored by Lukas Stockner (lukasstockner97)). · Explain WhyMay 3 2017, 4:52 PM

This revision was automatically updated to reflect the committed changes.

Stefan Werner (swerner) mentioned this in rB7ffc46bde79f: D2443.May 29 2019, 1:21 PM

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

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 8756

intern/cycles/blender/addon/__init__.py

intern/cycles/blender/addon/engine.py

intern/cycles/blender/addon/properties.py

intern/cycles/blender/addon/ui.py

intern/cycles/blender/blender_python.cpp

intern/cycles/blender/blender_session.cpp

intern/cycles/blender/blender_sync.h

intern/cycles/blender/blender_sync.cpp

source/blender/blenkernel/BKE_blender_version.h

source/blender/blenkernel/BKE_node.h

source/blender/blenkernel/intern/image.c

source/blender/blenkernel/intern/scene.c

source/blender/blenloader/intern/readfile.c

source/blender/blenloader/intern/versioning_270.c

source/blender/blenloader/intern/writefile.c

source/blender/compositor/CMakeLists.txt

source/blender/compositor/intern/COM_NodeGraph.cpp

source/blender/compositor/nodes/COM_ImageNode.cpp

source/blender/compositor/nodes/COM_RenderLayersNode.h

source/blender/compositor/nodes/COM_RenderLayersNode.cpp

source/blender/compositor/operations/COM_RenderLayersProg.h

source/blender/compositor/operations/COM_RenderLayersProg.cpp

source/blender/editors/render/render_internal.c

source/blender/editors/render/render_update.c

source/blender/editors/space_image/image_buttons.c

source/blender/editors/space_node/drawnode.c

source/blender/editors/space_node/node_edit.c

source/blender/freestyle/intern/blender_interface/FRS_freestyle.cpp

source/blender/makesdna/DNA_node_types.h

source/blender/makesdna/DNA_scene_types.h

source/blender/makesrna/intern/CMakeLists.txt

source/blender/makesrna/intern/rna_nodetree.c

source/blender/makesrna/intern/rna_render.c

source/blender/makesrna/intern/rna_scene.c

source/blender/nodes/CMakeLists.txt

source/blender/nodes/NOD_composite.h

source/blender/nodes/composite/node_composite_tree.c

source/blender/nodes/composite/nodes/node_composite_image.c

source/blender/render/CMakeLists.txt

source/blender/render/extern/include/RE_engine.h

source/blender/render/extern/include/RE_pipeline.h

source/blender/render/intern/include/render_result.h

source/blender/render/intern/include/rendercore.h

source/blender/render/intern/source/envmap.c

source/blender/render/intern/source/external_engine.c

source/blender/render/intern/source/pipeline.c

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

source/blender/render/intern/source/rendercore.c

source/blender/render/intern/source/zbuf.c

source/blenderplayer/bad_level_call_stubs/stubs.c

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

Revision Contents
Changeset List

intern/cycles/blender/addon/init.py