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

[Jason Ekstrand]

On Thu, Jun 15, 2017 at 9:49 PM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> 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.
>

It's also worth pointing out that there's no difference between logical and
physical with 3D since 3D can't be multisampled.  For 3D, I think logical
is the only one that makes sense.


> >
> > > +            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?
>

No, you want to return.  The whole point of this function is to determine
if this gl_texture_image matches the given level of the miptree.  This is
used, for instence, when someone does glTexImage2D and changes the size of
one LOD.  In that case, we can't upload to the miptree and we have to split
that LOD out into it's own.  Later on, assuming the texture is complete, we
recombine them into a single miptree.  It's aweful, but welcome to OpenGL.

--Jason


> > > +             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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170615/ce8781d0/attachment.html>