On Fri, Feb 08, 2019 at 12:55:20PM +0200, Eleni Maria Stea wrote:
> Hi Nanley,
>
> On Thu, 7 Feb 2019 15:46:29 -0800
> Nanley Chery <nanleych...@gmail.com> wrote:
> >
> > > @@ -3825,10 +3849,20 @@ intel_miptree_unmap(struct brw_context *brw,
> > > DBG("%s: mt %p (%s) level %d slice %d\n", __func__,
> > > mt, _mesa_get_format_name(mt->format), level, slice);
> > >
> > > + level_w = minify(mt->surf.phys_level0_sa.width,
> > > + level - mt->first_level);
> > > + level_h = minify(mt->surf.phys_level0_sa.height,
> > > + level - mt->first_level);
> > > +
> > > if (map->unmap)
> > > map->unmap(brw, mt, map, level, slice);
> > >
> > > intel_miptree_release_map(mt, level, slice);
> > > +
> > > + if (intel_miptree_has_etc_shadow(brw, mt) &&
> > > mt->shadow_needs_update) {
> > > + intel_miptree_update_etc_shadow(brw, mt, level, slice,
> > > level_w,
> > > + level_h);
> > > + }
> >
> > With the next patch applied, the change in this function becomes
> > unnecessary. Is there any reason you're leaving it around?
>
> After a second thought, I believe that this change wasn't unnecessary.
> There is a problem if we remove it:
>
> When we generate mipmaps we need to update the shadow for each level.
> As the update is done per level during unmap, if we remove the call we
> end-up with the first level correctly updated but all the others empty.
>
> An example:
> git clone https://github.com/hikiko/test-compression.git
> make
> ./test compressed/full.tex
>
That's a nice test :)
> This test loads dumped compressed mipmap levels from the full.tex and
> displays them, if you run it with the per level update inside the unmap
> you will see all the mipmap levels. Without, you will see only the
> first, like here: https://imgur.com/a/VvS0CYC
>
> Do you have any suggestion on how I could bypass this problem?
>
Yes. The cause of this problem is that after
intel_miptree_update_etc_shadow_levels() calls
intel_miptree_update_etc_shadow() on the first level, the miptree is
marked as not needing an update. Therefore subsequent calls to
intel_miptree_update_etc_shadow() return early without updating other
levels. One way to fix this is to move the responsibility of marking the
miptree as updated from intel_miptree_update_etc_shadow() to it's
callers.
I also found another bug. I'll add a comment on the problematic patch.
> Thanks again,
> Eleni
>
>
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
