[PATCH 1/4] d3d10: Get resources used by effect shaders.

[Connor McAdams]

Signed-off-by: Connor McAdams 
---
 dlls/d3d10/d3d10_private.h | 12 ++++++
 dlls/d3d10/effect.c        | 87 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+)

diff --git a/dlls/d3d10/d3d10_private.h b/dlls/d3d10/d3d10_private.h
index e02ea376bb..21fae15505 100644
--- a/dlls/d3d10/d3d10_private.h
+++ b/dlls/d3d10/d3d10_private.h
@@ -81,6 +81,15 @@ struct d3d10_effect_object
     } object;
 };

+struct d3d10_effect_shader_resource
+{
+    D3D10_SHADER_INPUT_TYPE in_type;
+    UINT bind_point;
+    UINT bind_count;
+
+    struct d3d10_effect_variable *resource_variable;
+};
+
 struct d3d10_effect_shader_signature
 {
     char *signature;
@@ -99,6 +108,9 @@ struct d3d10_effect_shader_variable
         ID3D10PixelShader *ps;
         ID3D10GeometryShader *gs;
     } shader;
+
+    UINT resource_count;
+    struct d3d10_effect_shader_resource *shader_resource;
 };

struct d3d10_effect_state_object_variable
diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
index f620191955..580b6079cf 100644
--- a/dlls/d3d10/effect.c
+++ b/dlls/d3d10/effect.c
@@ -656,6 +656,87 @@ static HRESULT shader_chunk_handler(const char *data, DWORD data_size, DWORD tag
     return S_OK;
 }

+static HRESULT get_fx10_shader_resources(struct d3d10_effect_variable *v, const void *data, size_t data_size)
+{
+    struct d3d10_effect_shader_variable *sv = &v->u.shader;
+    struct d3d10_effect_shader_resource *sr;
+    D3D10_SHADER_INPUT_BIND_DESC rsrc_desc;
+    struct d3d10_effect_variable *var;
+    ID3D10ShaderReflection *reflector;
+    D3D10_SHADER_DESC desc;
+    unsigned int i, y;
+    HRESULT hr;
+
+    hr = D3D10ReflectShader(data, data_size, &reflector);
+    if (FAILED(hr)) return hr;
+
+    reflector->lpVtbl->GetDesc(reflector, &desc);
+    sv->resource_count = desc.BoundResources;
+
+    if (!(sv->shader_resource = heap_calloc(sv->resource_count, sizeof(*sv->shader_resource))))
+    {
+        ERR("Failed to allocate shader resource binding information memory.\n");
+        hr = E_OUTOFMEMORY;
+        goto exit;
+    }
+
+    for (i = 0; i < desc.BoundResources; ++i)
+    {
+        reflector->lpVtbl->GetResourceBindingDesc(reflector, i, &rsrc_desc);
+        sr = &sv->shader_resource[i];
+
+        sr->in_type = rsrc_desc.Type;
+        sr->bind_point = rsrc_desc.BindPoint;
+        sr->bind_count = rsrc_desc.BindCount;
+
+        switch (rsrc_desc.Type)
+        {
+            case D3D10_SIT_CBUFFER:
+            case D3D10_SIT_TBUFFER:
+                for (y = 0; y < v->effect->local_buffer_count; ++y)
+                {
+                    var = &v->effect->local_buffers[y];
+
+                    if (!strcmp(rsrc_desc.Name, var->name))
+                    {
+                        sr->resource_variable = var;
+                        break;
+                    }
+                }
+                break;
+
+            case D3D10_SIT_SAMPLER:
+            case D3D10_SIT_TEXTURE:
+                for (y = 0; y < v->effect->local_variable_count; ++y)
+                {
+                    var = &v->effect->local_variables[y];
+
+                    if (!strcmp(rsrc_desc.Name, var->name))
+                    {
+                        sr->resource_variable = var;
+                        break;
+                    }
+                }
+                break;
+
+            default:
+                break;
+        }
+
+        if (!sr->resource_variable)
+        {
+            ERR("Failed to find shader resource.\n");
+            hr = E_FAIL;
+            goto exit;
+        }
+    }
+
+exit:
+    reflector->lpVtbl->Release(reflector);
+
+    return hr;
+}
+
 static HRESULT parse_fx10_shader(const char *data, size_t data_size, DWORD offset, struct d3d10_effect_variable *v)
 {
     ID3D10Device *device = v->effect->device;
@@ -691,6 +772,9 @@ static HRESULT parse_fx10_shader(const char *data, size_t data_size, DWORD offse
     /* We got a shader VertexShader vs = NULL, so it is fine to skip this. */
     if (!dxbc_size) return S_OK;

+    hr = get_fx10_shader_resources(v, (const void *)ptr, dxbc_size);
+    if (FAILED(hr)) return hr;
+
     switch (v->type->basetype)
     {
         case D3D10_SVT_VERTEXSHADER:
@@ -2623,6 +2707,9 @@ static void d3d10_effect_shader_variable_destroy(struct d3d10_effect_shader_vari
             FIXME("Unhandled shader type %s.\n", debug_d3d10_shader_variable_type(type));
             break;
     }
+
+    if (s->resource_count)
+        heap_free(s->shader_resource);
 }

static void d3d10_effect_variable_destroy(struct d3d10_effect_variable *v)

[PATCH 2/4] d3d10: Apply shader resources for shaders used in pass.

[Connor McAdams]

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

diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
index 580b6079cf..f86ec5ce9f 100644
--- a/dlls/d3d10/effect.c
+++ b/dlls/d3d10/effect.c
@@ -3617,9 +3617,107 @@ static struct ID3D10EffectVariable * STDMETHODCALLTYPE d3d10_effect_pass_GetAnno
     return &null_variable.ID3D10EffectVariable_iface;
 }

+static void update_buffer(ID3D10Device *device, struct d3d10_effect_variable *v)
+{
+    struct d3d10_effect_buffer_variable *b = &v->u.buffer;
+
+    if (!b->changed)
+        return;
+
+    ID3D10Device_UpdateSubresource(device, (ID3D10Resource *)b->buffer, 0, NULL,
+            (const void *)b->local_buffer, v->data_size, 0);
+
+    b->changed = FALSE;
+}
+
+static void apply_shader_resources(ID3D10Device *device, struct ID3D10EffectShaderVariable *variable)
+{
+    struct d3d10_effect_variable *v = impl_from_ID3D10EffectVariable((ID3D10EffectVariable *)variable);
+    struct d3d10_effect_shader_variable *sv = &v->u.shader;
+    struct d3d10_effect_shader_resource *sr;
+    ID3D10ShaderResourceView *const *srv;
+    struct d3d10_effect_variable *rsrc_v;
+
+    unsigned int i;
+
+    for (i = 0; i < sv->resource_count; ++i)
+    {
+        sr = &sv->shader_resource[i];
+        rsrc_v = sr->resource_variable;
+
+        switch (sr->in_type)
+        {
+            case D3D10_SIT_CBUFFER:
+                update_buffer(device, rsrc_v);
+                switch (v->type->basetype)
+                {
+                    case D3D10_SVT_VERTEXSHADER:
+                        ID3D10Device_VSSetConstantBuffers(device, sr->bind_point, sr->bind_count,
+                                (ID3D10Buffer *const *)&rsrc_v->u.buffer.buffer);
+                        break;
+                    case D3D10_SVT_PIXELSHADER:
+                        ID3D10Device_PSSetConstantBuffers(device, sr->bind_point, sr->bind_count,
+                                (ID3D10Buffer *const *)&rsrc_v->u.buffer.buffer);
+                        break;
+                    case D3D10_SVT_GEOMETRYSHADER:
+                        ID3D10Device_GSSetConstantBuffers(device, sr->bind_point, sr->bind_count,
+                                (ID3D10Buffer *const *)&rsrc_v->u.buffer.buffer);
+                        break;
+                    default:
+                        break;
+                }
+                break;
+
+            case D3D10_SIT_TBUFFER:
+                update_buffer(device, rsrc_v);
+                srv = (ID3D10ShaderResourceView *const *)&rsrc_v->u.buffer.resource_view;
+
+                switch (v->type->basetype)
+                {
+                    case D3D10_SVT_VERTEXSHADER:
+                        ID3D10Device_VSSetShaderResources(device, sr->bind_point, sr->bind_count, srv);
+                        break;
+                    case D3D10_SVT_PIXELSHADER:
+                        ID3D10Device_PSSetShaderResources(device, sr->bind_point, sr->bind_count, srv);
+                        break;
+                    case D3D10_SVT_GEOMETRYSHADER:
+                        ID3D10Device_GSSetShaderResources(device, sr->bind_point, sr->bind_count, srv);
+                        break;
+                    default:
+                        break;
+                }
+                break;
+
+            case D3D10_SIT_SAMPLER:
+                switch (v->type->basetype)
+                {
+                    case D3D10_SVT_VERTEXSHADER:
+                        ID3D10Device_VSSetSamplers(device, sr->bind_point, sr->bind_count,
+                                (ID3D10SamplerState *const *)&rsrc_v->u.state.object.sampler);
+                        break;
+                    case D3D10_SVT_PIXELSHADER:
+                        ID3D10Device_PSSetSamplers(device, sr->bind_point, sr->bind_count,
+                                (ID3D10SamplerState *const *)&rsrc_v->u.state.object.sampler);
+                        break;
+                    case D3D10_SVT_GEOMETRYSHADER:
+                        ID3D10Device_GSSetSamplers(device, sr->bind_point, sr->bind_count,
+                                (ID3D10SamplerState *const *)&rsrc_v->u.state.object.sampler);
+                        break;
+                    default:
+                        break;
+                }
+                break;
+
+            default:
+                break;
+        }
+    }
+}
+
 static HRESULT STDMETHODCALLTYPE d3d10_effect_pass_Apply(ID3D10EffectPass *iface, UINT flags)
 {
     struct d3d10_effect_pass *This = impl_from_ID3D10EffectPass(iface);
+    ID3D10Device *device = This->technique->effect->device;
     HRESULT hr = S_OK;
     unsigned int i;

@@ -3627,6 +3725,13 @@ static HRESULT STDMETHODCALLTYPE d3d10_effect_pass_Apply(ID3D10EffectPass *iface

if (flags) FIXME("Ignoring flags (%#x)\n", flags);

+    if (This->vs.pShaderVariable != (ID3D10EffectShaderVariable *)&null_shader_variable.ID3D10EffectVariable_iface)
+        apply_shader_resources(device, This->vs.pShaderVariable);
+    if (This->gs.pShaderVariable != (ID3D10EffectShaderVariable *)&null_shader_variable.ID3D10EffectVariable_iface)
+        apply_shader_resources(device, This->gs.pShaderVariable);
+    if (This->ps.pShaderVariable != (ID3D10EffectShaderVariable *)&null_shader_variable.ID3D10EffectVariable_iface)
+        apply_shader_resources(device, This->ps.pShaderVariable);
+
     for (i = 0; i < This->object_count; ++i)
     {
         hr = d3d10_effect_object_apply(&This->objects[i]);

[PATCH 2/4] d3d10: Apply shader resources for shaders used in pass.

[Matteo Bruni]

On Sat, Mar 21, 2020 at 7:30 PM Connor McAdams  wrote:
> Signed-off-by: Connor McAdams
>
> dlls/d3d10/effect.c | 105 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 105 insertions(+)
>
> diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
> index 580b6079cf..f86ec5ce9f 100644
> --- a/dlls/d3d10/effect.c
> +++ b/dlls/d3d10/effect.c
> @@ -3617,9 +3617,107 @@ static struct ID3D10EffectVariable * STDMETHODCALLTYPE d3d10_effect_pass_GetAnno
>      return &null_variable.ID3D10EffectVariable_iface;
>  }
>
> +static void update_buffer(ID3D10Device *device, struct d3d10_effect_variable *v)
> +{
> - struct d3d10_effect_buffer_variable *b = &v->u.buffer;
> -
> - if (!b->changed)
> -
>    return;
> -
> - ID3D10Device_UpdateSubresource(device, (ID3D10Resource *)b->buffer, 0, NULL,
> -
>        (const void *)b->local_buffer, v->data_size, 0);

The (const void *) cast is unnecessary.
> -
> - b->changed = FALSE;
>
> +}
> -
>
> +static void apply_shader_resources(ID3D10Device *device, struct ID3D10EffectShaderVariable *variable)
> +{
> - struct d3d10_effect_variable *v = impl_from_ID3D10EffectVariable((ID3D10EffectVariable *)variable);

I think we should introduce a impl_from_ID3D10EffectShaderVariable(),
like we recently did for ID3D10EffectScalarVariable and such. Probably
as a separate patch, replacing the existing cases at the same time.
> - struct d3d10_effect_shader_variable *sv = &v->u.shader;
> - struct d3d10_effect_shader_resource *sr;
> - ID3D10ShaderResourceView *const *srv;

I see no reason for the const there, it's not really protecting much.
> - struct d3d10_effect_variable *rsrc_v;
> -
> - unsigned int i;

Blank line?
> -
> - for (i = 0; i < sv->resource_count; ++i)
> - {
> -
>    sr = &sv->shader_resource[i];
> -
>    rsrc_v = sr->resource_variable;
> -
> -
>    switch (sr->in_type)
> -
>    {
> -
>        case D3D10_SIT_CBUFFER:
> -
>            update_buffer(device, rsrc_v);
> -
>            switch (v->type->basetype)
> -
>            {
> -
>                case D3D10_SVT_VERTEXSHADER:
> -
>                    ID3D10Device_VSSetConstantBuffers(device, sr->bind_point, sr->bind_count,
> -
>                            (ID3D10Buffer *const *)&rsrc_v->u.buffer.buffer);

Those casts are unnecessary AFAICS.
> -
>                    break;
> -
>                case D3D10_SVT_PIXELSHADER:
> -
>                    ID3D10Device_PSSetConstantBuffers(device, sr->bind_point, sr->bind_count,
> -
>                            (ID3D10Buffer *const *)&rsrc_v->u.buffer.buffer);
> -
>                    break;
> -
>                case D3D10_SVT_GEOMETRYSHADER:
> -
>                    ID3D10Device_GSSetConstantBuffers(device, sr->bind_point, sr->bind_count,
> -
>                            (ID3D10Buffer *const *)&rsrc_v->u.buffer.buffer);
> -
>                    break;
> -
>                default:
> -
>                    break;

It might be worthwhile to add a WARN() or something for the default case.
> @@ -3627,6 +3725,13 @@ static HRESULT STDMETHODCALLTYPE d3d10_effect_pass_Apply(ID3D10EffectPass *iface
>  if (flags) FIXME("Ignoring flags (%#x)\n", flags);
>
> - if (This->vs.pShaderVariable != (ID3D10EffectShaderVariable *)&null_shader_variable.ID3D10EffectVariable_iface)
> -
>    apply_shader_resources(device, This->vs.pShaderVariable);

These casts also look ugly but, as I understand it, they're caused by
the idiotic d3d10 effect variable object model and are basically
unavoidable :/

While touching the function, though, you could rename "This" to "pass".

[PATCH 3/4] d3d10: Implement ShaderResource effect variable set method.

[Connor McAdams]

Signed-off-by: Connor McAdams 
---
 dlls/d3d10/d3d10_private.h |   7 +++
 dlls/d3d10/effect.c        | 111 ++++++++++++++++++++++++++++++++++---
 2 files changed, 111 insertions(+), 7 deletions(-)

diff --git a/dlls/d3d10/d3d10_private.h b/dlls/d3d10/d3d10_private.h
index 21fae15505..35fc9993db 100644
--- a/dlls/d3d10/d3d10_private.h
+++ b/dlls/d3d10/d3d10_private.h
@@ -131,6 +131,12 @@ struct d3d10_effect_state_object_variable
     } object;
 };

+struct d3d10_effect_resource_variable
+{
+    ID3D10ShaderResourceView **resource_view;
+    BOOL parent;
+};
+
 struct d3d10_effect_buffer_variable
 {
     ID3D10Buffer *buffer;
@@ -196,6 +202,7 @@ struct d3d10_effect_variable
         struct d3d10_effect_state_object_variable state;
         struct d3d10_effect_shader_variable shader;
         struct d3d10_effect_buffer_variable buffer;
+        struct d3d10_effect_resource_variable resource;
     } u;
 };

diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
index f86ec5ce9f..0654d28fc2 100644
--- a/dlls/d3d10/effect.c
+++ b/dlls/d3d10/effect.c
@@ -2077,6 +2077,28 @@ static HRESULT parse_fx10_local_variable(const char *data, size_t data_size,
         case D3D10_SVT_TEXTURE2DMSARRAY:
         case D3D10_SVT_TEXTURE3D:
         case D3D10_SVT_TEXTURECUBE:
+            if (!v->type->element_count)
+                i = 1;
+            else
+                i = v->type->element_count;
+
+            if (!(v->u.resource.resource_view = heap_calloc(i, sizeof(ID3D10ShaderResourceView *))))
+            {
+                ERR("Failed to allocate shader resource view array memory.\n");
+                return E_OUTOFMEMORY;
+            }
+            v->u.resource.parent = TRUE;
+
+            if (v->elements)
+            {
+                for (i = 0; i < v->type->element_count; ++i)
+                {
+                    v->elements[i].u.resource.resource_view = &v->u.resource.resource_view[i];
+                    v->elements[i].u.resource.parent = FALSE;
+                }
+            }
+            break;
+
         case D3D10_SVT_RENDERTARGETVIEW:
         case D3D10_SVT_DEPTHSTENCILVIEW:
         case D3D10_SVT_BUFFER:
@@ -2714,7 +2736,7 @@ static void d3d10_effect_shader_variable_destroy(struct d3d10_effect_shader_vari

static void d3d10_effect_variable_destroy(struct d3d10_effect_variable *v)
 {
-    unsigned int i;
+    unsigned int i, elem_count;

TRACE("variable %p.\n", v);

@@ -2777,6 +2799,31 @@ static void d3d10_effect_variable_destroy(struct d3d10_effect_variable *v)
                     ID3D10SamplerState_Release(v->u.state.object.sampler);
                 break;

+            case D3D10_SVT_TEXTURE1D:
+            case D3D10_SVT_TEXTURE1DARRAY:
+            case D3D10_SVT_TEXTURE2D:
+            case D3D10_SVT_TEXTURE2DARRAY:
+            case D3D10_SVT_TEXTURE2DMS:
+            case D3D10_SVT_TEXTURE2DMSARRAY:
+            case D3D10_SVT_TEXTURE3D:
+            case D3D10_SVT_TEXTURECUBE:
+                if (!v->u.resource.parent)
+                    break;
+
+                if (!v->type->element_count)
+                    elem_count = 1;
+                else
+                    elem_count = v->type->element_count;
+
+                for (i = 0; i < elem_count; ++i)
+                {
+                    if (v->u.resource.resource_view[i])
+                        ID3D10ShaderResourceView_Release(v->u.resource.resource_view[i]);
+                }
+
+                heap_free(v->u.resource.resource_view);
+                break;
+
             default:
                 break;
         }
@@ -3669,8 +3716,14 @@ static void apply_shader_resources(ID3D10Device *device, struct ID3D10EffectShad
                 break;

case D3D10_SIT_TBUFFER:
-                update_buffer(device, rsrc_v);
-                srv = (ID3D10ShaderResourceView *const *)&rsrc_v->u.buffer.resource_view;
+            case D3D10_SIT_TEXTURE:
+                if (sr->in_type == D3D10_SIT_TEXTURE)
+                    srv = (ID3D10ShaderResourceView *const *)rsrc_v->u.resource.resource_view;
+                else
+                {
+                    update_buffer(device, sr->resource_variable);
+                    srv = (ID3D10ShaderResourceView *const *)&rsrc_v->u.buffer.resource_view;
+                }

switch (v->type->basetype)
                 {
@@ -5908,8 +5961,24 @@ static const struct ID3D10EffectStringVariableVtbl d3d10_effect_string_variable_
     d3d10_effect_string_variable_GetStringArray,
 };

+static void set_shader_resource_variable(ID3D10ShaderResourceView **src, ID3D10ShaderResourceView **dst)
+{
+    if (*dst)
+        ID3D10ShaderResourceView_Release(*dst);
+    if (*src)
+        ID3D10ShaderResourceView_AddRef(*src);
+
+    *dst = *src;
+}
+
 /* ID3D10EffectVariable methods */

+static inline struct d3d10_effect_variable *impl_from_ID3D10EffectShaderResourceVariable(
+        ID3D10EffectShaderResourceVariable *iface)
+{
+    return CONTAINING_RECORD(iface, struct d3d10_effect_variable, ID3D10EffectVariable_iface);
+}
+
 static BOOL STDMETHODCALLTYPE d3d10_effect_shader_resource_variable_IsValid(ID3D10EffectShaderResourceVariable *iface)
 {
     TRACE("iface %p\n", iface);
@@ -6066,9 +6135,13 @@ static HRESULT STDMETHODCALLTYPE d3d10_effect_shader_resource_variable_GetRawVal
 static HRESULT STDMETHODCALLTYPE d3d10_effect_shader_resource_variable_SetResource(
         ID3D10EffectShaderResourceVariable *iface, ID3D10ShaderResourceView *resource)
 {
-    FIXME("iface %p, resource %p stub!\n", iface, resource);
+    struct d3d10_effect_variable *effect_var = impl_from_ID3D10EffectShaderResourceVariable(iface);

-    return E_NOTIMPL;
+    TRACE("iface %p, resource %p.\n", iface, resource);
+
+    set_shader_resource_variable(&resource, effect_var->u.resource.resource_view);
+
+    return S_OK;
 }

static HRESULT STDMETHODCALLTYPE d3d10_effect_shader_resource_variable_GetResource(
@@ -6082,9 +6155,33 @@ static HRESULT STDMETHODCALLTYPE d3d10_effect_shader_resource_variable_GetResour
 static HRESULT STDMETHODCALLTYPE d3d10_effect_shader_resource_variable_SetResourceArray(
         ID3D10EffectShaderResourceVariable *iface, ID3D10ShaderResourceView **resources, UINT offset, UINT count)
 {
-    FIXME("iface %p, resources %p, offset %u, count %u stub!\n", iface, resources, offset, count);
+    struct d3d10_effect_variable *effect_var = impl_from_ID3D10EffectShaderResourceVariable(iface);
+    ID3D10ShaderResourceView **rsrc_view;
+    UINT i;

-    return E_NOTIMPL;
+    TRACE("iface %p, resources %p, offset %u, count %u.\n", iface, resources, offset, count);
+
+    if (!effect_var->type->element_count)
+        return d3d10_effect_shader_resource_variable_SetResource(iface, *resources);
+
+    if (offset >= effect_var->type->element_count)
+    {
+        WARN("Offset %u larger than element count %u, ignoring.\n", offset, effect_var->type->element_count);
+        return S_OK;
+    }
+
+    if (count > effect_var->type->element_count - offset)
+    {
+        WARN("Offset %u, count %u overruns the variable (element count %u), fixing up.\n",
+             offset, count, effect_var->type->element_count);
+        count = effect_var->type->element_count - offset;
+    }
+
+    rsrc_view = &effect_var->u.resource.resource_view[offset];
+    for (i = 0; i < count; ++i)
+        set_shader_resource_variable(&resources[i], &rsrc_view[i]);
+
+    return S_OK;
 }

static HRESULT STDMETHODCALLTYPE d3d10_effect_shader_resource_variable_GetResourceArray(

[PATCH 3/4] d3d10: Implement ShaderResource effect variable set method.

[Matteo Bruni]

On Sat, Mar 21, 2020 at 7:31 PM Connor McAdams  wrote:
> @@ -5908,8 +5961,24 @@ static const struct ID3D10EffectStringVariableVtbl d3d10_effect_string_variable_
>      d3d10_effect_string_variable_GetStringArray,
>  };
>
> +static void set_shader_resource_variable(ID3D10ShaderResourceView **src, ID3D10ShaderResourceView **dst)
> +{
> - if (*dst)
> -
>    ID3D10ShaderResourceView_Release(*dst);
> - if (*src)
> -
>    ID3D10ShaderResourceView_AddRef(*src);
> -
> - *dst = *src;
>
> +}

When we have to handle something like this in other places we usually
check that *dst != *src first, as an optimization. Also, can either of
them be NULL?

[PATCH 3/4] d3d10: Implement ShaderResource effect variable set method.

[Connor McAdams]

On Tue, Mar 24, 2020 at 02:12:23PM +0100, Matteo Bruni wrote:
> On Sat, Mar 21, 2020 at 7:31 PM Connor McAdams  wrote:
> > @@ -5908,8 +5961,24 @@ static const struct ID3D10EffectStringVariableVtbl d3d10_effect_string_variable_
> >      d3d10_effect_string_variable_GetStringArray,
> >  };
> >
> > +static void set_shader_resource_variable(ID3D10ShaderResourceView **src, ID3D10ShaderResourceView **dst)
> > +{
> > - if (*dst)
> > -
> >    ID3D10ShaderResourceView_Release(*dst);
> > - if (*src)
> > -
> >    ID3D10ShaderResourceView_AddRef(*src);
> > -
> > - *dst = *src;
> >
> > +}
>
> When we have to handle something like this in other places we usually
> check that *dst != *src first, as an optimization. Also, can either of
> them be NULL?

I will add the check for *dst != *src, hadn't thought of that.

src or dst can be NULL, depending on the circumstance. To clear a bound
resource, SetResource is called with the SRV set to NULL, and in that
case, if the resource was set to a valid SRV previously, you'd have *dst
be non-NULL and src be NULL, then the opposite case could be true as
well.

[PATCH 3/4] d3d10: Implement ShaderResource effect variable set method.

[Henri Verbeet]

On Tue, 24 Mar 2020 at 17:42, Matteo Bruni  wrote:
> On Sat, Mar 21, 2020 at 7:31 PM Connor McAdams  wrote:
> > @@ -5908,8 +5961,24 @@ static const struct ID3D10EffectStringVariableVtbl d3d10_effect_string_variable_
> >      d3d10_effect_string_variable_GetStringArray,
> >  };
> >
> > +static void set_shader_resource_variable(ID3D10ShaderResourceView **src, ID3D10ShaderResourceView **dst)
> > +{
> > - if (*dst)
> > -
> >    ID3D10ShaderResourceView_Release(*dst);
> > - if (*src)
> > -
> >    ID3D10ShaderResourceView_AddRef(*src);
> > -
> > - *dst = *src;
> >
> > +}
>
> When we have to handle something like this in other places we usually
> check that *dst != *src first, as an optimization. Also, can either of
> them be NULL?

It's perhaps worth pointing out that it's potentially more than an
optimisation. If *dst and *src point to the same object, there's a
risk that doing the Release() before the AddRef() ends up destroying
that object.

[PATCH 4/4] d3d10: Add ID3D10EffectShaderResourceVariable tests.

[Connor McAdams]

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

diff --git a/dlls/d3d10/tests/effect.c b/dlls/d3d10/tests/effect.c
index 06397d7aa5..3465653575 100644
--- a/dlls/d3d10/tests/effect.c
+++ b/dlls/d3d10/tests/effect.c
@@ -5356,6 +5356,231 @@ static void test_effect_matrix_variable(void)
     ok(!refcount, "Device has %u references left.\n", refcount);
 }

+/*
+ * test_effect_resource_variable
+ */
+#if 0
+Texture2D t0;
+Texture2D t_a[2];
+
+float4 VS( float4 Pos : POSITION ) : SV_POSITION { return float4(1.0f, 1.0f, 1.0f, 1.0f); }
+
+float4 PS( float4 Pos : SV_POSITION ) : SV_Target
+{
+    uint4 tmp;
+
+    tmp = t0.Load(int3(0, 0, 0));
+    tmp = t_a[0].Load(int4(0, 0, 0, 0));
+    tmp = t_a[1].Load(int4(0, 0, 0, 0));
+    return float4(1.0f, 1.0f, 0.0f, 1.0f) + tmp;
+}
+
+technique10 rsrc_test
+{
+    pass p0
+    {
+        SetVertexShader(CompileShader(vs_4_0, VS()));
+        SetPixelShader(CompileShader(ps_4_0, PS()));
+    }
+}
+#endif
+static DWORD fx_test_resource_variable[] =
+{
+    0x43425844, 0x767a8421, 0xdcbfffe6, 0x83df123d, 0x189ce72a, 0x00000001, 0x0000065a, 0x00000001,
+    0x00000024, 0x30315846, 0x0000062e, 0xfeff1001, 0x00000000, 0x00000000, 0x00000002, 0x00000000,
+    0x00000000, 0x00000000, 0x00000001, 0x00000582, 0x00000000, 0x00000003, 0x00000000, 0x00000000,
+    0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000002, 0x00000002, 0x00000000, 0x74786554,
+    0x32657275, 0x00040044, 0x00020000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x000c0000,
+    0x30740000, 0x00000400, 0x00000200, 0x00000200, 0x00000000, 0x00000000, 0x00000000, 0x00000c00,
+    0x615f7400, 0x72737200, 0x65745f63, 0x70007473, 0x01b40030, 0x58440000, 0x338d4342, 0xc5a69f46,
+    0x56883ae5, 0xa98fccc2, 0x00018ead, 0x01b40000, 0x00050000, 0x00340000, 0x008c0000, 0x00c00000,
+    0x00f40000, 0x01380000, 0x44520000, 0x00504645, 0x00000000, 0x00000000, 0x00000000, 0x001c0000,
+    0x04000000, 0x0100fffe, 0x001c0000, 0x694d0000, 0x736f7263, 0x2074666f, 0x20295228, 0x4c534c48,
+    0x61685320, 0x20726564, 0x706d6f43, 0x72656c69, 0x322e3920, 0x35392e39, 0x31332e32, 0xab003131,
+    0x5349abab, 0x002c4e47, 0x00010000, 0x00080000, 0x00200000, 0x00000000, 0x00000000, 0x00030000,
+    0x00000000, 0x000f0000, 0x4f500000, 0x49544953, 0xab004e4f, 0x534fabab, 0x002c4e47, 0x00010000,
+    0x00080000, 0x00200000, 0x00000000, 0x00010000, 0x00030000, 0x00000000, 0x000f0000, 0x56530000,
+    0x534f505f, 0x4f495449, 0x4853004e, 0x003c5244, 0x00400000, 0x000f0001, 0x00670000, 0x20f20400,
+    0x00000010, 0x00010000, 0x00360000, 0x20f20800, 0x00000010, 0x40020000, 0x00000000, 0x00003f80,
+    0x00003f80, 0x00003f80, 0x003e3f80, 0x54530100, 0x00745441, 0x00020000, 0x00000000, 0x00000000,
+    0x00010000, 0x00000000, 0x00000000, 0x00000000, 0x00010000, 0x00000000, 0x00000000, 0x00000000,
+    0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+    0x00010000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+    0x00000000, 0x00000000, 0x005a0000, 0x00000000, 0x035c0000, 0x58440000, 0xe3754342, 0x3e477f40,
+    0xed6f143f, 0xf16d26ce, 0x00010c3a, 0x035c0000, 0x00050000, 0x00340000, 0x00d00000, 0x01040000,
+    0x01380000, 0x02e00000, 0x44520000, 0x00944645, 0x00000000, 0x00000000, 0x00020000, 0x001c0000,
+    0x04000000, 0x0100ffff, 0x00630000, 0x005c0000, 0x00020000, 0x00050000, 0x00040000, 0xffff0000,
+    0x0000ffff, 0x00010000, 0x000c0000, 0x005f0000, 0x00020000, 0x00050000, 0x00040000, 0xffff0000,
+    0x0001ffff, 0x00020000, 0x000c0000, 0x30740000, 0x615f7400, 0x63694d00, 0x6f736f72, 0x28207466,
+    0x48202952, 0x204c534c, 0x64616853, 0x43207265, 0x69706d6f, 0x2072656c, 0x39322e39, 0x3235392e,
+    0x3131332e, 0x53490031, 0x002c4e47, 0x00010000, 0x00080000, 0x00200000, 0x00000000, 0x00010000,
+    0x00030000, 0x00000000, 0x000f0000, 0x56530000, 0x534f505f, 0x4f495449, 0x534f004e, 0x002c4e47,
+    0x00010000, 0x00080000, 0x00200000, 0x00000000, 0x00000000, 0x00030000, 0x00000000, 0x000f0000,
+    0x56530000, 0x7261545f, 0x00746567, 0x4853abab, 0x01a05244, 0x00400000, 0x00680000, 0x18580000,
+    0x70000400, 0x00000010, 0x55550000, 0x18580000, 0x70000400, 0x00010010, 0x55550000, 0x18580000,
+    0x70000400, 0x00020010, 0x55550000, 0x00650000, 0x20f20300, 0x00000010, 0x00680000, 0x00020200,
+    0x002d0000, 0x00f20a00, 0x00000010, 0x40020000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+    0x7e460000, 0x00000010, 0x001c0000, 0x00f20500, 0x00000010, 0x0e460000, 0x00000010, 0x00560000,
+    0x00f20500, 0x00000010, 0x0e460000, 0x00000010, 0x002d0000, 0x00f20a00, 0x00010010, 0x40020000,
+    0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x7e460000, 0x00010010, 0x00000000, 0x00f20700,
+    0x00000010, 0x0e460000, 0x00000010, 0x0e460000, 0x00010010, 0x001c0000, 0x00f20500, 0x00000010,
+    0x0e460000, 0x00000010, 0x00560000, 0x00f20500, 0x00000010, 0x0e460000, 0x00000010, 0x002d0000,
+    0x00f20a00, 0x00010010, 0x40020000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x7e460000,
+    0x00020010, 0x00000000, 0x00f20700, 0x00000010, 0x0e460000, 0x00000010, 0x0e460000, 0x00010010,
+    0x001c0000, 0x00f20500, 0x00000010, 0x0e460000, 0x00000010, 0x00560000, 0x00f20500, 0x00000010,
+    0x0e460000, 0x00000010, 0x00000000, 0x20f20a00, 0x00000010, 0x0e460000, 0x00000010, 0x40020000,
+    0x00000000, 0x00003f80, 0x00003f80, 0x00000000, 0x003e3f80, 0x54530100, 0x00745441, 0x000d0000,
+    0x00020000, 0x00000000, 0x00010000, 0x00030000, 0x00000000, 0x00000000, 0x00010000, 0x00000000,
+    0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00000000,
+    0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x00060000, 0x00000000, 0x00000000, 0x00000000,
+    0x00000000, 0x00000000, 0x00000000, 0x00000000, 0x021a0000, 0x00000000, 0x002a0000, 0x000e0000,
+    0x00000000, 0xffff0000, 0x0000ffff, 0x00490000, 0x002d0000, 0x00000000, 0xffff0000, 0x0000ffff,
+    0x004d0000, 0x00010000, 0x00000000, 0x00570000, 0x00020000, 0x00000000, 0x00060000, 0x00000000,
+    0x00070000, 0x02120000, 0x00070000, 0x00000000, 0x00070000, 0x057a0000, 0x00000000,
+};
+
+static void create_effect_texture_resource(ID3D10Device *device, ID3D10ShaderResourceView **srv,
+        ID3D10Texture2D **tex)
+{
+    D3D10_TEXTURE2D_DESC tex_desc;
+    HRESULT hr;
+
+    tex_desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM;
+    tex_desc.Width  = 8;
+    tex_desc.Height = 8;
+    tex_desc.ArraySize = 1;
+    tex_desc.MipLevels = 0;
+    tex_desc.BindFlags = D3D10_BIND_SHADER_RESOURCE;
+    tex_desc.Usage = D3D10_USAGE_DEFAULT;
+    tex_desc.CPUAccessFlags = 0;
+    tex_desc.SampleDesc.Count = 1;
+    tex_desc.SampleDesc.Quality = 0;
+    tex_desc.MiscFlags = 0;
+
+    hr = ID3D10Device_CreateTexture2D(device, &tex_desc, NULL, tex);
+    ok(SUCCEEDED(hr), "Failed to create a 2D texture, hr %#x.\n", hr);
+    hr = ID3D10Device_CreateShaderResourceView(device, (ID3D10Resource *)*tex, NULL, srv);
+    ok(SUCCEEDED(hr), "Failed to create a shader resource view, hr %#x.\n", hr);
+}
+
+static void get_effect_shader_resource_variable(ID3D10EffectVariable *var,
+        ID3D10EffectShaderResourceVariable **srv)
+{
+    D3D10_EFFECT_TYPE_DESC type_desc;
+    ID3D10EffectType *type;
+    HRESULT hr;
+
+    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_TEXTURE2D, "Type is %x, expected %x.\n", type_desc.Type, D3D10_SVT_TEXTURE2D);
+    *srv = var->lpVtbl->AsShaderResource(var);
+}
+
+static void test_effect_resource_variable(void)
+{
+    ID3D10ShaderResourceView *srv0, *srv_a[2], *srv_tmp[2];
+    ID3D10EffectShaderResourceVariable *t0, *t_a, *t_a_0;
+    ID3D10EffectTechnique *technique;
+    ID3D10Texture2D *tex0, *tex_a[2];
+    ID3D10EffectVariable *var;
+    ID3D10EffectPass *pass;
+    ID3D10Device *device;
+    ID3D10Effect *effect;
+    unsigned int i;
+    ULONG refcount;
+    HRESULT hr;
+
+    if (!(device = create_device()))
+    {
+        skip("Failed to create device, skipping tests.\n");
+        return;
+    }
+
+    hr = create_effect(fx_test_resource_variable, 0, device, NULL, &effect);
+    ok(SUCCEEDED(hr), "D3D10CreateEffectFromMemory failed (%x).\n", hr);
+
+    var = effect->lpVtbl->GetVariableByName(effect, "t0");
+    get_effect_shader_resource_variable(var, &t0);
+    create_effect_texture_resource(device, &srv0, &tex0);
+    hr = t0->lpVtbl->SetResource(t0, srv0);
+    ok(hr == S_OK, "Failed to SetResource, hr %#x.\n", hr);
+
+    var = effect->lpVtbl->GetVariableByName(effect, "t_a");
+    get_effect_shader_resource_variable(var, &t_a);
+    for (i = 0; i < 2; ++i)
+        create_effect_texture_resource(device, &srv_a[i], &tex_a[i]);
+    hr = t_a->lpVtbl->SetResourceArray(t_a, srv_a, 0, 2);
+    ok(hr == S_OK, "Failed to SetResourceArray, hr %#x.\n", hr);
+
+    /* Apply the pass to bind the resource to the shader. */
+    technique = effect->lpVtbl->GetTechniqueByName(effect, "rsrc_test");
+    ok(!!technique, "Failed to get technique.\n");
+    pass = technique->lpVtbl->GetPassByName(technique, "p0");
+    ok(!!pass, "Failed to get pass.\n");
+    hr = pass->lpVtbl->Apply(pass, 0);
+    ok(SUCCEEDED(hr), "Failed to apply pass, hr %#x.\n", hr);
+
+    ID3D10Device_PSGetShaderResources(device, 0, 1, &srv_tmp[0]);
+    ok(srv0 == srv_tmp[0], "Shader resource view was not bound to the shader.\n");
+    ID3D10ShaderResourceView_Release(srv_tmp[0]);
+
+    ID3D10Device_PSGetShaderResources(device, 1, 2, srv_tmp);
+    for (i = 0; i < 2; ++i)
+    {
+        ok(srv_a[i] == srv_tmp[i], "Shader resource view was not bound to the shader.\n");
+        ID3D10ShaderResourceView_Release(srv_tmp[i]);
+    }
+
+    /* Test individual array element variable SetResource. */
+    var = t_a->lpVtbl->GetElement(t_a, 1);
+    get_effect_shader_resource_variable(var, &t_a_0);
+    hr = t_a_0->lpVtbl->SetResource(t_a_0, srv0);
+    ok(hr == S_OK, "Failed to SetResource, hr %#x.\n", hr);
+
+    hr = pass->lpVtbl->Apply(pass, 0);
+    ok(SUCCEEDED(hr), "Failed to apply pass, hr %#x.\n", hr);
+
+    ID3D10Device_PSGetShaderResources(device, 1, 2, srv_tmp);
+    ok(srv0 == srv_tmp[1], "Shader resource view was not bound to the shader.\n");
+    for (i = 0; i < 2; ++i)
+        ID3D10ShaderResourceView_Release(srv_tmp[i]);
+
+    /* Test offset. */
+    hr = t_a->lpVtbl->SetResourceArray(t_a, srv_a, 1, 1);
+    ok(hr == S_OK, "Failed to SetResourceArray, hr %#x.\n", hr);
+
+    hr = pass->lpVtbl->Apply(pass, 0);
+    ok(SUCCEEDED(hr), "Failed to apply pass, hr %#x.\n", hr);
+
+    ID3D10Device_PSGetShaderResources(device, 1, 2, srv_tmp);
+    ok(srv_a[0] == srv_tmp[1], "Shader resource view was not bound to the shader.\n");
+    for (i = 0; i < 2; ++i)
+        ID3D10ShaderResourceView_Release(srv_tmp[i]);
+
+    /* If the offset or write size is larger than the element size of the
+     * texture, due to there being no bounds checking, an access violation
+     * occurs. */
+    if (0)
+    {
+        hr = t_a->lpVtbl->SetResourceArray(t_a, srv_a, 2, 2);
+        ok(hr == S_OK, "Failed to SetResourceArray, hr %#x.\n", hr);
+    }
+
+    ID3D10ShaderResourceView_Release(srv0);
+    ID3D10Texture2D_Release(tex0);
+    for (i = 0; i < 2; ++i)
+    {
+        ID3D10ShaderResourceView_Release(srv_a[i]);
+        ID3D10Texture2D_Release(tex_a[i]);
+    }
+
+    effect->lpVtbl->Release(effect);
+
+    refcount = ID3D10Device_Release(device);
+    ok(!refcount, "Device has %u references left.\n", refcount);
+}
+
 START_TEST(effect)
 {
     test_effect_constant_buffer_type();
@@ -5371,4 +5596,5 @@ START_TEST(effect)
     test_effect_scalar_variable();
     test_effect_vector_variable();
     test_effect_matrix_variable();
+    test_effect_resource_variable();
 }

[PATCH 4/4] d3d10: Add ID3D10EffectShaderResourceVariable tests.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=67744

Your paranoid android.

=== build (build log) ===

error: patch failed: dlls/d3d10/tests/effect.c:5371
Task: Patch failed to apply

=== debiant (build log) ===

error: patch failed: dlls/d3d10/tests/effect.c:5371
Task: Patch failed to apply

=== debiant (build log) ===

error: patch failed: dlls/d3d10/tests/effect.c:5371
Task: Patch failed to apply