Re: [PATCH 08/10] d3d10: Implement ShaderResource effect variable set method.

[Matteo Bruni]

On Sat, Dec 7, 2019 at 7:24 PM Connor McAdams  wrote:
> Implement the SetResource method for the
> ID3D10EffectShaderResourceVariable interface.
> Signed-off-by: Connor McAdams
>
> dlls/d3d10/d3d10_private.h |  6 ++++
>  dlls/d3d10/effect.c        | 71 +++++++++++++++++++++++++++++++++++---
>  2 files changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/dlls/d3d10/d3d10_private.h b/dlls/d3d10/d3d10_private.h
> index 5c6c7a2d72..f3b4e65e1c 100644
> --- a/dlls/d3d10/d3d10_private.h
> +++ b/dlls/d3d10/d3d10_private.h
> @@ -114,6 +114,11 @@ struct d3d10_effect_state_object_variable
>      } object;
>  };
>
> +struct d3d10_effect_resource_variable
> +{
> - ID3D10ShaderResourceView **resource_view;
>
> +};
> -
>
> struct d3d10_effect_buffer_variable
>  {
>      ID3D10Buffer *buffer;
> @@ -179,6 +184,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 1e1d5d7f4c..9898b7f79f 100644
> --- a/dlls/d3d10/effect.c
> +++ b/dlls/d3d10/effect.c
> @@ -1994,6 +1994,19 @@ 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:
> -
>        TRACE("SVT could not have elements.\n");

That trace doesn't seem to apply. I guess it's an accidental copypaste
of the similar one below (not visible in the patch).
> -
>        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;
> -
>        }
> -
>        break;
> -
>    case D3D10_SVT_RENDERTARGETVIEW:
>    case D3D10_SVT_DEPTHSTENCILVIEW:
>    case D3D10_SVT_BUFFER:
>
> @@ -2689,6 +2702,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->type->element_count)
> -
>            {
> -
>                if (*v->u.resource.resource_view)
> -
>                    ID3D10ShaderResourceView_Release(*v->u.resource.resource_view);
> -
>            }
> -
>            else
> -
>            {
> -
>                for (i = 0; i < v->type->element_count; i++)
> -
>                {
> -
>                    if (v->u.resource.resource_view[i])
> -
>                        ID3D10ShaderResourceView_Release(v->u.resource.resource_view[i]);
> -
> -
>                }
> -
>            }

I think it would be cleaner to use the for loop in the else branch for
element_count == 0, fixing up the iteration count accordingly (like
you do in parse_fx10_local_variable() above).