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 <iammisc(a)gmail.com> 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.