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

[Iago Toral]

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

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;
> > > > >      }