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

[Matteo Bruni]

2016-03-03 13:22 GMT+01:00 Paul Gofman :
> Signed-off-by: Paul Gofman
> - 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.