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

[Matteo Bruni]

2017-03-09 23:15 GMT+01:00 Paul Gofman :
> Signed-off-by: Paul Gofman
>
> 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 :
> > Signed-off-by: Paul Gofman
> >   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.