Re: [PATCH 3/3] d3dx9: Apply changed states only in CommitChanges.

[Matteo Bruni]

2017-03-09 23:15 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>:
> Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
> ---
>  dlls/d3dx9_36/d3dx9_private.h |   8 ++-
>  dlls/d3dx9_36/effect.c        | 146 ++++++++++++++++++++++++++++--------------
>  dlls/d3dx9_36/preshader.c     |  40 ++++++++++--
>  dlls/d3dx9_36/tests/effect.c  |  13 ----
>  4 files changed, 137 insertions(+), 70 deletions(-)
>
> diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h
> index 27b91a7..53cf99f 100644
> --- a/dlls/d3dx9_36/d3dx9_private.h
> +++ b/dlls/d3dx9_36/d3dx9_private.h
> @@ -222,6 +222,11 @@ struct d3dx_parameter
>      DWORD *dirty_flag_ptr;
>  };
>
> +static inline BOOL is_param_dirty(struct d3dx_parameter *param)
> +{
> +    return (*param->dirty_flag_ptr & PARAMETER_FLAG_DIRTY);
> +}
> +
>  struct d3dx9_base_effect;
>
> struct d3dx_parameter *get_parameter_by_name(struct d3dx9_base_effect *base,
> @@ -233,6 +238,7 @@ void d3dx_free_param_eval(struct d3dx_param_eval *peval) DECLSPEC_HIDDEN;
>  HRESULT d3dx_evaluate_parameter(struct d3dx_param_eval *peval,
>          const struct d3dx_parameter *param, void *param_value) DECLSPEC_HIDDEN;
>  HRESULT d3dx_param_eval_set_shader_constants(struct IDirect3DDevice9 *device,
> -        struct d3dx_param_eval *peval) DECLSPEC_HIDDEN;
> +        struct d3dx_param_eval *peval, BOOL update_all) DECLSPEC_HIDDEN;
> +BOOL is_param_eval_input_dirty(struct d3dx_param_eval *peval) DECLSPEC_HIDDEN;
>
> #endif /* __WINE_D3DX9_PRIVATE_H */
> diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c
> index ada78f9..b18465c 100644
> --- a/dlls/d3dx9_36/effect.c
> +++ b/dlls/d3dx9_36/effect.c
> @@ -123,6 +123,9 @@ struct d3dx_pass
>
> struct d3dx_state *states;
>      struct d3dx_parameter *annotations;
> +
> +    D3DLIGHT9 current_light[8];
> +    D3DMATERIAL9 current_material;
>  };

I think these might be stored in the effect struct instead.
> +static void clear_dirty_all(struct d3dx9_base_effect *base)

clear_dirty_params() or something like that would be a clearer name.
> @@ -1570,8 +1587,12 @@ static HRESULT d3dx9_base_effect_set_int(struct d3dx9_base_effect *base, D3DXHAN
>      {
>          if (param->rows == 1 && param->columns == 1)
>          {
> -            set_number(param->data, param->type, &n, D3DXPT_INT);
> -            set_dirty(param);
> +            DWORD value;
> +
> +            set_number(&value, param->type, &n, D3DXPT_INT);
> +            if (value != *(DWORD *)param->data)
> +                set_dirty(param);
> +             *(DWORD *)param->data = value;

Shouldn't this go in the previous patch?

[PATCH 3/3] d3dx9: Apply changed states only in CommitChanges.

[Paul Gofman]

On 03/14/2017 12:47 AM, Matteo Bruni wrote:
> 2017-03-09 23:15 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>:
> > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
> > ---
> >
> > struct d3dx_state *states;
> >       struct d3dx_parameter *annotations;
> > +
> > +    D3DLIGHT9 current_light[8];
> > +    D3DMATERIAL9 current_material;
> >   };
> > I think these might be stored in the effect struct instead.
> > Yes actually, too passes can't be active at a time and each BeginPass 
> > should initialize this in effect structure correctly.
> > +static void clear_dirty_all(struct d3dx9_base_effect *base)
> > clear_dirty_params() or something like that would be a clearer name.
> > I will change the name.
> > @@ -1570,8 +1587,12 @@ static HRESULT d3dx9_base_effect_set_int(struct d3dx9_base_effect *base, D3DXHAN
> >       {
> >           if (param->rows == 1 && param->columns == 1)
> >           {
> > -            set_number(param->data, param->type, &n, D3DXPT_INT);
> > -            set_dirty(param);
> > +            DWORD value;
> > +
> > +            set_number(&value, param->type, &n, D3DXPT_INT);
> > +            if (value != *(DWORD *)param->data)
> > +                set_dirty(param);
> > +             *(DWORD *)param->data = value;
> > Shouldn't this go in the previous patch?

Yes, sure. There is actually another fix I made in the next patches 
which makes SetLight go in one call for all light attributes, probably I 
should bring it here at once too? Just the test for that goes later in 
my patches when I test & implement effect state manager.