From ed1a356c5e21a8d19287fea2512d3b3a82171e9f Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Mon, 3 Dec 2018 22:26:47 -0800 Subject: [PATCH] st/mesa: Drop !passColor optimization in drawpixels shaders. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The glDrawPixels passthrough vertex shader copies position and texcoord vertex attributes to varying outputs. It also optionally copies a third gl_Color attribute, which sometimes is unnecessary. Until now, we've compiled separate variants of the shader, one of which does this extra copy, and the other of which doesn't. We have done this since 2007. But, the vertex shader runs for a whopping four vertices, and so the cost of a copying a single input to output is likely inconsequential. In theory, we could bind one fewer vertex element - but we always bind all three regardless. So, we don't even get that savings. This patch unifies the two, so we always copy the optional color, and save having to compile the variant. It also makes the VS input interface match up with the vertex element state without any dead (unused) input attributes. Reviewed-by: Marek Olšák --- src/mesa/state_tracker/st_cb_drawpixels.c | 46 ++++++++++------------- src/mesa/state_tracker/st_context.h | 2 +- 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c index 4f08e751393..0bb2caf5edc 100644 --- a/src/mesa/state_tracker/st_cb_drawpixels.c +++ b/src/mesa/state_tracker/st_cb_drawpixels.c @@ -191,32 +191,29 @@ get_drawpix_z_stencil_program(struct st_context *st, /** * Create a simple vertex shader that just passes through the - * vertex position and texcoord (and optionally, color). + * vertex position, texcoord, and color. */ -static void * -make_passthrough_vertex_shader(struct st_context *st, - GLboolean passColor) +static void +make_passthrough_vertex_shader(struct st_context *st) { const enum tgsi_semantic texcoord_semantic = st->needs_texcoord_semantic ? TGSI_SEMANTIC_TEXCOORD : TGSI_SEMANTIC_GENERIC; - if (!st->drawpix.vert_shaders[passColor]) { + if (!st->drawpix.vert_shader) { struct ureg_program *ureg = ureg_create( PIPE_SHADER_VERTEX ); if (ureg == NULL) - return NULL; + return; /* MOV result.pos, vertex.pos; */ ureg_MOV(ureg, ureg_DECL_output( ureg, TGSI_SEMANTIC_POSITION, 0 ), ureg_DECL_vs_input( ureg, 0 )); - if (passColor) { - /* MOV result.color0, vertex.attr[1]; */ - ureg_MOV(ureg, - ureg_DECL_output( ureg, TGSI_SEMANTIC_COLOR, 0 ), - ureg_DECL_vs_input( ureg, 1 )); - } + /* MOV result.color0, vertex.attr[1]; */ + ureg_MOV(ureg, + ureg_DECL_output( ureg, TGSI_SEMANTIC_COLOR, 0 ), + ureg_DECL_vs_input( ureg, 1 )); /* MOV result.texcoord0, vertex.attr[2]; */ ureg_MOV(ureg, @@ -225,11 +222,9 @@ make_passthrough_vertex_shader(struct st_context *st, ureg_END( ureg ); - st->drawpix.vert_shaders[passColor] = + st->drawpix.vert_shader = ureg_create_shader_and_destroy( ureg, st->pipe ); } - - return st->drawpix.vert_shaders[passColor]; } @@ -1135,7 +1130,7 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y, GLenum format, GLenum type, const struct gl_pixelstore_attrib *unpack, const void *pixels) { - void *driver_vp, *driver_fp; + void *driver_fp; struct st_context *st = st_context(ctx); struct pipe_context *pipe = st->pipe; GLboolean write_stencil = GL_FALSE, write_depth = GL_FALSE; @@ -1185,19 +1180,19 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y, return; } + make_passthrough_vertex_shader(st); + /* * Get vertex/fragment shaders */ if (write_depth || write_stencil) { driver_fp = get_drawpix_z_stencil_program(st, write_depth, write_stencil); - driver_vp = make_passthrough_vertex_shader(st, GL_TRUE); } else { fpv = get_color_fp_variant(st); driver_fp = fpv->driver_shader; - driver_vp = make_passthrough_vertex_shader(st, GL_FALSE); if (ctx->Pixel.MapColorFlag) { pipe_sampler_view_reference(&sv[1], @@ -1246,7 +1241,7 @@ st_DrawPixels(struct gl_context *ctx, GLint x, GLint y, ctx->Pixel.ZoomX, ctx->Pixel.ZoomY, sv, num_sampler_view, - driver_vp, + st->drawpix.vert_shader, driver_fp, fpv, ctx->Current.RasterColor, GL_FALSE, write_depth, write_stencil); @@ -1506,7 +1501,7 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy, struct pipe_context *pipe = st->pipe; struct pipe_screen *screen = pipe->screen; struct st_renderbuffer *rbRead; - void *driver_vp, *driver_fp; + void *driver_fp; struct pipe_resource *pt; struct pipe_sampler_view *sv[2] = { NULL }; struct st_fp_variant *fpv = NULL; @@ -1547,6 +1542,7 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy, * are handled. */ + make_passthrough_vertex_shader(st); /* * Get vertex/fragment shaders @@ -1557,7 +1553,6 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy, rbRead = st_get_color_read_renderbuffer(ctx); driver_fp = fpv->driver_shader; - driver_vp = make_passthrough_vertex_shader(st, GL_FALSE); if (ctx->Pixel.MapColorFlag) { pipe_sampler_view_reference(&sv[1], @@ -1576,7 +1571,6 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy, Attachment[BUFFER_DEPTH].Renderbuffer); driver_fp = get_drawpix_z_stencil_program(st, GL_TRUE, GL_FALSE); - driver_vp = make_passthrough_vertex_shader(st, GL_TRUE); } /* Choose the format for the temporary texture. */ @@ -1703,7 +1697,7 @@ st_CopyPixels(struct gl_context *ctx, GLint srcx, GLint srcy, width, height, ctx->Pixel.ZoomX, ctx->Pixel.ZoomY, sv, num_sampler_view, - driver_vp, + st->drawpix.vert_shader, driver_fp, fpv, ctx->Current.Attrib[VERT_ATTRIB_COLOR0], invertTex, GL_FALSE, GL_FALSE); @@ -1732,10 +1726,8 @@ st_destroy_drawpix(struct st_context *st) st->drawpix.zs_shaders[i]); } - if (st->drawpix.vert_shaders[0]) - cso_delete_vertex_shader(st->cso_context, st->drawpix.vert_shaders[0]); - if (st->drawpix.vert_shaders[1]) - cso_delete_vertex_shader(st->cso_context, st->drawpix.vert_shaders[1]); + if (st->drawpix.vert_shader) + cso_delete_vertex_shader(st->cso_context, st->drawpix.vert_shader); /* Free cache data */ for (i = 0; i < ARRAY_SIZE(st->drawpix_cache.entries); i++) { diff --git a/src/mesa/state_tracker/st_context.h b/src/mesa/state_tracker/st_context.h index 78e962dec00..622b7a40dc4 100644 --- a/src/mesa/state_tracker/st_context.h +++ b/src/mesa/state_tracker/st_context.h @@ -231,7 +231,7 @@ struct st_context /** for glDraw/CopyPixels */ struct { void *zs_shaders[4]; - void *vert_shaders[2]; /**< ureg shaders */ + void *vert_shader; /**< ureg shader */ } drawpix; /** Cache of glDrawPixels images */ -- 2.30.2