From 995d55b34c2ccd56e03d9e246983775935ecc732 Mon Sep 17 00:00:00 2001
From: Lepton Wu <lepton@chromium.org>
Date: Tue, 19 May 2020 16:25:59 -0700
Subject: [PATCH] shader: Use integer type for ARM (MALI GPU)

Currently, even the input/output buffers are integer formats, we still
use float type for them. This is actually undefined behavior and MALI
GPU will do type conversion and cause unexpected results. To fix this,
we check the input buffer format for vertex shader and also color buffer
format for fragment shader, if they are integer types, just use integer
types in generated shaders. Since the old way works fine on other GPU,
only enable this fix for ARM MALI.

Signed-off-by: Lepton Wu <lepton@chromium.org>
---
 src/vrend_renderer.c | 43 +++++++++++++++++++++++++--
 src/vrend_shader.c   | 71 ++++++++++++++++++++++++++++++++++++++++----
 src/vrend_shader.h   |  5 ++++
 3 files changed, 111 insertions(+), 8 deletions(-)

diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
index 5bf2b3be..33042be4 100644
--- a/src/vrend_renderer.c
+++ b/src/vrend_renderer.c
@@ -283,6 +283,7 @@ struct global_renderer_state {
    bool finishing;
    bool use_gles;
    bool use_core_profile;
+   bool use_integer;
 
    bool features[feat_last];
 
@@ -482,6 +483,8 @@ struct vrend_vertex_element_array {
    unsigned count;
    struct vrend_vertex_element elements[PIPE_MAX_ATTRIBS];
    GLuint id;
+   uint32_t signed_int_bitmask;
+   uint32_t unsigned_int_bitmask;
 };
 
 struct vrend_constants {
@@ -646,6 +649,7 @@ struct vrend_context {
    bool in_error;
    bool ctx_switch_pending;
    bool pstip_inited;
+   bool drawing;
 
    GLuint pstipple_tex_id;
 
@@ -2657,8 +2661,15 @@ int vrend_create_vertex_elements_state(struct vrend_context *ctx,
       for (i = 0; i < num_elements; i++) {
          struct vrend_vertex_element *ve = &v->elements[i];
 
-         if (util_format_is_pure_integer(ve->base.src_format))
+         if (util_format_is_pure_integer(ve->base.src_format)) {
+            if (vrend_state.use_integer) {
+               if (util_format_is_pure_uint(ve->base.src_format))
+                  v->unsigned_int_bitmask |= (1 << i);
+               else
+                  v->signed_int_bitmask |= (1 << i);
+            }
             glVertexAttribIFormat(i, ve->nr_chan, ve->type, ve->base.src_offset);
+         }
          else
             glVertexAttribFormat(i, ve->nr_chan, ve->type, ve->norm, ve->base.src_offset);
          glVertexAttribBinding(i, ve->base.vertex_buffer_index);
@@ -3129,13 +3140,25 @@ static inline void vrend_fill_shader_key(struct vrend_context *ctx,
       int i;
       bool add_alpha_test = true;
       key->cbufs_are_a8_bitmask = 0;
+      // Only use integer info when drawing to avoid stale info.
+      if (vrend_state.use_integer && ctx->drawing) {
+         key->attrib_signed_int_bitmask = ctx->sub->ve->signed_int_bitmask;
+         key->attrib_unsigned_int_bitmask = ctx->sub->ve->unsigned_int_bitmask;
+      }
       for (i = 0; i < ctx->sub->nr_cbufs; i++) {
          if (!ctx->sub->surf[i])
             continue;
          if (vrend_format_is_emulated_alpha(ctx->sub->surf[i]->format))
             key->cbufs_are_a8_bitmask |= (1 << i);
-         if (util_format_is_pure_integer(ctx->sub->surf[i]->format))
+         if (util_format_is_pure_integer(ctx->sub->surf[i]->format)) {
             add_alpha_test = false;
+            if (vrend_state.use_integer) {
+               if (util_format_is_pure_uint(ctx->sub->surf[i]->format))
+                  key->cbufs_unsigned_int_bitmask |= (1 << i);
+               else
+                  key->cbufs_signed_int_bitmask |= (1 << i);
+            }
+         }
          key->surface_component_bits[i] = util_format_get_component_bits(ctx->sub->surf[i]->format, UTIL_FORMAT_COLORSPACE_RGB, 0);
       }
       if (add_alpha_test) {
@@ -4372,7 +4395,9 @@ int vrend_draw_vbo(struct vrend_context *ctx,
          return 0;
       }
 
+      ctx->drawing = true;
       vrend_shader_select(ctx, ctx->sub->shaders[PIPE_SHADER_VERTEX], &vs_dirty);
+      ctx->drawing = false;
 
       if (ctx->sub->shaders[PIPE_SHADER_TESS_CTRL] && ctx->sub->shaders[PIPE_SHADER_TESS_CTRL]->tokens)
          vrend_shader_select(ctx, ctx->sub->shaders[PIPE_SHADER_TESS_CTRL], &tcs_dirty);
@@ -5850,6 +5875,16 @@ vrend_renderer_get_pipe_callbacks(void)
    return &callbacks;
 }
 
+static bool use_integer() {
+   if (getenv("VIRGL_USE_INTEGER"))
+      return true;
+
+   const char * a = (const char *) glGetString(GL_VENDOR);
+   if (strcmp(a, "ARM") == 0)
+      return true;
+   return false;
+}
+
 int vrend_renderer_init(struct vrend_if_cbs *cbs, uint32_t flags)
 {
    bool gles;
@@ -5910,6 +5945,9 @@ int vrend_renderer_init(struct vrend_if_cbs *cbs, uint32_t flags)
       vrend_printf( "gl_version %d - compat profile\n", gl_ver);
    }
 
+   if (use_integer())
+      vrend_state.use_integer = true;
+
    init_features(gles ? 0 : gl_ver,
                  gles ? gl_ver : 0);
 
@@ -6142,6 +6180,7 @@ struct vrend_context *vrend_create_context(int id, uint32_t nlen, const char *de
    grctx->shader_cfg.has_gpu_shader5 = has_feature(feat_gpu_shader5);
    grctx->shader_cfg.has_es31_compat = has_feature(feat_gles31_compatibility);
    grctx->shader_cfg.has_conservative_depth = has_feature(feat_conservative_depth);
+   grctx->shader_cfg.use_integer = vrend_state.use_integer;
 
    vrend_renderer_create_sub_ctx(grctx, 0);
    vrend_renderer_set_sub_ctx(grctx, 0);
diff --git a/src/vrend_shader.c b/src/vrend_shader.c
index 03ad71bd..aae4f2fc 100644
--- a/src/vrend_shader.c
+++ b/src/vrend_shader.c
@@ -100,6 +100,8 @@ struct vrend_shader_io {
    bool glsl_gl_block;
    bool override_no_wm;
    bool is_int;
+   bool is_ivec;
+   bool is_uvec;
    bool fbfetch_used;
    char glsl_name[128];
    unsigned stream;
@@ -915,6 +917,12 @@ iter_declaration(struct tgsi_iterate_context *iter,
       }
       if (iter->processor.Processor == TGSI_PROCESSOR_VERTEX) {
          ctx->attrib_input_mask |= (1 << decl->Range.First);
+         if (ctx->key->attrib_signed_int_bitmask &
+             (1 << decl->Range.First))
+            ctx->inputs[i].is_ivec = true;
+         else if (ctx->key->attrib_unsigned_int_bitmask &
+                  (1 << decl->Range.First))
+            ctx->inputs[i].is_uvec = true;
       }
       ctx->inputs[i].name = decl->Semantic.Name;
       ctx->inputs[i].sid = decl->Semantic.Index;
@@ -1265,6 +1273,13 @@ iter_declaration(struct tgsi_iterate_context *iter,
          }
          break;
       case TGSI_SEMANTIC_COLOR:
+         if (iter->processor.Processor == TGSI_PROCESSOR_FRAGMENT) {
+            if (ctx->key->cbufs_signed_int_bitmask & (1 << ctx->outputs[i].sid))
+                ctx->outputs[i].is_ivec = true;
+            else if (ctx->key->cbufs_unsigned_int_bitmask & (1 << ctx->outputs[i].sid))
+                ctx->outputs[i].is_uvec = true;
+          }
+
          if (iter->processor.Processor == TGSI_PROCESSOR_VERTEX) {
             if (ctx->glsl_ver_required < 140) {
                ctx->outputs[i].glsl_no_index = true;
@@ -3572,6 +3587,16 @@ get_destination_info(struct dump_ctx *ctx,
                         dinfo->dtypeprefix = FLOAT_BITS_TO_INT;
                      dinfo->dstconv = INT;
                   }
+                  else if (ctx->outputs[j].is_uvec) {
+                     if (dinfo->dtypeprefix == TYPE_CONVERSION_NONE)
+                        dinfo->dtypeprefix = FLOAT_BITS_TO_UINT;
+                     dinfo->dstconv = dinfo->udstconv;
+                  }
+                  else if (ctx->outputs[j].is_ivec) {
+                     if (dinfo->dtypeprefix == TYPE_CONVERSION_NONE)
+                        dinfo->dtypeprefix = FLOAT_BITS_TO_INT;
+                     dinfo->dstconv = dinfo->idstconv;
+                  }
                   if (ctx->outputs[j].name == TGSI_SEMANTIC_PSIZE) {
                      dinfo->dstconv = FLOAT;
                      break;
@@ -3868,6 +3893,16 @@ get_source_info(struct dump_ctx *ctx,
                   if ((stype == TGSI_TYPE_UNSIGNED || stype == TGSI_TYPE_SIGNED) &&
                       ctx->inputs[j].is_int)
                      srcstypeprefix = TYPE_CONVERSION_NONE;
+                  else if (ctx->inputs[j].is_ivec || ctx->inputs[j].is_uvec) {
+                     if (stype == TGSI_TYPE_UNSIGNED)
+                        srcstypeprefix = UVEC4;
+                     else if (stype == TGSI_TYPE_SIGNED)
+                        srcstypeprefix = IVEC4;
+                     else if (ctx->inputs[j].is_ivec)
+                        srcstypeprefix = INT_BITS_TO_FLOAT;
+                     else // ctx->inputs[j].is_uvec
+                        srcstypeprefix = UINT_BITS_TO_FLOAT;
+                  }
 
                   if (inst->Instruction.Opcode == TGSI_OPCODE_INTERP_SAMPLE && i == 1) {
                      strbuf_fmt(src_buf, "floatBitsToInt(%s%s%s%s)", prefix, ctx->inputs[j].glsl_name, arrayname, swizzle);
@@ -5902,11 +5937,22 @@ emit_ios_generic(struct dump_ctx *ctx, enum io_type iot,  const char *prefix,
                  const struct vrend_shader_io *io, const char *inout,
                  const char *postfix)
 {
-   const char type[4][6] = {"float", " vec2", " vec3", " vec4"};
+   const char *itype[4] = {"  int", "ivec2", "ivec3", "ivec4"};
+   const char *utype[4] = {" uint", "uvec2", "uvec3", "uvec4"};
+   const char *ftype[4] = {"float", " vec2", " vec3", " vec4"};
    const char *t = " vec4";
+   const char **type = ftype;
 
    char layout[128] = "";
 
+   if (io->is_uvec) {
+      t = "uvec4";
+      type = utype;
+   } else if (io->is_ivec) {
+      t = "ivec4";
+      type = itype;
+   }
+
    if (io->layout_location > 0) {
       /* we need to define a layout here because interleaved arrays might be emited */
       if (io->swizzle_offset)
@@ -6072,7 +6118,13 @@ static void emit_ios_vs(struct dump_ctx *ctx)
          }
          if (ctx->inputs[i].first != ctx->inputs[i].last)
             snprintf(postfix, sizeof(postfix), "[%d]", ctx->inputs[i].last - ctx->inputs[i].first + 1);
-         emit_hdrf(ctx, "in vec4 %s%s;\n", ctx->inputs[i].glsl_name, postfix);
+         const char *vtype = "vec4";
+         if (ctx->inputs[i].is_ivec)
+             vtype = "ivec4";
+         else if (ctx->inputs[i].is_uvec)
+             vtype = "uvec4";
+         emit_hdrf(ctx, "in %s %s%s;\n",
+                   vtype, ctx->inputs[i].glsl_name, postfix);
       }
    }
 
@@ -6208,18 +6260,25 @@ static void emit_ios_fs(struct dump_ctx *ctx)
    }
 
    if (ctx->write_all_cbufs) {
+      const char* type = "vec4";
+      if (ctx->key->cbufs_unsigned_int_bitmask)
+         type = "uvec4";
+      else if (ctx->key->cbufs_signed_int_bitmask)
+         type = "ivec4";
+
       for (i = 0; i < (uint32_t)ctx->cfg->max_draw_buffers; i++) {
          if (ctx->cfg->use_gles) {
             if (ctx->key->fs_logicop_enabled)
-               emit_hdrf(ctx, "vec4 fsout_tmp_c%d;\n", i);
+               emit_hdrf(ctx, "%s fsout_tmp_c%d;\n", type, i);
 
             if (logiop_require_inout(ctx->key)) {
                const char *noncoherent = ctx->key->fs_logicop_emulate_coherent ? "" : ", noncoherent";
-               emit_hdrf(ctx, "layout (location=%d%s) inout highp vec4 fsout_c%d;\n", i, noncoherent, i);
+               emit_hdrf(ctx, "layout (location=%d%s) inout highp %s fsout_c%d;\n", i, noncoherent, type, i);
             } else
-               emit_hdrf(ctx, "layout (location=%d) out vec4 fsout_c%d;\n", i, i);
+               emit_hdrf(ctx, "layout (location=%d) out %s fsout_c%d;\n", i,
+			 type, i);
          } else
-            emit_hdrf(ctx, "out vec4 fsout_c%d;\n", i);
+            emit_hdrf(ctx, "out %s fsout_c%d;\n", type, i);
       }
    } else {
       for (i = 0; i < ctx->num_outputs; i++) {
diff --git a/src/vrend_shader.h b/src/vrend_shader.h
index 8c94e250..3a098280 100644
--- a/src/vrend_shader.h
+++ b/src/vrend_shader.h
@@ -118,6 +118,10 @@ struct vrend_shader_key {
    uint8_t prev_stage_num_cull_out;
    bool next_stage_pervertex_in;
    uint32_t cbufs_are_a8_bitmask;
+   uint32_t cbufs_signed_int_bitmask;
+   uint32_t cbufs_unsigned_int_bitmask;
+   uint32_t attrib_signed_int_bitmask;
+   uint32_t attrib_unsigned_int_bitmask;
    uint8_t num_indirect_generic_outputs;
    uint8_t num_indirect_patch_outputs;
    uint8_t num_indirect_generic_inputs;
@@ -137,6 +141,7 @@ struct vrend_shader_cfg {
    bool has_gpu_shader5;
    bool has_es31_compat;
    bool has_conservative_depth;
+   bool use_integer;
 };
 
 struct vrend_context;
-- 
2.26.2