Re: [v2 2/3] d3dx9: Implement IsParameterUsed function in effect.

[Matteo Bruni]

2016-05-23 12:40 GMT+02:00 Paul Gofman :
> Signed-off-by: Paul Gofman

It looks generally good, I only have minor "complaints" (and sorry for
the delay in reviewing...).
> dlls/d3dx9_36/effect.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 110 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
> index 31aa2d6..cdfc494 100644
> --- a/dlls/d3dx9_36/effect.c
> +++ b/dlls/d3dx9_36/effect.c
> @@ -3545,15 +3545,122 @@ static HRESULT WINAPI ID3DXEffectImpl_FindNextValidTechnique(ID3DXEffect* iface,
>      return E_NOTIMPL;
>  }
>
> +typedef BOOL (*walk_parameter_dep_func)(void *data, struct d3dx_parameter *param);
> +static BOOL walk_state_dep(struct d3dx_state *state, walk_parameter_dep_func param_func,
> -
>    void *data);
>
> +static BOOL walk_param_eval_dep(struct d3dx_param_eval *param_eval, walk_parameter_dep_func param_func,
> -
>    void *data);

I guess you intend to reuse all these functions down the line for some
other API, what are your plans?
> +static BOOL compare_param_ptr(void *param_comp, struct d3dx_parameter *param)
> +{
> - return param_comp == (void *)param;

I think the cast is unnecessary here.
> +}
> -
>
> static BOOL WINAPI ID3DXEffectImpl_IsParameterUsed(ID3DXEffect* iface, D3DXHANDLE parameter, D3DXHANDLE technique)
>  {
>      struct ID3DXEffectImpl *effect = impl_from_ID3DXEffect(iface);
> - unsigned int i, j;
> struct d3dx_parameter *param = get_valid_parameter(&effect->base_effect, parameter);
> - struct d3dx_technique *tech = get_valid_technique(&effect->base_effect, technique);
> - struct d3dx_pass *pass;
> - FIXME("iface %p, parameter %p, technique %p stub.\n", iface, parameter, technique);
> - TRACE("param %p (%s).\n", param, param ? debugstr_a(param->name) : "");
> - TRACE("iface %p, parameter %p, technique %p.\n", iface, parameter, technique);
> - TRACE("param %p, name %s, tech %p.\n", param, param ? debugstr_a(param->name) : "", tech);
> - if (!tech || !param)
> -
>    return FALSE;
>
> - return TRUE;
> - for (i = 0; i < tech->pass_count; ++i)
> - {
> -
>    pass = &tech->passes[i];
> -
>    for (j = 0; j < pass->state_count; ++j)
> -
>    {
> -
>        if (walk_state_dep(&pass->states[j], compare_param_ptr, param))
> -
>        {
> -
>            TRACE("technique %p, parameter %s, TRUE.\n", tech, param ? debugstr_a(param->name) : "");

I don't think you need to trace technique and parameter again, they
are in the immediately preceding ID3DXEffectImpl_IsParameterUsed()
TRACE anyway. Maybe the message could simply be "Returning TRUE.\n" or
similar.
> -
>            return TRUE;
> -
>        }
> -
>    }
> - }
> - TRACE("technique %p, parameter %s, FALSE.\n", tech, param ? debugstr_a(param->name) : "");

Same here.
> - return FALSE;
>
> }
> static HRESULT WINAPI ID3DXEffectImpl_Begin(ID3DXEffect *iface, UINT *passes, DWORD flags)
>
> 2.5.5

[v2 2/3] d3dx9: Implement IsParameterUsed function in effect.

[Paul Gofman]

On 05/26/2016 06:10 PM, Matteo Bruni wrote:
> 2016-05-23 12:40 GMT+02:00 Paul Gofman :
> > Signed-off-by: Paul Gofman
>
> It looks generally good, I only have minor "complaints" (and sorry for
> the delay in reviewing...).
> > dlls/d3dx9_36/effect.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 110 insertions(+), 3 deletions(-)
> >
> > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
> > index 31aa2d6..cdfc494 100644
> > --- a/dlls/d3dx9_36/effect.c
> > +++ b/dlls/d3dx9_36/effect.c
> > @@ -3545,15 +3545,122 @@ static HRESULT WINAPI ID3DXEffectImpl_FindNextValidTechnique(ID3DXEffect* iface,
> >      return E_NOTIMPL;
> >  }
> >
> > +typedef BOOL (*walk_parameter_dep_func)(void *data, struct d3dx_parameter *param);
> > +static BOOL walk_state_dep(struct d3dx_state *state, walk_parameter_dep_func param_func,
> > -
> >    void *data);
> >
> > +static BOOL walk_param_eval_dep(struct d3dx_param_eval *param_eval, walk_parameter_dep_func param_func,
> > -
> >    void *data);
>
> I guess you intend to reuse all these functions down the line for some
> other API, what are your plans?

Well, I intended to reuse, but later realized that the other "walk"
function I need in 'dirty' param tracking on CommitChanges
implementation are quite different as walk parameter members tree and
not dependencies. Now I am reusing just typedef for the tree mapped
function, I will remove one prototype and move another one near to the
function (I will still need one as there are recursive calls between the
two functions).