GPUOpen-Effects / TressFX

DirectX 12 and Vulkan libraries that provide convenient access to realistically rendered and simulated hair and fur
MIT License
766 stars 129 forks source link

Design clarification regarding the reason for using single global TressFX_OpaqueDesc #14

Open lion03 opened 8 years ago

lion03 commented 8 years ago

Hello!

Could you please clarify why you chose to use a single TressFX_OpaqueDesc for the lifetime of the program?

jstewart-amd commented 8 years ago

@khillesl-AMD will have more to say on this soon, but this decision was ... not universally beloved by the AMDers currently involved with TressFX. We may refactor this.

khillesl-AMD commented 8 years ago

We'd like to fix things like that, and make the library easier to integrate for production use. This is just one of many things that could change. Knowing you've worked on an Unreal integration, we'd like to hear what you think about that.

lion03 commented 8 years ago

@khillesl-AMD The way TressFX currently written it is very hard to integrate without rewriting TressFX Lib. The major issues with the current way TressFX is written is: 1) Asset loading and D3D resource creation are tightly coupled, while in UE4 asset loading and D3D api calls happen on different threads. 2) The single static TressFX_OpaqueDesc restricts us to a single TFX asset. 3) Asset export should be as a single file, otherwise the import into UE4 is a pain. 4) Animating skinned TFX asset using the vertex buffers of the skinned mesh is impossible since UE4 when build in shipping configuration optimizes them out on the cpu side, on the other hand the bone matrices remain accessible from the cpu.

khillesl-AMD commented 8 years ago

Good feedback. 1,2 and 4 I had mostly guessed (and argued the same fairly early on). 3, I hadn't.

for 3 - Is the issue that there's a .tfx file as well as a .tfxsim file, etc, for each section? or is it that the hair model is in multiple sections, and there's a file (set) for each section?

I have argued that we should just have one section = one object = one set of uniform settings (including LOD/section), rather than having objects made of pieces from a library perspective, just to simplify things. In other words, the current hair model woudl be something like 4 objects sharing the same transform, rather than 1 object with 4 sets of settings inside of it. There are a bunch of other factors here, but wondering if that is actually moving in the right or wrong direction, from your perspective, with respect to item 3.

lion03 commented 8 years ago

I ended up using the tfxproj file from the viewer to import the rest

vlj commented 8 years ago

On a separate note, why is there a global g_PPLBuffers defined in TressFXRenderer.cpp ? As far as I see it's always mirrored by TressFXRenderer::m_pHeadPPLL_Buffer, m_pPPLL_Buffer . When a new PPLLBuffer is requested, the global one is destroyed and replaced by the new one, and the member in TressFXRenderer are changed so it looks like it's not really used.

khillesl-AMD commented 8 years ago

As I understand it, it was a hastily done hack to try multiple objects.

vlj commented 8 years ago

There are several #define (SHORTCUT_WEIGHTED_AVERAGE, SHORTCUT_DETERMINISTIC) also used in shader, are they here for debug purpose or will they become runtime config toggle in an update ?