[Mesa-dev] [PATCH 2/6] st/mesa: Drop !passColor optimization in drawpixels shaders.

Kenneth Graunke kenneth at whitecape.org
Mon Dec 17 18:37:36 UTC 2018


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.
---
 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.19.1



More information about the mesa-dev mailing list