Re: [PATCH 4/5] d3dx9: Get rid of constant's length checking for matrix in set_constants().

[Matteo Bruni]

2017-06-06 14:28 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
> @@ -1026,10 +1027,22 @@ static void get_const_upload_info(struct d3dx_const_param_eval_output *const_set
>          info->major = param->rows;
>          info->minor = param->columns;
>      }
> -    info->major_stride = max(info->minor, get_offset_reg(table, 1));
> -    info->major_count = min(info->major * info->major_stride,
> -            get_offset_reg(table, const_set->register_count) + info->major_stride - 1) / info->major_stride;
> -    info->count = info->major_count * info->minor;
> +
> +    if (table == PRES_REGTAB_OBCONST)

Minor, but you could key this on component_count == 1 instead of
explicitly checking PRES_REGTAB_OBCONST.
> +    {
> +        unsigned int const_length = get_offset_reg(table, const_set->register_count);
> +
> +        info->major_stride = info->minor;
> +        info->major_count = const_length / info->major_stride;
> +        info->minor_remainder = const_length % info->major_stride;
> +    }
> +    else
> +    {
> +        info->major_stride = get_offset_reg(table, 1);

Another potential use for the get_reg_components(table) helper :)

[PATCH 4/5] d3dx9: Get rid of constant's length checking for matrix in set_constants().

[Paul Gofman]

On 06/09/2017 08:50 PM, Matteo Bruni wrote:
> 2017-06-06 14:28 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
> > @@ -1026,10 +1027,22 @@ static void get_const_upload_info(struct d3dx_const_param_eval_output *const_set
> >           info->major = param->rows;
> >           info->minor = param->columns;
> >       }
> > -    info->major_stride = max(info->minor, get_offset_reg(table, 1));
> > -    info->major_count = min(info->major * info->major_stride,
> > -            get_offset_reg(table, const_set->register_count) + info->major_stride - 1) / info->major_stride;
> > -    info->count = info->major_count * info->minor;
> > +
> > +    if (table == PRES_REGTAB_OBCONST)
> > Minor, but you could key this on component_count == 1 instead of
> > explicitly checking PRES_REGTAB_OBCONST.
> > +    {
> > +        unsigned int const_length = get_offset_reg(table, const_set->register_count);
> > +
> > +        info->major_stride = info->minor;
> > +        info->major_count = const_length / info->major_stride;
> > +        info->minor_remainder = const_length % info->major_stride;
> > +    }
> > +    else
> > +    {
> > +        info->major_stride = get_offset_reg(table, 1);
> > Another potential use for the get_reg_components(table) helper :)

Do you think it is better to add this helper as a preliminary patch to 
this series, and use it right away for these cases (as well as other 
places instead of get_offset_reg(table, 1))?

[PATCH 4/5] d3dx9: Get rid of constant's length checking for matrix in set_constants().

[Matteo Bruni]

2017-06-09 20:57 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
> On 06/09/2017 08:50 PM, Matteo Bruni wrote:
> > 2017-06-06 14:28 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
> > > @@ -1026,10 +1027,22 @@ static void get_const_upload_info(struct
> > > d3dx_const_param_eval_output *const_set
> > >           info->major = param->rows;
> > >           info->minor = param->columns;
> > >       }
> > > -    info->major_stride = max(info->minor, get_offset_reg(table, 1));
> > > -    info->major_count = min(info->major * info->major_stride,
> > > -            get_offset_reg(table, const_set->register_count) +
> > > info->major_stride - 1) / info->major_stride;
> > > -    info->count = info->major_count * info->minor;
> > > +
> > > +    if (table == PRES_REGTAB_OBCONST)
> >
> > Minor, but you could key this on component_count == 1 instead of
> > explicitly checking PRES_REGTAB_OBCONST.
> > > +    {
> > > +        unsigned int const_length = get_offset_reg(table,
> > > const_set->register_count);
> > > +
> > > +        info->major_stride = info->minor;
> > > +        info->major_count = const_length / info->major_stride;
> > > +        info->minor_remainder = const_length % info->major_stride;
> > > +    }
> > > +    else
> > > +    {
> > > +        info->major_stride = get_offset_reg(table, 1);
> >
> > Another potential use for the get_reg_components(table) helper :)
>
> Do you think it is better to add this helper as a preliminary patch to this
> series, and use it right away for these cases (as well as other places
> instead of get_offset_reg(table, 1))?

Probably, yes.