Re: [PATCH 1/5] d3dcompiler_43/tests: Added HLSL test suite

[Henri Verbeet]

I think this is going in the right direction, but I still have a
couple of comments.

On 1 October 2010 00:43, Travis Athougies  wrote:
> +/* Macro to compare two floating point numbers */
> +#define F_EQ(a, b, epsilon) (fabs(a - b) <= epsilon)

There's no reason this can't be an inline function.
> +/* Color macro to simplify handling of D3DXCOLOR */
> +#define COLOR(r, g, b, a) {r, g, b, a}

This looks pretty pointless.
> +/* Signals the end of a probe list */
> +#define PROBE_END {-1, 0, {0.0f, 0.0f, 0.0f, 0.0f}, 0.0f, ""}

You'll probably want to just pass a count to compute_shader_probe9().
> - /* The x and y coordinate, the width and the height of the region to check.
> - When x < 0, this is the last probe */
> - int x, y;

There's no width and height here.
> - setup_device9(device, &render_target, &readback, D3DFMT_A32B32G32R32F,
> - width, height, vshader, pshader);

You can't use D3DFMT_A32B32G32R32F without checking if it's available
using CheckDeviceFormat() first.
> - /* Now go through the probes and check each one */
> - for(;probes->x >= 0;probes++) {
> - int index = probes->x + (probes->y * lr.Pitch);
> - ok(F_EQ(probes->c.a, pbits_data[index].a, probes->epsilon),
> - "In alpha component at (%d, %d): %s: Expected %f, got %f\n", probes->x, probes->y,
> - probes->message, probes->c.a, pbits_data[index].a);
> - ok(F_EQ(probes->c.b, pbits_data[index].b, probes->epsilon),
> - "In blue component at (%d, %d): %s: Expected %f, got %f\n", probes->x, probes->y,
> - probes->message, probes->c.b, pbits_data[index].b);
> - ok(F_EQ(probes->c.g, pbits_data[index].g, probes->epsilon),
> - "In green component at (%d, %d): %s: Expected %f, got %f\n", probes->x, probes->y,
> - probes->message, probes->c.g, pbits_data[index].g);
> - ok(F_EQ(probes->c.r, pbits_data[index].r, probes->epsilon),
> - "In blue component at (%d, %d): %s: Expected %f, got %f\n", probes->x, probes->y,
> - probes->message, probes->c.r, pbits_data[index].r);
> - }

You'll probably want to have a function to compare two D3DXCOLORs.
Also, please include the line number of the caller of
compute_shader_probe9() somewhere.
> - if (caps.PixelShaderVersion >= 0xffff0200)

You can still just use D3DPS_VERSION(2, 0) here.