Re: [PATCH 6/7] d3dx9: Implement d3dx_shader_get_raw_constant_info() function.

[Matteo Bruni]

2017-07-06 13:15 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
> Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
> ---
>  dlls/d3dx9_36/d3dx9_private.h |  3 +++
>  dlls/d3dx9_36/shader.c        | 18 ++++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h
> index db80490..bfb3909 100644
> --- a/dlls/d3dx9_36/d3dx9_private.h
> +++ b/dlls/d3dx9_36/d3dx9_private.h
> @@ -355,4 +355,7 @@ HRESULT d3dx_param_eval_set_shader_constants(ID3DXEffectStateManager *manager, s
>          struct d3dx_param_eval *peval, BOOL update_all) DECLSPEC_HIDDEN;
>  BOOL is_param_eval_input_dirty(struct d3dx_param_eval *peval, ULONG64 update_version) DECLSPEC_HIDDEN;
>
> +const D3DXSHADER_CONSTANTINFO *d3dx_shader_get_raw_constant_info(ID3DXConstantTable *iface,
> +        D3DXHANDLE constant) DECLSPEC_HIDDEN;
> +
>  #endif /* __WINE_D3DX9_PRIVATE_H */
> diff --git a/dlls/d3dx9_36/shader.c b/dlls/d3dx9_36/shader.c
> index 4dbecb1..11527c8 100644
> --- a/dlls/d3dx9_36/shader.c
> +++ b/dlls/d3dx9_36/shader.c
> @@ -682,6 +682,7 @@ HRESULT WINAPI D3DXPreprocessShaderFromResourceW(HMODULE module, const WCHAR *re
>  }
>
> struct ctab_constant {
> +    const D3DXSHADER_CONSTANTINFO *raw_constant_info;
>      D3DXCONSTANT_DESC desc;
>      struct ctab_constant *constants;
>  };
> @@ -946,6 +947,22 @@ static HRESULT WINAPI ID3DXConstantTableImpl_GetDesc(ID3DXConstantTable *iface,
>      return D3D_OK;
>  }
>
> +const D3DXSHADER_CONSTANTINFO *d3dx_shader_get_raw_constant_info(ID3DXConstantTable *iface, D3DXHANDLE constant)
> +{
> +    struct ID3DXConstantTableImpl *const_table = impl_from_ID3DXConstantTable(iface);
> +    struct ctab_constant *c = get_valid_constant(const_table, constant);
> +
> +    TRACE("const_table %p, constant %p.\n", const_table, constant);
> +
> +    if (!c)
> +    {
> +        WARN("Invalid argument specified.\n");
> +        return NULL;
> +    }
> +
> +    return c->raw_constant_info;
> +}
> +
>  static HRESULT WINAPI ID3DXConstantTableImpl_GetConstantDesc(ID3DXConstantTable *iface, D3DXHANDLE constant,
>                                                               D3DXCONSTANT_DESC *desc, UINT *count)
>  {
> @@ -2052,6 +2069,7 @@ HRESULT WINAPI D3DXGetShaderConstantTableEx(const DWORD *byte_code, DWORD flags,
>          {
>              object->constants[i].desc.RegisterCount = constant_info[i].RegisterCount;
>          }
> +        object->constants[i].raw_constant_info = &constant_info[i];
>      }
>
> *constant_table = &object->ID3DXConstantTable_iface;

I don't like this all that much.

If you need access to the original D3DXSHADER_CONSTANTINFO only for
the Reserved field, as in the next patch, I'd say just store that one
in struct ctab_constant.
Then, for the retrieval part, I don't think it makes much sense to
hide our internals from ourselves. You could move the definition of
struct ctab_constant to d3dx9_private.h and introduce an internal
function to return it from the constant table.
You could then entirely replace the use of
ID3DXConstantTable_GetConstantDesc(), if you want. Otherwise you can
keep that part as-is for the time being and make use of the new
function only to get to the Reserved data. Actually, if you go with
the latter, it probably makes sense to merge this patch into the next.