[Mesa-dev] [PATCH 05/15] i965: Prepare image validation for isl based miptrees

[Pohjolainen, Topi]

On Thu, Jun 15, 2017 at 05:21:42PM -0700, Nanley Chery wrote:
> On Tue, Jun 13, 2017 at 05:50:03PM +0300, Topi Pohjolainen wrote:
> > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index 061860cdf6..f44bac988f 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -1087,6 +1087,21 @@ intel_miptree_match_image(struct intel_mipmap_tree *mt,
> >     if (mt->target == GL_TEXTURE_CUBE_MAP)
> >        depth = 6;
> >  
> > +   if (mt->surf.size > 0) {
> > +      if (level >= mt->surf.levels)
> > +         return false;
> > +
> > +      const unsigned level_depth =
> > +         mt->surf.dim == ISL_SURF_DIM_3D ?
> > +            minify(mt->surf.logical_level0_px.depth, level) :
> 
> We should be looking up the physical depth shouldn't we? I haven't yet
> looked into what width or height should be. I'm done for today, so I
> will revisit this series tomorrow.

Hmm, you are correct when comparing to the existing code - it uses the table
which contains physical values. The given depth, however, comes from
gl_texture_image::Depth which is core concept and deals with logical
values.

The confusing bit here is (which I should have written into the commit
message) is that number of layers can't be altered, only levels. The existing
code checks for depth because it uses that to catch missing levels. The table
is initialised to zeroes and finding depth of zero is taken as meaning that
the desired level doesn't exist.

> 
> > +            mt->surf.logical_level0_px.array_len;
> > +
> > +      return width == minify(mt->surf.logical_level0_px.width, level) &&
> > +             height == minify(mt->surf.logical_level0_px.height, level) &&
> > +             depth == level_depth &&

I should probably assert this instead of failing - as I said I don't see how
number of layers could change. Ian, Ken, Jason, how do you see this?

> > +             MAX2(image->NumSamples, 1) == mt->surf.samples;
> > +   }
> > +
> >     int level_depth = mt->level[level].depth;
> >     if (mt->num_samples > 1) {
> >        switch (mt->msaa_layout) {
> > -- 
> > 2.11.0
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev