GPUOpen-Effects/TressFX

API documentation

mrgreywater opened this issue · 3 comments

It would be nice to have documentation for the API and maybe some samples. Right now all the functions take a TressFX_Desc which contains public state about the Hair asset, but it is unknown which fields need to be set before each function call, and which fields are modified.

Additionally, there seem to be unused members in the TressFX_Desc, such as mWorld, which make it quite frustrating to work with TressFX. The name mWorld suggests it's supposed to be the world transform for the model, but in fact it does nothing, it's not used anywhere in the code, and modelTransformForHead has to be used instead.

The design philosophy was to allow maximum flexibility, which means passing in as much info as possible, even if not used. It also means these functions do quite a bit.

What do you think of a more drastic refactoring, where we break these functions down a bit, and move away from the "give us everything, and a raw D3D11 device pointer, and we'll take care of everything" API philosophy?

move away from the "give us everything, and a raw D3D11 device pointer, and we'll take care of everything" API philosophy?

In my opinion, the problem is not the philosophy itself, but the fact that there is one big undocumented god-object 'TressFxDesc'.

A few examples of difficulties I had are:

  • I cannot load my own assets that are generated procedurally from memory. TressFX_LoadRawAsset takes an opaque blob, instead of a documented structure.
  • Before my last PR, just drawing the Hair without simulating didn't actually draw the Hair at the correct position. (Because the simulation was actually the function setting the position of the final vertices)
  • Uncertainty which members are required to be set before each TressFX function.
  • Uncertainty which members are modified by TressFX functions themselves.
  • Having members such as mWorld that don't do anything are highly misleading.

Either there needs to be a good documentation on how to handle the 'TressFXDesc', or the API really needs refactoring. If I would have to design such API, It would probably look like this

struct TressFX_HairAsset {
    size_t numGuideStrands;
    float *guideStrands;
    bool singleHeadTransform;
    bool enableSkinning;
    // ... all asset information that is not supposed to change at runtime (aka the tfx file content)
}

HRESULT TressFX_Initialize(); //global initialization
HRESULT TressFX_CreateHairAsset(TressFX_HairAsset const&, int* hairID);
HRESULT TressFX_SetDevice(int hairID, ID3D11Device *device, ID3D11DeviceContext *context);
HRESULT TressFX_Begin(int hairID);
HRESULT TressFX_End(int hairID);
HRESULT TressFX_SetRenderAttributes(int hairID, TressFX_HairParams const& params);
HRESULT TressFX_SetSimulationAttributes(int hairID, TressFX_SimulationParams const& params, TressFX_CollisionCapsule const& capsule);
HRESULT TressFX_SetTransforms(int hairID, DirectX::XMMATRIX const& world, DirectX::XMMATRIX const& view, DirectX::XMMATRIX const& proj);
HRESULT TressFX_GenerateAnimationTransforms(int hairID, TressFX_SceneMesh const&sceneMesh);
HRESULT TressFX_GenerateAnimationTransforms(int hairID, float* bones, size_t numBones);
HRESULT TressFX_Simulate(int hairID, float elapsedTime, bool warp = false);
HRESULT TressFX_RenderShadowMap(int hairID);
HRESULT TressFX_Render(int hairID, int antialias = 0);
HRESULT TressFX_Resize(int hairID, int width, int height);
HRESULT TressFX_Free(int hairID);
HRESULT TressFX_Uninitialize(); //global uninitialization, releases stray hair instances

Note this approach removes 'TressFXDesc' from the API completely and makes it relatively obvious how to interface with TressFX. Additionally each function should return proper error messages if it is called with arguments that are invalid - right now if often just doesn't work, or it hits an assertion in the TressFX code, which is annoying and shouldn't happen.

This is good. We'll try to move in this direction, although I can't give you an ETA yet.