[virglrenderer-devel] [PATCH] Add support for PIPE_FORMAT_A4B4G4R4_UNORM

[Gert Wollny]

Just some little nitpicks, apart from that the patch is 

Reviewed-By: Gert Wollny <gert.wollny at collabora.com>

Am Mittwoch, den 16.05.2018, 15:34 +0200 schrieb Tomeu Vizoso:
> and make sure that on GLES it it chosen for GL_RGBA4 over
> VIRGL_FORMAT_B4G4R4A4_UNORM, by removing support for the latter.
> 
> This is needed because on GLES3 GL_BGRA isn't a supported format to
> pass
> to glTexImage3D.
> 
> Fixes the test dEQP-
> GLES3.functional.texture.format.sized.3d.rgba4_pot
> on GLES hosts.
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso at collabora.com>
> Suggested-by: Jakob Bornecrantz <jakob at collabora.com>
> ---
>  src/gallium/auxiliary/util/u_format.csv |  1 +
>  src/virgl_hw.h                          |  2 +-
>  src/vrend_formats.c                     | 21 +++++++++++++++++++--
>  src/vrend_renderer.c                    |  2 +-
>  src/vrend_renderer.h                    |  2 +-
>  5 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/util/u_format.csv
> b/src/gallium/auxiliary/util/u_format.csv
> index d409693ab0fc..e985d8761652 100644
> --- a/src/gallium/auxiliary/util/u_format.csv
> +++ b/src/gallium/auxiliary/util/u_format.csv
> @@ -76,6 +76,7 @@ PIPE_FORMAT_B5G5R5X1_UNORM        , plain, 1, 1,
> un5 , un5 , un5 , x1  , zyx1, r
>  PIPE_FORMAT_B5G5R5A1_UNORM        , plain, 1, 1, un5 , un5 , un5 ,
> un1 , zyxw, rgb, un1 , un5 , un5 , un5 , yzwx
>  PIPE_FORMAT_B4G4R4A4_UNORM        , plain, 1, 1, un4 , un4 , un4 ,
> un4 , zyxw, rgb, un4 , un4 , un4 , un4 , yzwx
>  PIPE_FORMAT_B4G4R4X4_UNORM        , plain, 1, 1, un4 , un4 , un4 ,
> x4  , zyx1, rgb, x4  , un4 , un4 , un4 , yzw1
> +PIPE_FORMAT_A4B4G4R4_UNORM        , plain, 1, 1, un4 , un4 , un4 ,
> un4 , wzyx, rgb, un4 , un4 , un4 , un4 , xyzw
>  PIPE_FORMAT_B5G6R5_UNORM          , plain, 1, 1, un5 , un6 , un5
> ,     , zyx1, rgb, un5 , un6 , un5 ,     , xyz1
>  PIPE_FORMAT_R10G10B10A2_UNORM     , plain, 1, 1, un10, un10, un10,
> un2 , xyzw, rgb, un2 , un10, un10, un10, wzyx
>  PIPE_FORMAT_R10G10B10X2_UNORM     , plain, 1, 1, un10, un10, un10,
> x2,   xyz1, rgb, x2  , un10, un10, un10, wzy1
> diff --git a/src/virgl_hw.h b/src/virgl_hw.h
> index 1701bc93552d..8e173878796f 100644
> --- a/src/virgl_hw.h
> +++ b/src/virgl_hw.h
> @@ -205,7 +205,7 @@ enum virgl_formats {
>     VIRGL_FORMAT_BPTC_RGB_UFLOAT         = 258,
>  
>     VIRGL_FORMAT_R10G10B10X2_UNORM       = 308,
> -
> +   VIRGL_FORMAT_A4B4G4R4_UNORM          = 311,
>     VIRGL_FORMAT_MAX,
>  };
>  
> diff --git a/src/vrend_formats.c b/src/vrend_formats.c
> index cf32f6fd5e95..f03ffd8c438a 100644
> --- a/src/vrend_formats.c
> +++ b/src/vrend_formats.c
> @@ -47,8 +47,8 @@ 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, RGB1_SWIZZLE },
> +    { VIRGL_FORMAT_A4B4G4R4_UNORM, GL_RGBA4, GL_RGBA,
> GL_UNSIGNED_SHORT_4_4_4_4, NO_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 },
>  
> @@ -60,6 +60,11 @@ static struct vrend_format_table
> base_rgba_formats[] =
>      { VIRGL_FORMAT_R16G16B16A16_UNORM, GL_RGBA16, GL_RGBA,
> GL_UNSIGNED_SHORT, NO_SWIZZLE },
>    };
>  
> +static struct vrend_format_table gl_base_rgba_formats[] =
> +  {
> +    { VIRGL_FORMAT_B4G4R4A4_UNORM, GL_RGBA4, GL_BGRA,
> GL_UNSIGNED_SHORT_4_4_4_4_REV, NO_SWIZZLE },
> +  };
> +
>  static struct vrend_format_table base_depth_formats[] =
>    {
>      { VIRGL_FORMAT_Z16_UNORM, GL_DEPTH_COMPONENT16,
> GL_DEPTH_COMPONENT, GL_UNSIGNED_SHORT, NO_SWIZZLE },
> @@ -351,7 +356,7 @@ static void vrend_add_formats(struct
> vrend_format_table *table, int num_entries)
>  
>  #define add_formats(x) vrend_add_formats((x), ARRAY_SIZE((x)))
>  
> -void vrend_build_format_list(void)
> +static void vrend_build_format_list(void)
Maybe rename to vrend_build_format_list_common(void)?  

>  {
>    add_formats(base_rgba_formats);
>    add_formats(base_depth_formats);
> @@ -393,6 +398,18 @@ void vrend_build_format_list(void)
>    add_formats(bptc_formats);
>  }
>  
> +
> +void vrend_build_format_list_gl(void)
> +{
> +  vrend_build_format_list();
Instead of calling this here I'd move this call up into
vrend_renderer_init()

> +
> +  /* We don't want VIRGL_FORMAT_B4G4R4A4_UNORM to be supported on
> GLES because
> +   * it's not as well supported as VIRGL_FORMAT_A4B4G4R4_UNORM in
> some
> +   * operations.
> +   */
> +  add_formats(gl_base_rgba_formats);
> +}
> +
>  void vrend_build_format_list_gles(void)
>  {
>    vrend_build_format_list();
Same as above.  

> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index d20b411dc566..6fd8a631d222 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -4243,7 +4243,7 @@ int vrend_renderer_init(struct vrend_if_cbs
> *cbs, uint32_t flags)

i.e. add the call to vrend_build_format_list() here. 

>     if (vrend_state.use_gles) {
>        vrend_build_format_list_gles();
>     } else {
> -      vrend_build_format_list();
> +      vrend_build_format_list_gl();
>     }
>  
>     /* disable for format testing */
> diff --git a/src/vrend_renderer.h b/src/vrend_renderer.h
> index 0a139c758e61..09b4603e6835 100644
> --- a/src/vrend_renderer.h
> +++ b/src/vrend_renderer.h
> @@ -312,7 +312,7 @@ void vrend_renderer_fill_caps(uint32_t set,
> uint32_t version,
>  
>  GLint64 vrend_renderer_get_timestamp(void);
>  
> -void vrend_build_format_list(void);
> +void vrend_build_format_list_gl(void);
>  void vrend_build_format_list_gles(void);
>  
>  int vrend_renderer_resource_attach_iov(int res_handle, struct iovec
> *iov,