Cycles: Don't sync unneccessary attributes for final renders
ClosedPublic
Actions

Authored by Lukas Stockner (lukasstockner97) on Oct 9 2016, 2:14 PM.

Details

Reviewers

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

Commits

rBS5aaa64394792: Cycles: Optimize shaders earlier to skip unneccessary attributes for…
rCcf907c6569fc: Optimize shaders earlier to skip unneccessary attributes for noninteractive…
rB5aaa64394792: Cycles: Optimize shaders earlier to skip unneccessary attributes for…

Summary

Currently, the required attributes aren't updated after shader optimization to avoid repeated re-evaluation of them while tweaking shaders in the viewport.
However, that's not a concern in regular F12 renders, so this commit avoids the unneccessary attributes in that case.

Diff Detail

Repository: rB Blender

Event Timeline

Lukas Stockner (lukasstockner97) updated this revision to Diff 7587.Oct 9 2016, 2:14 PM

Lukas Stockner (lukasstockner97) retitled this revision from to Cycles: Don't sync unneccessary attributes for final renders.

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

Lukas Stockner (lukasstockner97) updated this object.

I'm not sure this works? Shaders are compiled after the scene has already been exported, so changing the requested attributes at that point would have no effect.

Ronny G (nutel) added a subscriber: Ronny G (nutel).Oct 11 2016, 1:08 PM

it works for attributes like uv map and vertex color

test file

nodeopti.blend411 KBDownload

optimization for uv tangent / normal map is still missing (:

Hm, I looked into this again and it really was the wrong approach...
This is both shorter and actually works as far as I can see.

mathieu menuet (bliblubli) mentioned this in T50181: Cycles-Regression: invisible shaders are optimized and compiled although they shouldn't.Dec 17 2016, 9:13 AM

Seems like a good approach. The downside of this patch is that the simplification is no longer multithreaded. It should be possible to multithreaded BlenderSync::sync_materials though, so that it's even faster than before due to multithreaded Blender material export.

Ah, yes, I forgot to commit that change.
If it's possible to multithread all of sync_materials, that's even better, but I wasn't sure about the Blender-side code...

Sergey Sharybin (sergey) added inline comments.Dec 20 2016, 9:45 AM

intern/cycles/blender/blender_shader.cpp
1212	Please bother to do benchmarks on real scenes rather than just wacking threading code into somewhere inside of an inner loop. Ideally threading should start as early as possible (in this case, export the whole material), otherwise synchronization overhead in the current task scheduler implementation might make things slower than they were before making things threaded.
intern/cycles/render/graph.cpp
212	Why that's needed here for `simplified` but not for `finalized`?

Lukas Stockner (lukasstockner97) mentioned this in rB05aefabb7b9c: Cycles Experimental Build v4.Dec 20 2016, 4:16 PM

Brecht Van Lommel (brecht) mentioned this in T50974: Normal map tangent computation is slow.Mar 17 2017, 3:09 AM

Please double-check out testsuite is till passing, otherwise LGTM after more careful and complete dig.

intern/cycles/blender/blender_shader.cpp
1212	Ok, re-read code more carefully. This is pushing the outer loop data. While in theory we can do the whole material sync from thread current code should be good enough already.
1221	IMO safer to always for pool.wait_work(). If there is no work to be done it'll be almost NOOP. ut if we'll be using pool for something else than non-preview tasks it'll save us from possible issues related on missing wait_work.