[Mesa-dev] [PATCH 1/2] anv: allow blue in alpha component in swizzle for render

[Nanley Chery]

On Wed, Feb 08, 2017 at 06:42:44PM +0100, Juan A. Suarez Romero wrote:
> On Wed, 2017-02-08 at 09:27 -0800, Nanley Chery wrote:
> > On Wed, Feb 08, 2017 at 01:31:54PM +0100, Juan A. Suarez Romero wrote:
> > > In pre-Broadwell devices, as B4G4R4A4 is not supported natively, we
> > > workaround it by using a format with a more complex swizzle, that uses
> > > blue in alpha component.
> > > 
> > > Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> > > ---
> > >  src/intel/vulkan/anv_private.h | 14 ++++++++++----
> > >  1 file changed, 10 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> > > index 51e85c7..e9bf13c 100644
> > > --- a/src/intel/vulkan/anv_private.h
> > > +++ b/src/intel/vulkan/anv_private.h
> > > @@ -1559,13 +1559,19 @@ static inline struct isl_swizzle
> > >  anv_swizzle_for_render(struct isl_swizzle swizzle)
> > >  {
> > >     /* Sometimes the swizzle will have alpha map to one.  We do this to fake
> > > -    * RGB as RGBA for texturing
> > > +    * RGB as RGBA for texturing.
> > > +    *
> > > +    * It can have also alpha to blue. This happens to workaround support for
> > > +    * B4G4R4A4 in gen < 8 devices, as this not supported natively.
> > >      */
> > >     assert(swizzle.a == ISL_CHANNEL_SELECT_ONE ||
> > > -          swizzle.a == ISL_CHANNEL_SELECT_ALPHA);
> > > +          swizzle.a == ISL_CHANNEL_SELECT_ALPHA ||
> > > +          swizzle.a == ISL_CHANNEL_SELECT_BLUE);
> > >  
> > > -   /* But it doesn't matter what we render to that channel */
> > > -   swizzle.a = ISL_CHANNEL_SELECT_ALPHA;
> > > +   /* But it doesn't matter what we render to that channel, except for the
> > > +    * B4G4R4A4 workaround */
> > 
> > I'm finding this commit message a tad confusing. Would you agree that 
> > replacing it with the following would be better?
> > 
> > For RGB formats that have alpha mapped to one, we must remap the channel
> > to alpha to satisfy the BDW+ surface state restriction.
> > 
> 
> Uhm... I don't think so. That is what the code was doing so far.
> 
> The commit just takes in account that alpha can be mapped to blue too.
> And in this case, we keep the blue.
> 

Oh, sorry, I meant code comment, not commit message. That would
definitely not work as a commit message. Thoughts?

> 
> > Aside from the comment, this patch looks good to me.
> > 
> > > +   if (swizzle.a == ISL_CHANNEL_SELECT_ONE)
> > > +      swizzle.a = ISL_CHANNEL_SELECT_ALPHA;
> > >  
> > >     return swizzle;
> > >  }
> > > -- 
> > > 2.9.3
> > > 
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > 
> >