Re: [PATCH 1/4] dx8vb: Add stubs for D3DX8 interface

[Henri Verbeet]

Perhaps this is sufficiently far removed from core Direct3D that
nobody cares, but for what it's worth:

On 31 October 2017 at 03:01, Fabian Maurer  wrote:
> +typedef struct
> +{
> - ID3DX8 ID3DX8_iface;
> - LONG ref;
>
> +} d3dx8;

Please don't typedef structures just to save keystrokes.
> +static HRESULT WINAPI d3dx8_QueryInterface(ID3DX8 *iface, REFIID riid, void **ppv)
> +{
> - d3dx8 *This = impl_from_ID3DX8(iface);

Please use lowercase variable names.
> -
> - TRACE("(%p/%p)->(%s,%p)\n", iface, This, debugstr_guid(riid), ppv);
> -
> - *ppv = NULL;
> -
> - if (IsEqualGUID(riid, &IID_IUnknown))
> -
>    *ppv = &This->ID3DX8_iface;
> - else if(IsEqualGUID(riid, &IID_ID3DX8))
> -
>    *ppv = &This->ID3DX8_iface;
> - else if(IsEqualGUID(riid, &IID_IPersistPropertyBag))
> -
>    FIXME("No interface for IID_IPersistPropertyBag\n");
> - else if(IsEqualGUID(riid, &IID_IPersistStreamInit))
> -
>    FIXME("No interface for IID_IPersistStreamInit\n");
> - else
> -
>    FIXME("No interface for %s\n", debugstr_guid(riid));

Not implementing an arbitrary interface is a WARN at best.
> -
> - if (*ppv)
> - {
> -
>    IUnknown_AddRef((IUnknown *)(*ppv));
> -
>    return S_OK;
> - }
> -
> - return E_NOINTERFACE;
>
> +}

I think Nikolay already hinted at this, but this is fairly different
from the canonical QueryInterface() implementation, especially for an
object that only implements a single interface.
> +static HRESULT WINAPI d3dx8_CreateFont(ID3DX8 *iface, Direct3DDevice8 *device,
> -
>      LONG hFont, D3DXFont **retFont)

Please use lowercase parameter names, and avoid the stray 'h'.
> +static HRESULT WINAPI d3dx8_AssembleShaderFromFile(ID3DX8 *iface, BSTR file, LONG flags, BSTR *log,
> -
>    D3DXBuffer **constants, D3DXBuffer **ppVertexShader)
>
> +{
> - FIXME("(%s, %i, %p, %p, %p): stub!\n", debugstr_w(file), flags, log, constants, ppVertexShader);

I certainly have my preferences when it comes to trace formatting, but
I think "%i" is questionable for flags regardless of preference.
> +static const ID3DX8Vtbl ID3DX8_Vtbl =

"d3dx8_vtbl"
> +HRESULT d3dx8_create(IUnknown *outer_unk, void **ppv)
> +{
> - d3dx8 *object;
> -
> - TRACE("(%p,%p)\n", outer_unk, ppv);
> -
> - object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(d3dx8));

"sizeof(*object)"