Re: [v3 3/7] d3dx9: implement light and material effect states application.

[Matteo Bruni]

2016-03-03 13:22 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>:
> Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
> ---
> +    light_tbl[] =
> +    {
> +       {FIELD_OFFSET(D3DLIGHT9, Type),         "LC_TYPE"},
> +       {FIELD_OFFSET(D3DLIGHT9, Diffuse),      "LT_DIFFUSE"},
> +       {FIELD_OFFSET(D3DLIGHT9, Specular),     "LT_SPECULAR"},
> +       {FIELD_OFFSET(D3DLIGHT9, Ambient),      "LT_AMBIENT"},
> +       {FIELD_OFFSET(D3DLIGHT9, Position),     "LT_POSITION"},
> +       {FIELD_OFFSET(D3DLIGHT9, Direction),    "LT_DIRECTION"},
> +       {FIELD_OFFSET(D3DLIGHT9, Range),        "LT_RANGE"},
> +       {FIELD_OFFSET(D3DLIGHT9, Falloff),      "LT_FALLOFF"},
> +       {FIELD_OFFSET(D3DLIGHT9, Attenuation0), "LT_ATTENUATION0"},
> +       {FIELD_OFFSET(D3DLIGHT9, Attenuation1), "LT_ATTENUATION1"},
> +       {FIELD_OFFSET(D3DLIGHT9, Attenuation2), "LT_ATTENUATION2"},
> +       {FIELD_OFFSET(D3DLIGHT9, Theta),        "LT_THETA"},
> +       {FIELD_OFFSET(D3DLIGHT9, Phi),          "LT_PHI"}
> +    };

This is misindented.
> +    switch (op)
> +    {
> +        case LT_TYPE:
> +            TRACE("LT_TYPE %u.\n", *(D3DLIGHTTYPE *)value);
> +            light->Type = *(D3DLIGHTTYPE *)value;
> +            break;
> +        case LT_DIFFUSE:
> +        case LT_SPECULAR:
> +        case LT_AMBIENT:
> +        {
> +            D3DCOLORVALUE c = *(D3DCOLORVALUE *)value;
> +
> +            TRACE("%s (%f %f %f %f).\n", light_tbl[op].name, c.r, c.g, c.b, c.a);
> +            *(D3DCOLORVALUE *)((char *)light + light_tbl[op].offset) = c;

It's probably a bit nicer to cast to (BYTE *) instead of (char *).
> diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
> index 81a7d12..3bcba72 100644
> --- a/dlls/d3dx9_36/tests/effect.c
> +++ b/dlls/d3dx9_36/tests/effect.c
> @@ -2910,11 +2910,11 @@ static void test_effect_states(IDirect3DDevice9 *device)
>      }
>
> hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval);
> -    todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
> +    ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
>      if (hr == D3D_OK)
>          ok(bval, "Got result %u, expected TRUE.\n", bval);
>      hr = IDirect3DDevice9_GetLight(device, 2, &light);
> -    todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
> +    ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
>      if (hr == D3D_OK)
>          ok(light.Position.x == 4.0f && light.Position.y == 5.0f && light.Position.z == 6.0f,
>                  "Got unexpected light position (%f, %f, %f).\n", light.Position.x, light.Position.y, light.Position.z);
> @@ -2950,9 +2950,9 @@ static void test_effect_states(IDirect3DDevice9 *device)
>      todo_wine ok(!memcmp(mat.m, test_mat.m, sizeof(mat)), "World matrix not restored.\n");
>
> hr = IDirect3DDevice9_GetLightEnable(device, 2, &bval);
> -    todo_wine ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
> +    ok(hr == D3D_OK, "Got result %x, expected 0 (D3D_OK).\n", hr);
>      if (hr == D3D_OK)
> -        ok(!bval, "Got result %u, expected 0.\n", bval);
> +        todo_wine ok(!bval, "Got result %u, expected 0.\n", bval);

It would be nice if the test also checked the material.