Mesa (master): etnaviv: Fix point sprite Z,W coordinate replacement

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Wed Mar 10 11:53:30 UTC 2021


Module: Mesa
Branch: master
Commit: f7dc0520d9045b424748855be103ee5affc53235
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=f7dc0520d9045b424748855be103ee5affc53235

Author: Marek Vasut <marex at denx.de>
Date:   Thu Jan 21 02:12:31 2021 +0100

etnaviv: Fix point sprite Z,W coordinate replacement

Mesa fixed pipeline texture loading on programmable pipeline hardware emits
a generic fragment shader program which contains gl_TexCoord.xyzw as a vec4
and then expects to configure the varying assignments to the shader in the
pipeline command stream, to select what is wired to the XYZW fragment shader
inputs.

This gl_TexCoord.xyzw is turned into texture load with projection (TGSI TXP
opcode, similar for NIR). Texture load with projection does not exist in the
Vivante GPU as a dedicated opcode and is emulated. The shader program first
divides texture coordinates XYZ by projector W and then applies regular TEX
opcode to load the texture (i.e. TEX(gl_TexCoord.xyzw/gl_TexCoord.wwww)).

For point sprites, XY are the point coordinates from VS, Z=0 and W=1, always.
The Vivante GPU can only configure varying to be either of -- point coord X,
point coord Y, used, unused -- which covers XYZ, but not W. Z is fine because
unused means 0.

W used to be 0 too before this patch and that led to division by 0 in shader.
The only known way to solve this is to set Z=0, W=1 in the shader program
itself if the point sprites are enabled. This means we have to generate a
special shader variant which does extra SET to set the W=1 in case the point
sprites are enabled.

In case of TGSI, emitting the SET.TRUE opcode permits setting W=1 without
allocating additional constants. With NIR, use nir_lower_texcoord_replace()
to lower TEXn to PNTC, which sets Z=0, W=1, and let NIR optimize the shader.
Note that nir_lower_texcoord_replace() must be called before input linking
is set up, as it might add new FS input.

Also note that it should be possible to simply drop PIPE_CAP_POINT_SPRITE
in the long run, ST would then apply the same optimization pass, but that
option is so far misbehaving. And for etnaviv TGSI this is not applicable
yet.

This fixes neverball point sprites (exit cylinder stars) and eglretrace of
gl4es pointsprite test:
https://github.com/ptitSeb/gl4es/blob/master/traces/pointsprite.tgz

Signed-off-by: Marek Vasut <marex at denx.de>
Reviewed-by: Christian Gmeiner <christian.gmeiner at gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8618>

---

 src/gallium/drivers/etnaviv/etnaviv_compiler_nir.c | 18 +++++++++++++++++-
 .../drivers/etnaviv/etnaviv_compiler_tgsi.c        | 22 +++++++++++++++++++---
 src/gallium/drivers/etnaviv/etnaviv_context.c      |  2 ++
 src/gallium/drivers/etnaviv/etnaviv_shader.h       |  4 ++++
 4 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.c b/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.c
index 684b6093a57..81f4c3696d5 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.c
@@ -1073,6 +1073,15 @@ etna_compile_shader_nir(struct etna_shader_variant *v)
    v->ps_color_out_reg = 0; /* 0 for shader that doesn't write fragcolor.. */
    v->ps_depth_out_reg = -1;
 
+   /*
+    * Lower glTexCoord, fixes e.g. neverball point sprite (exit cylinder stars)
+    * and gl4es pointsprite.trace apitrace
+    */
+   if (s->info.stage == MESA_SHADER_FRAGMENT && v->key.sprite_coord_enable) {
+      NIR_PASS_V(s, nir_lower_texcoord_replace, v->key.sprite_coord_enable,
+                 false, v->key.sprite_coord_yinvert);
+   }
+
    /* setup input linking */
    struct etna_shader_io_file *sf = &v->infile;
    if (s->info.stage == MESA_SHADER_VERTEX) {
@@ -1241,7 +1250,7 @@ etna_link_shader_nir(struct etna_shader_link_info *info,
       varying->use[2] = VARYING_COMPONENT_USE_UNUSED;
       varying->use[3] = VARYING_COMPONENT_USE_UNUSED;
 
-      /* point coord is an input to the PS without matching VS output,
+      /* point/tex coord is an input to the PS without matching VS output,
        * so it gets a varying slot without being assigned a VS register.
        */
       if (fsio->slot == VARYING_SLOT_PNTC) {
@@ -1249,6 +1258,13 @@ etna_link_shader_nir(struct etna_shader_link_info *info,
          varying->use[1] = VARYING_COMPONENT_USE_POINTCOORD_Y;
 
          info->pcoord_varying_comp_ofs = comp_ofs;
+      } else if (util_varying_is_point_coord(fsio->slot, fs->key.sprite_coord_enable)) {
+         /*
+	  * Do nothing, TexCoord is lowered to PointCoord above
+	  * and the TexCoord here is just a remnant. This needs
+	  * to be removed with some nir_remove_dead_variables(),
+	  * but that one removes all FS inputs ... why?
+	  */
       } else {
          if (vsio == NULL) { /* not found -- link error */
             BUG("Semantic value not found in vertex shader outputs\n");
diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler_tgsi.c b/src/gallium/drivers/etnaviv/etnaviv_compiler_tgsi.c
index 743ee0b5ace..7f7e0b6704c 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_compiler_tgsi.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_compiler_tgsi.c
@@ -1882,12 +1882,27 @@ etna_compile_pass_generate_code(struct etna_compile *c)
 
          for (int i = 0; i < tgsi->num_src && i < ETNA_NUM_SRC; i++) {
             const struct tgsi_full_src_register *reg = &inst->Src[i];
-            const struct etna_native_reg *n = &etna_get_src_reg(c, reg->Register)->native;
+            const struct etna_reg_desc *srcreg = etna_get_src_reg(c, reg->Register);
+            const struct etna_native_reg *n = &srcreg->native;
 
             if (!n->valid || n->is_tex)
                continue;
 
             src[i] = etna_create_src(reg, n);
+
+            /*
+	     * Replace W=1.0 for point sprite coordinates, since hardware
+	     * can only replace X,Y and leaves Z,W=0,0 instead of Z,W=0,1
+	     */
+            if (srcreg && srcreg->has_semantic &&
+                srcreg->semantic.Name == TGSI_SEMANTIC_TEXCOORD &&
+                (c->key->sprite_coord_enable & BITFIELD_BIT(srcreg->semantic.Index))) {
+               emit_inst(c, &(struct etna_inst) {
+                  .opcode = INST_OPCODE_SET,
+                  .cond = INST_CONDITION_TRUE,
+                  .dst = etna_native_to_dst(srcreg->native, INST_COMPS_W),
+               });
+            }
          }
 
          const unsigned opc = inst->Instruction.Opcode;
@@ -2084,6 +2099,7 @@ permute_ps_inputs(struct etna_compile *c)
     * gl_FragCoord   VARYING_SLOT_POS   TGSI_SEMANTIC_POSITION
     * gl_FrontFacing VARYING_SLOT_FACE  TGSI_SEMANTIC_FACE
     * gl_PointCoord  VARYING_SLOT_PNTC  TGSI_SEMANTIC_PCOORD
+    * gl_TexCoord    VARYING_SLOT_TEX   TGSI_SEMANTIC_TEXCOORD
     */
    uint native_idx = 1;
 
@@ -2551,10 +2567,10 @@ etna_link_shader(struct etna_shader_link_info *info,
       varying->use[2] = VARYING_COMPONENT_USE_UNUSED;
       varying->use[3] = VARYING_COMPONENT_USE_UNUSED;
 
-      /* point coord is an input to the PS without matching VS output,
+      /* point/tex coord is an input to the PS without matching VS output,
        * so it gets a varying slot without being assigned a VS register.
        */
-      if (fsio->slot == VARYING_SLOT_PNTC) {
+      if (util_varying_is_point_coord(fsio->slot, fs->key.sprite_coord_enable)) {
          varying->use[0] = VARYING_COMPONENT_USE_POINTCOORD_X;
          varying->use[1] = VARYING_COMPONENT_USE_POINTCOORD_Y;
 
diff --git a/src/gallium/drivers/etnaviv/etnaviv_context.c b/src/gallium/drivers/etnaviv/etnaviv_context.c
index 6589f033d92..b1a223de19c 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_context.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_context.c
@@ -308,6 +308,8 @@ etna_draw_vbo(struct pipe_context *pctx, const struct pipe_draw_info *info,
 
    struct etna_shader_key key = {
       .front_ccw = ctx->rasterizer->front_ccw,
+      .sprite_coord_enable = ctx->rasterizer->sprite_coord_enable,
+      .sprite_coord_yinvert = !!ctx->rasterizer->sprite_coord_mode,
    };
 
    if (pfb->cbufs[0])
diff --git a/src/gallium/drivers/etnaviv/etnaviv_shader.h b/src/gallium/drivers/etnaviv/etnaviv_shader.h
index ccba5c858db..3d244c11214 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_shader.h
+++ b/src/gallium/drivers/etnaviv/etnaviv_shader.h
@@ -27,6 +27,7 @@
 #ifndef H_ETNAVIV_SHADER
 #define H_ETNAVIV_SHADER
 
+#include "mesa/main/config.h"
 #include "pipe/p_state.h"
 #include "util/disk_cache.h"
 
@@ -46,6 +47,9 @@ struct etna_shader_key
          unsigned frag_rb_swap : 1;
          /* do we need to invert front facing value? */
          unsigned front_ccw : 1;
+         /* do we need to replace glTexCoord.xy ? */
+         unsigned sprite_coord_enable : MAX_TEXTURE_COORD_UNITS;
+         unsigned sprite_coord_yinvert : 1;
       };
       uint32_t global;
    };



More information about the mesa-commit mailing list