[PATCH v2 0/4] MR7505: d3dx9: Partially implement ID3DXEffect::SetRawValue().

[Connor McAdams (＠cmcadams)]

This patch series implements `ID3DXEffect::SetRawValue()` for the simplest cases, which are vec4s and matrix 4x4s.

--
  v2: d3dx9: Fixup return values for D3DXPT_BOOL parameters in ID3DXEffect::GetValue().
      d3dx9: Add support for setting 4x4 matrices in ID3DXEffect::SetRawValue().
      d3dx9: Partially implement ID3DXEffect::SetRawValue().
      d3dx9/tests: Add tests for ID3DXEffect::SetRawValue().

https://gitlab.winehq.org/wine/wine/-/merge_requests/7505

[PATCH v2 1/4] d3dx9/tests: Add tests for ID3DXEffect::SetRawValue().

[Connor McAdams]

From: Connor McAdams

Signed-off-by: Connor McAdams 
---
 dlls/d3dx9_36/tests/effect.c | 302 +++++++++++++++++++++++++++++++++++
 1 file changed, 302 insertions(+)

diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
index c55019f0f7a..9e46a0c6b38 100644
--- a/dlls/d3dx9_36/tests/effect.c
+++ b/dlls/d3dx9_36/tests/effect.c
@@ -8890,6 +8890,305 @@ static void test_effect_parameter_block(void)
     DestroyWindow(window);
 }

+#if D3DX_SDK_VERSION >= 27
+static void test_effect_set_raw_value(IDirect3DDevice9 *device)
+{
+    static const char test_set_raw_value_shader[] =
+        "bool b;\n"
+        "bool b_2[2];\n"
+        "bool2 b2;\n"
+        "bool2 b2_2[2];\n"
+        "bool4 b4;\n"
+        "bool4 b4_2[2];\n"
+        "bool2x2 b22;\n"
+        "bool2x2 b22_2[2];\n"
+        "bool4x4 b44;\n"
+        "bool4x4 b44_2[2];\n"
+        "int i;\n"
+        "int i_2[2];\n"
+        "int2 i2;\n"
+        "int2 i2_2[2];\n"
+        "int4 i4;\n"
+        "int4 i4_2[2];\n"
+        "int2x2 i22;\n"
+        "int2x2 i22_2[2];\n"
+        "int4x4 i44;\n"
+        "int4x4 i44_2[2];\n"
+        "float f;\n"
+        "float f_2[2];\n"
+        "float2 f2;\n"
+        "float2 f2_2[2];\n"
+        "float4 f4;\n"
+        "float4 f4_2[2];\n"
+        "float2x2 f22;\n"
+        "float2x2 f22_2[2];\n"
+        "float4x4 f44;\n"
+        "float4x4 f44_2[2];\n"
+        "technique t { pass p { } };\n";
+    static const char *param_type_str[] = { "b", "i", "f" };
+    static const struct
+    {
+        const char *suffix;
+        UINT value_offset;
+        UINT value_bytes;
+        union
+        {
+            float f[32];
+            DWORD dword[32];
+        } value;
+        union
+        {
+            float f[32];
+            DWORD dword[32];
+        } expected_value;
+        BOOL todo_hr;
+    } tests[] =
+    {
+        { NULL, 0, 4,
+          { .f = { 1.0f } },
+          { .f = { 1.0f } },
+          .todo_hr = TRUE
+        },
+        { "_2", 0, 8,
+          { .f = { 1.0f, 2.0f } },
+          { .f = { 1.0f, 0.0f } },
+          .todo_hr = TRUE
+        },
+        { "_2", 0, 20,
+          { .f = { 1.0f, 2.0f, 0.0f, 0.0f, 3.0f } },
+          { .f = { 1.0f, 3.0f, 0.0f, 0.0f, 0.0f } },
+          .todo_hr = TRUE
+        },
+        /* Offset of 4, nothing gets set. */
+        { "_2", 4, 4,
+          { .f = { 1.0f } },
+          { .f = { 0.0f, 0.0f } },
+          .todo_hr = TRUE
+        },
+        /*
+         * To set the second value in the array, we need to start at an offset
+         * of 16.
+         */
+        { "_2", 16, 4,
+          { .f = { 2.0f } },
+          { .f = { 0.0f, 2.0f } },
+          .todo_hr = TRUE
+        },
+        /* 5. */
+        { "2", 0, 8,
+          { .f = { 0.0f, 1.0f } },
+          { .f = { 0.0f, 1.0f } },
+          .todo_hr = TRUE
+        },
+        { "2_2", 0, 16,
+          { .f = { 0.0f, 1.0f, 0.0f, 2.0f } },
+          { .f = { 0.0f, 1.0f, 0.0f, 0.0f } },
+          .todo_hr = TRUE
+        },
+        { "2_2", 0, 24,
+          { .f = { 0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 0.0f } },
+          { .f = { 0.0f, 1.0f, 4.0f, 0.0f } },
+          .todo_hr = TRUE
+        },
+        { "4", 0, 16,
+          { .f = { 1.0f, 2.0f, 0.0f, 3.0f } },
+          { .f = { 1.0f, 2.0f, 0.0f, 3.0f } },
+          .todo_hr = TRUE
+        },
+        { "4_2", 0, 32,
+          { .f = { 2.0f, 0.0f, 0.0f, 8.0f, 3.0f, 4.0f, 0.0f, 5.0f } },
+          { .f = { 2.0f, 0.0f, 0.0f, 8.0f, 3.0f, 4.0f, 0.0f, 5.0f } },
+          .todo_hr = TRUE
+        },
+        /* 10. */
+        { "22", 0, 64,
+          { .f = {  1.0f,  2.0f,  3.0f,  4.0f,
+                    5.0f,  6.0f,  7.0f,  0.0f,
+                    8.0f,  9.0f, 10.0f, 11.0f,
+                   12.0f, 13.0f, 14.0f,  0.0f }
+          },
+          { .f = { 1.0f, 5.0f, 2.0f, 6.0f } },
+          .todo_hr = TRUE
+        },
+        { "22_2", 0, 128,
+          { .f = {  1.0f,  2.0f,  3.0f,  4.0f,
+                    5.0f,  6.0f,  7.0f,  0.0f,
+                    8.0f,  9.0f, 10.0f, 11.0f,
+                   12.0f, 13.0f, 14.0f,  0.0f,
+                   /* Matrix 2. */
+                   15.0f, 16.0f, 17.0f, 18.0f,
+                   19.0f, 20.0f, 21.0f, 22.0f,
+                   23.0f, 24.0f, 25.0f, 26.0f,
+                   27.0f, 28.0f, 29.0f, 30.0f }
+          },
+          { .f = { 1.0f, 5.0f, 2.0f, 6.0f, 15.0f, 19.0f, 16.0f, 20.0f } },
+          .todo_hr = TRUE
+        },
+        { "44", 0, 64,
+          { .f = {  1.0f,  2.0f,  3.0f,  4.0f,
+                    5.0f,  6.0f,  7.0f,  0.0f,
+                    8.0f,  9.0f, 10.0f, 11.0f,
+                   12.0f, 13.0f, 14.0f,  0.0f }
+          },
+          { .f = {  1.0f,  5.0f,  8.0f, 12.0f,
+                    2.0f,  6.0f,  9.0f, 13.0f,
+                    3.0f,  7.0f, 10.0f, 14.0f,
+                    4.0f,  0.0f, 11.0f,  0.0f }
+          },
+          .todo_hr = TRUE
+        },
+        { "44_2", 0, 128,
+          { .f = {  1.0f,  2.0f,  3.0f,  4.0f,
+                    5.0f,  6.0f,  7.0f,  0.0f,
+                    8.0f,  9.0f, 10.0f, 11.0f,
+                   12.0f, 13.0f, 14.0f,  0.0f,
+                   /* Matrix 2. */
+                   15.0f, 16.0f, 17.0f, 18.0f,
+                   19.0f, 20.0f, 21.0f, 22.0f,
+                   23.0f, 24.0f, 25.0f, 26.0f,
+                   27.0f, 28.0f, 29.0f, 30.0f }
+          },
+          { .f = {  1.0f,  5.0f,  8.0f, 12.0f,
+                    2.0f,  6.0f,  9.0f, 13.0f,
+                    3.0f,  7.0f, 10.0f, 14.0f,
+                    4.0f,  0.0f, 11.0f,  0.0f,
+                   /* Matrix 2. */
+                   15.0f, 19.0f, 23.0f, 27.0f,
+                   16.0f, 20.0f, 24.0f, 28.0f,
+                   17.0f, 21.0f, 25.0f, 29.0f,
+                   18.0f, 22.0f, 26.0f, 30.0f }
+          },
+          .todo_hr = TRUE
+        },
+        /* Set second element. */
+        { "44_2", 64, 64,
+          { .f = {  1.0f,  2.0f,  3.0f,  4.0f,
+                    5.0f,  6.0f,  7.0f,  0.0f,
+                    8.0f,  9.0f, 10.0f, 11.0f,
+                   12.0f, 13.0f, 14.0f,  0.0f }
+          },
+          { .f = {  0.0f,  0.0f,  0.0f,  0.0f,
+                    0.0f,  0.0f,  0.0f,  0.0f,
+                    0.0f,  0.0f,  0.0f,  0.0f,
+                    0.0f,  0.0f,  0.0f,  0.0f,
+                   /* Matrix 2. */
+                    1.0f,  5.0f,  8.0f, 12.0f,
+                    2.0f,  6.0f,  9.0f, 13.0f,
+                    3.0f,  7.0f, 10.0f, 14.0f,
+                    4.0f,  0.0f, 11.0f,  0.0f }
+          },
+          .todo_hr = TRUE
+        },
+    };
+    static const DWORD test_int_val[] = { 1, 2, 3, 4 };
+    D3DXPARAMETER_DESC param_desc;
+    D3DXHANDLE param, param2;
+    unsigned int i, j, k;
+    char param_name[64];
+    ID3DXEffect *effect;
+    ULONG refcount;
+    DWORD tmp[32];
+    HRESULT hr;
+
+    hr = D3DXCreateEffect(device, test_set_raw_value_shader,
+            sizeof(test_set_raw_value_shader), NULL, NULL, 0, NULL, &effect, NULL);
+    ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+
+    for (i = 0; i < ARRAY_SIZE(tests); ++i)
+    {
+        for (j = 0; j < ARRAY_SIZE(param_type_str); ++j)
+        {
+            strcpy(param_name, param_type_str[j]);
+            if (tests[i].suffix)
+                strcat(param_name, tests[i].suffix);
+
+            winetest_push_context("Test %u (param %s)", i, param_name);
+            param = effect->lpVtbl->GetParameterByName(effect, NULL, param_name);
+
+            hr = effect->lpVtbl->GetParameterDesc(effect, param, &param_desc);
+            ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+
+            /* Clear our values first. */
+            memset(tmp, 0, sizeof(tmp));
+            hr = effect->lpVtbl->SetValue(effect, param, tmp, sizeof(tmp));
+            ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+
+            hr = effect->lpVtbl->SetRawValue(effect, param, tests[i].value.dword, tests[i].value_offset,
+                    tests[i].value_bytes);
+            todo_wine_if(tests[i].todo_hr) ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+            if (SUCCEEDED(hr))
+            {
+                unsigned int unexpected_values = 0;
+
+                memset(tmp, 0xff, sizeof(tmp));
+                hr = effect->lpVtbl->GetValue(effect, param, tmp, sizeof(tmp));
+                ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+
+                for (k = 0; k < (param_desc.Bytes / sizeof(*tmp)); ++k)
+                {
+                    const DWORD *expected_val = &tests[i].expected_value.dword[k];
+                    DWORD *ret_val = &tmp[k];
+
+                    if (param_desc.Type == D3DXPT_BOOL && (!!(*expected_val) != (*ret_val)))
+                        unexpected_values++;
+                    else if (param_desc.Type != D3DXPT_BOOL && memcmp(ret_val, expected_val, sizeof(*ret_val)))
+                        unexpected_values++;
+                }
+
+                for (; k < ARRAY_SIZE(tmp); ++k)
+                {
+                    if (tmp[k] != 0xffffffffu)
+                        unexpected_values++;
+                }
+
+                todo_wine_if(param_desc.Type == D3DXPT_BOOL) ok(!unexpected_values,
+                        "Got %u unexpected values.\n", unexpected_values);
+            }
+            winetest_pop_context();
+        }
+    }
+
+    /* Test passing a NULL parameter. */
+    hr = effect->lpVtbl->SetRawValue(effect, NULL, tmp, 4, 0);
+    todo_wine ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr);
+
+    param = effect->lpVtbl->GetParameterByName(effect, NULL, "i4");
+    param2 = effect->lpVtbl->GetParameterByName(effect, NULL, "i44");
+
+    /* Test setting with a size of 0. */
+    hr = effect->lpVtbl->SetRawValue(effect, param, tmp, 0, 0);
+    todo_wine ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = effect->lpVtbl->SetRawValue(effect, param2, tmp, 0, 0);
+    todo_wine ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+
+    param = effect->lpVtbl->GetParameterByName(effect, NULL, "i4");
+    param2 = effect->lpVtbl->GetParameterByName(effect, NULL, "i4_2");
+
+    /* Test setting adjacent variables. */
+    memset(tmp, 0, sizeof(tmp));
+    hr = effect->lpVtbl->SetValue(effect, param, tmp, sizeof(tmp));
+    ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+    hr = effect->lpVtbl->SetValue(effect, param2, tmp, sizeof(tmp));
+    ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+
+    /* An offset of 16 bytes on variable i4 sets adjacent variable i4_2. */
+    hr = effect->lpVtbl->SetRawValue(effect, param, test_int_val, 16, sizeof(test_int_val));
+    todo_wine ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = effect->lpVtbl->GetValue(effect, param, tmp, sizeof(tmp));
+    ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!tmp[0], "Unexpected value.\n");
+
+    hr = effect->lpVtbl->GetValue(effect, param2, tmp, sizeof(tmp));
+    ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine ok(!memcmp(tmp, test_int_val, sizeof(test_int_val)), "Got unexpected values.\n");
+
+    refcount = effect->lpVtbl->Release(effect);
+    ok(!refcount, "Unexpected refcount %lu.\n", refcount);
+}
+#endif /* D3DX_SDK_VERSION >= 27 */
+
 START_TEST(effect)
 {
     IDirect3DDevice9 *device;
@@ -8918,6 +9217,9 @@ START_TEST(effect)
     test_effect_large_address_aware_flag(device);
     test_effect_get_pass_desc(device);
     test_effect_skip_constants(device);
+#if D3DX_SDK_VERSION >= 27
+    test_effect_set_raw_value(device);
+#endif

refcount = IDirect3DDevice9_Release(device);
     ok(!refcount, "Device has %lu references left.\n", refcount);

[PATCH v2 2/4] d3dx9: Partially implement ID3DXEffect::SetRawValue().

[Connor McAdams]

From: Connor McAdams

Add support for setting vec4 effect parameters.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=44795
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46012
Signed-off-by: Connor McAdams 
---
 dlls/d3dx9_36/effect.c       | 42 +++++++++++++++++++++++++++++++++---
 dlls/d3dx9_36/tests/effect.c |  6 ++----
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
index 628d8ebc3eb..67eaf2b5d63 100644
--- a/dlls/d3dx9_36/effect.c
+++ b/dlls/d3dx9_36/effect.c
@@ -4431,10 +4431,46 @@ static HRESULT WINAPI d3dx_effect_CloneEffect(ID3DXEffect *iface, IDirect3DDevic
 static HRESULT WINAPI d3dx_effect_SetRawValue(ID3DXEffect *iface, D3DXHANDLE parameter, const void *data,
         UINT byte_offset, UINT bytes)
 {
-    FIXME("iface %p, parameter %p, data %p, byte_offset %u, bytes %u stub!\n",
-            iface, parameter, data, byte_offset, bytes);
+    struct d3dx_effect *effect = impl_from_ID3DXEffect(iface);
+    struct d3dx_parameter *param = get_valid_parameter(effect, parameter);

-    return E_NOTIMPL;
+    TRACE("iface %p, parameter %p, data %p, byte_offset %u, bytes %u.\n", iface, parameter, data, byte_offset, bytes);
+
+    if (!param)
+    {
+        WARN("Invalid parameter %p specified.\n", parameter);
+        return D3DERR_INVALIDCALL;
+    }
+
+    switch (param->class)
+    {
+        case D3DXPC_VECTOR:
+        {
+            uint8_t *dst_data;
+
+            if (param->columns != 4)
+            {
+                FIXME("Vec%u parameters are currently unsupported.\n", param->columns);
+                return E_NOTIMPL;
+            }
+
+            if ((byte_offset + bytes) > param->bytes)
+            {
+                FIXME("Writing adjacent parameters is currently unsupported.\n");
+                return E_NOTIMPL;
+            }
+
+            dst_data = param_get_data_and_dirtify(effect, param, !byte_offset ? bytes : param->bytes, TRUE);
+            memcpy(dst_data + byte_offset, data, bytes);
+            break;
+        }
+
+        default:
+            FIXME("Unhandled parameter class %s.\n", debug_d3dxparameter_class(param->class));
+            return E_NOTIMPL;
+    }
+
+    return D3D_OK;
 }
 #endif

diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
index 9e46a0c6b38..9b166384364 100644
--- a/dlls/d3dx9_36/tests/effect.c
+++ b/dlls/d3dx9_36/tests/effect.c
@@ -8993,12 +8993,10 @@ static void test_effect_set_raw_value(IDirect3DDevice9 *device)
         { "4", 0, 16,
           { .f = { 1.0f, 2.0f, 0.0f, 3.0f } },
           { .f = { 1.0f, 2.0f, 0.0f, 3.0f } },
-          .todo_hr = TRUE
         },
         { "4_2", 0, 32,
           { .f = { 2.0f, 0.0f, 0.0f, 8.0f, 3.0f, 4.0f, 0.0f, 5.0f } },
           { .f = { 2.0f, 0.0f, 0.0f, 8.0f, 3.0f, 4.0f, 0.0f, 5.0f } },
-          .todo_hr = TRUE
         },
         /* 10. */
         { "22", 0, 64,
@@ -9150,14 +9148,14 @@ static void test_effect_set_raw_value(IDirect3DDevice9 *device)

/* Test passing a NULL parameter. */
     hr = effect->lpVtbl->SetRawValue(effect, NULL, tmp, 4, 0);
-    todo_wine ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr);
+    ok(hr == D3DERR_INVALIDCALL, "Unexpected hr %#lx.\n", hr);

param = effect->lpVtbl->GetParameterByName(effect, NULL, "i4");
     param2 = effect->lpVtbl->GetParameterByName(effect, NULL, "i44");

/* Test setting with a size of 0. */
     hr = effect->lpVtbl->SetRawValue(effect, param, tmp, 0, 0);
-    todo_wine ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+    ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);

hr = effect->lpVtbl->SetRawValue(effect, param2, tmp, 0, 0);
     todo_wine ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);

[PATCH v2 3/4] d3dx9: Add support for setting 4x4 matrices in ID3DXEffect::SetRawValue().

[Connor McAdams]

From: Connor McAdams

Signed-off-by: Connor McAdams 
---
 dlls/d3dx9_36/effect.c       | 31 +++++++++++++++++++++++++++++++
 dlls/d3dx9_36/tests/effect.c |  5 +----
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
index 67eaf2b5d63..75ce872e2c4 100644
--- a/dlls/d3dx9_36/effect.c
+++ b/dlls/d3dx9_36/effect.c
@@ -4465,6 +4465,37 @@ static HRESULT WINAPI d3dx_effect_SetRawValue(ID3DXEffect *iface, D3DXHANDLE par
             break;
         }

+        case D3DXPC_MATRIX_ROWS:
+        {
+            D3DXMATRIX *dst_elem;
+            uint8_t *dst_data;
+            unsigned int i;
+
+            if (param->columns != 4 || param->rows != 4)
+            {
+                FIXME("%ux%u matrix parameters are currently unsupported.\n", param->rows, param->columns);
+                return E_NOTIMPL;
+            }
+
+            if (byte_offset & 0x3f || bytes & 0x3f)
+            {
+                FIXME("Partial matrix updates are currently unsupported.\n");
+                return E_NOTIMPL;
+            }
+
+            if ((byte_offset + bytes) > param->bytes)
+            {
+                FIXME("Writing adjacent parameters is currently unsupported.\n");
+                return E_NOTIMPL;
+            }
+
+            dst_data = param_get_data_and_dirtify(effect, param, !byte_offset ? bytes : param->bytes, TRUE);
+            dst_elem = (D3DXMATRIX *)(dst_data + byte_offset);
+            for (i = 0; i < (bytes / sizeof(D3DXMATRIX)); ++i)
+                D3DXMatrixTranspose(&dst_elem[i], &((const D3DXMATRIX *)data)[i]);
+            break;
+        }
+
         default:
             FIXME("Unhandled parameter class %s.\n", debug_d3dxparameter_class(param->class));
             return E_NOTIMPL;
diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
index 9b166384364..939dbc5a1be 100644
--- a/dlls/d3dx9_36/tests/effect.c
+++ b/dlls/d3dx9_36/tests/effect.c
@@ -9033,7 +9033,6 @@ static void test_effect_set_raw_value(IDirect3DDevice9 *device)
                     3.0f,  7.0f, 10.0f, 14.0f,
                     4.0f,  0.0f, 11.0f,  0.0f }
           },
-          .todo_hr = TRUE
         },
         { "44_2", 0, 128,
           { .f = {  1.0f,  2.0f,  3.0f,  4.0f,
@@ -9056,7 +9055,6 @@ static void test_effect_set_raw_value(IDirect3DDevice9 *device)
                    17.0f, 21.0f, 25.0f, 29.0f,
                    18.0f, 22.0f, 26.0f, 30.0f }
           },
-          .todo_hr = TRUE
         },
         /* Set second element. */
         { "44_2", 64, 64,
@@ -9075,7 +9073,6 @@ static void test_effect_set_raw_value(IDirect3DDevice9 *device)
                     3.0f,  7.0f, 10.0f, 14.0f,
                     4.0f,  0.0f, 11.0f,  0.0f }
           },
-          .todo_hr = TRUE
         },
     };
     static const DWORD test_int_val[] = { 1, 2, 3, 4 };
@@ -9158,7 +9155,7 @@ static void test_effect_set_raw_value(IDirect3DDevice9 *device)
     ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);

hr = effect->lpVtbl->SetRawValue(effect, param2, tmp, 0, 0);
-    todo_wine ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+    ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);

param = effect->lpVtbl->GetParameterByName(effect, NULL, "i4");
     param2 = effect->lpVtbl->GetParameterByName(effect, NULL, "i4_2");

[PATCH v2 4/4] d3dx9: Fixup return values for D3DXPT_BOOL parameters in ID3DXEffect::GetValue().

[Connor McAdams]

From: Connor McAdams

Signed-off-by: Connor McAdams 
---
 dlls/d3dx9_36/effect.c       | 17 ++++++++++++-----
 dlls/d3dx9_36/tests/effect.c |  3 +--
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
index 75ce872e2c4..d5442777afd 100644
--- a/dlls/d3dx9_36/effect.c
+++ b/dlls/d3dx9_36/effect.c
@@ -2453,6 +2453,7 @@ static HRESULT WINAPI d3dx_effect_GetValue(ID3DXEffect *iface, D3DXHANDLE parame
 {
     struct d3dx_effect *effect = impl_from_ID3DXEffect(iface);
     struct d3dx_parameter *param = get_valid_parameter(effect, parameter);
+    unsigned int i;

TRACE("iface %p, parameter %p, data %p, bytes %u.\n", iface, parameter, data, bytes);

@@ -2474,12 +2475,22 @@ static HRESULT WINAPI d3dx_effect_GetValue(ID3DXEffect *iface, D3DXHANDLE parame
         switch (param->type)
         {
             case D3DXPT_VOID:
-            case D3DXPT_BOOL:
             case D3DXPT_INT:
             case D3DXPT_FLOAT:
             case D3DXPT_STRING:
                 break;

+            case D3DXPT_BOOL:
+            {
+                BOOL *src = (BOOL *)param->data;
+                BOOL *dst = (BOOL *)data;
+
+                for (i = 0; i < (param->bytes / sizeof(*src)); ++i)
+                    dst[i] = !!src[i];
+
+                return D3D_OK;
+            }
+
             case D3DXPT_VERTEXSHADER:
             case D3DXPT_PIXELSHADER:
             case D3DXPT_TEXTURE:
@@ -2487,9 +2498,6 @@ static HRESULT WINAPI d3dx_effect_GetValue(ID3DXEffect *iface, D3DXHANDLE parame
             case D3DXPT_TEXTURE2D:
             case D3DXPT_TEXTURE3D:
             case D3DXPT_TEXTURECUBE:
-            {
-                unsigned int i;
-
                 for (i = 0; i < (param->element_count ? param->element_count : 1); ++i)
                 {
                     IUnknown *unk = ((IUnknown **)param->data)[i];
@@ -2497,7 +2505,6 @@ static HRESULT WINAPI d3dx_effect_GetValue(ID3DXEffect *iface, D3DXHANDLE parame
                         IUnknown_AddRef(unk);
                 }
                 break;
-            }

default:
                 FIXME("Unhandled type %s.\n", debug_d3dxparameter_type(param->type));
diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
index 939dbc5a1be..028b112cb0b 100644
--- a/dlls/d3dx9_36/tests/effect.c
+++ b/dlls/d3dx9_36/tests/effect.c
@@ -9136,8 +9136,7 @@ static void test_effect_set_raw_value(IDirect3DDevice9 *device)
                         unexpected_values++;
                 }

-                todo_wine_if(param_desc.Type == D3DXPT_BOOL) ok(!unexpected_values,
-                        "Got %u unexpected values.\n", unexpected_values);
+                ok(!unexpected_values, "Got %u unexpected values.\n", unexpected_values);
             }
             winetest_pop_context();
         }

[Connor McAdams (＠cmcadams)]

On Mon Mar 10 14:35:00 2025 +0000, Connor McAdams wrote:
> changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/7505/diffs?diff_id=162829&start_sha=859269fc255fc9913e70bcd347c43c197bf1f9cc#6af8d13245d87d86339fd93c308f9d2332f22092_8930_8894)

It compiles, I've switched from using a binary blob to passing in the shader text.

[Connor McAdams (＠cmcadams)]

On Mon Mar 10 14:35:00 2025 +0000, Connor McAdams wrote:
> changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/7505/diffs?diff_id=162829&start_sha=859269fc255fc9913e70bcd347c43c197bf1f9cc#6af8d13245d87d86339fd93c308f9d2332f22092_9036_8945)

No reason in particular, I never really noticed but I seem to do it in other tests as well. I've changed it to just `tests` now, shortens some lines. :)

[Connor McAdams (＠cmcadams)]

On Sat Mar  8 23:54:10 2025 +0000, Matteo Bruni wrote:
> FWIW, I added one more test here before reading further (test 6
> basically covers the same):
>         { "_2", 0, 8,
>           { .f = { 1.0f, 2.0f } },
>           { .f = { 1.0f, 0.0f } },
>           .todo_hr = TRUE
>         },
>         { "_2", 0, 20,
>           { .f = { 1.0f, 2.0f, 0.0f, 0.0f, 3.0f } },
>           { .f = { 1.0f, 3.0f, 0.0f, 0.0f, 0.0f } },
>           .todo_hr = TRUE
>         },
>
> I also changed the code below to have the `memcmp()` go for
> `param_desc.Rows * 4 * param_desc.Elements` DWORDs and cleared `tmp`
> before each iteration.
> I don't think adding this test is particularly necessary, or even a good
> idea. Same WRT changing the inner `for` loop end condition. I'll come
> back to the other change below.

I added the test you've posted above, not sure if that's what you wanted or not :D I can remove it if need be.

[Connor McAdams (＠cmcadams)]

On Mon Mar 10 14:39:41 2025 +0000, Matteo Bruni wrote:
>                 }
>                 for (; k < ARRAY_SIZE(tmp); ++k)
>                 {
>                     if (tmp[k] != 0xffffffffu)
>                         unexpected_values++;
>                 }
>
> I haven't checked if we already have a similar test for `GetValue()` though.

We have a similar test in `test_effect_parameter_value_GetValue()` which sets everything to `0xab`, but I went ahead and added it here anyways since it shouldn't harm anything.

[Connor McAdams (＠cmcadams)]

On Mon Mar 10 14:40:53 2025 +0000, Aida Jonikienė wrote:
> For reference here are the bug reports mentioning `SetRawValue()`:
> https://bugs.winehq.org/show_bug.cgi?id=46012
> https://bugs.winehq.org/show_bug.cgi?id=44795

I've added `Wine-Bug:` tags for both of the above bugs in the current revision.

[Matteo Bruni (＠Mystral)]

On Tue Mar 11 12:52:03 2025 +0000, Connor McAdams wrote:
> I added the test you've posted above, not sure if that's what you wanted
> or not :D I can remove it if need be.

As usual I wasn't clear at all :sweat_smile: I don't think my test is necessary, but it doesn't hurt either, so this is absolutely fine :smile:

[PATCH v2 0/4] MR7505: d3dx9: Partially implement ID3DXEffect::SetRawValue(). - approved

[Matteo Bruni (＠Mystral)]

This merge request was approved by Matteo Bruni.