Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lion03 opened this issue Mar 25, 2016 · 8 comments

Comments

@lion03
Copy link

lion03 commented Mar 25, 2016

Hello!

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

@jstewart-amd
Copy link
Contributor

jstewart-amd commented Apr 28, 2016

@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
Copy link
Contributor

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
Copy link
Author

lion03 commented Apr 29, 2016

@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
Copy link
Contributor

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
Copy link
Author

lion03 commented Apr 30, 2016

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

@vlj
Copy link

vlj commented Jun 2, 2016

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
Copy link
Contributor

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

@vlj
Copy link

vlj commented Jun 8, 2016

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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants