[PATCH 1/3] d3dx9: Implement D3DXCreateAnimationController with a stubbed ID3DXAnimationController interface. (try 2)

[Matteo Bruni]

2016-01-06 3:52 GMT+01:00 Alistair Leslie-Hughes <leslie_alistair at hotmail.com>:
> From: Christian Costa <titan.costa at gmail.com>
>
> Correct QI to be like the IDevice11 one.
> Changed DOUBLE to double in the interface.
> Adjusted parameter names in D3DXCreateAnimationController
>
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair at hotmail.com>

> +static inline struct d3dx9_animation_controller *impl_from_ID3DXAnimationController(ID3DXAnimationController *iface)

Not a big deal but I think it's better to avoid the "inline" here,
it's unnecessary in this case since the function isn't in a .h file.

> +static HRESULT WINAPI d3dx9_animation_controller_AdvanceTime(ID3DXAnimationController *iface, double time_delta,
> +        ID3DXAnimationCallbackHandler **callback_handler)

I think callback_handler only wants one '*' here.

> +static HRESULT WINAPI d3dx9_animation_controller_GetTrackPriority(ID3DXAnimationController *iface,
> +        UINT track, D3DXPRIORITY_TYPE *priority)

This one should be called SetTrackPriority, at least according to the
header from the June 2010 DirectX SDK. The header currently in Wine
has the same GetTrackPriority prototype you used here (it's there
since the header was first introduced by Dylan Smith in
5cccad1311745117628b9a5f10652b5211151b51). I guess this is simply a
typo in our header and it "propagated" here but if anyone knows better
please speak up ;)

Quite reasonably, in a sense, the "priority" parameter for the
SetTrackPriority method is not supposed to be a pointer.

> +static HRESULT WINAPI d3dx9_animation_controller_SetTrackPosition(ID3DXAnimationController *iface,
> +        UINT track, double position)
> +{
> +    FIXME("iface %p, track %u, position %g stub.\n", iface, track, position);

It doesn't seem a good choice to me to use %g instead of %f for double
parameters.

> +static HRESULT WINAPI d3dx9_animation_controller_SetTrackEnable(ID3DXAnimationController *iface,
> +        UINT track, BOOL enable)
> +{
> +    FIXME("iface %p, track %u, enable %u stub.\n", iface, track, enable);

BOOL is defined as long i.e. signed.

> +static D3DXEVENTHANDLE WINAPI d3dx9_animation_controller_GetUpcomingPriorityBlend(ID3DXAnimationController *iface,
> +        D3DXEVENTHANDLE handle)
> +{
> +    FIXME("iface %p, handle %u stub.\n", iface, handle);

Not too important but you called all the other occurrences "event"
instead of "handle".

> +static HRESULT WINAPI d3dx9_animation_controller_CloneAnimationController(ID3DXAnimationController *iface, UINT max_num_anim_outputs,
> +        UINT max_num_anim_sets, UINT max_num_tracks, UINT max_num_events, ID3DXAnimationController **anim_controller)

You can probably drop the "num_" part of those parameter names for brevity.