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

[Jason Ekstrand]

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

> On Thu, Jun 15, 2017 at 09:57:03PM -0700, Jason Ekstrand wrote:
> > 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.
>
> Right, sorry, should have been clearer. I meant:
>
>          /* Only number of levels, width and height can be altered. */
>          assert(depth == level_depth);
>
>          return width == minify(mt->surf.logical_level0_px.width, level)
> &&
>                 height == minify(mt->surf.logical_level0_px.height,
> level);
>

What I said above still holds, you just now need glTexImage3D to trigger
it. :-)


> >
> > --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/52fab754/attachment.html>