From: Matteo Bruni <matteo.mystral@gmail.com>
Subject: Re: [PATCH 3/4] d3d10: Implement ShaderResource effect variable set method.
Message-Id: <CABvNrtOtLkKKfSA5woJC4UQNuZE=ATbX9aBz3CNS5CWOTETPtw@mail.gmail.com>
Date: Wed, 25 Mar 2020 22:31:41 +0100
In-Reply-To: <20200325181629.25837-3-conmanx360@gmail.com>
References: <20200325181629.25837-1-conmanx360@gmail.com> <20200325181629.25837-3-conmanx360@gmail.com>

On Wed, Mar 25, 2020 at 7:17 PM Connor McAdams <conmanx360@gmail.com> wrote:
>
> Signed-off-by: Connor McAdams <conmanx360@gmail.com>
> ---
>  dlls/d3d10/d3d10_private.h |   7 +++
>  dlls/d3d10/effect.c        | 114 ++++++++++++++++++++++++++++++++++---
>  2 files changed, 114 insertions(+), 7 deletions(-)
>
> diff --git a/dlls/d3d10/d3d10_private.h b/dlls/d3d10/d3d10_private.h
> index 21d7f964e8..5d7c4e128b 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;
> +};

Just to give a couple more naming options: that could also be srv or view.

> +
>  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 3d397a38eb..91cb189362 100644
> --- a/dlls/d3d10/effect.c
> +++ b/dlls/d3d10/effect.c
> @@ -2079,6 +2079,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 *))))

sizeof(*v->u.resource.resource_view)

> @@ -5924,8 +5977,27 @@ 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 == *src)
> +        return;
> +
> +    if (*dst)
> +        ID3D10ShaderResourceView_Release(*dst);
> +    if (*src)
> +        ID3D10ShaderResourceView_AddRef(*src);
> +
> +    *dst = *src;
> +}

Nitpick: can you swap the order of the if (*src) / if (*dst)? I know
it doesn't matter since there is the equality check just above, but
with the AddRef() before the Release() it becomes obviously correct
and a bit easier to read.

> @@ -6082,9 +6154,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);

Just var is fine.

>
> -    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(
> @@ -6098,9 +6174,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;

unsigned int i;