Cycles: Remove meaningless volume shaders
ClosedPublic
Actions

Authored by Sergey Sharybin (sergey) on Mar 21 2017, 5:23 PM.

Details

Reviewers

Thomas Dinges (dingto)
Brecht Van Lommel (brecht)
Hristo Gueorguiev (nirved)
Mai Lavelle (maiself)
Lukas Stockner (lukasstockner97)

Commits

rBSdd356ec08d07: Cycles: Remove meaningless volume shaders
rC6db6a7b6d100: Remove meaningless volume shaders
rBdd356ec08d07: Cycles: Remove meaningless volume shaders

Summary

This is possible to use surface-only nodes and connect them to volume output.
If there was something connected to surface output those extra connections
will not chang anything visually but will force volume features to be included
into feature-adaptive kernels.

In fact, this exact reason seems to be causing slowdown of Barcelone file
comparing AMD OpenCL to NVidia CUDA.

This patch requires D2285 from Lukas.

Diff Detail

Repository

rB Blender

Branch

cycles_remove_meaningles_volume

Build Status

Buildable 485
Build 485: arc lint + arc unit

Event Timeline

Sergey Sharybin (sergey) created this revision.Mar 21 2017, 5:23 PM

Harbormaster completed remote builds in B484: Diff 8454.Mar 21 2017, 5:23 PM

Fix typo in scheduling

Harbormaster completed remote builds in B485: Diff 8455.Mar 21 2017, 5:47 PM

Sergey Sharybin (sergey) mentioned this in T50808: Volume rendering with GPU bug.Mar 22 2017, 2:29 PM

Usefull patch.
Diff is not made against a master commit and doesn't apply on latest master.

Brecht Van Lommel (brecht) requested changes to this revision.Apr 14 2017, 5:01 PM

Brecht Van Lommel (brecht) added inline comments.

intern/cycles/blender/blender_shader.cpp
1165	As far as I can tell these flags are not yet expected to be set at this point, they are only computed during SVM/OSL compilation.
intern/cycles/render/graph.cpp
716	I think this should happen before or as part of constant folding, so that mix/add nodes can get removed as well?
intern/cycles/render/osl.cpp
1108	A few lines below here variables like `has_surface`, `has_volume` and `has_displacement` are set to false, and then computed later. So as far as I can tell `update_flags()` will have no effect and could just be removed?
intern/cycles/render/svm.cpp
828	Same comment as for OSL.

This revision now requires changes to proceed.Apr 14 2017, 5:01 PM

@Brecht Van Lommel (brecht),

intern/cycles/render/graph.cpp
716	Hrm, not sure it fits in constant folding: as far as i know, you'll need to recurse into Volume input of material output node. and do the same logic as here. Otherwise you don't know whether you can remove Diffuse BSDF or not, because it can be used for both Surface and Volume, so you cant' configure constant folder to do special folding for volume output.
intern/cycles/render/osl.cpp
1108	Indeed seems this can be removed.
intern/cycles/render/svm.cpp
828	Indeed.