[virglrenderer-devel] [PATCH v2 7/7] vrend: Use RGB1 swizzle for formats with "don't care" components

Alexandros Frantzis alexandros.frantzis at collabora.com
Thu May 10 15:17:44 UTC 2018


On Thu, May 10, 2018 at 11:24:47AM +0300, Alexandros Frantzis wrote:
> On Thu, May 10, 2018 at 06:19:34AM +1000, Dave Airlie wrote:
> > On 9 May 2018 at 18:52, Alexandros Frantzis
> > <alexandros.frantzis at collabora.com> wrote:
> > > On Wed, May 09, 2018 at 03:11:31PM +1000, Dave Airlie wrote:
> > >> On 24 April 2018 at 01:46, Alexandros Frantzis
> > >> <alexandros.frantzis at collabora.com> wrote:
> > >> > The swizzle ensures we read/write correct values from/to such formats
> > >> > during blitting.
> > >> >
> > >> > Fixes:
> > >> >
> > >> > dEQP-GLES3.functional.fbo.blit.default_framebuffer.srgb8_alpha8_linear_out_of_bounds_blit_to_default
> > >> > dEQP-GLES3.functional.fbo.blit.default_framebuffer.rgb8_linear_out_of_bounds_blit_to_default
> > >> >
> > >> > Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> > >> > ---
> > >> >  src/vrend_formats.c | 25 +++++++++++++------------
> > >> >  1 file changed, 13 insertions(+), 12 deletions(-)
> > >> >
> > >> > diff --git a/src/vrend_formats.c b/src/vrend_formats.c
> > >> > index 724551a..999dc41 100644
> > >> > --- a/src/vrend_formats.c
> > >> > +++ b/src/vrend_formats.c
> > >> > @@ -31,14 +31,15 @@
> > >> >  #define NO_SWIZZLE { SWIZZLE_INVALID, SWIZZLE_INVALID, SWIZZLE_INVALID, SWIZZLE_INVALID }
> > >> >  #define RRR1_SWIZZLE { PIPE_SWIZZLE_RED, PIPE_SWIZZLE_RED, PIPE_SWIZZLE_RED, PIPE_SWIZZLE_ONE }
> > >> >  #define RRRG_SWIZZLE { PIPE_SWIZZLE_RED, PIPE_SWIZZLE_RED, PIPE_SWIZZLE_RED, PIPE_SWIZZLE_GREEN }
> > >> > +#define RGB1_SWIZZLE { PIPE_SWIZZLE_RED, PIPE_SWIZZLE_GREEN, PIPE_SWIZZLE_BLUE, PIPE_SWIZZLE_ONE }
> > >> >
> > >> >  /* fill the format table */
> > >> >  static struct vrend_format_table base_rgba_formats[] =
> > >> >    {
> > >> > -    { VIRGL_FORMAT_B8G8R8X8_UNORM, GL_RGBA8, GL_BGRA, GL_UNSIGNED_BYTE, NO_SWIZZLE },
> > >> > +    { VIRGL_FORMAT_B8G8R8X8_UNORM, GL_RGBA8, GL_BGRA, GL_UNSIGNED_BYTE, RGB1_SWIZZLE },
> > >> >      { VIRGL_FORMAT_B8G8R8A8_UNORM, GL_RGBA8, GL_BGRA, GL_UNSIGNED_BYTE, NO_SWIZZLE },
> > >> >
> > >> > -    { VIRGL_FORMAT_R8G8B8X8_UNORM, GL_RGBA8, GL_RGBA, GL_UNSIGNED_BYTE, NO_SWIZZLE },
> > >> > +    { VIRGL_FORMAT_R8G8B8X8_UNORM, GL_RGBA8, GL_RGBA, GL_UNSIGNED_BYTE, RGB1_SWIZZLE },
> > >> >      { VIRGL_FORMAT_R8G8B8A8_UNORM, GL_RGBA8, GL_RGBA, GL_UNSIGNED_BYTE, NO_SWIZZLE },
> > >> >
> > >> >      { VIRGL_FORMAT_A8R8G8B8_UNORM, GL_RGBA8, GL_BGRA, GL_UNSIGNED_INT_8_8_8_8, NO_SWIZZLE },
> > >> > @@ -47,14 +48,14 @@ static struct vrend_format_table base_rgba_formats[] =
> > >> >      { VIRGL_FORMAT_A8B8G8R8_UNORM, GL_RGBA8, GL_ABGR_EXT, GL_UNSIGNED_BYTE, NO_SWIZZLE },
> > >> >
> > >> >      { VIRGL_FORMAT_B4G4R4A4_UNORM, GL_RGBA4, GL_BGRA, GL_UNSIGNED_SHORT_4_4_4_4_REV, NO_SWIZZLE },
> > >> > -    { VIRGL_FORMAT_B4G4R4X4_UNORM, GL_RGBA4, GL_BGRA, GL_UNSIGNED_SHORT_4_4_4_4_REV, NO_SWIZZLE },
> > >> > -    { VIRGL_FORMAT_B5G5R5X1_UNORM, GL_RGB5_A1, GL_BGRA, GL_UNSIGNED_SHORT_1_5_5_5_REV, NO_SWIZZLE },
> > >> > +    { VIRGL_FORMAT_B4G4R4X4_UNORM, GL_RGBA4, GL_BGRA, GL_UNSIGNED_SHORT_4_4_4_4_REV, RGB1_SWIZZLE },
> > >> > +    { VIRGL_FORMAT_B5G5R5X1_UNORM, GL_RGB5_A1, GL_BGRA, GL_UNSIGNED_SHORT_1_5_5_5_REV, RGB1_SWIZZLE },
> > >> >      { VIRGL_FORMAT_B5G5R5A1_UNORM, GL_RGB5_A1, GL_BGRA, GL_UNSIGNED_SHORT_1_5_5_5_REV, NO_SWIZZLE },
> > >> >
> > >> >      { VIRGL_FORMAT_B5G6R5_UNORM, GL_RGB565, GL_RGB, GL_UNSIGNED_SHORT_5_6_5, NO_SWIZZLE },
> > >> >      { VIRGL_FORMAT_B2G3R3_UNORM, GL_R3_G3_B2, GL_RGB, GL_UNSIGNED_BYTE_3_3_2, NO_SWIZZLE },
> > >> >
> > >> > -    { VIRGL_FORMAT_R16G16B16X16_UNORM, GL_RGBA16, GL_RGBA, GL_UNSIGNED_SHORT, NO_SWIZZLE },
> > >> > +    { VIRGL_FORMAT_R16G16B16X16_UNORM, GL_RGBA16, GL_RGBA, GL_UNSIGNED_SHORT, RGB1_SWIZZLE },
> > >> >
> > >> >      { VIRGL_FORMAT_R16G16B16A16_UNORM, GL_RGBA16, GL_RGBA, GL_UNSIGNED_SHORT, NO_SWIZZLE },
> > >> >    };
> > >> > @@ -184,13 +185,13 @@ static struct vrend_format_table snorm_formats[] = {
> > >> >    { VIRGL_FORMAT_R8G8_SNORM, GL_RG8_SNORM, GL_RG, GL_BYTE, NO_SWIZZLE },
> > >> >
> > >> >    { VIRGL_FORMAT_R8G8B8A8_SNORM, GL_RGBA8_SNORM, GL_RGBA, GL_BYTE, NO_SWIZZLE },
> > >> > -  { VIRGL_FORMAT_R8G8B8X8_SNORM, GL_RGBA8_SNORM, GL_RGBA, GL_BYTE, NO_SWIZZLE },
> > >> > +  { VIRGL_FORMAT_R8G8B8X8_SNORM, GL_RGBA8_SNORM, GL_RGBA, GL_BYTE, RGB1_SWIZZLE },
> > >> >
> > >> >    { VIRGL_FORMAT_R16_SNORM, GL_R16_SNORM, GL_RED, GL_SHORT, NO_SWIZZLE },
> > >> >    { VIRGL_FORMAT_R16G16_SNORM, GL_RG16_SNORM, GL_RG, GL_SHORT, NO_SWIZZLE },
> > >> >    { VIRGL_FORMAT_R16G16B16A16_SNORM, GL_RGBA16_SNORM, GL_RGBA, GL_SHORT, NO_SWIZZLE },
> > >> >
> > >> > -  { VIRGL_FORMAT_R16G16B16X16_SNORM, GL_RGBA16_SNORM, GL_RGBA, GL_SHORT, NO_SWIZZLE },
> > >> > +  { VIRGL_FORMAT_R16G16B16X16_SNORM, GL_RGBA16_SNORM, GL_RGBA, GL_SHORT, RGB1_SWIZZLE },
> > >> >  };
> > >> >
> > >> >  static struct vrend_format_table snorm_la_formats[] = {
> > >> > @@ -226,19 +227,19 @@ static struct vrend_format_table rgtc_formats[] = {
> > >> >
> > >> >  static struct vrend_format_table srgb_formats[] = {
> > >> >
> > >> > -  { VIRGL_FORMAT_B8G8R8X8_SRGB, GL_SRGB8_ALPHA8, GL_BGRA, GL_UNSIGNED_BYTE, NO_SWIZZLE },
> > >> > +  { VIRGL_FORMAT_B8G8R8X8_SRGB, GL_SRGB8_ALPHA8, GL_BGRA, GL_UNSIGNED_BYTE, RGB1_SWIZZLE },
> > >> >    { VIRGL_FORMAT_B8G8R8A8_SRGB, GL_SRGB8_ALPHA8, GL_BGRA, GL_UNSIGNED_BYTE, NO_SWIZZLE },
> > >> > -  { VIRGL_FORMAT_R8G8B8X8_SRGB, GL_SRGB8_ALPHA8, GL_RGBA, GL_UNSIGNED_BYTE, NO_SWIZZLE },
> > >> > +  { VIRGL_FORMAT_R8G8B8X8_SRGB, GL_SRGB8_ALPHA8, GL_RGBA, GL_UNSIGNED_BYTE, RGB1_SWIZZLE },
> > >> >
> > >> >    { VIRGL_FORMAT_L8_SRGB, GL_SR8_EXT, GL_RED, GL_UNSIGNED_BYTE, RRR1_SWIZZLE },
> > >> >    { VIRGL_FORMAT_L8A8_SRGB, GL_SRG8_EXT, GL_RG, GL_UNSIGNED_BYTE, RRRG_SWIZZLE },
> > >> >  };
> > >> >
> > >> >  static struct vrend_format_table bit10_formats[] = {
> > >> > -  { VIRGL_FORMAT_B10G10R10X2_UNORM, GL_RGB10_A2, GL_BGRA, GL_UNSIGNED_INT_2_10_10_10_REV, NO_SWIZZLE },
> > >> > +  { VIRGL_FORMAT_B10G10R10X2_UNORM, GL_RGB10_A2, GL_BGRA, GL_UNSIGNED_INT_2_10_10_10_REV, RGB1_SWIZZLE },
> > >> >    { VIRGL_FORMAT_B10G10R10A2_UNORM, GL_RGB10_A2, GL_BGRA, GL_UNSIGNED_INT_2_10_10_10_REV, NO_SWIZZLE },
> > >> >    { VIRGL_FORMAT_B10G10R10A2_UINT, GL_RGB10_A2UI, GL_BGRA_INTEGER, GL_UNSIGNED_INT_2_10_10_10_REV, NO_SWIZZLE },
> > >> > -  { VIRGL_FORMAT_R10G10B10X2_UNORM, GL_RGB10_A2, GL_RGBA, GL_UNSIGNED_INT_2_10_10_10_REV, NO_SWIZZLE },
> > >> > +  { VIRGL_FORMAT_R10G10B10X2_UNORM, GL_RGB10_A2, GL_RGBA, GL_UNSIGNED_INT_2_10_10_10_REV, RGB1_SWIZZLE },
> > >> >    { VIRGL_FORMAT_R10G10B10A2_UNORM, GL_RGB10_A2, GL_RGBA, GL_UNSIGNED_INT_2_10_10_10_REV, NO_SWIZZLE },
> > >> >  };
> > >> >
> > >> > @@ -258,7 +259,7 @@ static struct vrend_format_table bptc_formats[] = {
> > >> >  };
> > >> >
> > >> >  static struct vrend_format_table gles_bgra_formats[] = {
> > >> > -  { VIRGL_FORMAT_B8G8R8X8_UNORM, GL_BGRA_EXT, GL_BGRA_EXT, GL_UNSIGNED_BYTE, NO_SWIZZLE },
> > >> > +  { VIRGL_FORMAT_B8G8R8X8_UNORM, GL_BGRA_EXT, GL_BGRA_EXT, GL_UNSIGNED_BYTE, RGB1_SWIZZLE },
> > >> >    { VIRGL_FORMAT_B8G8R8A8_UNORM, GL_BGRA_EXT, GL_BGRA_EXT, GL_UNSIGNED_BYTE, NO_SWIZZLE },
> > >> >  };
> > >> >
> > >> > --
> > >> > 2.17.0
> > >> >
> > >> This appears to cause some regressions in piglit.
> > >>
> > >> fbo-blit-stretch for example.
> > >>
> > >> Dave.
> > >
> > > Hi Dave,
> > >
> > > thanks for the heads-up. I will investigate.
> > 
> > I'm seeing some other piglit regressions around this series previous patches,
> > 
> > I'll try and narrow it down a bit further.
> > 
> > Dave.
> 
> Hi Dave,
> 
> I haven't looked in depth yet, but my first guess would be "[PATCH v2
> 4/7] vrend: Properly handle out-of-bounds blits when blitting with GL"
> [1].
> 
> I introduced this patch to fix some dEQP regressions I found when the
> subsequent patches changed some formats to use swizzles and thus led to
> the use of the GL blit path for these formats. The patch implements a
> clip/stretch behavior similar to how glBlitFramebuffer works, but it's
> possible that there is some error there or some cases I am not handling
> properly and which are exercised by piglit.
> 
> Thanks,
> Alexandros
> 
> [1] https://lists.freedesktop.org/archives/virglrenderer-devel/2018-April/000387.html

Hi,

I have found the issue and I am working on a solution. Stay tuned.

Thanks,
Alexandros


More information about the virglrenderer-devel mailing list