On 22/12/15 03:00, [email protected] wrote:
From: Roland Scheidegger <[email protected]>

They can't actually be 0 (as position is there) but should avoid confusion.

This was supposed to have been done by af7ba989fb5a39925a0a1261ed281fe7f48a16cf
but I accidentally pushed an older version of the patch in the end...
Also prettify slightly. And make some notes about the confusing and useless
fs input "map".
---
  src/gallium/drivers/llvmpipe/lp_context.h       | 12 ++--
  src/gallium/drivers/llvmpipe/lp_setup_context.h |  8 +--
  src/gallium/drivers/llvmpipe/lp_state_derived.c | 73 +++++++++++++------------
  src/gallium/drivers/llvmpipe/lp_state_fs.c      | 35 ++++++------
  src/gallium/drivers/llvmpipe/lp_state_setup.c   |  4 +-
  src/gallium/drivers/llvmpipe/lp_state_setup.h   |  8 +--
  6 files changed, 73 insertions(+), 67 deletions(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_context.h 
b/src/gallium/drivers/llvmpipe/lp_context.h
index b1cb102..62d99bb 100644
--- a/src/gallium/drivers/llvmpipe/lp_context.h
+++ b/src/gallium/drivers/llvmpipe/lp_context.h
@@ -108,22 +108,22 @@ struct llvmpipe_context {
     struct vertex_info vertex_info;

     /** Which vertex shader output slot contains color */
-   uint8_t color_slot[2];
+   int8_t color_slot[2];

     /** Which vertex shader output slot contains bcolor */
-   uint8_t bcolor_slot[2];
+   int8_t bcolor_slot[2];

     /** Which vertex shader output slot contains point size */
-   uint8_t psize_slot;
+   int8_t psize_slot;

     /** Which vertex shader output slot contains viewport index */
-   uint8_t viewport_index_slot;
+   int8_t viewport_index_slot;

     /** Which geometry shader output slot contains layer */
-   uint8_t layer_slot;
+   int8_t layer_slot;

     /** A fake frontface output for unfilled primitives */
-   uint8_t face_slot;
+   int8_t face_slot;

     /** Depth format and bias settings. */
     boolean floating_point_depth;
diff --git a/src/gallium/drivers/llvmpipe/lp_setup_context.h 
b/src/gallium/drivers/llvmpipe/lp_setup_context.h
index 4451284..80acd74 100644
--- 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;
-   uint8_t psize_slot;
-   uint8_t viewport_index_slot;
-   uint8_t layer_slot;
-   uint8_t face_slot;
+   int8_t psize_slot;
+   int8_t viewport_index_slot;
+   int8_t layer_slot;
+   int8_t face_slot;

     struct pipe_framebuffer_state fb;
     struct u_rect framebuffer;
diff --git a/src/gallium/drivers/llvmpipe/lp_state_derived.c 
b/src/gallium/drivers/llvmpipe/lp_state_derived.c
index fbc2e18..34961cb 100644
--- a/src/gallium/drivers/llvmpipe/lp_state_derived.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_derived.c
@@ -48,21 +48,26 @@
  static void
  compute_vertex_info(struct llvmpipe_context *llvmpipe)
  {
-   const struct lp_fragment_shader *lpfs = llvmpipe->fs;
+   const struct tgsi_shader_info *fsInfo = &llvmpipe->fs->info.base;
     struct vertex_info *vinfo = &llvmpipe->vertex_info;
     int vs_index;
     uint i;

     draw_prepare_shader_outputs(llvmpipe->draw);

-   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;
+   /*
+    * Those can't actually be 0 (because pos is always at 0).
+    * But use ints anyway to avoid confusion (in vs outputs, they
+    * can very well be at pos 0).
+    */
+   llvmpipe->color_slot[0] = -1;
+   llvmpipe->color_slot[1] = -1;
+   llvmpipe->bcolor_slot[0] = -1;
+   llvmpipe->bcolor_slot[1] = -1;
+   llvmpipe->viewport_index_slot = -1;
+   llvmpipe->layer_slot = -1;
+   llvmpipe->face_slot = -1;
+   llvmpipe->psize_slot = -1;

     /*
      * Match FS inputs against VS outputs, emitting the necessary
@@ -73,30 +78,26 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
     vinfo->num_attribs = 0;

     vs_index = draw_find_shader_output(llvmpipe->draw,
-                                      TGSI_SEMANTIC_POSITION,
-                                      0);
+                                      TGSI_SEMANTIC_POSITION, 0);

     draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index);

-   for (i = 0; i < lpfs->info.base.num_inputs; i++) {
+   for (i = 0; i < fsInfo->num_inputs; i++) {
        /*
         * Search for each input in current vs output:
         */
-
        vs_index = draw_find_shader_output(llvmpipe->draw,
-                                         
lpfs->info.base.input_semantic_name[i],
-                                         
lpfs->info.base.input_semantic_index[i]);
+                                         fsInfo->input_semantic_name[i],
+                                         fsInfo->input_semantic_index[i]);

-      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] = vinfo->num_attribs;
+      if (fsInfo->input_semantic_name[i] == TGSI_SEMANTIC_COLOR &&
+          fsInfo->input_semantic_index[i] < 2) {
+         int idx = fsInfo->input_semantic_index[i];
+         llvmpipe->color_slot[idx] = (int)vinfo->num_attribs;
        }

-      if (lpfs->info.base.input_semantic_name[i] == TGSI_SEMANTIC_FACE) {
-         llvmpipe->face_slot = vinfo->num_attribs;
-         draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index);
-      } else if (lpfs->info.base.input_semantic_name[i] == 
TGSI_SEMANTIC_PRIMID) {
+      if (fsInfo->input_semantic_name[i] == TGSI_SEMANTIC_FACE) {
+         llvmpipe->face_slot = (int)vinfo->num_attribs;
           draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index);
        /*
         * For vp index and layer, if the fs requires them but the vs doesn't
@@ -104,16 +105,20 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
         * (This means in this case we'll also use those slots in setup, which
         * isn't necessary but they'll contain the correct (0) value.)
         */
-      } else if (lpfs->info.base.input_semantic_name[i] ==
+      } else if (fsInfo->input_semantic_name[i] ==
                   TGSI_SEMANTIC_VIEWPORT_INDEX) {
-         llvmpipe->viewport_index_slot = vinfo->num_attribs;
+         llvmpipe->viewport_index_slot = (int)vinfo->num_attribs;
           draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index);
-      } else if (lpfs->info.base.input_semantic_name[i] == 
TGSI_SEMANTIC_LAYER) {
-         llvmpipe->layer_slot = vinfo->num_attribs;
+      } else if (fsInfo->input_semantic_name[i] == TGSI_SEMANTIC_LAYER) {
+         llvmpipe->layer_slot = (int)vinfo->num_attribs;
           draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index);
        } else {
           /*
-          * Emit the requested fs attribute for all but position.
+          * Note that we'd actually want to skip position (as we won't use
+          * the attribute in the fs) but can't. The reason is that we don't
+          * actually have a input/output map for setup (even though it looks
+          * like we do...). Could adjust for this though even without a map
+          * (in llvmpipe_create_fs_state()).
            */
           draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index);
        }
@@ -126,7 +131,7 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
                                           TGSI_SEMANTIC_BCOLOR, i);

        if (vs_index >= 0) {
-         llvmpipe->bcolor_slot[i] = vinfo->num_attribs;
+         llvmpipe->bcolor_slot[i] = (int)vinfo->num_attribs;
           draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index);
        }
     }
@@ -137,28 +142,28 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
                                        TGSI_SEMANTIC_PSIZE, 0);

     if (vs_index >= 0) {
-      llvmpipe->psize_slot = vinfo->num_attribs;
+      llvmpipe->psize_slot = (int)vinfo->num_attribs;
        draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index);
     }

     /* Figure out if we need viewport index (if it wasn't already in fs input) 
*/
-   if (llvmpipe->viewport_index_slot == 0) {
+   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;
+         llvmpipe->viewport_index_slot =(int)vinfo->num_attribs;
           draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index);
        }
     }

     /* Figure out if we need layer (if it wasn't already in fs input) */
-   if (llvmpipe->layer_slot == 0) {
+   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;
+         llvmpipe->layer_slot = (int)vinfo->num_attribs;
           draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index);
        }
     }
diff --git a/src/gallium/drivers/llvmpipe/lp_state_fs.c 
b/src/gallium/drivers/llvmpipe/lp_state_fs.c
index 079083e..83ff976 100644
--- a/src/gallium/drivers/llvmpipe/lp_state_fs.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c
@@ -2695,34 +2695,35 @@ llvmpipe_create_fs_state(struct pipe_context *pipe,

        switch (shader->info.base.input_interpolate[i]) {
        case TGSI_INTERPOLATE_CONSTANT:
-        shader->inputs[i].interp = LP_INTERP_CONSTANT;
-        break;
+         shader->inputs[i].interp = LP_INTERP_CONSTANT;
+         break;
        case TGSI_INTERPOLATE_LINEAR:
-        shader->inputs[i].interp = LP_INTERP_LINEAR;
-        break;
+         shader->inputs[i].interp = LP_INTERP_LINEAR;
+         break;
        case TGSI_INTERPOLATE_PERSPECTIVE:
-        shader->inputs[i].interp = LP_INTERP_PERSPECTIVE;
-        break;
+         shader->inputs[i].interp = LP_INTERP_PERSPECTIVE;
+         break;
        case TGSI_INTERPOLATE_COLOR:
-        shader->inputs[i].interp = LP_INTERP_COLOR;
-        break;
+         shader->inputs[i].interp = LP_INTERP_COLOR;
+         break;
        default:
-        assert(0);
-        break;
+         assert(0);
+         break;
        }

        switch (shader->info.base.input_semantic_name[i]) {
        case TGSI_SEMANTIC_FACE:
-        shader->inputs[i].interp = LP_INTERP_FACING;
-        break;
+         shader->inputs[i].interp = LP_INTERP_FACING;
+         break;
        case TGSI_SEMANTIC_POSITION:
-        /* Position was already emitted above
-         */
-        shader->inputs[i].interp = LP_INTERP_POSITION;
-        shader->inputs[i].src_index = 0;
-        continue;
+         /* Position was already emitted above
+          */
+         shader->inputs[i].interp = LP_INTERP_POSITION;
+         shader->inputs[i].src_index = 0;
+         continue;
        }

+      /* XXX this is a completely pointless index map... */
        shader->inputs[i].src_index = i+1;
     }

diff --git a/src/gallium/drivers/llvmpipe/lp_state_setup.c 
b/src/gallium/drivers/llvmpipe/lp_state_setup.c
index 20e177f..6a4fbbb 100644
--- 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);
     }
  }
diff --git a/src/gallium/drivers/llvmpipe/lp_state_setup.h 
b/src/gallium/drivers/llvmpipe/lp_state_setup.h
index 6cee6fe..9ad2444 100644
--- a/src/gallium/drivers/llvmpipe/lp_state_setup.h
+++ b/src/gallium/drivers/llvmpipe/lp_state_setup.h
@@ -17,10 +17,10 @@ struct lp_setup_variant_list_item
  struct lp_setup_variant_key {
     unsigned size:16;
     unsigned num_inputs:8;
-   unsigned color_slot:8;
-   unsigned bcolor_slot:8;
-   unsigned spec_slot:8;
-   unsigned bspec_slot:8;
+   int color_slot:8;
+   int bcolor_slot:8;
+   int spec_slot:8;
+   int bspec_slot:8;
     unsigned flatshade_first:1;
     unsigned pixel_center_half:1;
     unsigned twoside:1;


Reviewed-by: Jose Fonseca <[email protected]>
_______________________________________________
mesa-dev mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to