[PATCH 1/3] glamor: Just set the logic op to what we want at the start of all rendering.

Eric Anholt eric at anholt.net
Wed Feb 4 17:25:10 PST 2015


By dropping the unconditional logic op disable at the end of
rendering, this fixes GL errors being thrown in GLES2 contexts (which
don't have logic ops).  On desktop, this also means a little less
overhead per draw call from taking one less trip through the
glEnable/glDisable switch statement of doom in Mesa.

The exchange here is that we end up taking a trip through it in the
XV, Render, and gradient-generation paths.  If the glEnable() is
actually costly, we should probably cache our logic op state in our
screen, since there's no way the GL could make that switch statement
as cheap as the caller caching it would be.

v2: Don't forget to set the logic op in Xephyr's drawing.

Signed-off-by: Eric Anholt <eric at anholt.net>
---

This is 2/3 a resend of patches that got swamped by a discussion that
wasn't related to the patches themselves.  The only feedback there
that I see missing is "Ideally, we'd write down the rules and then
construct assertions to validate them at each entry point."  But we
don't have a particular good place to put documentation that I can
see.  Any suggestions?

 glamor/glamor_copy.c               |  4 ----
 glamor/glamor_dash.c               | 13 +++++--------
 glamor/glamor_glyphblt.c           | 10 ++--------
 glamor/glamor_gradient.c           |  4 ++++
 glamor/glamor_lines.c              |  5 +----
 glamor/glamor_pixmap.c             |  3 +++
 glamor/glamor_points.c             |  9 +++------
 glamor/glamor_rects.c              |  7 ++-----
 glamor/glamor_render.c             |  1 +
 glamor/glamor_segs.c               |  2 --
 glamor/glamor_spans.c              |  7 ++-----
 glamor/glamor_text.c               |  8 +-------
 glamor/glamor_xv.c                 |  1 +
 hw/kdrive/ephyr/ephyr_glamor_glx.c |  2 ++
 14 files changed, 27 insertions(+), 49 deletions(-)

diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c
index 609fcb3..1660ffd 100644
--- a/glamor/glamor_copy.c
+++ b/glamor/glamor_copy.c
@@ -404,11 +404,9 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src,
         glDisable(GL_SCISSOR_TEST);
     glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 
-    glDisable(GL_COLOR_LOGIC_OP);
     return TRUE;
 
 bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
     return FALSE;
 }
 
@@ -452,7 +450,6 @@ glamor_copy_fbo_fbo_temp(DrawablePtr src,
 
     if (!glamor_set_alu(screen, gc ? gc->alu : GXcopy))
         goto bail_ctx;
-    glDisable(GL_COLOR_LOGIC_OP);
 
     /* Find the size of the area to copy
      */
@@ -521,7 +518,6 @@ bail:
     return FALSE;
 
 bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
     return FALSE;
 }
 
diff --git a/glamor/glamor_dash.c b/glamor/glamor_dash.c
index 49ad3b6..4281ff0 100644
--- a/glamor/glamor_dash.c
+++ b/glamor/glamor_dash.c
@@ -159,11 +159,11 @@ glamor_dash_setup(DrawablePtr drawable, GCPtr gc)
                                        &glamor_priv->on_off_dash_line_progs,
                                        &glamor_facet_on_off_dash_lines);
         if (!prog)
-            goto bail_ctx;
+            goto bail;
         break;
     case LineDoubleDash:
         if (gc->fillStyle != FillSolid)
-            goto bail_ctx;
+            goto bail;
 
         prog = &glamor_priv->double_dash_line_prog;
 
@@ -171,18 +171,18 @@ glamor_dash_setup(DrawablePtr drawable, GCPtr gc)
             if (!glamor_build_program(screen, prog,
                                       &glamor_facet_double_dash_lines,
                                       NULL))
-                goto bail_ctx;
+                goto bail;
         }
 
         if (!glamor_use_program(pixmap, gc, prog, NULL))
-            goto bail_ctx;
+            goto bail;
 
         glamor_set_color(pixmap, gc->fgPixel, prog->fg_uniform);
         glamor_set_color(pixmap, gc->bgPixel, prog->bg_uniform);
         break;
 
     default:
-        goto bail_ctx;
+        goto bail;
     }
 
 
@@ -195,8 +195,6 @@ glamor_dash_setup(DrawablePtr drawable, GCPtr gc)
 
     return prog;
 
-bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
 bail:
     return NULL;
 }
@@ -230,7 +228,6 @@ glamor_dash_loop(DrawablePtr drawable, GCPtr gc, glamor_program *prog,
     }
 
     glDisable(GL_SCISSOR_TEST);
-    glDisable(GL_COLOR_LOGIC_OP);
     glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 }
 
diff --git a/glamor/glamor_glyphblt.c b/glamor/glamor_glyphblt.c
index 8c73f48..1791f6d 100644
--- a/glamor/glamor_glyphblt.c
+++ b/glamor/glamor_glyphblt.c
@@ -60,7 +60,7 @@ glamor_poly_glyph_blt_gl(DrawablePtr drawable, GCPtr gc,
                                    &glamor_priv->poly_glyph_blt_progs,
                                    &glamor_facet_poly_glyph_blt);
     if (!prog)
-        goto bail_ctx;
+        goto bail;
 
     glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
 
@@ -138,12 +138,9 @@ glamor_poly_glyph_blt_gl(DrawablePtr drawable, GCPtr gc,
         }
     }
 
-    glDisable(GL_COLOR_LOGIC_OP);
     glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 
     return TRUE;
-bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
 bail:
     return FALSE;
 }
@@ -191,7 +188,7 @@ glamor_push_pixels_gl(GCPtr gc, PixmapPtr bitmap,
                                    &glamor_priv->poly_glyph_blt_progs,
                                    &glamor_facet_poly_glyph_blt);
     if (!prog)
-        goto bail_ctx;
+        goto bail;
 
     glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
 
@@ -230,12 +227,9 @@ glamor_push_pixels_gl(GCPtr gc, PixmapPtr bitmap,
         glDrawArrays(GL_POINTS, 0, num_points);
     }
 
-    glDisable(GL_COLOR_LOGIC_OP);
     glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
     return TRUE;
 
-bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
 bail:
     return FALSE;
 }
diff --git a/glamor/glamor_gradient.c b/glamor/glamor_gradient.c
index 232b402..8ea645e 100644
--- a/glamor/glamor_gradient.c
+++ b/glamor/glamor_gradient.c
@@ -993,6 +993,8 @@ glamor_generate_radial_gradient_picture(ScreenPtr screen,
          vertices, tex_vertices, 0))
         goto GRADIENT_FAIL;
 
+    glamor_set_alu(screen, GXcopy);
+
     /* Set all the stops and colors to shader. */
     if (stops_count > RADIAL_SMALL_STOPS) {
         stop_colors = malloc(4 * stops_count * sizeof(float));
@@ -1309,6 +1311,8 @@ glamor_generate_linear_gradient_picture(ScreenPtr screen,
          vertices, tex_vertices, 1))
         goto GRADIENT_FAIL;
 
+    glamor_set_alu(screen, GXcopy);
+
     /* Normalize the PTs. */
     glamor_set_normalize_pt(xscale, yscale,
                             pixman_fixed_to_double(src_picture->pSourcePict->
diff --git a/glamor/glamor_lines.c b/glamor/glamor_lines.c
index 6c8bb88..2dd9c07 100644
--- a/glamor/glamor_lines.c
+++ b/glamor/glamor_lines.c
@@ -65,7 +65,7 @@ glamor_poly_lines_solid_gl(DrawablePtr drawable, GCPtr gc,
                                    &glamor_facet_poly_lines);
 
     if (!prog)
-        goto bail_ctx;
+        goto bail;
 
     /* Set up the vertex buffers for the points */
 
@@ -117,12 +117,9 @@ glamor_poly_lines_solid_gl(DrawablePtr drawable, GCPtr gc,
     }
 
     glDisable(GL_SCISSOR_TEST);
-    glDisable(GL_COLOR_LOGIC_OP);
     glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 
     return TRUE;
-bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
 bail:
     return FALSE;
 }
diff --git a/glamor/glamor_pixmap.c b/glamor/glamor_pixmap.c
index 0224b34..89b4c36 100644
--- a/glamor/glamor_pixmap.c
+++ b/glamor/glamor_pixmap.c
@@ -748,6 +748,7 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format,
                                       int swap_rb, int x, int y, int w, int h,
                                       int stride, void *bits, int pbo)
 {
+    ScreenPtr screen = pixmap->drawable.pScreen;
     glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap);
     glamor_screen_private *glamor_priv =
         glamor_get_screen_private(pixmap->drawable.pScreen);
@@ -833,6 +834,7 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format,
         glEnableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
 
         glamor_set_destination_pixmap_priv_nc(glamor_priv, pixmap, pixmap_priv);
+        glamor_set_alu(screen, GXcopy);
         __glamor_upload_pixmap_to_texture(pixmap, &tex,
                                           format, type, 0, 0, w, h, bits, pbo);
         glActiveTexture(GL_TEXTURE0);
@@ -1131,6 +1133,7 @@ glamor_es2_pixmap_read_prepare(PixmapPtr source, int x, int y, int w, int h,
     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
 
     glamor_set_destination_pixmap_fbo(glamor_priv, temp_fbo, 0, 0, w, h);
+    glamor_set_alu(screen, GXcopy);
     glUseProgram(glamor_priv->finish_access_prog[no_alpha]);
     glUniform1i(glamor_priv->finish_access_revert[no_alpha], revert);
     glUniform1i(glamor_priv->finish_access_swap_rb[no_alpha], swap_rb);
diff --git a/glamor/glamor_points.c b/glamor/glamor_points.c
index 01a1c7e..df7e5a2 100644
--- a/glamor/glamor_points.c
+++ b/glamor/glamor_points.c
@@ -55,17 +55,17 @@ glamor_poly_point_gl(DrawablePtr drawable, GCPtr gc, int mode, int npt, DDXPoint
     glamor_make_current(glamor_priv);
 
     if (prog->failed)
-        goto bail_ctx;
+        goto bail;
 
     if (!prog->prog) {
         if (!glamor_build_program(screen, prog,
                                   &glamor_facet_point,
                                   &glamor_fill_solid))
-            goto bail_ctx;
+            goto bail;
     }
 
     if (!glamor_use_program(pixmap, gc, prog, NULL))
-        goto bail_ctx;
+        goto bail;
 
     vbo_ppt = glamor_get_vbo_space(screen, npt * (2 * sizeof (INT16)), &vbo_offset);
     glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
@@ -102,13 +102,10 @@ glamor_poly_point_gl(DrawablePtr drawable, GCPtr gc, int mode, int npt, DDXPoint
     }
 
     glDisable(GL_SCISSOR_TEST);
-    glDisable(GL_COLOR_LOGIC_OP);
     glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 
     return TRUE;
 
-bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
 bail:
     return FALSE;
 }
diff --git a/glamor/glamor_rects.c b/glamor/glamor_rects.c
index 7161921..c8c92be 100644
--- a/glamor/glamor_rects.c
+++ b/glamor/glamor_rects.c
@@ -65,7 +65,7 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
                                        &glamor_facet_polyfillrect_130);
 
         if (!prog)
-            goto bail_ctx;
+            goto bail;
 
         /* Set up the vertex buffers for the points */
 
@@ -87,7 +87,7 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
                                        &glamor_facet_polyfillrect_120);
 
         if (!prog)
-            goto bail_ctx;
+            goto bail;
 
         /* Set up the vertex buffers for the points */
 
@@ -139,14 +139,11 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
     }
 
     glDisable(GL_SCISSOR_TEST);
-    glDisable(GL_COLOR_LOGIC_OP);
     if (glamor_priv->glsl_version >= 130)
         glVertexAttribDivisor(GLAMOR_VERTEX_POS, 0);
     glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 
     return TRUE;
-bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
 bail:
     return FALSE;
 }
diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
index 6669f48..27c09fd 100644
--- a/glamor/glamor_render.c
+++ b/glamor/glamor_render.c
@@ -1172,6 +1172,7 @@ glamor_composite_with_shader(CARD8 op,
 
     glamor_set_destination_pixmap_priv_nc(glamor_priv, dest_pixmap, dest_pixmap_priv);
     glamor_composite_set_shader_blend(glamor_priv, dest_pixmap_priv, &key, shader, &op_info);
+    glamor_set_alu(screen, GXcopy);
 
     glamor_make_current(glamor_priv);
 
diff --git a/glamor/glamor_segs.c b/glamor/glamor_segs.c
index 6d469ce..e167325 100644
--- a/glamor/glamor_segs.c
+++ b/glamor/glamor_segs.c
@@ -109,12 +109,10 @@ glamor_poly_segment_solid_gl(DrawablePtr drawable, GCPtr gc,
     }
 
     glDisable(GL_SCISSOR_TEST);
-    glDisable(GL_COLOR_LOGIC_OP);
     glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 
     return TRUE;
 bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
 bail:
     return FALSE;
 }
diff --git a/glamor/glamor_spans.c b/glamor/glamor_spans.c
index fe8c7dc..7138db5 100644
--- a/glamor/glamor_spans.c
+++ b/glamor/glamor_spans.c
@@ -68,7 +68,7 @@ glamor_fill_spans_gl(DrawablePtr drawable,
                                        &glamor_facet_fillspans_130);
 
         if (!prog)
-            goto bail_ctx;
+            goto bail;
 
         /* Set up the vertex buffers for the points */
 
@@ -93,7 +93,7 @@ glamor_fill_spans_gl(DrawablePtr drawable,
                                        &glamor_facet_fillspans_120);
 
         if (!prog)
-            goto bail_ctx;
+            goto bail;
 
         /* Set up the vertex buffers for the points */
 
@@ -147,14 +147,11 @@ glamor_fill_spans_gl(DrawablePtr drawable,
     }
 
     glDisable(GL_SCISSOR_TEST);
-    glDisable(GL_COLOR_LOGIC_OP);
     if (glamor_priv->glsl_version >= 130)
         glVertexAttribDivisor(GLAMOR_VERTEX_POS, 0);
     glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 
     return TRUE;
-bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
 bail:
     return FALSE;
 }
diff --git a/glamor/glamor_text.c b/glamor/glamor_text.c
index 68593a4..c7c1ab7 100644
--- a/glamor/glamor_text.c
+++ b/glamor/glamor_text.c
@@ -210,7 +210,6 @@ glamor_text(DrawablePtr drawable, GCPtr gc,
     glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
     glVertexAttribDivisor(GLAMOR_VERTEX_POS, 0);
     glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
-    glDisable(GL_COLOR_LOGIC_OP);
 
     return x;
 }
@@ -286,18 +285,14 @@ glamor_poly_text(DrawablePtr drawable, GCPtr gc,
     prog = glamor_use_program_fill(pixmap, gc, &glamor_priv->poly_text_progs, &glamor_facet_poly_text);
 
     if (!prog)
-        goto bail_ctx;
+        goto bail;
 
     x = glamor_text(drawable, gc, glamor_font, prog,
                     x, y, count, chars, charinfo, sixteen);
 
-    glDisable(GL_COLOR_LOGIC_OP);
-
     *final_pos = x;
     return TRUE;
 
-bail_ctx:
-    glDisable(GL_COLOR_LOGIC_OP);
 bail:
     return FALSE;
 }
@@ -468,7 +463,6 @@ glamor_image_text(DrawablePtr drawable, GCPtr gc,
     return TRUE;
 
 bail:
-    glDisable(GL_COLOR_LOGIC_OP);
     return FALSE;
 }
 
diff --git a/glamor/glamor_xv.c b/glamor/glamor_xv.c
index 2d5b8e1..364104d 100644
--- a/glamor/glamor_xv.c
+++ b/glamor/glamor_xv.c
@@ -286,6 +286,7 @@ glamor_xv_render(glamor_port_private *port_priv)
     glamor_get_drawable_deltas(port_priv->pDraw, port_priv->pPixmap, &dst_x_off,
                                &dst_y_off);
     glamor_set_destination_pixmap_priv_nc(glamor_priv, pixmap, pixmap_priv);
+    glamor_set_alu(screen, GXcopy);
 
     for (i = 0; i < 3; i++) {
         if (port_priv->src_pix[i]) {
diff --git a/hw/kdrive/ephyr/ephyr_glamor_glx.c b/hw/kdrive/ephyr/ephyr_glamor_glx.c
index 8fe7516..582e3af 100644
--- a/hw/kdrive/ephyr/ephyr_glamor_glx.c
+++ b/hw/kdrive/ephyr/ephyr_glamor_glx.c
@@ -214,6 +214,8 @@ ephyr_glamor_damage_redisplay(struct ephyr_glamor *glamor,
     glBindFramebuffer(GL_FRAMEBUFFER, 0);
     glUseProgram(glamor->texture_shader);
     glViewport(0, 0, glamor->width, glamor->height);
+    if (!ephyr_glamor_gles2)
+        glDisable(GL_COLOR_LOGIC_OP);
 
     glVertexAttribPointer(glamor->texture_shader_position_loc,
                           2, GL_FLOAT, FALSE, 0, position);
-- 
2.1.4



More information about the xorg-devel mailing list