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

API documentation #13

Open
mrgreywater opened this issue Mar 20, 2016 · 3 comments
Open

API documentation #13

mrgreywater opened this issue Mar 20, 2016 · 3 comments

Comments

@mrgreywater
Copy link
Contributor

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.

@khillesl-AMD
Copy link
Contributor

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?

@mrgreywater
Copy link
Contributor Author

mrgreywater commented Apr 29, 2016

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.

@khillesl-AMD
Copy link
Contributor

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

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

3 participants