Re: [PATCH 2/5] d3dx9_36: Implemented ID3DXConstantTable_SetIntArray and ID3DXConstantTable_SetInt (Try 5)

[Rico Schüller]

Hi,

this and the previous patch (1/5) are doing the same thing? Is there a 
reason why they could not use the same code base?

e.g.
ID3DXConstantTable_SetIntArray(iface, device, constant, n, count)
{
     return ID3DXConstantTable_SetFloatArray(iface, device, constant, n, 
count);
}

Also I'd prefer if there would be TRACEs in each function.

It might be useful to add the RegisterSet in the FIXME for the default 
case, to see which one is missing in the special case.

Cheers
Rico

Am 27.03.2011 22:22, schrieb Travis Athougies:
> dlls/d3dx9_36/shader.c |   37 ++++++++++++++++++++++++++++++-------
>   1 files changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/dlls/d3dx9_36/shader.c b/dlls/d3dx9_36/shader.c
> index 4a3caab..874d916 100644
> --- a/dlls/d3dx9_36/shader.c
> +++ b/dlls/d3dx9_36/shader.c
> @@ -847,11 +847,7 @@ static HRESULT WINAPI ID3DXConstantTableImpl_SetBoolArray(ID3DXConstantTable* if
>
> static HRESULT WINAPI ID3DXConstantTableImpl_SetInt(ID3DXConstantTable* iface, LPDIRECT3DDEVICE9 device, D3DXHANDLE constant, INT n)
>   {
> - ID3DXConstantTableImpl *This = impl_from_ID3DXConstantTable(iface);
> -
> - FIXME("(%p)->(%p, %p, %d): stub\n", This, device, constant, n);
> -
> - return E_NOTIMPL;
> -
>
> return ID3DXConstantTable_SetIntArray(iface, device, constant,&n, 1);
> }
>
> static HRESULT WINAPI ID3DXConstantTableImpl_SetIntArray(ID3DXConstantTable* iface, LPDIRECT3DDEVICE9 device,
>
> @@ -859,9 +855,36 @@ static HRESULT WINAPI ID3DXConstantTableImpl_SetIntArray(ID3DXConstantTable* ifa
>   {
>       ID3DXConstantTableImpl *This = impl_from_ID3DXConstantTable(iface);
> - FIXME("(%p)->(%p, %p, %p, %d): stub\n", This, device, constant, n, count);
> - D3DXCONSTANT_DESC desc;
> - HRESULT hr;
> - UINT i, desc_count = 1;
> - float row[4] = {0.0f, 0.0f, 0.0f, 0.0f};
> -
> - hr = ID3DXConstantTable_GetConstantDesc(iface, constant,&desc,&desc_count);
> - if (FAILED(hr))
> - {
> -
>    TRACE("ID3DXConstantTable_GetConstantDesc failed: %08x", hr);
> -
>    return D3DERR_INVALIDCALL;
> - }
> - return E_NOTIMPL;
> -
>
> switch (desc.RegisterSet)
> -
>
> {
> -
>
> case D3DXRS_FLOAT4:
> -
>    for (i = 0; i<  count&&  i<  desc.RegisterCount; i++)
> -
>    {
> -
>        row[0] = n[i];
> -
>        if (is_vertex_shader(This->desc.Version))
> -
>            IDirect3DDevice9_SetVertexShaderConstantF(device, desc.RegisterIndex + i, row, 1);
> -
>        else
> -
>            IDirect3DDevice9_SetPixelShaderConstantF(device, desc.RegisterIndex + i, row, 1);
> -
>    }
> -
>    break;
> -
>
> default:
> -
>    FIXME("Can't handle RegisterSets other than D3DXRS_FlOAT4\n");
> -
>    return E_NOTIMPL;
> -
>
> }
> -
> -
>
> return D3D_OK;
> }
>
> static HRESULT WINAPI ID3DXConstantTableImpl_SetFloat(ID3DXConstantTable* iface, LPDIRECT3DDEVICE9 device,

[PATCH 2/5] d3dx9_36: Implemented ID3DXConstantTable_SetIntArray and ID3DXConstantTable_SetInt (Try 5)

[Travis Athougies]

2011/3/28 Rico Schüller :
> Hi,
>
> this and the previous patch (1/5) are doing the same thing? Is there a
> reason why they could not use the same code base?
>
> e.g.
> ID3DXConstantTable_SetIntArray(iface, device, constant, n, count)
> {
>    return ID3DXConstantTable_SetFloatArray(iface, device, constant, n,
> count);
> }

Ints and floats are represented differently and so, if we want to put
an integer array into floating point registers, we need to have casts.
If you were to simply call SetFloatArray, you would get nonsense
values in the floating point registers.
> Also I'd prefer if there would be TRACEs in each function.
>
> It might be useful to add the RegisterSet in the FIXME for the default case,
> to see which one is missing in the special case.

Okay.

Travis.
> Cheers
> Rico
>
> Am 27.03.2011 22:22, schrieb Travis Athougies:
> > dlls/d3dx9_36/shader.c |   37 ++++++++++++++++++++++++++++++-------
> >  1 files changed, 30 insertions(+), 7 deletions(-)
> >
> > diff --git a/dlls/d3dx9_36/shader.c b/dlls/d3dx9_36/shader.c
> > index 4a3caab..874d916 100644
> > --- a/dlls/d3dx9_36/shader.c
> > +++ b/dlls/d3dx9_36/shader.c
> > @@ -847,11 +847,7 @@ static HRESULT WINAPI
> > ID3DXConstantTableImpl_SetBoolArray(ID3DXConstantTable* if
> >
> > static HRESULT WINAPI ID3DXConstantTableImpl_SetInt(ID3DXConstantTable*
> > iface, LPDIRECT3DDEVICE9 device, D3DXHANDLE constant, INT n)
> >  {
> > - ID3DXConstantTableImpl *This = impl_from_ID3DXConstantTable(iface);
> > -
> > - FIXME("(%p)->(%p, %p, %d): stub\n", This, device, constant, n);
> > -
> > - return E_NOTIMPL;
> > - return ID3DXConstantTable_SetIntArray(iface, device, constant,&n, 1);
> >
> > }
> >
> > static HRESULT WINAPI
> > ID3DXConstantTableImpl_SetIntArray(ID3DXConstantTable* iface,
> > LPDIRECT3DDEVICE9 device,
> > @@ -859,9 +855,36 @@ static HRESULT WINAPI
> > ID3DXConstantTableImpl_SetIntArray(ID3DXConstantTable* ifa
> >  {
> >      ID3DXConstantTableImpl *This = impl_from_ID3DXConstantTable(iface);
> > - FIXME("(%p)->(%p, %p, %p, %d): stub\n", This, device, constant, n,
> >
> > count);
> > - D3DXCONSTANT_DESC desc;
> > - HRESULT hr;
> > - UINT i, desc_count = 1;
> > - float row[4] = {0.0f, 0.0f, 0.0f, 0.0f};
> > -
> > - hr = ID3DXConstantTable_GetConstantDesc(iface,
> >
> > constant,&desc,&desc_count);
> > - if (FAILED(hr))
> > - {
> > - TRACE("ID3DXConstantTable_GetConstantDesc failed: %08x", hr);
> > - return D3DERR_INVALIDCALL;
> > - }
> > - return E_NOTIMPL;
> > - switch (desc.RegisterSet)
> > - {
> > - case D3DXRS_FLOAT4:
> > - for (i = 0; i<  count&&  i<  desc.RegisterCount; i++)
> > - {
> > - row[0] = n[i];
> > - if (is_vertex_shader(This->desc.Version))
> > - IDirect3DDevice9_SetVertexShaderConstantF(device,
> >
> > desc.RegisterIndex + i, row, 1);
> > - else
> > - IDirect3DDevice9_SetPixelShaderConstantF(device,
> >
> > desc.RegisterIndex + i, row, 1);
> > - }
> > - break;
> > - default:
> > - FIXME("Can't handle RegisterSets other than D3DXRS_FlOAT4\n");
> > - return E_NOTIMPL;
> > - }
> > -
> > - return D3D_OK;
> >
> > }
> >
> > static HRESULT WINAPI ID3DXConstantTableImpl_SetFloat(ID3DXConstantTable*
> > iface, LPDIRECT3DDEVICE9 device,

[PATCH 2/5] d3dx9_36: Implemented ID3DXConstantTable_SetIntArray and ID3DXConstantTable_SetInt (Try 5)

[David Laight]

> Ints and floats are represented differently and so, if we want to put
> an integer array into floating point registers, we need to have casts.
> If you were to simply call SetFloatArray, you would get nonsense
> values in the floating point registers.

Even using casts will fall foul of the C aliasing rules - which
mean that the compiler can assume that a read of type will not be
modified by a write to a different type so can be reorderd (unless 
one of the types is 'char').

Not only that, but there are systems that will fault is an invalid
FP bit pattern is loaded into an FP register.

David