llvmpipe: fix layer/vp input into fs when not written by prior stages

ARB_fragment_layer_viewport requires that if a fs reads layer or viewport
index but it wasn't output by gs (or vs with other extensions), then it reads
0. This never worked for llvmpipe, and is surprisingly non-trivial to fix.
The problem is the mechanism to handle non-existing outputs in draw is rather
crude, it will simply redirect them to whatever is at output 0, thus later
stages will just get garbage. So, rather than trying to fix this up (which
looks non-trivial), fix this up in llvmpipe setup by detecting this case there
and output a fixed zero directly.
While here, also optimize the hw vertex layout a bit - previously if the gs
outputted layer (or vp) and the fs read those inputs, we'd add them twice
to the vertex layout, which is unnecessary.
And do some minor cleanup, slots don't require that many bits, there was some
bogus (but harmless) float/int mixup for psize slot too, make the slots all
unsigned (we always put pos at pos zero thus everything else has to be positive
if it exists), and make sure they are properly initialized (layer and vp index
slot were not which looked fishy as they might not have got set back to zero
when changing from a gs which outputs them to one which does not).

This fixes the failures in piglit's arb_fragment_layer_viewport group
(3 each for layer and vp).

Reviewed-by: Jose Fonseca <jfonseca@vmware.com>

diff --git a/src/gallium/drivers/llvmpipe/lp_bld_interp.h b/src/gallium/drivers/llvmpipe/lp_bld_interp.h
index 9029d2a41808e1713650224edfc0aea07a619a48..0a52642e3957160b4361585db2445697c058c5dc 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_bld_interp.h
+++ b/src/gallium/drivers/llvmpipe/lp_bld_interp.h
@@ -63,7 +63,8 @@ enum lp_interp {
    LP_INTERP_LINEAR,
    LP_INTERP_PERSPECTIVE,
    LP_INTERP_POSITION,
-   LP_INTERP_FACING
+   LP_INTERP_FACING,
+   LP_INTERP_ZERO
 };
 
 struct lp_shader_input {

diff --git a/src/gallium/drivers/llvmpipe/lp_context.h b/src/gallium/drivers/llvmpipe/lp_context.h
index c9a5d6782447c267061978f1dcd0e616a3ae367a..9dcc102e7580f453a0dc2e2a0b1a9d9e9bc41dc0 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_context.h
+++ b/src/gallium/drivers/llvmpipe/lp_context.h
@@ -108,22 +108,28 @@ struct llvmpipe_context {
    struct vertex_info vertex_info;
    
    /** Which vertex shader output slot contains color */
-   int color_slot[2];
+   uint8_t color_slot[2];
 
    /** Which vertex shader output slot contains bcolor */
-   int bcolor_slot[2];
+   uint8_t bcolor_slot[2];
 
    /** Which vertex shader output slot contains point size */
-   int psize_slot;
+   uint8_t psize_slot;
 
    /** Which vertex shader output slot contains viewport index */
-   int viewport_index_slot;
+   uint8_t viewport_index_slot;
 
    /** Which geometry shader output slot contains layer */
-   int layer_slot;
+   uint8_t layer_slot;
 
    /** A fake frontface output for unfilled primitives */
-   int face_slot;
+   uint8_t face_slot;
+
+   /** Which output slot is used for the fake vp index info */
+   uint8_t fake_vpindex_slot;
+
+   /** Which output slot is used for the fake layer info */
+   uint8_t fake_layer_slot;
 
    /** Depth format and bias settings. */
    boolean floating_point_depth;

diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
index 1778b13f9ddc4d7bd9acd308fc03a3f32a311757..ddbb88eb1074e4b2b02b4da24cb53c3a9bd99da0 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_setup.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup.c
@@ -1207,7 +1207,7 @@ lp_setup_update_state( struct lp_setup_context *setup,
       /* Will probably need to move this somewhere else, just need  
        * to know about vertex shader point size attribute.
        */
-      setup->psize = lp->psize_slot;
+      setup->psize_slot = lp->psize_slot;
       setup->viewport_index_slot = lp->viewport_index_slot;
       setup->layer_slot = lp->layer_slot;
       setup->face_slot = lp->face_slot;

diff --git a/src/gallium/drivers/llvmpipe/lp_setup_context.h b/src/gallium/drivers/llvmpipe/lp_setup_context.h
index 2410e23840bafc7c12364c7a1a9bfc975541a1c8..4451284c30397785c15d207165335e29f7901cc9 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_setup_context.h
+++ b/src/gallium/drivers/llvmpipe/lp_setup_context.h
@@ -105,10 +105,10 @@ struct lp_setup_context
    float pixel_offset;
    float line_width;
    float point_size;
-   float psize;
-   unsigned viewport_index_slot;
-   unsigned layer_slot;
-   int face_slot;
+   uint8_t psize_slot;
+   uint8_t viewport_index_slot;
+   uint8_t layer_slot;
+   uint8_t face_slot;
 
    struct pipe_framebuffer_state fb;
    struct u_rect framebuffer;

diff --git a/src/gallium/drivers/llvmpipe/lp_setup_point.c b/src/gallium/drivers/llvmpipe/lp_setup_point.c
index 75544b524939e27125ddd37fdd7c18a529ce648e..14c389f7ce0c2dfbd1facb662fd8820a99982cad 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_setup_point.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup_point.c
@@ -328,7 +328,7 @@ try_setup_point( struct lp_setup_context *setup,
    struct llvmpipe_context *lp_context = (struct llvmpipe_context *)setup->pipe;
    /* x/y positions in fixed point */
    const struct lp_setup_variant_key *key = &setup->setup.variant->key;
-   const int sizeAttr = setup->psize;
+   const int sizeAttr = setup->psize_slot;
    const float size
       = (setup->point_size_per_vertex && sizeAttr > 0) ? v0[sizeAttr][0]
       : setup->point_size;

diff --git a/src/gallium/drivers/llvmpipe/lp_state_derived.c b/src/gallium/drivers/llvmpipe/lp_state_derived.c
index a25e8326188070218b0406ddd44f4cd6db4da300..f5bcfb2b511b08a0c8c93adada23587c5afef6c7 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_state_derived.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_derived.c
@@ -55,10 +55,14 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
 
    draw_prepare_shader_outputs(llvmpipe->draw);
 
-   llvmpipe->color_slot[0] = -1;
-   llvmpipe->color_slot[1] = -1;
-   llvmpipe->bcolor_slot[0] = -1;
-   llvmpipe->bcolor_slot[1] = -1;
+   llvmpipe->color_slot[0] = 0;
+   llvmpipe->color_slot[1] = 0;
+   llvmpipe->bcolor_slot[0] = 0;
+   llvmpipe->bcolor_slot[1] = 0;
+   llvmpipe->viewport_index_slot = 0;
+   llvmpipe->layer_slot = 0;
+   llvmpipe->face_slot = 0;
+   llvmpipe->psize_slot = 0;
 
    /*
     * Match FS inputs against VS outputs, emitting the necessary
@@ -86,7 +90,7 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
       if (lpfs->info.base.input_semantic_name[i] == TGSI_SEMANTIC_COLOR &&
           lpfs->info.base.input_semantic_index[i] < 2) {
          int idx = lpfs->info.base.input_semantic_index[i];
-         llvmpipe->color_slot[idx] = (int)vinfo->num_attribs;
+         llvmpipe->color_slot[idx] = vinfo->num_attribs;
       }
 
       if (lpfs->info.base.input_semantic_name[i] == TGSI_SEMANTIC_FACE) {
@@ -94,6 +98,30 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
          draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_CONSTANT, vs_index);
       } else if (lpfs->info.base.input_semantic_name[i] == TGSI_SEMANTIC_PRIMID) {
          draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_CONSTANT, vs_index);
+      /*
+       * For vp index and layer, if the fs requires them but the vs doesn't
+       * provide them, store the slot - we'll later replace the data directly
+       * with zero (as required by ARB_fragment_layer_viewport). This is
+       * because draw itself just redirects them to whatever was at output 0.
+       * We'll also store the real vpindex/layer slot for setup use.
+       */
+      } else if (lpfs->info.base.input_semantic_name[i] ==
+                 TGSI_SEMANTIC_VIEWPORT_INDEX) {
+         if (vs_index >= 0) {
+            llvmpipe->viewport_index_slot = vinfo->num_attribs;
+         }
+         else {
+            llvmpipe->fake_vpindex_slot = vinfo->num_attribs;
+         }
+         draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_CONSTANT, vs_index);
+      } else if (lpfs->info.base.input_semantic_name[i] == TGSI_SEMANTIC_LAYER) {
+         if (vs_index >= 0) {
+            llvmpipe->layer_slot = vinfo->num_attribs;
+         }
+         else {
+            llvmpipe->fake_layer_slot = vinfo->num_attribs;
+         }
+         draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_CONSTANT, vs_index);
       } else {
          /*
           * Emit the requested fs attribute for all but position.
@@ -101,6 +129,7 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
          draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_PERSPECTIVE, vs_index);
       }
    }
+
    /* Figure out if we need bcolor as well.
     */
    for (i = 0; i < 2; i++) {
@@ -108,12 +137,11 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
                                          TGSI_SEMANTIC_BCOLOR, i);
 
       if (vs_index >= 0) {
-         llvmpipe->bcolor_slot[i] = (int)vinfo->num_attribs;
+         llvmpipe->bcolor_slot[i] = vinfo->num_attribs;
          draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_PERSPECTIVE, vs_index);
       }
    }
 
-
    /* Figure out if we need pointsize as well.
     */
    vs_index = draw_find_shader_output(llvmpipe->draw,
@@ -124,26 +152,26 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
       draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_CONSTANT, vs_index);
    }
 
-   /* Figure out if we need viewport index */
-   vs_index = draw_find_shader_output(llvmpipe->draw,
-                                      TGSI_SEMANTIC_VIEWPORT_INDEX,
-                                      0);
-   if (vs_index >= 0) {
-      llvmpipe->viewport_index_slot = vinfo->num_attribs;
-      draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_CONSTANT, vs_index);
-   } else {
-      llvmpipe->viewport_index_slot = 0;
+   /* Figure out if we need viewport index (if it wasn't already in fs input) */
+   if (llvmpipe->viewport_index_slot == 0) {
+      vs_index = draw_find_shader_output(llvmpipe->draw,
+                                         TGSI_SEMANTIC_VIEWPORT_INDEX,
+                                         0);
+      if (vs_index >= 0) {
+         llvmpipe->viewport_index_slot = vinfo->num_attribs;
+         draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_CONSTANT, vs_index);
+      }
    }
 
-   /* Figure out if we need layer */
-   vs_index = draw_find_shader_output(llvmpipe->draw,
-                                      TGSI_SEMANTIC_LAYER,
-                                      0);
-   if (vs_index >= 0) {
-      llvmpipe->layer_slot = vinfo->num_attribs;
-      draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_CONSTANT, vs_index);
-   } else {
-      llvmpipe->layer_slot = 0;
+   /* Figure out if we need layer (if it wasn't already in fs input) */
+   if (llvmpipe->layer_slot == 0) {
+      vs_index = draw_find_shader_output(llvmpipe->draw,
+                                         TGSI_SEMANTIC_LAYER,
+                                         0);
+      if (vs_index >= 0) {
+         llvmpipe->layer_slot = vinfo->num_attribs;
+         draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_CONSTANT, vs_index);
+      }
    }
 
    draw_compute_vertex_size(vinfo);

diff --git a/src/gallium/drivers/llvmpipe/lp_state_setup.c b/src/gallium/drivers/llvmpipe/lp_state_setup.c
index 64215be91afb67c979e13b724fc52cbef4e4779e..d7ba5c8ad8eaa9b6e1eaaba7cf904be7d079d117 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_state_setup.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_setup.c
@@ -372,9 +372,9 @@ load_attribute(struct gallivm_state *gallivm,
    /* Potentially modify it according to twoside, etc:
     */
    if (key->twoside) {
-      if (vert_attr == key->color_slot && key->bcolor_slot >= 0)
+      if (vert_attr == key->color_slot && key->bcolor_slot > 0)
          lp_twoside(gallivm, args, key, key->bcolor_slot, attribv);
-      else if (vert_attr == key->spec_slot && key->bspec_slot >= 0)
+      else if (vert_attr == key->spec_slot && key->bspec_slot > 0)
          lp_twoside(gallivm, args, key, key->bspec_slot, attribv);
    }
 }
@@ -602,6 +602,13 @@ emit_tri_coef( struct gallivm_state *gallivm,
           */
          break;
 
+      case LP_INTERP_ZERO:
+         /*
+          * The information we get from the output is bogus, replace it
+          * with zero.
+          */
+         emit_constant_coef4(gallivm, args, slot+1, args->bld.zero);
+         break;
       case LP_INTERP_FACING:
          emit_facing_coef(gallivm, args, slot+1);
          break;
@@ -848,14 +855,10 @@ lp_make_setup_variant_key(struct llvmpipe_context *lp,
    key->size = Offset(struct lp_setup_variant_key,
                       inputs[key->num_inputs]);
 
-   key->color_slot  = lp->color_slot [0];
+   key->color_slot = lp->color_slot[0];
    key->bcolor_slot = lp->bcolor_slot[0];
-   key->spec_slot   = lp->color_slot [1];
-   key->bspec_slot  = lp->bcolor_slot[1];
-   assert(key->color_slot  == lp->color_slot [0]);
-   assert(key->bcolor_slot == lp->bcolor_slot[0]);
-   assert(key->spec_slot   == lp->color_slot [1]);
-   assert(key->bspec_slot  == lp->bcolor_slot[1]);
+   key->spec_slot = lp->color_slot[1];
+   key->bspec_slot = lp->bcolor_slot[1];
 
    /*
     * If depth is floating point, depth bias is calculated with respect
@@ -876,7 +879,13 @@ lp_make_setup_variant_key(struct llvmpipe_context *lp,
    key->pad = 0;
    memcpy(key->inputs, fs->inputs, key->num_inputs * sizeof key->inputs[0]);
    for (i = 0; i < key->num_inputs; i++) {
-      if (key->inputs[i].interp == LP_INTERP_COLOR) {
+      if (key->inputs[i].interp == LP_INTERP_CONSTANT) {
+         if (key->inputs[i].src_index == lp->fake_vpindex_slot ||
+             key->inputs[i].src_index == lp->fake_layer_slot) {
+            key->inputs[i].interp = LP_INTERP_ZERO;
+         }
+      }
+      else if (key->inputs[i].interp == LP_INTERP_COLOR) {
          if (lp->rasterizer->flatshade)
             key->inputs[i].interp = LP_INTERP_CONSTANT;
          else

diff --git a/src/gallium/drivers/llvmpipe/lp_state_setup.h b/src/gallium/drivers/llvmpipe/lp_state_setup.h
index 82af8350fb7090676d2827491f90a68a3146ab25..6cee6fe5eb57682a49b09015af22219f27a4d991 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_state_setup.h
+++ b/src/gallium/drivers/llvmpipe/lp_state_setup.h
@@ -17,11 +17,10 @@ struct lp_setup_variant_list_item
 struct lp_setup_variant_key {
    unsigned size:16;
    unsigned num_inputs:8;
-   int color_slot:8;
-
-   int bcolor_slot:8;
-   int spec_slot:8;
-   int bspec_slot:8;
+   unsigned color_slot:8;
+   unsigned bcolor_slot:8;
+   unsigned spec_slot:8;
+   unsigned bspec_slot:8;
    unsigned flatshade_first:1;
    unsigned pixel_center_half:1;
    unsigned twoside:1;