Re: [v2 3/3] d3dx9/tests: Add more tests for preshader relative addressing.

[Matteo Bruni]

2017-03-23 16:25 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>:
> Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
> ---
> +static void test_effect_preshader_relative_addressing(IDirect3DDevice9 *device)
> +{
> +    static const struct
> +    {
> +        D3DXVECTOR4 opvect2;
> +        D3DXVECTOR4 g_ivect;
> +        unsigned int result[4];
> +    }
> +    test_out_of_bounds_index[] =

Better as out_of_bounds_index_tests[] or similar.
> +    static const struct
> +    {
> +        unsigned int zw[2];
> +    }
> +    test_index_range[] =

This one would be clearer as expected_light_specular[] or something like that.
> +    fvect.x = 1001.f; fvect.y = 1002.f; fvect.z = 1003.f; fvect.w = 1004.f;

For consistency, please write out the 0 after the decimal separator.
> +        for (j = 0; j < 4; ++j)
> +        {
> +            ok(compare_float(v[j], ((const float *)test_out_of_bounds_index[i].result)[j], 0),
> +                    "Test %u, component %u, expected %#x (%g), got %#x (%g).\n",
> +                    i, j, test_out_of_bounds_index[i].result[j],
> +                    ((const float *)test_out_of_bounds_index[i].result)[j],
> +                    ((const unsigned int *)v)[j], v[j]);

You could factor out a variable for the const float *
(expected_float?) and use it twice here.
> +        for (j = 0; j < 2; ++j)
> +        {
> +            ok(compare_float(v[j + 2], ((const float *)test_index_range[(unsigned int)i & 7].zw)[j], 0),
> +                    "Test %u, component %u, expected %#x (%g), got %#x (%g).\n",
> +                    i, j, test_index_range[(unsigned int)i & 7].zw[j],
> +                    ((const float *)test_index_range[(unsigned int)i & 7].zw)[j],
> +                    ((const unsigned int *)v)[j + 2], v[j + 2]);

Same here. In addition you could set v to &light.Specular.b before
this part of the test and avoid those "+ 2".