<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jun 15, 2017 at 10:00 PM, Pohjolainen, Topi <span dir="ltr"><<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, Jun 15, 2017 at 09:57:03PM -0700, Jason Ekstrand wrote:<br>
> On Thu, Jun 15, 2017 at 9:49 PM, Pohjolainen, Topi <<br>
> <a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> wrote:<br>
><br>
> > On Thu, Jun 15, 2017 at 05:21:42PM -0700, Nanley Chery wrote:<br>
> > > On Tue, Jun 13, 2017 at 05:50:03PM +0300, Topi Pohjolainen wrote:<br>
> > > > Signed-off-by: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>><br>
> > > > ---<br>
> > > >  src/mesa/drivers/dri/i965/<wbr>intel_mipmap_tree.c | 15 +++++++++++++++<br>
> > > >  1 file changed, 15 insertions(+)<br>
> > > ><br>
> > > > diff --git a/src/mesa/drivers/dri/i965/<wbr>intel_mipmap_tree.c<br>
> > b/src/mesa/drivers/dri/i965/<wbr>intel_mipmap_tree.c<br>
> > > > index 061860cdf6..f44bac988f 100644<br>
> > > > --- a/src/mesa/drivers/dri/i965/<wbr>intel_mipmap_tree.c<br>
> > > > +++ b/src/mesa/drivers/dri/i965/<wbr>intel_mipmap_tree.c<br>
> > > > @@ -1087,6 +1087,21 @@ intel_miptree_match_image(<wbr>struct<br>
> > intel_mipmap_tree *mt,<br>
> > > >     if (mt->target == GL_TEXTURE_CUBE_MAP)<br>
> > > >        depth = 6;<br>
> > > ><br>
> > > > +   if (mt->surf.size > 0) {<br>
> > > > +      if (level >= mt->surf.levels)<br>
> > > > +         return false;<br>
> > > > +<br>
> > > > +      const unsigned level_depth =<br>
> > > > +         mt->surf.dim == ISL_SURF_DIM_3D ?<br>
> > > > +            minify(mt->surf.logical_<wbr>level0_px.depth, level) :<br>
> > ><br>
> > > We should be looking up the physical depth shouldn't we? I haven't yet<br>
> > > looked into what width or height should be. I'm done for today, so I<br>
> > > will revisit this series tomorrow.<br>
> ><br>
> > Hmm, you are correct when comparing to the existing code - it uses the<br>
> > table<br>
> > which contains physical values. The given depth, however, comes from<br>
> > gl_texture_image::Depth which is core concept and deals with logical<br>
> > values.<br>
> ><br>
> > The confusing bit here is (which I should have written into the commit<br>
> > message) is that number of layers can't be altered, only levels. The<br>
> > existing<br>
> > code checks for depth because it uses that to catch missing levels. The<br>
> > table<br>
> > is initialised to zeroes and finding depth of zero is taken as meaning that<br>
> > the desired level doesn't exist.<br>
> ><br>
><br>
> It's also worth pointing out that there's no difference between logical and<br>
> physical with 3D since 3D can't be multisampled.  For 3D, I think logical<br>
> is the only one that makes sense.<br>
><br>
><br>
> > ><br>
> > > > +            mt->surf.logical_level0_px.<wbr>array_len;<br>
> > > > +<br>
> > > > +      return width == minify(mt->surf.logical_<wbr>level0_px.width,<br>
> > level) &&<br>
> > > > +             height == minify(mt->surf.logical_<wbr>level0_px.height,<br>
> > level) &&<br>
> > > > +             depth == level_depth &&<br>
> ><br>
> > I should probably assert this instead of failing - as I said I don't see<br>
> > how<br>
> > number of layers could change. Ian, Ken, Jason, how do you see this?<br>
> ><br>
><br>
> No, you want to return.  The whole point of this function is to determine<br>
> if this gl_texture_image matches the given level of the miptree.  This is<br>
> used, for instence, when someone does glTexImage2D and changes the size of<br>
> one LOD.  In that case, we can't upload to the miptree and we have to split<br>
> that LOD out into it's own.  Later on, assuming the texture is complete, we<br>
> recombine them into a single miptree.  It's aweful, but welcome to OpenGL.<br>
<br>
</div></div>Right, sorry, should have been clearer. I meant:<br>
<br>
         /* Only number of levels, width and height can be altered. */<br>
         assert(depth == level_depth);<br>
<span class=""><br>
         return width == minify(mt->surf.logical_<wbr>level0_px.width, level) &&<br>
</span>                height == minify(mt->surf.logical_<wbr>level0_px.height, level);<br><div class="HOEnZb"><div class="h5"></div></div></blockquote><div><br></div><div>What I said above still holds, you just now need glTexImage3D to trigger it. :-)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
><br>
> --Jason<br>
><br>
><br>
> > > > +             MAX2(image->NumSamples, 1) == mt->surf.samples;<br>
> > > > +   }<br>
> > > > +<br>
> > > >     int level_depth = mt->level[level].depth;<br>
> > > >     if (mt->num_samples > 1) {<br>
> > > >        switch (mt->msaa_layout) {<br>
> > > > --<br>
> > > > 2.11.0<br>
> > > ><br>
> > > > ______________________________<wbr>_________________<br>
> > > > mesa-dev mailing list<br>
> > > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> > > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
> ><br>
</div></div></blockquote></div><br></div></div>