[virglrenderer-devel] [PATCH] formats: include compressed formats in the copy-compatibility check

[Gurchetan Singh]

On Fri, Aug 3, 2018 at 2:02 PM Gert Wollny <gert.wollny at collabora.com> wrote:
>
> Add the combinations that include compressed format to the check to use
> glCopyImageSubData. This fixes a number tests of "arb_copy_image-formats"
> were compressed textures are the source and uncompressed textures the
> destination (no regressions).
>
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> ---
> Two remarks:
>
> * Given the way the piglit is written it will be impossible to fix the tests
>   where the destination texture is a compressed texture when run on a GLES
>   host, because this would require glGetCompressedTexImage, (unless of course
>   one would want to mirror each copy operation on the backing IOVs.
>   On a GL host the ways readbacks are done would need an overhaul to give
>   precedence to the  gettextimage code path.


> * All the dEQP copy_image tests are already passing,

Interesting.  Even with this patch applied, I'm seeing many copy_image
failures (see https://gitlab.freedesktop.org/virgl/virglrenderer/issues/1
for a list).  Do you not see these failures in your setup?  Those
results are from a Nvidia blob driver.

but with this piglit
>   all tests that include snorm images fail, because the negative values
>   are clamped to zero. I think this is an issue worth investigating, because it
>   might point to a more important bug.
>
>  src/vrend_formats.c | 75 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 72 insertions(+), 3 deletions(-)
>
> diff --git a/src/vrend_formats.c b/src/vrend_formats.c
> index 4647c43..7975823 100644
> --- a/src/vrend_formats.c
> +++ b/src/vrend_formats.c
> @@ -488,6 +488,67 @@ unsigned vrend_renderer_query_multisample_caps(unsigned max_samples, struct virg
>     return max_samples_confirmed;
>  }
>
> +/* returns: 1 = compatible, -1 = not compatible, 0 = undecided */
> +static int format_uncompressed_compressed_copy_compatible(enum pipe_format src,
> +                                                          enum pipe_format dst)
> +{
> +   switch (src) {
> +   case PIPE_FORMAT_R32G32B32A32_UINT:
> +   case PIPE_FORMAT_R32G32B32A32_SINT:
> +   case PIPE_FORMAT_R32G32B32A32_FLOAT:
> +   case PIPE_FORMAT_R32G32B32A32_SNORM:
> +   case PIPE_FORMAT_R32G32B32A32_UNORM:
> +      switch (dst) {
> +      case PIPE_FORMAT_DXT3_RGBA:
> +      case PIPE_FORMAT_DXT3_SRGBA:
> +      case PIPE_FORMAT_DXT5_RGBA:
> +      case PIPE_FORMAT_DXT5_SRGBA:
> +      case PIPE_FORMAT_RGTC2_UNORM:
> +      case PIPE_FORMAT_RGTC2_SNORM:
> +      case PIPE_FORMAT_BPTC_RGBA_UNORM:
> +      case PIPE_FORMAT_BPTC_SRGBA:
> +      case PIPE_FORMAT_BPTC_RGB_FLOAT:
> +      case PIPE_FORMAT_BPTC_RGB_UFLOAT:
> +         return 1;
> +      default:
> +         return -1;
> +      }
> +   case PIPE_FORMAT_R16G16B16A16_UINT:
> +   case PIPE_FORMAT_R16G16B16A16_SINT:
> +   case PIPE_FORMAT_R16G16B16A16_FLOAT:
> +   case PIPE_FORMAT_R16G16B16A16_SNORM:
> +   case PIPE_FORMAT_R16G16B16A16_UNORM:
> +   case PIPE_FORMAT_R32G32_UINT:
> +   case PIPE_FORMAT_R32G32_SINT:
> +   case PIPE_FORMAT_R32G32_FLOAT:
> +   case PIPE_FORMAT_R32G32_UNORM:
> +   case PIPE_FORMAT_R32G32_SNORM:
> +      switch (dst) {
> +      case PIPE_FORMAT_DXT1_RGBA:
> +      case PIPE_FORMAT_DXT1_SRGBA:
> +      case PIPE_FORMAT_DXT1_RGB:
> +      case PIPE_FORMAT_DXT1_SRGB:
> +      case PIPE_FORMAT_RGTC1_UNORM:
> +      case PIPE_FORMAT_RGTC1_SNORM:
> +         return 1;
> +      default:
> +         return -1;
> +      }
> +   default:
> +      return 0;
> +   }
> +}
> +
> +static boolean format_compressed_compressed_copy_compatible(enum pipe_format src, enum pipe_format dst)
> +{
> +   if ((src == PIPE_FORMAT_RGTC1_UNORM && dst == PIPE_FORMAT_RGTC1_SNORM) ||
> +       (src == PIPE_FORMAT_RGTC2_UNORM && dst == PIPE_FORMAT_RGTC2_SNORM) ||
> +       (src == PIPE_FORMAT_BPTC_RGBA_UNORM && dst == PIPE_FORMAT_BPTC_SRGBA) ||
> +       (src == PIPE_FORMAT_BPTC_RGB_FLOAT && dst == PIPE_FORMAT_BPTC_RGB_UFLOAT))
> +      return true;
> +   return false;
> +}
> +
>  boolean format_is_copy_compatible(enum pipe_format src, enum pipe_format dst)
>  {
>     int r;
> @@ -501,6 +562,14 @@ boolean format_is_copy_compatible(enum pipe_format src, enum pipe_format dst)
>        return util_is_format_compatible(src_desc, dst_desc);
>     }
>
> -   /* compressed formats will be supported later */
> -   return false;
> -}
> \ No newline at end of file
> +   /* compressed-uncompressed */
> +   r = format_uncompressed_compressed_copy_compatible(src, dst);
> +   if (r)
> +      return r > 0;
> +
> +   r = format_uncompressed_compressed_copy_compatible(dst, src);
> +   if (r)
> +      return r > 0;
> +
> +   return format_compressed_compressed_copy_compatible(dst, src);
> +}
> --
> 2.17.1
>
> _______________________________________________
> virglrenderer-devel mailing list
> virglrenderer-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel