Re: [PATCH v5 5/5] d3d10: Add tests for scalar and vector effect variables.

[Matteo Bruni]

Hi Connor,

You were asking whether this is too many tests. I'd say not, the tests
all look into interesting details of the implementation. There is even
room for more tests :)

This looks good overall, although I have comments on a number of details.

On Fri, Feb 28, 2020 at 7:52 PM Connor McAdams  wrote:
> Add tests for the ID3D10EffectScalarVariable and
> ID3D10EffectVectorVariable set/get functions.

These commit messages add nothing of value. In general this is where
the "why" of the patch would be explained, if necessary (it usually
isn't).
> Signed-off-by: Connor McAdams
>
> v5: Add tests for each set method and then all of the possible get
> methods, to test all possible set/get combinations.
>
> dlls/d3d10/tests/effect.c | 835 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 835 insertions(+)
>
> diff --git a/dlls/d3d10/tests/effect.c b/dlls/d3d10/tests/effect.c
> index e2b27cf0c8..0c91aafa7a 100644
> --- a/dlls/d3d10/tests/effect.c
> +++ b/dlls/d3d10/tests/effect.c
> @@ -4273,6 +4273,839 @@ static void test_effect_state_group_defaults(void)
>      ok(!refcount, "Device has %u references left.\n", refcount);
>  }
>
> +/*
> -
> - test_effect_scalar_variable
> - */
>
> +#if 0
> +cbuffer cb
> +{
> - float f0, f_a[2];
> - int i0, i_a[2];
> - bool b0, b_a[2];
>
> +};
> +#endif
> +static DWORD fx_test_scalar_variable[] =
> +{
> - 0x43425844, 0xe4da4aa6, 0x1380ddc5, 0x445edad5,
> - 0x08581666, 0x00000001, 0x0000020b, 0x00000001,
> - 0x00000024, 0x30315846, 0x000001df, 0xfeff1001,
> - 0x00000001, 0x00000006, 0x00000000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00000000, 0x000000d3,
> - 0x00000000, 0x00000000, 0x00000000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00000000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00000000, 0x66006263,
> - 0x74616f6c, 0x00000700, 0x00000100, 0x00000000,
> - 0x00000400, 0x00001000, 0x00000400, 0x00090900,
> - 0x00306600, 0x00000007, 0x00000001, 0x00000002,
> - 0x00000014, 0x00000010, 0x00000008, 0x00000909,
> - 0x00615f66, 0x00746e69, 0x0000004c, 0x00000001,
> - 0x00000000, 0x00000004, 0x00000010, 0x00000004,
> - 0x00000911, 0x4c003069, 0x01000000, 0x02000000,
> - 0x14000000, 0x10000000, 0x08000000, 0x11000000,
> - 0x69000009, 0x6200615f, 0x006c6f6f, 0x0000008f,
> - 0x00000001, 0x00000000, 0x00000004, 0x00000010,
> - 0x00000004, 0x00000921, 0x8f003062, 0x01000000,
> - 0x02000000, 0x14000000, 0x10000000, 0x08000000,
> - 0x21000000, 0x62000009, 0x0400615f, 0x70000000,
> - 0x00000000, 0x06000000, 0xff000000, 0x00ffffff,
> - 0x29000000, 0x0d000000, 0x00000000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00000000, 0x48000000,
> - 0x2c000000, 0x00000000, 0x10000000, 0x00000000,
> - 0x00000000, 0x00000000, 0x6c000000, 0x50000000,
> - 0x00000000, 0x24000000, 0x00000000, 0x00000000,
> - 0x00000000, 0x8b000000, 0x6f000000, 0x00000000,
> - 0x30000000, 0x00000000, 0x00000000, 0x00000000,
> - 0xb0000000, 0x94000000, 0x00000000, 0x44000000,
> - 0x00000000, 0x00000000, 0x00000000, 0xcf000000,
> - 0xb3000000, 0x00000000, 0x50000000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00000000,
>
> +};
> -
>
> +static void test_scalar_methods(ID3D10EffectScalarVariable *var, D3D10_SHADER_VARIABLE_TYPE type)
> +{
> - float ret_f;
> - int ret_i;
> - BOOL ret_b;
> - HRESULT hr;
> -
> - /* SetFloat test. */

This kind of "section" comments might be useful sometimes but I don't
feel like this is one of such cases.
> - hr = var->lpVtbl->SetFloat(var, 5.0f);
> - ok(SUCCEEDED(hr), "SetFloat failed (%x).\n", hr);

This gives no context on the variable being tested. There are a couple
of options here (and in the other ok() calls in these helper
functions): either print the variable name in the error message or
pass and print the line number in the caller function. The former
seems better in this case, because of another suggestion further
below.
> -
> - hr = var->lpVtbl->GetFloat(var, &ret_f);
> - ok(SUCCEEDED(hr), "GetFloat failed (%x).\n", hr);

It's unlikely that anything surprising comes up but, generally, please
check the exact return value instead:

ok(hr == S_OK, "Got unexpected hr %#x.\n");
> - if (type == D3D10_SVT_BOOL)
> -
>    ok(ret_f == -1.0f, "Got unexpected value %.8e.\n", ret_f);
> - else
> -
>    ok(ret_f == 5.0f, "Got unexpected value %.8e.\n", ret_f);

Instead of duplicating the whole ok() call I'd prefer something like:

expected_f = type == D3D10_SVT_BOOL ? -1.0f : 5.0f;
ok(ret_f == expected_f, "Got unexpected value %.8e.\n", ret_f);
> -
> - hr = var->lpVtbl->GetInt(var, &ret_i);
> - ok(SUCCEEDED(hr), "GetInt failed (%x).\n", hr);
> - if (type == D3D10_SVT_BOOL)
> -
>    ok(ret_i == -1, "Got unexpected value %#x.\n", ret_i);
> - else
> -
>    ok(ret_i == 5, "Got unexpected value %#x.\n", ret_i);
> -
> - hr = var->lpVtbl->GetBool(var, &ret_b);
> - ok(SUCCEEDED(hr), "GetBool failed (%x).\n", hr);
> - ok(ret_b == -1, "Got unexpected value %#x.\n", ret_b);
> -
> - /* SetInt test. */
> - hr = var->lpVtbl->SetInt(var, 2);
> - ok(SUCCEEDED(hr), "SetInt failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloat(var, &ret_f);
> - ok(SUCCEEDED(hr), "GetFloat failed (%x).\n", hr);
> - if (type == D3D10_SVT_BOOL)
> -
>    ok(ret_f == -1.0f, "Got unexpected value %.8e.\n", ret_f);
> - else
> -
>    ok(ret_f == 2.0f, "Got unexpected value %.8e.\n", ret_f);
> -
> - hr = var->lpVtbl->GetInt(var, &ret_i);
> - ok(SUCCEEDED(hr), "GetInt failed (%x).\n", hr);
> - if (type == D3D10_SVT_BOOL)
> -
>    ok(ret_i == -1, "Got unexpected value %#x.\n", ret_i);
> - else
> -
>    ok(ret_i == 2, "Got unexpected value %#x.\n", ret_i);
> -
> - hr = var->lpVtbl->GetBool(var, &ret_b);
> - ok(SUCCEEDED(hr), "GetBool failed (%x).\n", hr);
> - ok(ret_b == -1, "Got unexpected value %#x.\n", ret_b);
> -
> - /* SetBool test. */
> - hr = var->lpVtbl->SetBool(var, TRUE);
> - ok(SUCCEEDED(hr), "SetBool failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloat(var, &ret_f);
> - ok(SUCCEEDED(hr), "GetFloat failed (%x).\n", hr);
> - ok(ret_f == -1.0f, "Got unexpected value %.8e.\n", ret_f);
> -
> - hr = var->lpVtbl->GetInt(var, &ret_i);
> - ok(SUCCEEDED(hr), "GetInt failed (%x).\n", hr);
> - ok(ret_i == -1, "Got unexpected value %#x.\n", ret_i);
> -
> - hr = var->lpVtbl->GetBool(var, &ret_b);
> - ok(SUCCEEDED(hr), "GetBool failed (%x).\n", hr);
> - if (type == D3D10_SVT_BOOL)
> -
>    ok(ret_b == TRUE, "Got unexpected value %#x.\n", ret_b);
> - else
> -
>    ok(ret_b == -1, "Got unexpected value %#x.\n", ret_b);
> -
> - /* See what happens if we use SetBool to set a value that isn't TRUE or FALSE.
> -
> * Tests show that on a bool variable, the value set is what is returned
> -
> * on a call to GetBool. However, on int and float types, the value
> -
> * returned is -1. */

I don't like having this kind of comments saying what's apparent from
the code. Generally, useful comments are those that tell something
that's not immediately obvious to the reader, perhaps related to why
something is done in a certain way.

FWIW, as you can see from those tests the d3d10 effect / shader bool
has no relation with the Windows BOOL type, thus TRUE has no
particular meaning (FALSE happens to match, but that's somewhat
incidental).
> - hr = var->lpVtbl->SetBool(var, 32);
> - ok(SUCCEEDED(hr), "SetBool failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloat(var, &ret_f);
> - ok(SUCCEEDED(hr), "GetFloat failed (%x).\n", hr);
> - ok(ret_f == -1.0f, "Got unexpected value %.8e.\n", ret_f);
> -
> - hr = var->lpVtbl->GetInt(var, &ret_i);
> - ok(SUCCEEDED(hr), "GetInt failed (%x).\n", hr);
> - ok(ret_i == -1, "Got unexpected value %#x.\n", ret_i);
> -
> - hr = var->lpVtbl->GetBool(var, &ret_b);
> - ok(SUCCEEDED(hr), "GetBool failed (%x).\n", hr);
> - if (type == D3D10_SVT_BOOL)
> -
>    ok(ret_b == 32, "Got unexpected value %#x.\n", ret_b);
> - else
> -
>    ok(ret_b == -1, "Got unexpected value %#x.\n", ret_b);
>
> +}
> -
>
> +static void test_scalar_array_methods(ID3D10EffectScalarVariable *var, D3D10_SHADER_VARIABLE_TYPE type)
> +{
> - float set_f[2], ret_f[2];
> - int set_i[2], ret_i[2];
> - BOOL set_b[2], ret_b[2];
> - unsigned int i;
> - HRESULT hr;
> -
> - /* SetFloatArray tests. */
> - set_f[0] = 10.0f; set_f[1] = 20.0f;
> - hr = var->lpVtbl->SetFloatArray(var, set_f, 0, 2);
> - ok(SUCCEEDED(hr), "SetFloatArray failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatArray(var, ret_f, 0, 2);
> - ok(SUCCEEDED(hr), "GetFloatArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_f[i] == -1.0f, "Got unexpected value %.8e.\n", ret_f[i]);
> -
>    else
> -
>        ok(ret_f[i] == set_f[i], "Got unexpected value %.8e.\n", ret_f[i]);
> - }
> -
> - hr = var->lpVtbl->GetIntArray(var, ret_i, 0, 2);
> - ok(SUCCEEDED(hr), "GetIntArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_i[i] == -1, "Got unexpected value %#x.\n", ret_i[i]);
> -
>    else
> -
>        ok(ret_i[i] == (int)set_f[i], "Got unexpected value %#x.\n", ret_i[i]);
> - }
> -
> - hr = var->lpVtbl->GetBoolArray(var, ret_b, 0, 2);
> - ok(SUCCEEDED(hr), "GetBoolArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> -
>    ok(ret_b[i] == -1, "Got unexpected value %#x.\n", ret_b[i]);
> -
> - /* SetIntArray tests. */
> - set_i[0] = 5; set_i[1] = 6;
> - hr = var->lpVtbl->SetIntArray(var, set_i, 0, 2);
> - ok(SUCCEEDED(hr), "SetIntArray failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatArray(var, ret_f, 0, 2);
> - ok(SUCCEEDED(hr), "GetFloatArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_f[i] == -1.0f, "Got unexpected value %.8e.\n", ret_f[i]);
> -
>    else
> -
>        ok(ret_f[i] == (float)set_i[i], "Got unexpected value %.8e.\n", ret_f[i]);
> - }
> -
> - hr = var->lpVtbl->GetIntArray(var, ret_i, 0, 2);
> - ok(SUCCEEDED(hr), "GetIntArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_i[i] == -1, "Got unexpected value %#x.\n", ret_i[i]);
> -
>    else
> -
>        ok(ret_i[i] == set_i[i], "Got unexpected value %#x.\n", ret_i[i]);
> - }
> -
> - hr = var->lpVtbl->GetBoolArray(var, ret_b, 0, 2);
> - ok(SUCCEEDED(hr), "GetBoolArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> -
>    ok(ret_b[i] == -1, "Got unexpected value %#x.\n", ret_b[i]);
> -
> - /* SetBoolArray tests. */
> - set_b[0] = TRUE; set_b[1] = TRUE;
> - hr = var->lpVtbl->SetBoolArray(var, set_b, 0, 2);
> - ok(SUCCEEDED(hr), "SetBoolArray failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatArray(var, ret_f, 0, 2);
> - ok(SUCCEEDED(hr), "GetFloatArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> -
>    ok(ret_f[i] == -1.0f, "Got unexpected value %.8e.\n", ret_f[i]);
> -
> - hr = var->lpVtbl->GetIntArray(var, ret_i, 0, 2);
> - ok(SUCCEEDED(hr), "GetIntArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> -
>    ok(ret_i[i] == -1, "Got unexpected value %#x.\n", ret_i[i]);
> -
> - hr = var->lpVtbl->GetBoolArray(var, ret_b, 0, 2);
> - ok(SUCCEEDED(hr), "GetBoolArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_b[i] == TRUE, "Got unexpected value %#x.\n", ret_b[i]);
> -
>    else
> -
>        ok(ret_b[i] == -1, "Got unexpected value %#x.\n", ret_b[i]);
> - }
> -
> - /* SetBoolArray non-boolean value tests. */
> - set_b[0] = 10; set_b[1] = 20;
> - hr = var->lpVtbl->SetBoolArray(var, set_b, 0, 2);
> - ok(SUCCEEDED(hr), "SetBoolArray failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatArray(var, ret_f, 0, 2);
> - ok(SUCCEEDED(hr), "GetFloatArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> -
>    ok(ret_f[i] == -1.0f, "Got unexpected value %.8e.\n", ret_f[i]);
> -
> - hr = var->lpVtbl->GetIntArray(var, ret_i, 0, 2);
> - ok(SUCCEEDED(hr), "GetIntArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> -
>    ok(ret_i[i] == -1, "Got unexpected value %#x.\n", ret_i[i]);
> -
> - hr = var->lpVtbl->GetBoolArray(var, ret_b, 0, 2);
> - ok(SUCCEEDED(hr), "GetBoolArray failed (%x).\n", hr);
> - for (i = 0; i < 2; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_b[i] == set_b[i], "Got unexpected value %#x.\n", ret_b[i]);
> -
>    else
> -
>        ok(ret_b[i] == -1, "Got unexpected value %#x.\n", ret_b[i]);
> - }
> -
> - /* Array offset tests. MSDN says offset argument goes unused, test and
> -
> * make sure that this is really the case.*/
> - /* Clear the value before trying to set an offset. */
> - set_i[0] = 0; set_i[1] = 0;
> - hr = var->lpVtbl->SetIntArray(var, set_i, 0, 2);
> - ok(SUCCEEDED(hr), "SetIntArray failed (%x).\n", hr);
> -
> - /* After this, if offset is in use, return should be { 0, 5 }.
> -
> * However, this test proves this is not the case, and scalars have offset
> -
> * go unused. */
> - set_i[0] = 5;
> - hr = var->lpVtbl->SetIntArray(var, set_i, 1, 1);
> - ok(SUCCEEDED(hr), "SetIntArray failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetIntArray(var, ret_i, 0, 2);
> - ok(SUCCEEDED(hr), "GetIntArray failed (%x).\n", hr);
> - ret_b[0] = -1; ret_b[1] = 0;
> - set_i[0] =  5; set_i[1] = 0;
> - for (i = 0; i < 2; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_i[i] == ret_b[i], "Got unexpected value %#x.\n", ret_i[i]);
> -
>    else
> -
>        ok(ret_i[i] == set_i[i], "Got unexpected value %#x.\n", ret_i[i]);
> - }
> -
> - /* Now test the offset on GetArray methods. If offset was in use, we'd get
> -
> * back 5 given that the variable was previously set to { 0, 5 }. However,
> -
> * since it is not in use, we get back 5 for non-bool variables, and -1
> -
> * for bools. */
> - hr = var->lpVtbl->GetIntArray(var, ret_i, 1, 1);
> - ok(SUCCEEDED(hr), "GetIntArray failed (%x).\n", hr);
> - if (type == D3D10_SVT_BOOL)
> -
>    ok(ret_i[0] == -1, "Got unexpected value %#x.\n", ret_i[0]);
> - else
> -
>    ok(ret_i[0] == 5, "Got unexpected value %#x.\n", ret_i[0]);
>
> +}
> -
>
> +static void test_effect_scalar_variable(void)
> +{
> - ID3D10Device *device;
> - ID3D10Effect *effect;
> - ID3D10EffectVariable *var;
> - ID3D10EffectType *type;
> - ID3D10EffectScalarVariable *f0, *f_a, *i0, *i_a, *b0, *b_a;
> - D3D10_EFFECT_TYPE_DESC type_desc;
> - ULONG refcount;
> - HRESULT hr;
> -
> - if (!(device = create_device()))
> - {
> -
>    skip("Failed to create device, skipping tests.\n");
> -
>    return;
> - }
> -
> - hr = create_effect(fx_test_scalar_variable, 0, device, NULL, &effect);
> - ok(SUCCEEDED(hr), "D3D10CreateEffectFromMemory failed (%x)\n", hr);
> -
> - /* Check each different scalar type, make sure the variable returned is
> -
> * valid, set it to a value, and make sure what we get back is the same
> -
> * as what we set it to. */
> -
> - /* Scalar floating point variable. */
> - var = effect->lpVtbl->GetVariableByName(effect, "f0");
> - type = var->lpVtbl->GetType(var);
> - hr = type->lpVtbl->GetDesc(type, &type_desc);
> - ok(SUCCEEDED(hr), "GetDesc failed (%x).\n", hr);
> - ok(type_desc.Type == D3D10_SVT_FLOAT, "Type is %x, expected %x\n", type_desc.Type, D3D10_SVT_FLOAT);
> - f0 = var->lpVtbl->AsScalar(var);
> - test_scalar_methods(f0, D3D10_SVT_FLOAT);
> -
> - /* Scalar floating point array variable. */
> - var = effect->lpVtbl->GetVariableByName(effect, "f_a");
> - type = var->lpVtbl->GetType(var);
> - hr = type->lpVtbl->GetDesc(type, &type_desc);
> - ok(SUCCEEDED(hr), "GetDesc failed (%x)\n", hr);
> - ok(type_desc.Type == D3D10_SVT_FLOAT, "Type is %x, expected %x\n", type_desc.Type, D3D10_SVT_FLOAT);
> - f_a = var->lpVtbl->AsScalar(var);
> - test_scalar_methods(f_a, D3D10_SVT_FLOAT);
> - test_scalar_array_methods(f_a, D3D10_SVT_FLOAT);
> -
> - /* Scalar int variable. */
> - var = effect->lpVtbl->GetVariableByName(effect, "i0");
> - type = var->lpVtbl->GetType(var);
> - hr = type->lpVtbl->GetDesc(type, &type_desc);
> - ok(SUCCEEDED(hr), "GetDesc failed (%x)\n", hr);
> - ok(type_desc.Type == D3D10_SVT_INT, "Type is %x, expected %x\n", type_desc.Type, D3D10_SVT_INT);
> - i0 = var->lpVtbl->AsScalar(var);
> - test_scalar_methods(i0, D3D10_SVT_INT);
> -
> - /* Scalar int array variable. */
> - var = effect->lpVtbl->GetVariableByName(effect, "i_a");
> - type = var->lpVtbl->GetType(var);
> - hr = type->lpVtbl->GetDesc(type, &type_desc);
> - ok(SUCCEEDED(hr), "GetDesc failed (%x)\n", hr);
> - ok(type_desc.Type == D3D10_SVT_INT, "Type is %x, expected %x\n", type_desc.Type, D3D10_SVT_INT);
> - i_a = var->lpVtbl->AsScalar(var);
> - test_scalar_methods(i_a, D3D10_SVT_INT);
> - test_scalar_array_methods(i_a, D3D10_SVT_INT);
> -
> - /* Scalar bool variable. */
> - var = effect->lpVtbl->GetVariableByName(effect, "b0");
> - type = var->lpVtbl->GetType(var);
> - hr = type->lpVtbl->GetDesc(type, &type_desc);
> - ok(SUCCEEDED(hr), "GetDesc failed (%x)\n", hr);
> - ok(type_desc.Type == D3D10_SVT_BOOL, "Type is %x, expected %x\n", type_desc.Type, D3D10_SVT_BOOL);
> - b0 = var->lpVtbl->AsScalar(var);
> - test_scalar_methods(b0, D3D10_SVT_BOOL);
> -
> - /* Scalar bool array variable. */
> - var = effect->lpVtbl->GetVariableByName(effect, "b_a");
> - type = var->lpVtbl->GetType(var);
> - hr = type->lpVtbl->GetDesc(type, &type_desc);
> - ok(SUCCEEDED(hr), "GetDesc failed (%x)\n", hr);
> - ok(type_desc.Type == D3D10_SVT_BOOL, "Type is %x, expected %x\n", type_desc.Type, D3D10_SVT_BOOL);
> - b_a = var->lpVtbl->AsScalar(var);
> - test_scalar_methods(b_a, D3D10_SVT_BOOL);
> - test_scalar_array_methods(b_a, D3D10_SVT_BOOL);

This repeats pretty much the same few function calls for each
variable. This should be rewritten to be table-based, like:

struct
{
    const char *name;
    D3D_SHADER_VARIABLE_TYPE type;
} tests[] = {{"f0", D3D10_SVT_FLOAT}, ...};

for (i = 0; i < ARRAY_SIZE(tests); ++i)
{
    var = effect->lpVtbl->GetVariableByName(effect, tests[i].name);
    ...
}
> -
> - effect->lpVtbl->Release(effect);
> -
> - refcount = ID3D10Device_Release(device);
> - ok(!refcount, "Device has %u references left.\n", refcount);
>
> +}
> -
>
> +/*
> -
> - test_effect_vector_variable
> - */
>
> +#if 0
> +cbuffer cb
> +{
> - float4 v_f0, v_f_a[2];
> - int3 v_i0, v_i_a[3];
> - bool2 v_b0, v_b_a[4];
>
> +};
> +#endif
> +static DWORD fx_test_vector_variable[] =
> +{
> - 0x43425844, 0x581ae0ae, 0xa906b020, 0x26bba03e,
> - 0x5d7dfba2, 0x00000001, 0x0000021a, 0x00000001,
> - 0x00000024, 0x30315846, 0x000001ee, 0xfeff1001,
> - 0x00000001, 0x00000006, 0x00000000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00000000, 0x000000e2,
> - 0x00000000, 0x00000000, 0x00000000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00000000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00000000, 0x66006263,
> - 0x74616f6c, 0x00070034, 0x00010000, 0x00000000,
> - 0x00100000, 0x00100000, 0x00100000, 0x210a0000,
> - 0x5f760000, 0x07003066, 0x01000000, 0x02000000,
> - 0x20000000, 0x10000000, 0x20000000, 0x0a000000,
> - 0x76000021, 0x615f665f, 0x746e6900, 0x00510033,
> - 0x00010000, 0x00000000, 0x000c0000, 0x00100000,
> - 0x000c0000, 0x19120000, 0x5f760000, 0x51003069,
> - 0x01000000, 0x03000000, 0x2c000000, 0x10000000,
> - 0x24000000, 0x12000000, 0x76000019, 0x615f695f,
> - 0x6f6f6200, 0x9900326c, 0x01000000, 0x00000000,
> - 0x08000000, 0x10000000, 0x08000000, 0x22000000,
> - 0x76000011, 0x0030625f, 0x00000099, 0x00000001,
> - 0x00000004, 0x00000038, 0x00000010, 0x00000020,
> - 0x00001122, 0x5f625f76, 0x00040061, 0x00c00000,
> - 0x00000000, 0x00060000, 0xffff0000, 0x0000ffff,
> - 0x002a0000, 0x000e0000, 0x00000000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00000000, 0x004b0000,
> - 0x002f0000, 0x00000000, 0x00100000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00720000, 0x00560000,
> - 0x00000000, 0x00300000, 0x00000000, 0x00000000,
> - 0x00000000, 0x00930000, 0x00770000, 0x00000000,
> - 0x00400000, 0x00000000, 0x00000000, 0x00000000,
> - 0x00bb0000, 0x009f0000, 0x00000000, 0x00700000,
> - 0x00000000, 0x00000000, 0x00000000, 0x00dc0000,
> - 0x00c00000, 0x00000000, 0x00800000, 0x00000000,
> - 0x00000000, 0x00000000, 0x00000000,
>
> +};
> -
>
> +static void test_vector_methods(ID3D10EffectVectorVariable *var, D3D10_SHADER_VARIABLE_TYPE type,
> -
>    unsigned int col_count)
>
> +{
> - float set_f[4], ret_f[4];
> - int set_i[4], ret_i[4];
> - BOOL set_b[4], ret_b[4];
> - unsigned int i;
> - HRESULT hr;
> -
> - /* SetFloatVector test. */
> - set_f[0] = 1.0f; set_f[1] = 2.0f; set_f[2] = 3.0f; set_f[3] = 4.0f;
> - hr = var->lpVtbl->SetFloatVector(var, set_f);
> - ok(SUCCEEDED(hr), "SetFloatVector failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatVector(var, ret_f);
> - ok(SUCCEEDED(hr), "GetFloatVector failed (%x).\n", hr);
> - for (i = 0; i < col_count; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_f[i] == -1.0f, "Got unexpected value %.8e.\n", ret_f[i]);
> -
>    else
> -
>        ok(ret_f[i] == set_f[i], "Got unexpected value %.8e.\n", ret_f[i]);
> - }
> -
> - hr = var->lpVtbl->GetIntVector(var, ret_i);
> - ok(SUCCEEDED(hr), "GetIntVector failed (%x).\n", hr);
> - for (i = 0; i < col_count; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_i[i] == -1, "Got unexpected value %#x.\n", ret_i[i]);
> -
>    else
> -
>        ok(ret_i[i] == (int)set_f[i], "Got unexpected value %#x.\n", ret_i[i]);
> - }
> -
> - hr = var->lpVtbl->GetBoolVector(var, ret_b);
> - ok(SUCCEEDED(hr), "GetBoolVector failed (%x).\n", hr);
> - for (i = 0; i < col_count; i++)
> -
>    ok(ret_b[i] == -1, "Got unexpected value %#x.\n", ret_b[i]);
> -
> - /* SetIntVector test. */
> - set_i[0] = 5; set_i[1] = 6; set_i[2] = 7; set_i[3] = 8;
> - hr = var->lpVtbl->SetIntVector(var, set_i);
> - ok(SUCCEEDED(hr), "SetIntVector failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatVector(var, ret_f);
> - ok(SUCCEEDED(hr), "GetFloatVector failed (%x).\n", hr);
> - for (i = 0; i < col_count; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_f[i] == -1.0f, "Got unexpected value %.8e.\n", ret_f[i]);
> -
>    else
> -
>        ok(ret_f[i] == (float)set_i[i], "Got unexpected value %.8e.\n", ret_f[i]);
> - }
> -
> - hr = var->lpVtbl->GetIntVector(var, ret_i);
> - ok(SUCCEEDED(hr), "GetIntVector failed (%x).\n", hr);
> - for (i = 0; i < col_count; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_i[i] == -1, "Got unexpected value %#x.\n", ret_i[i]);
> -
>    else
> -
>        ok(ret_i[i] == set_i[i], "Got unexpected value %#x.\n", ret_i[i]);
> - }
> -
> - hr = var->lpVtbl->GetBoolVector(var, ret_b);
> - ok(SUCCEEDED(hr), "GetBoolVector failed (%x).\n", hr);
> - for (i = 0; i < col_count; i++)
> -
>    ok(ret_b[i] == -1, "Got unexpected value %#x.\n", ret_b[i]);
> -
> - /* SetBoolVector test. */
> - set_b[0] = TRUE; set_b[1] = FALSE; set_b[2] = TRUE; set_b[3] = FALSE;
> - hr = var->lpVtbl->SetBoolVector(var, set_b);
> - ok(SUCCEEDED(hr), "SetBoolVector failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatVector(var, ret_f);
> - ok(SUCCEEDED(hr), "GetFloatVector failed (%x).\n", hr);
> - /* Use set_f as the array to test against. */
> - set_f[0] = -1.0f; set_f[1] = 0.0f; set_f[2] = -1.0f; set_f[3] = 0.0f;
> - for (i = 0; i < col_count; i++)
> -
>    ok(ret_f[i] == set_f[i], "Got unexpected value %.8e.\n", ret_f[i]);
> -
> - hr = var->lpVtbl->GetIntVector(var, ret_i);
> - ok(SUCCEEDED(hr), "GetIntVector failed (%x).\n", hr);
> - set_i[0] = -1; set_i[1] = 0; set_i[2] = -1; set_i[3] = 0;
> - for (i = 0; i < col_count; i++)
> -
>    ok(ret_i[i] == set_i[i], "Got unexpected value %#x.\n", ret_i[i]);
> -
> - hr = var->lpVtbl->GetBoolVector(var, ret_b);
> - ok(SUCCEEDED(hr), "GetBoolVector failed (%x).\n", hr);
> - for (i = 0; i < col_count; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_b[i] == set_b[i], "Got unexpected value %#x.\n", ret_b[i]);
> -
>    else
> -
>        ok(ret_b[i] == set_i[i], "Got unexpected value %#x.\n", ret_b[i]);
> - }
> -
> - /* See what happens if we use SetBoolVector with a value that isn't TRUE
> -
> * or FALSE. */
> - set_b[0] = 5; set_b[1] = 10; set_b[2] = 15; set_b[3] = 20;
> - hr = var->lpVtbl->SetBoolVector(var, set_b);
> - ok(SUCCEEDED(hr), "SetBoolVector failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatVector(var, ret_f);
> - ok(SUCCEEDED(hr), "GetFloatVector failed (%x).\n", hr);
> - for (i = 0; i < col_count; i++)
> -
>    ok(ret_f[i] == -1.0f, "Got unexpected value %.8e.\n", ret_f[i]);
> -
> - hr = var->lpVtbl->GetIntVector(var, ret_i);
> - ok(SUCCEEDED(hr), "GetIntVector failed (%x).\n", hr);
> - for (i = 0; i < col_count; i++)
> -
>    ok(ret_i[i] == -1, "Got unexpected value %#x.\n", ret_i[i]);
> -
> - hr = var->lpVtbl->GetBoolVector(var, ret_b);
> - ok(SUCCEEDED(hr), "GetBoolVector failed (%x).\n", hr);
> - for (i = 0; i < col_count; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_b[i] == set_b[i], "Got unexpected value %#x.\n", ret_b[i]);
> -
>    else
> -
>        ok(ret_b[i] == -1, "Got unexpected value %#x.\n", ret_b[i]);
> - }
>
> +}
> -
>
> +static void test_vector_array_methods(ID3D10EffectVectorVariable *var, D3D10_SHADER_VARIABLE_TYPE type,
> -
>    unsigned int col_count, unsigned int row_count)
>
> +{
> - float set_f[9], ret_f[9];
> - int set_i[9], ret_i[9];
> - BOOL set_b[9], ret_b[9];
> - unsigned int i;
> - HRESULT hr;
> -
> - /* SetFloatVectorArray test. */
> - set_f[0] = 1.0f; set_f[1] = 2.0f; set_f[2] = 3.0f; set_f[3] = 4.0f;
> - set_f[4] = 5.0f; set_f[5] = 6.0f; set_f[6] = 7.0f; set_f[7] = 8.0f;
> - set_f[8] = 9.0f;
> - hr = var->lpVtbl->SetFloatVectorArray(var, set_f, 0, row_count);
> - ok(SUCCEEDED(hr), "SetFloatVectorArray failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatVectorArray(var, ret_f, 0, row_count);
> - ok(SUCCEEDED(hr), "GetFloatVectorArray failed (%x).\n", hr);
> - for (i = 0; i < col_count * row_count; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_f[i] == -1.0f, "Got unexpected value %.8e.\n", ret_f[i]);
> -
>    else
> -
>        ok(ret_f[i] == set_f[i], "Got unexpected value %.8e.\n", ret_f[i]);
> - }
> -
> - hr = var->lpVtbl->GetIntVectorArray(var, ret_i, 0, row_count);
> - ok(SUCCEEDED(hr), "GetIntVectorArray failed (%x).\n", hr);
> - for (i = 0; i < col_count * row_count; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_i[i] == -1, "Got unexpected value %#x.\n", ret_i[i]);
> -
>    else
> -
>        ok(ret_i[i] == (int)set_f[i], "Got unexpected value %#x.\n", ret_i[i]);
> - }
> -
> - hr = var->lpVtbl->GetBoolVectorArray(var, ret_b, 0, row_count);
> - ok(SUCCEEDED(hr), "GetBoolVectorArray failed (%x).\n", hr);
> - for (i = 0; i < col_count * row_count; i++)
> -
>    ok(ret_b[i] == -1, "Got unexpected value %#x.\n", ret_b[i]);
> -
> - /* SetIntVectorArray test. */
> - set_i[0] = 10; set_i[1] = 11; set_i[2] = 12; set_i[3] = 13;
> - set_i[4] = 14; set_i[5] = 15; set_i[6] = 16; set_i[7] = 17;
> - set_i[8] = 18;
> - hr = var->lpVtbl->SetIntVectorArray(var, set_i, 0, row_count);
> - ok(SUCCEEDED(hr), "SetIntVectorArray failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatVectorArray(var, ret_f, 0, row_count);
> - ok(SUCCEEDED(hr), "GetFloatVectorArray failed (%x).\n", hr);
> - for (i = 0; i < col_count * row_count; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_f[i] == -1.0f, "Got unexpected value %.8e.\n", ret_f[i]);
> -
>    else
> -
>        ok(ret_f[i] == (float)set_i[i], "Got unexpected value %.8e.\n", ret_f[i]);
> - }
> -
> - hr = var->lpVtbl->GetIntVectorArray(var, ret_i, 0, row_count);
> - ok(SUCCEEDED(hr), "GetIntVectorArray failed (%x).\n", hr);
> - for (i = 0; i < col_count * row_count; i++)
> - {
> -
>    if (type == D3D10_SVT_BOOL)
> -
>        ok(ret_i[i] == -1, "Got unexpected value %#x.\n", ret_i[i]);
> -
>    else
> -
>        ok(ret_i[i] == set_i[i], "Got unexpected value %#x.\n", ret_i[i]);
> - }
> -
> - hr = var->lpVtbl->GetBoolVectorArray(var, ret_b, 0, row_count);
> - ok(SUCCEEDED(hr), "GetBoolVectorArray failed (%x).\n", hr);
> - for (i = 0; i < col_count * row_count; i++)
> -
>    ok(ret_b[i] == -1, "Got unexpected value %#x.\n", ret_b[i]);
> -
> - /* SetBoolVectorArray test. */
> - set_b[0] = TRUE; set_b[1] = FALSE; set_b[2] = TRUE;  set_b[3] = TRUE;
> - set_b[4] = TRUE; set_b[5] = FALSE; set_b[6] = FALSE; set_b[7] = TRUE;
> - set_b[8] = TRUE;
> - hr = var->lpVtbl->SetBoolVectorArray(var, set_b, 0, row_count);
> - ok(SUCCEEDED(hr), "SetBoolVectorArray failed (%x).\n", hr);
> -
> - hr = var->lpVtbl->GetFloatVectorArray(var, ret_f, 0, row_count);
> - ok(SUCCEEDED(hr), "GetFloatVectorArray failed (%x).\n", hr);
> - /* Use set_f as the array to test against. */
> - set_f[0] = -1.0f; set_f[1] = 0.0f; set_f[2] = -1.0f; set_f[3] = -1.0f;
> - set_f[4] = -1.0f; set_f[5] = 0.0f; set_f[6] =  0.0f; set_f[7] = -1.0f;
> - set_f[8] = -1.0f;
> - for (i = 0; i < col_count * row_count; i++)
> -
>    ok(ret_f[i] == set_f[i], "Got unexpected value %.8e.\n", ret_f[i]);

Not a big deal but you can use an expected_f[] array and remove the
need of having a comment here.

In general, please do have a look at your patches that I resent
recently and notice the bits I changed. Many are nitpicks that
wouldn't block a new patch submission but I'd be happier if your
patches followed the same style from the start.