[PATCH v2 1/4] d3dx9: Do not use d3dx_effect_cleanup() in D3DXCreateEffectEx().

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/d3dx9_36/effect.c | 57 +++++++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 14 deletions(-)

diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
index 310e5c0bf17..6ea138e632f 100644
--- a/dlls/d3dx9_36/effect.c
+++ b/dlls/d3dx9_36/effect.c
@@ -6422,15 +6422,6 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
     effect->ID3DXEffect_iface.lpVtbl = &ID3DXEffect_Vtbl;
     effect->ref = 1;
 
-    if (pool)
-    {
-        effect->pool = unsafe_impl_from_ID3DXEffectPool(pool);
-        pool->lpVtbl->AddRef(pool);
-    }
-
-    IDirect3DDevice9_AddRef(device);
-    effect->device = device;
-
     effect->flags = flags;
 
     list_init(&effect->parameter_block_list);
@@ -6487,6 +6478,15 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
         TRACE("Tag: %x\n", tag);
     }
 
+    if (pool)
+    {
+        effect->pool = unsafe_impl_from_ID3DXEffectPool(pool);
+        pool->lpVtbl->AddRef(pool);
+    }
+
+    IDirect3DDevice9_AddRef(device);
+    effect->device = device;
+
     if (skip_constants_string)
     {
         skip_constants_buffer = HeapAlloc(GetProcessHeap(), 0,
@@ -6495,7 +6495,8 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
         {
             if (bytecode)
                 ID3D10Blob_Release(bytecode);
-            return E_OUTOFMEMORY;
+            hr = E_OUTOFMEMORY;
+            goto fail;
         }
         strcpy(skip_constants_buffer, skip_constants_string);
 
@@ -6504,7 +6505,8 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
             HeapFree(GetProcessHeap(), 0, skip_constants_buffer);
             if (bytecode)
                 ID3D10Blob_Release(bytecode);
-            return E_OUTOFMEMORY;
+            hr = E_OUTOFMEMORY;
+            goto fail;
         }
     }
     read_dword(&ptr, &offset);
@@ -6518,7 +6520,7 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
         FIXME("Failed to parse effect.\n");
         HeapFree(GetProcessHeap(), 0, skip_constants_buffer);
         HeapFree(GetProcessHeap(), 0, skip_constants);
-        return hr;
+        goto fail;
     }
 
     for (i = 0; i < skip_constants_count; ++i)
@@ -6535,7 +6537,8 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
                             debugstr_a(skip_constants[i]), j);
                     HeapFree(GetProcessHeap(), 0, skip_constants_buffer);
                     HeapFree(GetProcessHeap(), 0, skip_constants);
-                    return D3DERR_INVALIDCALL;
+                    hr = D3DERR_INVALIDCALL;
+                    goto fail;
                 }
             }
         }
@@ -6557,6 +6560,33 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
     }
 
     return D3D_OK;
+
+fail:
+    if (effect->techniques)
+    {
+        for (i = 0; i < effect->technique_count; ++i)
+            free_technique(&effect->techniques[i]);
+        heap_free(effect->techniques);
+    }
+
+    if (effect->parameters)
+    {
+        for (i = 0; i < effect->parameter_count; ++i)
+            free_top_level_parameter(&effect->parameters[i]);
+        heap_free(effect->parameters);
+    }
+
+    if (effect->objects)
+    {
+        for (i = 0; i < effect->object_count; ++i)
+            free_object(&effect->objects[i]);
+        heap_free(effect->objects);
+    }
+
+    IDirect3DDevice9_Release(effect->device);
+    if (pool)
+        pool->lpVtbl->Release(pool);
+    return hr;
 }
 
 HRESULT WINAPI D3DXCreateEffectEx(struct IDirect3DDevice9 *device, const void *srcdata, UINT srcdatalen,
@@ -6593,7 +6623,6 @@ HRESULT WINAPI D3DXCreateEffectEx(struct IDirect3DDevice9 *device, const void *s
     if (FAILED(hr))
     {
         WARN("Failed to create effect object, hr %#x.\n", hr);
-        d3dx_effect_cleanup(object);
         return hr;
     }
 
-- 
2.34.1

[PATCH v2 2/4] d3dx9: Separate out d3x9_effect_init_from_dxbc().

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/d3dx9_36/effect.c | 144 +++++++++++++++++++++--------------------
 1 file changed, 75 insertions(+), 69 deletions(-)

diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
index 6ea138e632f..ccc8da5a9e8 100644
--- a/dlls/d3dx9_36/effect.c
+++ b/dlls/d3dx9_36/effect.c
@@ -6396,16 +6396,10 @@ static const char **parse_skip_constants_string(char *skip_constants_string, uns
     return new_alloc;
 }
 
-static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDevice9 *device,
-        const char *data, SIZE_T data_size, const D3D_SHADER_MACRO *defines, ID3DInclude *include,
-        UINT flags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string)
+static HRESULT d3dx9_effect_init_from_dxbc(struct d3dx_effect *effect,
+        struct IDirect3DDevice9 *device, const char *data, SIZE_T data_size,
+        unsigned int flags, struct ID3DXEffectPool *pool, const char *skip_constants_string)
 {
-#if D3DX_SDK_VERSION <= 36
-    UINT compile_flags = D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY;
-#else
-    UINT compile_flags = 0;
-#endif
-    ID3DBlob *bytecode = NULL, *temp_errors = NULL;
     unsigned int skip_constants_count = 0;
     char *skip_constants_buffer = NULL;
     const char **skip_constants = NULL;
@@ -6414,11 +6408,6 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
     unsigned int i, j;
     HRESULT hr;
 
-    TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, flags %#x, errors %p, "
-            "pool %p, skip_constants %s.\n",
-            effect, device, data, data_size, defines, include, flags, errors, pool,
-            debugstr_a(skip_constants_string));
-
     effect->ID3DXEffect_iface.lpVtbl = &ID3DXEffect_Vtbl;
     effect->ref = 1;
 
@@ -6429,55 +6418,6 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
     read_dword(&ptr, &tag);
     TRACE("Tag: %x\n", tag);
 
-    if (tag != d3dx9_effect_version(9, 1))
-    {
-        TRACE("HLSL ASCII effect, trying to compile it.\n");
-        compile_flags |= flags & ~(D3DXFX_NOT_CLONEABLE | D3DXFX_LARGEADDRESSAWARE);
-        hr = D3DCompile(data, data_size, NULL, defines, include,
-                NULL, "fx_2_0", compile_flags, 0, &bytecode, &temp_errors);
-        if (FAILED(hr))
-        {
-            WARN("Failed to compile ASCII effect.\n");
-            if (bytecode)
-                ID3D10Blob_Release(bytecode);
-            if (temp_errors)
-            {
-                const char *error_string = ID3D10Blob_GetBufferPointer(temp_errors);
-                const char *string_ptr;
-
-                while (*error_string)
-                {
-                    string_ptr = error_string;
-                    while (*string_ptr && *string_ptr != '\n' && *string_ptr != '\r'
-                           && string_ptr - error_string < 80)
-                        ++string_ptr;
-                    TRACE("%s\n", debugstr_an(error_string, string_ptr - error_string));
-                    error_string = string_ptr;
-                    while (*error_string == '\n' || *error_string == '\r')
-                        ++error_string;
-                }
-            }
-            if (errors)
-                *errors = temp_errors;
-            else if (temp_errors)
-                ID3D10Blob_Release(temp_errors);
-            return hr;
-        }
-        if (!bytecode)
-        {
-            FIXME("No output from effect compilation.\n");
-            return D3DERR_INVALIDCALL;
-        }
-        if (errors)
-            *errors = temp_errors;
-        else if (temp_errors)
-            ID3D10Blob_Release(temp_errors);
-
-        ptr = ID3D10Blob_GetBufferPointer(bytecode);
-        read_dword(&ptr, &tag);
-        TRACE("Tag: %x\n", tag);
-    }
-
     if (pool)
     {
         effect->pool = unsafe_impl_from_ID3DXEffectPool(pool);
@@ -6493,8 +6433,6 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
                 sizeof(*skip_constants_buffer) * (strlen(skip_constants_string) + 1));
         if (!skip_constants_buffer)
         {
-            if (bytecode)
-                ID3D10Blob_Release(bytecode);
             hr = E_OUTOFMEMORY;
             goto fail;
         }
@@ -6503,8 +6441,6 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
         if (!(skip_constants = parse_skip_constants_string(skip_constants_buffer, &skip_constants_count)))
         {
             HeapFree(GetProcessHeap(), 0, skip_constants_buffer);
-            if (bytecode)
-                ID3D10Blob_Release(bytecode);
             hr = E_OUTOFMEMORY;
             goto fail;
         }
@@ -6513,8 +6449,6 @@ static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDev
     TRACE("Offset: %x\n", offset);
 
     hr = d3dx_parse_effect(effect, ptr, data_size, offset, skip_constants, skip_constants_count);
-    if (bytecode)
-        ID3D10Blob_Release(bytecode);
     if (hr != D3D_OK)
     {
         FIXME("Failed to parse effect.\n");
@@ -6589,6 +6523,78 @@ fail:
     return hr;
 }
 
+static HRESULT d3dx9_effect_init(struct d3dx_effect *effect, struct IDirect3DDevice9 *device,
+        const char *data, SIZE_T data_size, const D3D_SHADER_MACRO *defines, ID3DInclude *include,
+        UINT flags, ID3DBlob **errors, struct ID3DXEffectPool *pool, const char *skip_constants_string)
+{
+#if D3DX_SDK_VERSION <= 36
+    UINT compile_flags = D3DCOMPILE_ENABLE_BACKWARDS_COMPATIBILITY;
+#else
+    UINT compile_flags = 0;
+#endif
+    ID3DBlob *bytecode = NULL, *temp_errors = NULL;
+    const char *ptr = data;
+    HRESULT hr;
+    DWORD tag;
+
+    TRACE("effect %p, device %p, data %p, data_size %lu, defines %p, include %p, flags %#x, errors %p, "
+            "pool %p, skip_constants %s.\n",
+            effect, device, data, data_size, defines, include, flags, errors, pool,
+            debugstr_a(skip_constants_string));
+
+    read_dword(&ptr, &tag);
+
+    if (tag == d3dx9_effect_version(9, 1))
+        return d3dx9_effect_init_from_dxbc(effect, device, data, data_size, flags, pool, skip_constants_string);
+
+    TRACE("HLSL ASCII effect, trying to compile it.\n");
+    compile_flags |= flags & ~(D3DXFX_NOT_CLONEABLE | D3DXFX_LARGEADDRESSAWARE);
+    hr = D3DCompile(data, data_size, NULL, defines, include,
+            NULL, "fx_2_0", compile_flags, 0, &bytecode, &temp_errors);
+    if (FAILED(hr))
+    {
+        WARN("Failed to compile ASCII effect.\n");
+        if (bytecode)
+            ID3D10Blob_Release(bytecode);
+        if (temp_errors)
+        {
+            const char *error_string = ID3D10Blob_GetBufferPointer(temp_errors);
+            const char *string_ptr;
+
+            while (*error_string)
+            {
+                string_ptr = error_string;
+                while (*string_ptr && *string_ptr != '\n' && *string_ptr != '\r'
+                       && string_ptr - error_string < 80)
+                    ++string_ptr;
+                TRACE("%s\n", debugstr_an(error_string, string_ptr - error_string));
+                error_string = string_ptr;
+                while (*error_string == '\n' || *error_string == '\r')
+                    ++error_string;
+            }
+        }
+        if (errors)
+            *errors = temp_errors;
+        else if (temp_errors)
+            ID3D10Blob_Release(temp_errors);
+        return hr;
+    }
+    if (!bytecode)
+    {
+        FIXME("No output from effect compilation.\n");
+        return D3DERR_INVALIDCALL;
+    }
+    if (errors)
+        *errors = temp_errors;
+    else if (temp_errors)
+        ID3D10Blob_Release(temp_errors);
+
+    hr = d3dx9_effect_init_from_dxbc(effect, device, ID3D10Blob_GetBufferPointer(bytecode),
+            ID3D10Blob_GetBufferSize(bytecode), flags, pool, skip_constants_string);
+    ID3D10Blob_Release(bytecode);
+    return hr;
+}
+
 HRESULT WINAPI D3DXCreateEffectEx(struct IDirect3DDevice9 *device, const void *srcdata, UINT srcdatalen,
         const D3DXMACRO *defines, struct ID3DXInclude *include, const char *skip_constants, DWORD flags,
         struct ID3DXEffectPool *pool, struct ID3DXEffect **effect, struct ID3DXBuffer **compilation_errors)
-- 
2.34.1

[PATCH v2 2/4] d3dx9: Separate out d3x9_effect_init_from_dxbc().

[Matteo Bruni]

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>

[PATCH v2 3/4] d3dx9/tests: Add more tests for ID3DXEffect::CloneEffect().

[Zebediah Figura]

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/d3dx9_36/tests/effect.c | 221 +++++++++++++++++++++++++++++++++--
 1 file changed, 210 insertions(+), 11 deletions(-)

diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
index 63321fba3b3..5a23819e28c 100644
--- a/dlls/d3dx9_36/tests/effect.c
+++ b/dlls/d3dx9_36/tests/effect.c
@@ -7413,20 +7413,91 @@ static void test_effect_null_shader(void)
 
 static void test_effect_clone(void)
 {
+    D3DXHANDLE parameter, parameter2, technique, technique2, block, annotation;
     IDirect3DDevice9 *device, *device2, *device3;
+    IDirect3DBaseTexture9 *texture, *texture2;
     ID3DXEffect *effect, *cloned;
+    unsigned int passes_count;
     HWND window, window2;
+    const char *string;
     ULONG refcount;
     HRESULT hr;
+    float f;
+
+    static const DWORD effect_code[] =
+    {
+#if 0
+        Texture2D tex;
+        float f <float a = 1.0>;
+
+        float4 vs_main(float4 pos : POSITION) : POSITION
+        {
+            return pos;
+        }
+
+        technique tech0 <float a = 1.0;>
+        {
+            pass p
+            {
+                VertexShader = compile vs_2_0 vs_main();
+            }
+        }
+
+        technique tech1
+        {
+            pass p
+            {
+                VertexShader = compile vs_2_0 vs_main();
+            }
+        }
+#endif
+        0xfeff0901, 0x00000114, 0x00000000, 0x00000007, 0x00000004, 0x0000001c, 0x00000000, 0x00000000,
+        0x00000001, 0x00000004, 0x00786574, 0x00000004, 0x00000004, 0x0000003c, 0x00000000, 0x00000000,
+        0x00000002, 0x00000002, 0x00000073, 0x00000003, 0x00000000, 0x0000008c, 0x00000000, 0x00000000,
+        0x00000001, 0x00000001, 0x00000000, 0x3f800000, 0x00000003, 0x00000000, 0x00000084, 0x00000000,
+        0x00000000, 0x00000001, 0x00000001, 0x00000002, 0x00000061, 0x00000002, 0x00000066, 0x3f800000,
+        0x00000003, 0x00000000, 0x000000b4, 0x00000000, 0x00000000, 0x00000001, 0x00000001, 0x00000002,
+        0x00000061, 0x00000003, 0x00000010, 0x00000004, 0x00000000, 0x00000000, 0x00000000, 0x00000002,
+        0x00000070, 0x00000006, 0x68636574, 0x00000030, 0x00000004, 0x00000010, 0x00000004, 0x00000000,
+        0x00000000, 0x00000000, 0x00000002, 0x00000070, 0x00000006, 0x68636574, 0x00000031, 0x00000003,
+        0x00000002, 0x00000004, 0x00000005, 0x00000004, 0x00000018, 0x00000000, 0x00000000, 0x00000024,
+        0x00000038, 0x00000000, 0x00000000, 0x00000044, 0x00000060, 0x00000000, 0x00000001, 0x00000068,
+        0x00000064, 0x000000dc, 0x00000001, 0x00000001, 0x00000098, 0x00000094, 0x000000d4, 0x00000000,
+        0x00000001, 0x00000092, 0x00000000, 0x000000c0, 0x000000bc, 0x00000108, 0x00000000, 0x00000001,
+        0x00000100, 0x00000000, 0x00000001, 0x00000092, 0x00000000, 0x000000ec, 0x000000e8, 0x00000002,
+        0x00000002, 0x00000002, 0x00000000, 0x00000001, 0x00000000, 0x00000001, 0x00000000, 0xffffffff,
+        0x00000000, 0x00000000, 0x00000074, 0xfffe0200, 0x0014fffe, 0x42415443, 0x0000001c, 0x00000023,
+        0xfffe0200, 0x00000000, 0x00000000, 0x20000400, 0x0000001c, 0x325f7376, 0x4d00305f, 0x6f726369,
+        0x74666f73, 0x29522820, 0x534c4820, 0x6853204c, 0x72656461, 0x6d6f4320, 0x656c6970, 0x30312072,
+        0xab00312e, 0x0200001f, 0x80000000, 0x900f0000, 0x02000001, 0xc00f0000, 0x90e40000, 0x0000ffff,
+        0x00000000, 0x00000000, 0xffffffff, 0x00000000, 0x00000000, 0x00000074, 0xfffe0200, 0x0014fffe,
+        0x42415443, 0x0000001c, 0x00000023, 0xfffe0200, 0x00000000, 0x00000000, 0x20000400, 0x0000001c,
+        0x325f7376, 0x4d00305f, 0x6f726369, 0x74666f73, 0x29522820, 0x534c4820, 0x6853204c, 0x72656461,
+        0x6d6f4320, 0x656c6970, 0x30312072, 0xab00312e, 0x0200001f, 0x80000000, 0x900f0000, 0x02000001,
+        0xc00f0000, 0x90e40000, 0x0000ffff,
+    };
 
     if (!(device = create_device(&window)))
         return;
 
+    hr = IDirect3DDevice9_CreateTexture(device, 16, 16, 1, 0, D3DFMT_X8R8G8B8,
+            D3DPOOL_DEFAULT, (IDirect3DTexture9 **)&texture, NULL);
+    ok(hr == D3D_OK, "Failed to create texture, hr %#x.\n", hr);
+
     /* D3DXFX_NOT_CLONEABLE */
-    hr = D3DXCreateEffect(device, test_effect_preshader_effect_blob, sizeof(test_effect_preshader_effect_blob),
+    hr = D3DXCreateEffect(device, effect_code, sizeof(effect_code),
             NULL, NULL, D3DXFX_NOT_CLONEABLE, NULL, &effect, NULL);
     ok(hr == D3D_OK, "Got result %#x.\n", hr);
 
+    technique = effect->lpVtbl->GetTechniqueByName(effect, "tech0");
+    ok(!!technique, "Expected a technique.\n");
+
+    technique2 = effect->lpVtbl->GetCurrentTechnique(effect);
+    ok(technique2 == technique, "Got unexpected technique %p.\n", technique2);
+
+    hr = effect->lpVtbl->SetTechnique(effect, "tech1");
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
     hr = effect->lpVtbl->CloneEffect(effect, NULL, NULL);
     ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr);
 
@@ -7445,10 +7516,33 @@ static void test_effect_clone(void)
 
     effect->lpVtbl->Release(effect);
 
-    hr = D3DXCreateEffect(device, test_effect_preshader_effect_blob, sizeof(test_effect_preshader_effect_blob),
+    hr = D3DXCreateEffect(device, effect_code, sizeof(effect_code),
             NULL, NULL, 0, NULL, &effect, NULL);
     ok(hr == D3D_OK, "Got result %#x.\n", hr);
 
+    hr = effect->lpVtbl->SetTexture(effect, "tex", texture);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
+    hr = effect->lpVtbl->SetFloat(effect, "f", 123.0f);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
+    hr = effect->lpVtbl->SetString(effect, "s", "tiny silver hammers");
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
+    hr = effect->lpVtbl->SetFloat(effect, "f(a)a", 4.0f);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
+    annotation = effect->lpVtbl->GetAnnotationByName(effect, "tech0", "a");
+    ok(!!annotation, "Failed to get annotation.\n");
+    hr = effect->lpVtbl->SetFloat(effect, annotation, 4.0f);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
+    parameter = effect->lpVtbl->GetParameterByName(effect, NULL, "tex");
+    ok(!!parameter, "Failed to get parameter.\n");
+
+    hr = effect->lpVtbl->Begin(effect, &passes_count, 0);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
     hr = effect->lpVtbl->CloneEffect(effect, NULL, NULL);
     ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr);
 
@@ -7461,20 +7555,57 @@ static void test_effect_clone(void)
     ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr);
 
     hr = effect->lpVtbl->CloneEffect(effect, device, &cloned);
-todo_wine
-    ok(hr == D3D_OK, "Got result %#x.\n", hr);
-if (hr == D3D_OK)
-{
+    todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    if (hr != D3D_OK)
+        goto out;
     ok(cloned != effect, "Expected new effect instance.\n");
+
+    hr = cloned->lpVtbl->GetTexture(cloned, "tex", &texture2);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    ok(texture2 == texture, "Expected the same texture.\n");
+    IDirect3DBaseTexture9_Release(texture2);
+
+    f = 0.0f;
+    hr = cloned->lpVtbl->GetFloat(cloned, "f", &f);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    ok(f == 123.0f, "Got float %.8e.\n", f);
+
+    hr = cloned->lpVtbl->GetString(cloned, "s", &string);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    ok(!strcmp(string, "tiny silver hammers"), "Got string %s.\n", debugstr_a(string));
+
+    f = 0.0f;
+    hr = cloned->lpVtbl->GetFloat(cloned, "f(a)a", &f);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    ok(f == 4.0f, "Got float %.8e.\n", f);
+
+    annotation = cloned->lpVtbl->GetAnnotationByName(cloned, "tech0", "a");
+    ok(!!annotation, "Failed to get annotation.\n");
+    f = 0.0f;
+    hr = cloned->lpVtbl->GetFloat(cloned, annotation, &f);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    ok(f == 4.0f, "Got float %.8e.\n", f);
+
+    parameter2 = cloned->lpVtbl->GetParameterByName(cloned, NULL, "tex");
+    ok(!!parameter2, "Failed to get parameter.\n");
+    ok(parameter2 != parameter, "Parameters should not match.\n");
+
+    hr = cloned->lpVtbl->BeginPass(cloned, 0);
+    ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr);
+
+    technique = cloned->lpVtbl->GetTechniqueByName(cloned, "tech0");
+    ok(!!technique, "Expected a technique.\n");
+
+    technique2 = cloned->lpVtbl->GetCurrentTechnique(cloned);
+    ok(technique2 == technique, "Got unexpected technique %p.\n", technique2);
+
     cloned->lpVtbl->Release(cloned);
-}
+
     /* Try with different device. */
     device2 = create_device(&window2);
+
     hr = effect->lpVtbl->CloneEffect(effect, device2, &cloned);
-todo_wine
     ok(hr == D3D_OK, "Got result %#x.\n", hr);
-if (hr == D3D_OK)
-{
     ok(cloned != effect, "Expected new effect instance.\n");
 
     hr = cloned->lpVtbl->GetDevice(cloned, &device3);
@@ -7482,11 +7613,79 @@ if (hr == D3D_OK)
     ok(device3 == device2, "Unexpected device instance.\n");
     IDirect3DDevice9_Release(device3);
 
+    hr = cloned->lpVtbl->GetTexture(cloned, "tex", &texture2);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    ok(!texture2, "Expected a NULL texture.\n");
+
+    f = 0.0f;
+    hr = cloned->lpVtbl->GetFloat(cloned, "f", &f);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    ok(f == 123.0f, "Got float %.8e.\n", f);
+
+    hr = cloned->lpVtbl->GetString(cloned, "s", &string);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    ok(!strcmp(string, "tiny silver hammers"), "Got string %s.\n", debugstr_a(string));
+
+    f = 0.0f;
+    hr = cloned->lpVtbl->GetFloat(cloned, "f(a)a", &f);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    ok(f == 4.0f, "Got float %.8e.\n", f);
+
+    annotation = cloned->lpVtbl->GetAnnotationByName(cloned, "tech0", "a");
+    ok(!!annotation, "Failed to get annotation.\n");
+    f = 0.0f;
+    hr = cloned->lpVtbl->GetFloat(cloned, annotation, &f);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    ok(f == 4.0f, "Got float %.8e.\n", f);
+
+    parameter2 = cloned->lpVtbl->GetParameterByName(cloned, NULL, "tex");
+    ok(!!parameter2, "Failed to get parameter.\n");
+    ok(parameter2 != parameter, "Parameters should not match.\n");
+
+    hr = cloned->lpVtbl->BeginPass(cloned, 0);
+    ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr);
+
+    technique = cloned->lpVtbl->GetTechniqueByName(cloned, "tech0");
+    ok(!!technique, "Expected a technique.\n");
+
+    technique2 = cloned->lpVtbl->GetCurrentTechnique(cloned);
+    ok(technique2 == technique, "Got unexpected technique %p.\n", technique2);
+
     cloned->lpVtbl->Release(cloned);
-}
+
+    hr = effect->lpVtbl->BeginPass(effect, 0);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
+    hr = effect->lpVtbl->EndPass(effect);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
+    hr = effect->lpVtbl->End(effect);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
+    /* Test parameter blocks (we can't do this above since we can't record a
+     * parameter block while started). */
+
+    hr = effect->lpVtbl->BeginParameterBlock(effect);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
+    hr = effect->lpVtbl->SetFloat(effect, "f", 456.0f);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+
+    hr = effect->lpVtbl->CloneEffect(effect, device, &cloned);
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
+    block = cloned->lpVtbl->EndParameterBlock(cloned);
+    ok(!block, "Expected no active parameter block.\n");
+
+    cloned->lpVtbl->Release(cloned);
+
+    block = effect->lpVtbl->EndParameterBlock(effect);
+    ok(!!block, "Expected an active parameter block.\n");
+
     IDirect3DDevice9_Release(device2);
     DestroyWindow(window2);
+out:
     effect->lpVtbl->Release(effect);
+    IDirect3DBaseTexture9_Release(texture);
     refcount = IDirect3DDevice9_Release(device);
     ok(!refcount, "Device has %u references left.\n", refcount);
     DestroyWindow(window);
-- 
2.34.1

[PATCH v2 3/4] d3dx9/tests: Add more tests for ID3DXEffect::CloneEffect().

[Matteo Bruni]

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>

[PATCH v2 4/4] d3dx9: Implement ID3DXEffect::CloneEffect().

[Zebediah Figura]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=44635
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
 dlls/d3dx9_36/effect.c       | 141 +++++++++++++++++++++++++++++++++--
 dlls/d3dx9_36/tests/effect.c |   5 +-
 2 files changed, 134 insertions(+), 12 deletions(-)

diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
index ccc8da5a9e8..e21ddac2869 100644
--- a/dlls/d3dx9_36/effect.c
+++ b/dlls/d3dx9_36/effect.c
@@ -17,7 +17,7 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  */
 
-
+#include <stdbool.h>
 #include <stdio.h>
 #include <assert.h>
 
@@ -199,6 +199,10 @@ struct d3dx_effect
 
     struct list parameter_block_list;
     struct d3dx_parameter_block *current_parameter_block;
+
+    char *source;
+    SIZE_T source_size;
+    char *skip_constants_string;
 };
 
 #define INITIAL_SHARED_DATA_SIZE 4
@@ -220,6 +224,9 @@ struct ID3DXEffectCompilerImpl
     LONG ref;
 };
 
+static HRESULT d3dx9_effect_init_from_dxbc(struct d3dx_effect *effect,
+        struct IDirect3DDevice9 *device, const char *data, SIZE_T data_size,
+        unsigned int flags, struct ID3DXEffectPool *pool, const char *skip_constants_string);
 static HRESULT d3dx_parse_state(struct d3dx_effect *effect, struct d3dx_state *state,
         const char *data, const char **ptr, struct d3dx_object *objects);
 static void free_parameter(struct d3dx_parameter *param, BOOL element, BOOL child);
@@ -4304,23 +4311,128 @@ static HRESULT WINAPI d3dx_effect_DeleteParameterBlock(ID3DXEffect *iface, D3DXH
 }
 #endif
 
-static HRESULT WINAPI d3dx_effect_CloneEffect(ID3DXEffect *iface, IDirect3DDevice9 *device,
-        ID3DXEffect **new_effect)
+static bool copy_parameter(struct d3dx_effect *dst_effect, const struct d3dx_effect *src_effect,
+        struct d3dx_parameter *dst, const struct d3dx_parameter *src)
 {
-    struct d3dx_effect *effect = impl_from_ID3DXEffect(iface);
+    IDirect3DBaseTexture9 *texture;
+    const char *src_string;
+    char *dst_string;
+    size_t len;
 
-    FIXME("iface %p, device %p, new_effect %p stub.\n", effect, device, new_effect);
+    if ((src->flags & PARAMETER_FLAG_SHARED) && dst_effect->pool)
+        return true;
 
-    if (!new_effect)
+    switch (src->type)
+    {
+        case D3DXPT_VOID:
+        case D3DXPT_BOOL:
+        case D3DXPT_INT:
+        case D3DXPT_FLOAT:
+            memcpy(dst->data, src->data, src->bytes);
+            break;
+
+        case D3DXPT_STRING:
+            src_string = *(char **)src->data;
+            len = strlen(src_string);
+            if (!(dst_string = heap_realloc(*(char **)dst->data, len + 1)))
+                return false;
+            *(char **)dst->data = dst_string;
+            memcpy(dst_string, src_string, len + 1);
+            break;
+
+        case D3DXPT_TEXTURE:
+        case D3DXPT_TEXTURE1D:
+        case D3DXPT_TEXTURE2D:
+        case D3DXPT_TEXTURE3D:
+        case D3DXPT_TEXTURECUBE:
+            texture = *(IDirect3DBaseTexture9 **)src->data;
+            if (src_effect->device == dst_effect->device && texture)
+            {
+                IDirect3DBaseTexture9_AddRef(texture);
+                *(IDirect3DBaseTexture9 **)dst->data = texture;
+            }
+            break;
+
+        case D3DXPT_SAMPLER:
+        case D3DXPT_SAMPLER1D:
+        case D3DXPT_SAMPLER2D:
+        case D3DXPT_SAMPLER3D:
+        case D3DXPT_SAMPLERCUBE:
+        case D3DXPT_PIXELSHADER:
+        case D3DXPT_VERTEXSHADER:
+            /* Nothing to do; these parameters are not mutable. */
+            break;
+
+        default:
+            FIXME("Unhandled parameter type %#x.\n", src->type);
+    }
+
+    return true;
+}
+
+static HRESULT WINAPI d3dx_effect_CloneEffect(ID3DXEffect *iface, IDirect3DDevice9 *device, ID3DXEffect **out)
+{
+    struct d3dx_effect *src = impl_from_ID3DXEffect(iface);
+    struct d3dx_effect *dst;
+    unsigned int i, j, k;
+    HRESULT hr;
+
+    TRACE("iface %p, device %p, out %p.\n", iface, device, out);
+
+    if (!out)
         return D3DERR_INVALIDCALL;
 
-    if (effect->flags & D3DXFX_NOT_CLONEABLE)
+    if (src->flags & D3DXFX_NOT_CLONEABLE)
         return E_FAIL;
 
     if (!device)
         return D3DERR_INVALIDCALL;
 
-    return E_NOTIMPL;
+    if (!(dst = heap_alloc_zero(sizeof(*dst))))
+        return E_OUTOFMEMORY;
+
+    if (FAILED(hr = d3dx9_effect_init_from_dxbc(dst, device, src->source, src->source_size,
+            src->flags, &src->pool->ID3DXEffectPool_iface, src->skip_constants_string)))
+    {
+        heap_free(dst);
+        return hr;
+    }
+
+    dst->manager = src->manager;
+    if (dst->manager)
+        dst->manager->lpVtbl->AddRef(dst->manager);
+
+    for (i = 0; i < src->parameter_count; ++i)
+    {
+        const struct d3dx_top_level_parameter *src_param = &src->parameters[i];
+        struct d3dx_top_level_parameter *dst_param = &dst->parameters[i];
+
+        copy_parameter(dst, src, &dst_param->param, &src_param->param);
+        for (j = 0; j < src_param->annotation_count; ++j)
+            copy_parameter(dst, src, &dst_param->annotations[j], &src_param->annotations[j]);
+    }
+
+    for (i = 0; i < src->technique_count; ++i)
+    {
+        const struct d3dx_technique *src_technique = &src->techniques[i];
+        struct d3dx_technique *dst_technique = &dst->techniques[i];
+
+        for (j = 0; j < src_technique->annotation_count; ++j)
+            copy_parameter(dst, src, &dst_technique->annotations[j], &src_technique->annotations[j]);
+
+        for (j = 0; j < src_technique->pass_count; ++j)
+        {
+            const struct d3dx_pass *src_pass = &src_technique->passes[j];
+            struct d3dx_pass *dst_pass = &dst_technique->passes[j];
+
+            for (k = 0; k < src_pass->annotation_count; ++k)
+                copy_parameter(dst, src, &dst_pass->annotations[k], &src_pass->annotations[k]);
+        }
+    }
+
+    *out = &dst->ID3DXEffect_iface;
+    TRACE("Created effect %p.\n", dst);
+    return D3D_OK;
 }
 
 #if D3DX_SDK_VERSION >= 27
@@ -6418,6 +6530,11 @@ static HRESULT d3dx9_effect_init_from_dxbc(struct d3dx_effect *effect,
     read_dword(&ptr, &tag);
     TRACE("Tag: %x\n", tag);
 
+    if (!(effect->source = malloc(data_size)))
+        return E_OUTOFMEMORY;
+    memcpy(effect->source, data, data_size);
+    effect->source_size = data_size;
+
     if (pool)
     {
         effect->pool = unsafe_impl_from_ID3DXEffectPool(pool);
@@ -6429,6 +6546,12 @@ static HRESULT d3dx9_effect_init_from_dxbc(struct d3dx_effect *effect,
 
     if (skip_constants_string)
     {
+        if (!(effect->skip_constants_string = strdup(skip_constants_string)))
+        {
+            hr = E_OUTOFMEMORY;
+            goto fail;
+        }
+
         skip_constants_buffer = HeapAlloc(GetProcessHeap(), 0,
                 sizeof(*skip_constants_buffer) * (strlen(skip_constants_string) + 1));
         if (!skip_constants_buffer)
@@ -6520,6 +6643,8 @@ fail:
     IDirect3DDevice9_Release(effect->device);
     if (pool)
         pool->lpVtbl->Release(pool);
+    free(effect->source);
+    free(effect->skip_constants_string);
     return hr;
 }
 
diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
index 5a23819e28c..99443085002 100644
--- a/dlls/d3dx9_36/tests/effect.c
+++ b/dlls/d3dx9_36/tests/effect.c
@@ -7555,9 +7555,7 @@ static void test_effect_clone(void)
     ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr);
 
     hr = effect->lpVtbl->CloneEffect(effect, device, &cloned);
-    todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr);
-    if (hr != D3D_OK)
-        goto out;
+    ok(hr == D3D_OK, "Got result %#x.\n", hr);
     ok(cloned != effect, "Expected new effect instance.\n");
 
     hr = cloned->lpVtbl->GetTexture(cloned, "tex", &texture2);
@@ -7683,7 +7681,6 @@ static void test_effect_clone(void)
 
     IDirect3DDevice9_Release(device2);
     DestroyWindow(window2);
-out:
     effect->lpVtbl->Release(effect);
     IDirect3DBaseTexture9_Release(texture);
     refcount = IDirect3DDevice9_Release(device);
-- 
2.34.1

[PATCH v2 4/4] d3dx9: Implement ID3DXEffect::CloneEffect().

[Matteo Bruni]

On Fri, Feb 11, 2022 at 5:17 AM Zebediah Figura <zfigura(a)codeweavers.com> wrote:
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=44635
> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
> ---
>  dlls/d3dx9_36/effect.c       | 141 +++++++++++++++++++++++++++++++++--
>  dlls/d3dx9_36/tests/effect.c |   5 +-
>  2 files changed, 134 insertions(+), 12 deletions(-)
>
> diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
> index ccc8da5a9e8..e21ddac2869 100644
> --- a/dlls/d3dx9_36/effect.c
> +++ b/dlls/d3dx9_36/effect.c
> @@ -17,7 +17,7 @@
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
>   */
>
> -
> +#include <stdbool.h>
>  #include <stdio.h>
>  #include <assert.h>
>
> @@ -199,6 +199,10 @@ struct d3dx_effect
>
> struct list parameter_block_list;
>      struct d3dx_parameter_block *current_parameter_block;
> +
> +    char *source;
> +    SIZE_T source_size;
> +    char *skip_constants_string;

Cloning the "skip constants" setting seems reasonable but I'd prefer
to have a test confirming that.
> };
>
> #define INITIAL_SHARED_DATA_SIZE 4
> @@ -220,6 +224,9 @@ struct ID3DXEffectCompilerImpl
>      LONG ref;
>  };
>
> +static HRESULT d3dx9_effect_init_from_dxbc(struct d3dx_effect *effect,
> +        struct IDirect3DDevice9 *device, const char *data, SIZE_T data_size,
> +        unsigned int flags, struct ID3DXEffectPool *pool, const char *skip_constants_string);
>  static HRESULT d3dx_parse_state(struct d3dx_effect *effect, struct d3dx_state *state,
>          const char *data, const char **ptr, struct d3dx_object *objects);
>  static void free_parameter(struct d3dx_parameter *param, BOOL element, BOOL child);
> @@ -4304,23 +4311,128 @@ static HRESULT WINAPI d3dx_effect_DeleteParameterBlock(ID3DXEffect *iface, D3DXH
>  }
>  #endif
>
> -static HRESULT WINAPI d3dx_effect_CloneEffect(ID3DXEffect *iface, IDirect3DDevice9 *device,
> -        ID3DXEffect **new_effect)
> +static bool copy_parameter(struct d3dx_effect *dst_effect, const struct d3dx_effect *src_effect,
> +        struct d3dx_parameter *dst, const struct d3dx_parameter *src)
>  {
> -    struct d3dx_effect *effect = impl_from_ID3DXEffect(iface);
> +    IDirect3DBaseTexture9 *texture;
> +    const char *src_string;
> +    char *dst_string;
> +    size_t len;
>
> -    FIXME("iface %p, device %p, new_effect %p stub.\n", effect, device, new_effect);
> +    if ((src->flags & PARAMETER_FLAG_SHARED) && dst_effect->pool)
> +        return true;
>
> -    if (!new_effect)
> +    switch (src->type)
> +    {
> +        case D3DXPT_VOID:
> +        case D3DXPT_BOOL:
> +        case D3DXPT_INT:
> +        case D3DXPT_FLOAT:
> +            memcpy(dst->data, src->data, src->bytes);
> +            break;
> +
> +        case D3DXPT_STRING:
> +            src_string = *(char **)src->data;
> +            len = strlen(src_string);
> +            if (!(dst_string = heap_realloc(*(char **)dst->data, len + 1)))
> +                return false;
> +            *(char **)dst->data = dst_string;
> +            memcpy(dst_string, src_string, len + 1);
> +            break;
> +
> +        case D3DXPT_TEXTURE:
> +        case D3DXPT_TEXTURE1D:
> +        case D3DXPT_TEXTURE2D:
> +        case D3DXPT_TEXTURE3D:
> +        case D3DXPT_TEXTURECUBE:
> +            texture = *(IDirect3DBaseTexture9 **)src->data;
> +            if (src_effect->device == dst_effect->device && texture)
> +            {
> +                IDirect3DBaseTexture9_AddRef(texture);
> +                *(IDirect3DBaseTexture9 **)dst->data = texture;
> +            }
> +            break;
> +
> +        case D3DXPT_SAMPLER:
> +        case D3DXPT_SAMPLER1D:
> +        case D3DXPT_SAMPLER2D:
> +        case D3DXPT_SAMPLER3D:
> +        case D3DXPT_SAMPLERCUBE:
> +        case D3DXPT_PIXELSHADER:
> +        case D3DXPT_VERTEXSHADER:
> +            /* Nothing to do; these parameters are not mutable. */
> +            break;

The test doesn't confirm that those are just recreated from scratch
(rather than e.g. shared with the original effect). Can we add some
tests for this?
> +
> +        default:
> +            FIXME("Unhandled parameter type %#x.\n", src->type);
> +    }
> +
> +    return true;
> +}
> +
> +static HRESULT WINAPI d3dx_effect_CloneEffect(ID3DXEffect *iface, IDirect3DDevice9 *device, ID3DXEffect **out)
> +{
> +    struct d3dx_effect *src = impl_from_ID3DXEffect(iface);
> +    struct d3dx_effect *dst;
> +    unsigned int i, j, k;
> +    HRESULT hr;
> +
> +    TRACE("iface %p, device %p, out %p.\n", iface, device, out);
> +
> +    if (!out)
>          return D3DERR_INVALIDCALL;
>
> -    if (effect->flags & D3DXFX_NOT_CLONEABLE)
> +    if (src->flags & D3DXFX_NOT_CLONEABLE)
>          return E_FAIL;
>
> if (!device)
>          return D3DERR_INVALIDCALL;
>
> -    return E_NOTIMPL;
> +    if (!(dst = heap_alloc_zero(sizeof(*dst))))
> +        return E_OUTOFMEMORY;
> +
> +    if (FAILED(hr = d3dx9_effect_init_from_dxbc(dst, device, src->source, src->source_size,
> +            src->flags, &src->pool->ID3DXEffectPool_iface, src->skip_constants_string)))
> +    {
> +        heap_free(dst);
> +        return hr;
> +    }
> +
> +    dst->manager = src->manager;
> +    if (dst->manager)
> +        dst->manager->lpVtbl->AddRef(dst->manager);

Again, this makes perfect sense but a test would make it foolproof.
> +
> +    for (i = 0; i < src->parameter_count; ++i)
> +    {
> +        const struct d3dx_top_level_parameter *src_param = &src->parameters[i];
> +        struct d3dx_top_level_parameter *dst_param = &dst->parameters[i];
> +
> +        copy_parameter(dst, src, &dst_param->param, &src_param->param);
> +        for (j = 0; j < src_param->annotation_count; ++j)
> +            copy_parameter(dst, src, &dst_param->annotations[j], &src_param->annotations[j]);
> +    }
> +
> +    for (i = 0; i < src->technique_count; ++i)
> +    {
> +        const struct d3dx_technique *src_technique = &src->techniques[i];
> +        struct d3dx_technique *dst_technique = &dst->techniques[i];
> +
> +        for (j = 0; j < src_technique->annotation_count; ++j)
> +            copy_parameter(dst, src, &dst_technique->annotations[j], &src_technique->annotations[j]);
> +
> +        for (j = 0; j < src_technique->pass_count; ++j)
> +        {
> +            const struct d3dx_pass *src_pass = &src_technique->passes[j];
> +            struct d3dx_pass *dst_pass = &dst_technique->passes[j];
> +
> +            for (k = 0; k < src_pass->annotation_count; ++k)
> +                copy_parameter(dst, src, &dst_pass->annotations[k], &src_pass->annotations[k]);
> +        }
> +    }
> +
> +    *out = &dst->ID3DXEffect_iface;
> +    TRACE("Created effect %p.\n", dst);
> +    return D3D_OK;
>  }
>
> #if D3DX_SDK_VERSION >= 27
> @@ -6418,6 +6530,11 @@ static HRESULT d3dx9_effect_init_from_dxbc(struct d3dx_effect *effect,
>      read_dword(&ptr, &tag);
>      TRACE("Tag: %x\n", tag);
>
> +    if (!(effect->source = malloc(data_size)))
> +        return E_OUTOFMEMORY;
> +    memcpy(effect->source, data, data_size);
> +    effect->source_size = data_size;
> +

We should probably do this only if D3DXFX_NOT_CLONEABLE is not set.
> if (pool)
>      {
>          effect->pool = unsafe_impl_from_ID3DXEffectPool(pool);
> @@ -6429,6 +6546,12 @@ static HRESULT d3dx9_effect_init_from_dxbc(struct d3dx_effect *effect,
>
> if (skip_constants_string)
>      {
> +        if (!(effect->skip_constants_string = strdup(skip_constants_string)))
> +        {
> +            hr = E_OUTOFMEMORY;
> +            goto fail;
> +        }
> +

Same here.
> skip_constants_buffer = HeapAlloc(GetProcessHeap(), 0,
>                  sizeof(*skip_constants_buffer) * (strlen(skip_constants_string) + 1));
>          if (!skip_constants_buffer)
> @@ -6520,6 +6643,8 @@ fail:
>      IDirect3DDevice9_Release(effect->device);
>      if (pool)
>          pool->lpVtbl->Release(pool);
> +    free(effect->source);
> +    free(effect->skip_constants_string);
>      return hr;
>  }
>
> diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c
> index 5a23819e28c..99443085002 100644
> --- a/dlls/d3dx9_36/tests/effect.c
> +++ b/dlls/d3dx9_36/tests/effect.c
> @@ -7555,9 +7555,7 @@ static void test_effect_clone(void)
>      ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr);
>
> hr = effect->lpVtbl->CloneEffect(effect, device, &cloned);
> -    todo_wine ok(hr == D3D_OK, "Got result %#x.\n", hr);
> -    if (hr != D3D_OK)
> -        goto out;
> +    ok(hr == D3D_OK, "Got result %#x.\n", hr);
>      ok(cloned != effect, "Expected new effect instance.\n");
>
> hr = cloned->lpVtbl->GetTexture(cloned, "tex", &texture2);
> @@ -7683,7 +7681,6 @@ static void test_effect_clone(void)
>
> IDirect3DDevice9_Release(device2);
>      DestroyWindow(window2);
> -out:
>      effect->lpVtbl->Release(effect);
>      IDirect3DBaseTexture9_Release(texture);
>      refcount = IDirect3DDevice9_Release(device);
> --
> 2.34.1

[PATCH v2 4/4] d3dx9: Implement ID3DXEffect::CloneEffect().

[Zebediah Figura]

On 2/12/22 13:54, Matteo Bruni wrote:
> > @@ -199,6 +199,10 @@ struct d3dx_effect
> >
> > struct list parameter_block_list;
> >       struct d3dx_parameter_block *current_parameter_block;
> > +
> > +    char *source;
> > +    SIZE_T source_size;
> > +    char *skip_constants_string;
>
> Cloning the "skip constants" setting seems reasonable but I'd prefer
> to have a test confirming that.

Done in v3.
> > +        case D3DXPT_SAMPLER:
> > +        case D3DXPT_SAMPLER1D:
> > +        case D3DXPT_SAMPLER2D:
> > +        case D3DXPT_SAMPLER3D:
> > +        case D3DXPT_SAMPLERCUBE:
> > +        case D3DXPT_PIXELSHADER:
> > +        case D3DXPT_VERTEXSHADER:
> > +            /* Nothing to do; these parameters are not mutable. */
> > +            break;
>
> The test doesn't confirm that those are just recreated from scratch
> (rather than e.g. shared with the original effect). Can we add some
> tests for this?

Shaders are in fact shared if possible. Unless I'm misreading though, 
there's no way for samplers to be shared.
> > +    dst->manager = src->manager;
> > +    if (dst->manager)
> > +        dst->manager->lpVtbl->AddRef(dst->manager);
>
> Again, this makes perfect sense but a test would make it foolproof.

This one, surprisingly enough, is wrong.
> > +    if (!(effect->source = malloc(data_size)))
> > +        return E_OUTOFMEMORY;
> > +    memcpy(effect->source, data, data_size);
> > +    effect->source_size = data_size;
> > +
>
> We should probably do this only if D3DXFX_NOT_CLONEABLE is not set.
> > if (pool)
> >       {
> >           effect->pool = unsafe_impl_from_ID3DXEffectPool(pool);
> > @@ -6429,6 +6546,12 @@ static HRESULT d3dx9_effect_init_from_dxbc(struct d3dx_effect *effect,
> >
> > if (skip_constants_string)
> >       {
> > +        if (!(effect->skip_constants_string = strdup(skip_constants_string)))
> > +        {
> > +            hr = E_OUTOFMEMORY;
> > +            goto fail;
> > +        }
> > +
>
> Same here.

Both done in v3.

[Matteo Bruni]

Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com>