Re: [v5 2/3] d3dx9: Get rid of constant's length checking for matrix in set_constants().

[Matteo Bruni]

2017-06-22 15:11 GMT+02:00 Paul Gofman :
> When count in const_upload_info is calculated precisely (considering actual
> constant set length), boolean matrix setting fall in direct copy path, unless
> transpose is required. The case of incomplete last row for matrix should be
> handled in set_constants() for transposed boolean constant only.
> Signed-off-by: Paul Gofman
>
> v5:
>     - added check for constant size not exceeding parameter size. The check for
>       non array, non struct constants should be sufficient as the dimensions of
>       arrays and structs are derived from const_set data and register counts
>       from constant description is not used for them.

I don't understand what you mean here.
> @@ -1417,6 +1433,16 @@ static HRESULT init_set_constants_param(struct d3dx_const_tab *const_tab, ID3DXC
>              && !info.transpose && info.minor == info.major_stride
>              && info.count == get_offset_reg(const_set.table, const_set.register_count)
>              && info.count * sizeof(unsigned int) <= param->bytes;
> - if (info.minor_remainder && !const_set.direct_copy && !info.transpose)
> -
>    FIXME("Incomplete last row for not transposed matrix which cannot be directly copied, parameter %s.\n",
> -
>            debugstr_a(param->name));
> -
> - if (info.major_count > info.major
> -
>        || (info.major_count == info.major && info.minor_remainder))
> - {
> -
>    WARN("Constant dimensions exceed parameter size.\n");
> -
>    return D3DERR_INVALIDCALL;
> - }

It seems to me that this catches all the problematic cases.

[v5 2/3] d3dx9: Get rid of constant's length checking for matrix in set_constants().

[Paul Gofman]

On 06/27/2017 09:34 PM, Matteo Bruni wrote:
> 2017-06-22 15:11 GMT+02:00 Paul Gofman :
> > When count in const_upload_info is calculated precisely (considering actual
> > constant set length), boolean matrix setting fall in direct copy path, unless
> > transpose is required. The case of incomplete last row for matrix should be
> > handled in set_constants() for transposed boolean constant only.
> > Signed-off-by: Paul Gofman
> >
> > v5:
> >      - added check for constant size not exceeding parameter size. The check for
> >        non array, non struct constants should be sufficient as the dimensions of
> >        arrays and structs are derived from const_set data and register counts
> >        from constant description is not used for them.
>
> I don't understand what you mean here.
> > @@ -1417,6 +1433,16 @@ static HRESULT init_set_constants_param(struct d3dx_const_tab *const_tab, ID3DXC
> >               && !info.transpose && info.minor == info.major_stride
> >               && info.count == get_offset_reg(const_set.table, const_set.register_count)
> >               && info.count * sizeof(unsigned int) <= param->bytes;
> > - if (info.minor_remainder && !const_set.direct_copy && !info.transpose)
> > -
> >    FIXME("Incomplete last row for not transposed matrix which cannot be directly copied, parameter %s.\n",
> > -
> >            debugstr_a(param->name));
> > -
> > - if (info.major_count > info.major
> > -
> >        || (info.major_count == info.major && info.minor_remainder))
> > - {
> > -
> >    WARN("Constant dimensions exceed parameter size.\n");
> > -
> >    return D3DERR_INVALIDCALL;
> > - }
>
> It seems to me that this catches all the problematic cases.

Yes, that what I mean above :) I tried to explain why I don't need a 
separate check that for arrays and structs, but probably the explanation 
is less clear than code itself.

[v5 2/3] d3dx9: Get rid of constant's length checking for matrix in set_constants().

[Matteo Bruni]

2017-06-27 20:38 GMT+02:00 Paul Gofman :
> On 06/27/2017 09:34 PM, Matteo Bruni wrote:
> > 2017-06-22 15:11 GMT+02:00 Paul Gofman :
> > > When count in const_upload_info is calculated precisely (considering
> > > actual
> > > constant set length), boolean matrix setting fall in direct copy path,
> > > unless
> > > transpose is required. The case of incomplete last row for matrix should
> > > be
> > > handled in set_constants() for transposed boolean constant only.
> > > Signed-off-by: Paul Gofman
> > >
> > > v5:
> > >      - added check for constant size not exceeding parameter size. The
> > > check for
> > >        non array, non struct constants should be sufficient as the
> > > dimensions of
> > >        arrays and structs are derived from const_set data and register
> > > counts
> > >        from constant description is not used for them.
> >
> > I don't understand what you mean here.
> > > @@ -1417,6 +1433,16 @@ static HRESULT init_set_constants_param(struct
> > > d3dx_const_tab *const_tab, ID3DXC
> > >               && !info.transpose && info.minor == info.major_stride
> > >               && info.count == get_offset_reg(const_set.table,
> > > const_set.register_count)
> > >               && info.count * sizeof(unsigned int) <= param->bytes;
> > > - if (info.minor_remainder && !const_set.direct_copy &&
> > >
> > > !info.transpose)
> > > -
> > >    FIXME("Incomplete last row for not transposed matrix which
> > >
> > > cannot be directly copied, parameter %s.\n",
> > > -
> > >            debugstr_a(param->name));
> > > -
> > > - if (info.major_count > info.major
> > > -
> > >        || (info.major_count == info.major && info.minor_remainder))
> > > - {
> > > -
> > >    WARN("Constant dimensions exceed parameter size.\n");
> > > -
> > >    return D3DERR_INVALIDCALL;
> > > - }
> >
> > It seems to me that this catches all the problematic cases.
>
> Yes, that what I mean above :) I tried to explain why I don't need a
> separate check that for arrays and structs, but probably the explanation is
> less clear than code itself.

Ahh, I see. Indeed the code speaks for itself :D