Page MenuHome

Cycles: Speedup Tangent calculation in Mikktspace
AbandonedPublic

Authored by Lukas Stockner (lukasstockner97) on Jun 4 2017, 11:58 PM.

Details

Summary

The current code is fairly inefficient for multiple reasons.
This commit addresses two of them:

  • The getter functions were directly accessing the RNA arrays, which causes a lot of overhead since they are called many times per vertex/face. By iterating over the RNA arrays once and caching the data in temporary storage, this overhead is avoided.
  • The step that merges duplicate vertices only performed binning in one spatial dimension and then looped over all vertices in each bin. By hashing all nine components, binning these hashes and then checking inside each bin the code is simplified a lot while also saving a huge amount of time.

There are many more optimization opportunities, but these two already halve
render times for my test file (suzanne with default unwrapping and subdivided
to level 6).

Diff Detail

Repository
rB Blender
Branch
tangent_speedup
Build Status
Buildable 980
Build 980: arc lint + arc unit

Event Timeline

Hi Lukas, great to see you back on other task than Denoiser :)
When trying to build on MSVC 2015, it prints some errors:

Ah, I see, you'll have to add 'typedef unsigned int uint;' above float_as_uint.

Tested this with 9-10 scene's and had no issues, Gives nice speedup, 5-20% generally for me.

Built OSX, added the 'typedef unsigned int uint;' above float_as_uint.
Then got these two errors in mikktspace.c (had to use screen capture as I didn't find a way to export error log yet).

If the C++ RNA is a bottleneck, then we should either look into ways to optimize it or to reformulate synchronization code in Cycles in a way that we don't need to duplicate data again and again: you can easily re-use attributes which already exists in Cycles. Additionally, it if it's RNA to blame, you still have overhead on synchronizing mesh itself.

One more thing: since rB5abae51 we can have access to all tangent maps generated on Blender side. If we are going to change how tangents are calculated in Cycles i would strongly suggest to make it so tangents are handled by Blender side. Would avoid the RNA bottleneck you've mentioned, avoid duplicated code, and additionally will make code more robust against further changes.

intern/cycles/blender/blender_mesh.cpp
152–155 ↗(On Diff #8906)

This adds yet another copy of data. How does this affect on memory usage when rendering production files?

Changes to mikkspace itself can go into master perhaps, but Cycles part i;d rather look into solution which will not involve data and code duplicaiton. Have some ideas which i'm experimenting with atm.

Now that we have D2810, here's a new version containing only the Mikktspace changes.

@Lukas Stockner (lukasstockner97) , those changes were good since last time i've cheched them. If nothing have changed, just commit them.

This revision is now accepted and ready to land.Nov 17 2017, 5:52 PM