[Mesa-dev] [PATCH] anv/state: if enabled, use anisotropic filtering also with VK_FILTER_NEAREST

[Jason Ekstrand]

On Nov 24, 2016 11:19 PM, "Iago Toral" <itoral at igalia.com> wrote:
>
> On Thu, 2016-11-24 at 16:28 -0800, Jason Ekstrand wrote:
> >
> > On Nov 24, 2016 7:12 AM, "Iago Toral" <itoral at igalia.com> wrote:
> > >
> > > On Thu, 2016-11-24 at 14:33 +0100, Iago Toral wrote:
> > > > Hi Lionel,
> > > >
> > > > On Thu, 2016-11-24 at 13:08 +0000, Lionel Landwerlin wrote:
> > > > >
> > > > > Hi Iago,
> > > > >
> > > > > Looking at the history, before
> > > > > ed4fe3e9ba9018e68afe6fdd4f267218a537fdaa
> > > > > we seem to set min/mag filter to MAPFILTER_ANISOTROPIC if
> > > > > maxAnisotropy
> > > > >  > 1. It seems your patch makes sense in using
> > > > > MAPFILTER_ANISOTROPIC
> > > > > in
> > > > > the NEAREST case, but I wonder whether we should also check
> > for
> > > > > maxAnisotropy > 1.
> > > > Right, good catch, although I think that if we do that it should
> > be a
> > > > separate change since we are not currently checking that for the
> > > > linear
> > > > filter either.
> > In GL, there is no explicit enable.  It's just assumed that it's
> > "always on" and the anisotropy value being 1.0 or > 1.0 enables and
> > disables it.  Vulkan has an explicit bit so we should use that.
> > > > It seems that we do check for this in OpenGL so I think we
> > probably
> > > > should do that here as well unless Jason dropped it for Vulkan on
> > > > purpose for some reason in that commit.
> > > >
> > > > I'll send a separate patch for this after I confirm that it does
> > not
> > > > alter the results for the tests in CTS if we add that check.
> > >
> > > Actually, thinking about this a bit more, I don't think we need
> >  That
> > > commit is about honoring SamplerCreateInfo.anisotropyEnable to
> > decide
> > > whether to activate anisotropic filtering, so if that is true we
> > want
> > > to use that. Notice that since that commit we clamp the anisotropy
> > > ratio to ensure it is in a valid range. If we pass a maxAnisotropy
> > > value < 1, it will clamp it to the minimum value we can work with,
> > so I
> > > think Jason did that change on purpose.
> > You overestimate the amount of thought I put into anisotropic
> > filtering.  :-)
> > I think this patch is probably correct.  At the very least, I don't
> > think it's wrong.  I'm not sure that it makes sense to use
> > anisotropic filtering with NEAREST.  I'm not even quite sure what
> > that would mean.  The spec certainly doesn't say.  Anyway, I think
> > I'm fine with this but a bit more digging to make sure we're actually
> > doing it right might be in order.
>
> If you're using anisotropic filtering to enhance texture quality it
> would seem odd that someone would use it with a nearest filter, but I
> have not seen any indication that this combination should not be
> allowed. After all, anisotropic filtering is conceptually orthogonal to
> nearest/linear filtering.
>
> I did not find any references in the Vulkan docs, but at least in
> OpenGL this is explained in EXT_texture_filter_anisotropic  and they
> actually mention the combination of nearest+anisotropic filtering
> explicitly:
>
> https://www.opengl.org/registry/specs/EXT/texture_filter_anisotropic.tx
> t
>
>    "A texture's maximum degree of anisotropy is specified independent
>     from the texture's minification and magnification filter (as
>     opposed to being supported as an entirely new filtering mode).
>     Implementations are free to use the specified minification and
>     magnification filter to select a particular anisotropic texture
>     filtering scheme.  For example, a NEAREST filter with a maximum
>     degree of anisotropy of two could be treated as a 2-tap filter that
>     accounts for the direction of anisotropy.  Implementations are also
>     permitted to ignore the minification or magnification filter and
>     implement the highest quality of anisotropic filtering possible."

I think the key there is "free to use".  In other words, we are also free
to ignore the minification and magnification filters when anisotropic
filtering is enabled which is what this patch does.  With anisotropic
filtering enabled, they just become implementation-specific knobs to adjust
the algorithm.  With that understanding, this patch is 100% correct.

> So basically, some drivers/hardware may have nearest + anisotropy work
> slightly different than linear + anisotropy. Not sure when that is
> helpful, but it seems that it should be possible to do it.
>
> Iago
>
> > Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
> > > Sounds reasonable?
> > >
> > > > Iago
> > > >
> > > > >
> > > > > On 24/11/16 11:30, Iago Toral Quiroga wrote:
> > > > > >
> > > > > >
> > > > > > Fixes multiple Vulkan CTS tests that combine anisotropy and
> > > > > > VK_FILTER_NEAREST
> > > > > > in dEQP-VK.texture.filtering_anisotropy.*
> > > > > > ---
> > > > > >   src/intel/vulkan/genX_state.c | 2 +-
> > > > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/src/intel/vulkan/genX_state.c
> > > > > > b/src/intel/vulkan/genX_state.c
> > > > > > index 4122395..0f621f9 100644
> > > > > > --- a/src/intel/vulkan/genX_state.c
> > > > > > +++ b/src/intel/vulkan/genX_state.c
> > > > > > @@ -101,7 +101,7 @@ vk_to_gen_tex_filter(VkFilter filter,
> > bool
> > > > > > anisotropyEnable)
> > > > > >      default:
> > > > > >         assert(!"Invalid filter");
> > > > > >      case VK_FILTER_NEAREST:
> > > > > > -      return MAPFILTER_NEAREST;
> > > > > > +      return anisotropyEnable ? MAPFILTER_ANISOTROPIC :
> > > > > > MAPFILTER_NEAREST;
> > > > > >      case VK_FILTER_LINEAR:
> > > > > >         return anisotropyEnable ? MAPFILTER_ANISOTROPIC :
> > > > > > MAPFILTER_LINEAR;
> > > > > >      }
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161125/3dab7014/attachment-0001.html>