Re: [PATCH 1/6] d3dx9_36/tests: Added HLSL test suite.

[Henri Verbeet]

On 5 August 2010 00:05, Travis Athougies  wrote:
> +#define CINTERFACE

Why do you need this?
> +#include <d3d9.h>

d3d9.h should be implied by d3dx9.h
> +#include <assert.h>
> +#include <stdio.h>

I don't think you need these either.
> +struct vertex
> +{
> - float x, y, z;
> - float tx, ty;
>
> +};
> -
>
> +const struct vertex quad_vertices[4] = {
> - {-1, -1, 0, 0, 1},
> - {-1, 1, 0, 0, 0},
> - {1, -1, 0, 1, 1},
> - {1, 1, 0, 1, 0}
>
> +};

Those aren't floating point values. Also, please use consistent formatting.
> +const struct vertex quad_vertices[4] = {

...
> +static IDirect3DVertexBuffer9 *quad_geometry;
> +static IDirect3DVertexShader9 *vshader_passthru;
> +static IDirect3DVertexDeclaration9 *vdeclaration;
> +static IDirect3DDevice9 *device;

Why is all of that global?
> - hres = IDirect3D9_CreateDevice(d3d9_ptr, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, NULL, D3DCREATE_HARDWARE_VERTEXPROCESSING, &present_parameters, &device_ptr);

Please break long lines. I realize this particular one is probably
somehow my fault because I added it back in 2005 with
c26ad82e75e408c302c21228ef08c1af0482326c, and everyone has been
(blindly) copying it since, but there are other examples like
setup_device() as well. Use 120 as maximum line length, 100 to be
safe.
> - hres = IDirect3DVertexBuffer9_Lock(quad_geometry, 0, sizeof(quad_vertices), (void**)&temp_geometry_vertices, 0);

You don't need that cast.
> - hres = D3DXCompileShader(vshader_passthru_hlsl, strlen(vshader_passthru_hlsl),
> - NULL, NULL, "vshader", "vs_1_1", 0,
> - &compiled, &errors, 0);
> - if(FAILED(hres))
> - {
> - vshader_passthru = NULL;

That doesn't make sense, vshader_passthru is already NULL at that point.
> - hr = D3DXCompileShader(shader, strlen(shader),
> - NULL, NULL, "test", profile,
> - 0, &compiled, &errors, constants);
> - ok (hr == D3D_OK, "Pixel shader %s compilation failed: %s\n", shader, (char*) (errors?ID3DXBuffer_GetBufferPointer(errors):""));

This casts away const.
> diff --git a/include/d3dx9shader.h b/include/d3dx9shader.h
> index c8380bc..c54d27c 100644
> --- a/include/d3dx9shader.h
> +++ b/include/d3dx9shader.h
> @@ -296,6 +296,17 @@ HRESULT WINAPI D3DXAssembleShader(LPCSTR data,
>                                   LPD3DXBUFFER* shader,
>                                   LPD3DXBUFFER* error_messages);
>
> +HRESULT WINAPI D3DXCompileShader(LPCSTR src_data,
> - UINT data_len,
> - const D3DXMACRO* defines,
> - LPD3DXINCLUDE include,
> - LPCSTR function_name,
> - LPCSTR profile,
> - DWORD flags,
> - LPD3DXBUFFER* shader,
> - LPD3DXBUFFER* error_messages,
> - LPD3DXCONSTANTTABLE* constant_table);
> -

This should be in a separate patch.

The patch has some more issues, they shouldn't be hard to spot.