From 8d4039ecdb167771d4b085f70b9666438be1c6ad Mon Sep 17 00:00:00 2001 From: Roland Scheidegger Date: Tue, 22 Dec 2015 03:42:33 +0100 Subject: [PATCH] softpipe: tell draw about the vertex layout we want This makes it more similar to llvmpipe. It also allows us to let draw emit code handle things like getting zeros for non-existing vs outputs automatically. There probably isn't really any overhead either way, there isn't really any "simply copy everything" code in the emit path it would copy each attrib individually just the same. Likewise, we still do another mapping step in softpipe as the layout may still not match exactly (same as in llvmpipe, should probably nuke the pointless mapping in both drivers). This fixes the piglit arb_fragment_layer_viewport no_gs/no_write tests. Reviewed-by: Brian Paul Reviewed-by: Edward O'Callaghan --- src/gallium/drivers/softpipe/sp_context.h | 8 +- src/gallium/drivers/softpipe/sp_prim_vbuf.c | 4 +- src/gallium/drivers/softpipe/sp_setup.h | 2 +- .../drivers/softpipe/sp_state_derived.c | 149 ++++++++++++------ 4 files changed, 105 insertions(+), 58 deletions(-) diff --git a/src/gallium/drivers/softpipe/sp_context.h b/src/gallium/drivers/softpipe/sp_context.h index 188cdeaf76f..d5c4aaae638 100644 --- a/src/gallium/drivers/softpipe/sp_context.h +++ b/src/gallium/drivers/softpipe/sp_context.h @@ -119,16 +119,16 @@ struct softpipe_context { /** Vertex format */ struct sp_setup_info setup_info; - struct vertex_info vertex_info_vbuf; + struct vertex_info vertex_info; /** Which vertex shader output slot contains point size */ - int psize_slot; + int8_t psize_slot; /** Which vertex shader output slot contains viewport index */ - int viewport_index_slot; + int8_t viewport_index_slot; /** Which vertex shader output slot contains layer */ - int layer_slot; + int8_t layer_slot; /** The reduced version of the primitive supplied by the state tracker */ unsigned reduced_api_prim; diff --git a/src/gallium/drivers/softpipe/sp_prim_vbuf.c b/src/gallium/drivers/softpipe/sp_prim_vbuf.c index f8a3eacdb37..95d1ac1514f 100644 --- a/src/gallium/drivers/softpipe/sp_prim_vbuf.c +++ b/src/gallium/drivers/softpipe/sp_prim_vbuf.c @@ -161,7 +161,7 @@ sp_vbuf_draw_elements(struct vbuf_render *vbr, const ushort *indices, uint nr) { struct softpipe_vbuf_render *cvbr = softpipe_vbuf_render(vbr); struct softpipe_context *softpipe = cvbr->softpipe; - const unsigned stride = softpipe->vertex_info_vbuf.size * sizeof(float); + const unsigned stride = softpipe->vertex_info.size * sizeof(float); const void *vertex_buffer = cvbr->vertex_buffer; struct setup_context *setup = cvbr->setup; const boolean flatshade_first = softpipe->rasterizer->flatshade_first; @@ -358,7 +358,7 @@ sp_vbuf_draw_arrays(struct vbuf_render *vbr, uint start, uint nr) struct softpipe_vbuf_render *cvbr = softpipe_vbuf_render(vbr); struct softpipe_context *softpipe = cvbr->softpipe; struct setup_context *setup = cvbr->setup; - const unsigned stride = softpipe->vertex_info_vbuf.size * sizeof(float); + const unsigned stride = softpipe->vertex_info.size * sizeof(float); const void *vertex_buffer = (void *) get_vert(cvbr->vertex_buffer, start, stride); const boolean flatshade_first = softpipe->rasterizer->flatshade_first; diff --git a/src/gallium/drivers/softpipe/sp_setup.h b/src/gallium/drivers/softpipe/sp_setup.h index 9efae1cb5ed..a54dc5dad0c 100644 --- a/src/gallium/drivers/softpipe/sp_setup.h +++ b/src/gallium/drivers/softpipe/sp_setup.h @@ -45,7 +45,7 @@ struct sp_setup_info { unsigned valid; struct { unsigned interp:8; /**< SP_INTERP_X */ - unsigned src_index:8; + int src_index:8; } attrib[PIPE_MAX_SHADER_OUTPUTS]; }; diff --git a/src/gallium/drivers/softpipe/sp_state_derived.c b/src/gallium/drivers/softpipe/sp_state_derived.c index ca29d76f8c2..d4d03f1be50 100644 --- a/src/gallium/drivers/softpipe/sp_state_derived.c +++ b/src/gallium/drivers/softpipe/sp_state_derived.c @@ -63,35 +63,47 @@ static void softpipe_compute_vertex_info(struct softpipe_context *softpipe) { struct sp_setup_info *sinfo = &softpipe->setup_info; - int vs_index; if (sinfo->valid == 0) { - /* compute vertex layout for vbuf now */ const struct tgsi_shader_info *fsInfo = &softpipe->fs_variant->info; - struct vertex_info *vinfo_vbuf = &softpipe->vertex_info_vbuf; - const uint num = draw_num_shader_outputs(softpipe->draw); + struct vertex_info *vinfo = &softpipe->vertex_info; uint i; + int vs_index; + /* + * This doesn't quite work right (wrt face injection, prim id, + * wide points) - hit a couple assertions, misrenderings plus + * memory corruption. Albeit could fix (the former two) by calling + * this "more often" (rasterizer changes etc.). (The latter would + * need to be included in draw_prepare_shader_outputs, but it looks + * like that would potentially allocate quite some unused additional + * vertex outputs.) + * draw_prepare_shader_outputs(softpipe->draw); + */ - /* Tell draw_vbuf to simply emit the whole post-xform vertex - * as-is. No longer any need to try and emit draw vertex_header - * info. + /* + * 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). */ - vinfo_vbuf->num_attribs = 0; - for (i = 0; i < num; i++) { - draw_emit_vertex_attr(vinfo_vbuf, EMIT_4F, i); - } - draw_compute_vertex_size(vinfo_vbuf); + softpipe->viewport_index_slot = -1; + softpipe->layer_slot = -1; + softpipe->psize_slot = -1; + + vinfo->num_attribs = 0; + + /* + * Put position always first (setup needs it there). + */ + vs_index = draw_find_shader_output(softpipe->draw, + TGSI_SEMANTIC_POSITION, 0); - softpipe->viewport_index_slot = 0; - softpipe->layer_slot = 0; - softpipe->psize_slot = 0; + draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index); /* - * Loop over fragment shader inputs, searching for the matching output - * from the vertex shader. + * Match FS inputs against VS outputs, emitting the necessary + * attributes. */ for (i = 0; i < fsInfo->num_inputs; i++) { - int src; enum sp_interp_mode interp = SP_INTERP_LINEAR; switch (fsInfo->input_interpolate[i]) { @@ -126,57 +138,93 @@ softpipe_compute_vertex_info(struct softpipe_context *softpipe) break; } - /* this includes texcoords and varying vars */ - src = draw_find_shader_output(softpipe->draw, - fsInfo->input_semantic_name[i], - fsInfo->input_semantic_index[i]); - if (fsInfo->input_semantic_name[i] == TGSI_SEMANTIC_COLOR && src == -1) + /* + * Search for each input in current vs output: + */ + vs_index = draw_find_shader_output(softpipe->draw, + fsInfo->input_semantic_name[i], + fsInfo->input_semantic_index[i]); + + if (fsInfo->input_semantic_name[i] == TGSI_SEMANTIC_COLOR && + vs_index == -1) { /* * try and find a bcolor. * Note that if there's both front and back color, draw will * have copied back to front color already. */ - src = draw_find_shader_output(softpipe->draw, - TGSI_SEMANTIC_BCOLOR, - fsInfo->input_semantic_index[i]); + vs_index = draw_find_shader_output(softpipe->draw, + TGSI_SEMANTIC_BCOLOR, + fsInfo->input_semantic_index[i]); + } sinfo->attrib[i].interp = interp; + /* extremely pointless index map */ + sinfo->attrib[i].src_index = i + 1; /* - * note src can be -1 if not found. Would need special handling, - * (as we don't tell draw anything about it) just force to 0. - * It's wrong either way but should be safer... + * For vp index and layer, if the fs requires them but the vs doesn't + * provide them, draw (vbuf) will give us the required 0 (slot -1). + * (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.) */ - if (src < 0) - src = 0; - sinfo->attrib[i].src_index = src; + if (fsInfo->input_semantic_name[i] == + TGSI_SEMANTIC_VIEWPORT_INDEX) { + softpipe->viewport_index_slot = (int)vinfo->num_attribs; + draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index); + } else if (fsInfo->input_semantic_name[i] == TGSI_SEMANTIC_LAYER) { + softpipe->layer_slot = (int)vinfo->num_attribs; + draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index); + /* + * 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. + */ + } else { + /* + * 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. + */ + draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index); + } } - /* Figure out if we need pointsize as well. */ + /* Figure out if we need pointsize as well. + */ vs_index = draw_find_shader_output(softpipe->draw, TGSI_SEMANTIC_PSIZE, 0); if (vs_index >= 0) { - softpipe->psize_slot = vs_index; + softpipe->psize_slot = (int)vinfo->num_attribs; + draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index); } - /* Figure out if we need viewport index */ - vs_index = draw_find_shader_output(softpipe->draw, - TGSI_SEMANTIC_VIEWPORT_INDEX, - 0); - if (vs_index >= 0) { - softpipe->viewport_index_slot = vs_index; + /* Figure out if we need viewport index (if it wasn't already in fs input) */ + if (softpipe->viewport_index_slot < 0) { + vs_index = draw_find_shader_output(softpipe->draw, + TGSI_SEMANTIC_VIEWPORT_INDEX, + 0); + if (vs_index >= 0) { + softpipe->viewport_index_slot =(int)vinfo->num_attribs; + draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index); + } } - /* Figure out if we need layer */ - vs_index = draw_find_shader_output(softpipe->draw, - TGSI_SEMANTIC_LAYER, - 0); - if (vs_index >= 0) { - softpipe->layer_slot = vs_index; + /* Figure out if we need layer (if it wasn't already in fs input) */ + if (softpipe->layer_slot < 0) { + vs_index = draw_find_shader_output(softpipe->draw, + TGSI_SEMANTIC_LAYER, + 0); + if (vs_index >= 0) { + softpipe->layer_slot = (int)vinfo->num_attribs; + draw_emit_vertex_attr(vinfo, EMIT_4F, vs_index); + } } + + draw_compute_vertex_size(vinfo); softpipe->setup_info.valid = 1; } - return; } @@ -184,15 +232,14 @@ softpipe_compute_vertex_info(struct softpipe_context *softpipe) /** * Called from vbuf module. * - * Note the vertex layout used for vbuf is simply telling it to pass - * through everything as is. The mapping actually used for setup is - * stored separately (but calculated here too at the same time). + * This will trigger validation of the vertex layout (and also compute + * the required information for setup). */ struct vertex_info * softpipe_get_vbuf_vertex_info(struct softpipe_context *softpipe) { softpipe_compute_vertex_info(softpipe); - return &softpipe->vertex_info_vbuf; + return &softpipe->vertex_info; } -- 2.30.2