st/mesa: fix SINT <-> UINT conversion during PBO upload / download

This fixes use cases like glReadPixels from an RGBA8I framebuffer into
a PBO with type GL_INT by clamping values appropriately when they fall
outside the range of the destination format.

Fixes parts of GL45-CTS.gtf32.GL3Tests.packed_pixels.packed_pixels_pbo.

Reviewed-by: Marek Olšák <[email protected]>
Reviewed-by: Edward O'Callaghan <[email protected]>

5 files changed, 89 insertions, 22 deletions

diff --git a/src/mesa/state_tracker/st_cb_readpixels.c b/src/mesa/state_tracker/st_cb_readpixels.c
index cab6abef323..ea91bb93b87 100644
--- a/src/mesa/state_tracker/st_cb_readpixels.c
+++ b/src/mesa/state_tracker/st_cb_readpixels.c
@@ -219,7 +219,7 @@ try_pbo_readpixels(struct st_context *st, struct st_renderbuffer *strb,
 
    /* Set up the fragment shader */
    {
-      void *fs = st_pbo_get_download_fs(st, view_target);
+      void *fs = st_pbo_get_download_fs(st, view_target, src_format, dst_format);
       if (!fs)
          goto fail;
 
diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
index ef59041d5ae..45f04487b38 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -1140,7 +1140,7 @@ try_pbo_upload_common(struct gl_context *ctx,
    bool success = false;
    void *fs;
 
-   fs = st_pbo_get_upload_fs(st);
+   fs = st_pbo_get_upload_fs(st, src_format, surface->format);
    if (!fs)
       return false;
 
diff --git a/src/mesa/state_tracker/st_context.h b/src/mesa/state_tracker/st_context.h
index 83d77fdf8f4..b3478ea26eb 100644
--- a/src/mesa/state_tracker/st_context.h
+++ b/src/mesa/state_tracker/st_context.h
@@ -224,8 +224,8 @@ struct st_context
       struct pipe_blend_state upload_blend;
       void *vs;
       void *gs;
-      void *upload_fs;
-      void *download_fs[PIPE_MAX_TEXTURE_TYPES];
+      void *upload_fs[3];
+      void *download_fs[3][PIPE_MAX_TEXTURE_TYPES];
       bool upload_enabled;
       bool download_enabled;
       bool rgba_only;
diff --git a/src/mesa/state_tracker/st_pbo.c b/src/mesa/state_tracker/st_pbo.c
index 1e215542bfb..a9ea6eae594 100644
--- a/src/mesa/state_tracker/st_pbo.c
+++ b/src/mesa/state_tracker/st_pbo.c
@@ -37,9 +37,19 @@
 #include "pipe/p_screen.h"
 #include "cso_cache/cso_context.h"
 #include "tgsi/tgsi_ureg.h"
+#include "util/u_format.h"
 #include "util/u_inlines.h"
 #include "util/u_upload_mgr.h"
 
+/* Conversion to apply in the fragment shader. */
+enum st_pbo_conversion {
+   ST_PBO_CONVERT_NONE = 0,
+   ST_PBO_CONVERT_UINT_TO_SINT,
+   ST_PBO_CONVERT_SINT_TO_UINT,
+
+   ST_NUM_PBO_CONVERSIONS
+};
+
 /* Final setup of buffer addressing information.
  *
  * buf_offset is in pixels.
@@ -376,8 +386,26 @@ st_pbo_create_gs(struct st_context *st)
    return ureg_create_shader_and_destroy(ureg, st->pipe);
 }
 
+static void
+build_conversion(struct ureg_program *ureg, const struct ureg_dst *temp,
+                 enum st_pbo_conversion conversion)
+{
+   switch (conversion) {
+   case ST_PBO_CONVERT_SINT_TO_UINT:
+      ureg_IMAX(ureg, *temp, ureg_src(*temp), ureg_imm1i(ureg, 0));
+      break;
+   case ST_PBO_CONVERT_UINT_TO_SINT:
+      ureg_UMIN(ureg, *temp, ureg_src(*temp), ureg_imm1u(ureg, (1u << 31) - 1));
+      break;
+   default:
+      /* no-op */
+      break;
+   }
+}
+
 static void *
-create_fs(struct st_context *st, bool download, enum pipe_texture_target target)
+create_fs(struct st_context *st, bool download, enum pipe_texture_target target,
+          enum st_pbo_conversion conversion)
 {
    struct pipe_context *pipe = st->pipe;
    struct pipe_screen *screen = pipe->screen;
@@ -495,6 +523,8 @@ create_fs(struct st_context *st, bool download, enum pipe_texture_target target)
       ureg_TXF(ureg, temp1, util_pipe_tex_to_tgsi_tex(target, 1),
                      ureg_src(temp1), sampler);
 
+      build_conversion(ureg, &temp1, conversion);
+
       /* store(out, temp0, temp1) */
       op[0] = ureg_src(temp0);
       op[1] = ureg_src(temp1);
@@ -504,7 +534,11 @@ create_fs(struct st_context *st, bool download, enum pipe_texture_target target)
       ureg_release_temporary(ureg, temp1);
    } else {
       /* out = txf(sampler, temp0.x) */
-      ureg_TXF(ureg, out, TGSI_TEXTURE_BUFFER, ureg_src(temp0), sampler);
+      ureg_TXF(ureg, temp0, TGSI_TEXTURE_BUFFER, ureg_src(temp0), sampler);
+
+      build_conversion(ureg, &temp0, conversion);
+
+      ureg_MOV(ureg, out, ureg_src(temp0));
    }
 
    ureg_release_temporary(ureg, temp0);
@@ -514,24 +548,49 @@ create_fs(struct st_context *st, bool download, enum pipe_texture_target target)
    return ureg_create_shader_and_destroy(ureg, pipe);
 }
 
+static enum st_pbo_conversion
+get_pbo_conversion(enum pipe_format src_format, enum pipe_format dst_format)
+{
+   if (util_format_is_pure_uint(src_format)) {
+      if (util_format_is_pure_sint(dst_format))
+         return ST_PBO_CONVERT_UINT_TO_SINT;
+   } else if (util_format_is_pure_sint(src_format)) {
+      if (util_format_is_pure_uint(dst_format))
+         return ST_PBO_CONVERT_SINT_TO_UINT;
+   }
+
+   return ST_PBO_CONVERT_NONE;
+}
+
 void *
-st_pbo_get_upload_fs(struct st_context *st)
+st_pbo_get_upload_fs(struct st_context *st,
+                     enum pipe_format src_format,
+                     enum pipe_format dst_format)
 {
-   if (!st->pbo.upload_fs)
-      st->pbo.upload_fs = create_fs(st, false, 0);
+   STATIC_ASSERT(ARRAY_SIZE(st->pbo.upload_fs) == ST_NUM_PBO_CONVERSIONS);
+
+   enum st_pbo_conversion conversion = get_pbo_conversion(src_format, dst_format);
 
-   return st->pbo.upload_fs;
+   if (!st->pbo.upload_fs[conversion])
+      st->pbo.upload_fs[conversion] = create_fs(st, false, 0, conversion);
+
+   return st->pbo.upload_fs[conversion];
 }
 
 void *
-st_pbo_get_download_fs(struct st_context *st, enum pipe_texture_target target)
+st_pbo_get_download_fs(struct st_context *st, enum pipe_texture_target target,
+                       enum pipe_format src_format,
+                       enum pipe_format dst_format)
 {
+   STATIC_ASSERT(ARRAY_SIZE(st->pbo.download_fs) == ST_NUM_PBO_CONVERSIONS);
    assert(target < PIPE_MAX_TEXTURE_TYPES);
 
-   if (!st->pbo.download_fs[target])
-      st->pbo.download_fs[target] = create_fs(st, true, target);
+   enum st_pbo_conversion conversion = get_pbo_conversion(src_format, dst_format);
+
+   if (!st->pbo.download_fs[conversion][target])
+      st->pbo.download_fs[conversion][target] = create_fs(st, true, target, conversion);
 
-   return st->pbo.download_fs[target];
+   return st->pbo.download_fs[conversion][target];
 }
 
 void
@@ -580,15 +639,19 @@ st_destroy_pbo_helpers(struct st_context *st)
 {
    unsigned i;
 
-   if (st->pbo.upload_fs) {
-      cso_delete_fragment_shader(st->cso_context, st->pbo.upload_fs);
-      st->pbo.upload_fs = NULL;
+   for (i = 0; i < ARRAY_SIZE(st->pbo.upload_fs); ++i) {
+      if (st->pbo.upload_fs[i]) {
+         cso_delete_fragment_shader(st->cso_context, st->pbo.upload_fs[i]);
+         st->pbo.upload_fs[i] = NULL;
+      }
    }
 
    for (i = 0; i < ARRAY_SIZE(st->pbo.download_fs); ++i) {
-      if (st->pbo.download_fs[i]) {
-         cso_delete_fragment_shader(st->cso_context, st->pbo.download_fs[i]);
-         st->pbo.download_fs[i] = NULL;
+      for (unsigned j = 0; j < ARRAY_SIZE(st->pbo.download_fs[0]); ++j) {
+         if (st->pbo.download_fs[i][j]) {
+            cso_delete_fragment_shader(st->cso_context, st->pbo.download_fs[i][j]);
+            st->pbo.download_fs[i][j] = NULL;
+         }
       }
    }
 
diff --git a/src/mesa/state_tracker/st_pbo.h b/src/mesa/state_tracker/st_pbo.h
index 6513093002d..54ae7768b4f 100644
--- a/src/mesa/state_tracker/st_pbo.h
+++ b/src/mesa/state_tracker/st_pbo.h
@@ -85,10 +85,14 @@ void *
 st_pbo_create_gs(struct st_context *st);
 
 void *
-st_pbo_get_upload_fs(struct st_context *st);
+st_pbo_get_upload_fs(struct st_context *st,
+                     enum pipe_format src_format,
+                     enum pipe_format dst_format);
 
 void *
-st_pbo_get_download_fs(struct st_context *st, enum pipe_texture_target target);
+st_pbo_get_download_fs(struct st_context *st, enum pipe_texture_target target,
+                       enum pipe_format src_format,
+                       enum pipe_format dst_format);
 
 extern void
 st_init_pbo_helpers(struct st_context *st);