[Mesa-dev] [PATCH 6/8] llvmpipe: use ints not unsigned for slots
Jose Fonseca
jfonseca at vmware.com
Tue Dec 22 05:38:28 PST 2015
On 22/12/15 03:00, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> 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 <jfonseca at vmware.com>
More information about the mesa-dev
mailing list