hdBlackbird
hdBlackbird copied to clipboard
New object management
New object management
Hey @bareya Just a summary, what's the purpose of this? I am guessing it is to help out instancing (having the reference concept et-all) and I guess just a better way of syncing data?
I guess the only thing I'd suggest is to check the latest Cycles API changes in the latest Blender, there's a concept of setting ownership of data so that Hydra or whatever can take ownership and not Cycles itself (it's used for procedurals) to see if it's compatible with what you're doing (I guess it is).
Cheers!
Hey @boberfly
It's a draft, and this draft is a result of the work I have done next to Transformation Motion Blur PR (#148). I decided to separate those two PRs.
Just a summary, what's the purpose of this? I am guessing it is to help out instancing (having the reference concept et-all) and I guess just a better way of syncing data?
I mentioned all the problems with current design in our standups, but I will write them down for you as a list:
-
Excessive locking - We should remove all direct access to the
ccl::Scene
from*::Sync
methods and replace it by sub-classes that derive fromHdBufferSource
.*::Sync
should spawn aforementioned sub-classes that are being collected byHdCyclesResourceRegistry
and should be committed to the scene in one place. Direct access toccl::Scene
forces us to lock during the*::Sync
calls. I am not sure if you remember, but I have done some profiling and during interactive Hydra session there is a lot of locking/unlocking happening just to check thatPRim
is not dirty, even if we don't commit any resources. -
Parallel object processing -
*::Sync
is being called from multiple threads, unfortunately we don't take advantage of it - scene locking. Deferring committing resources toHdCyclesResourceRegistry::CommitResources
allows us to have an explicit control over parts can be processed in parallel. -
Committing resources through
HdBufferSource
- Currently we have a lot of code duplication. In most of the places code is copied just to create attributes. I recommend you to read code for curves and how curves handle arbitrary attribute creation. We just submit an instance ofHdCyclesAttributeSource
toHdCyclesResourceRegistry
and attribute creation is handled independently of type ofccl::Geometry
. Also, resolving an attribute happens in parallel. I recommend reading the code inHdCyclesResourceRegistry::_CommitResources
andHdCyclesAttributeSource
. -
Daisy chaining of
HdBufferSource
- CheckHdBufferSource
header file documentation.HdBufferSource::GetChainedBuffers
comes super handy when it comes to chained computations such as normals creation for subdivision meshes. If we use chain ofHdBufferSources
we essentially build a dependency for computing attributes:Compute limit points -> Compute limit tangents -> Compute limit normals
In current implementation we store limit u and v as fields(extra memory overhead) and when normals are requested, we compute them from aforementioned fields.
-
Reduce memory - In many places we store data as fields for no reason.
HdCyclesAttributeSource
carriesVtValue
and the value is expired and removed right afterHdCyclesAttributeSource
instance is gone. -
Automatic binding/unbinding -
HdInstance<>
can automatically control lifespan of lights, materials and objects. Have a look intoHdCyclesResourceRegistry
committing code. Unbinding happens with help of unordered set, and I guess it comes down to linear complexity. Rather than being called byRPrims
directly each timeRPrim
is removed byRenderIndex
. -
Shared instances - I really recommend reading
HdStMesh
code. Topology, instances code can be shared between the objects. I really recommend reading aboutHdInstance
andHdInstanceRegistry
. -
Update only what is necessary - With explicit control of object creation/modification we can have centralized way of updating scene and session. Have a look into
HdCyclesResourceRegistry
committing code.
I guess the only thing I'd suggest is to check the latest Cycles API changes in the latest Blender, there's a concept of setting ownership of data so that Hydra or whatever can take ownership and not Cycles itself (it's used for procedurals) to see if it's compatible with what you're doing (I guess it is).
I will stick to current api for now, unless it becomes must have demand that comes from the production.
I hope that explains all the doubts you had about motivation for this PR.
Cheers!
Thank you for the clarity, I'm aware of your hard efforts addressing these things - it was just a little tricky to tell if this patch is directly related or something else/specific. Cheers!