[PATCH] glamor: Review changes for glamor_dash.c

[Eric Anholt]

3 major changes:

- Drops power-of-two alignment of our line vertex data, simplifying
  the code.

- Stops reading from the VBO.  While on keithp's and my machines the
  VBO is mapped cached, on many implementations it will be mapped WC,
  making those reads extremely expensive.

- Style fixes (line wrapping, spaces around operators).

No performance difference on x11perf -dline10 (n=20).
No performance difference on x11perf -dseg10 (n=20, or with warm-up
outliers trimmed, n=17).
---

Here's review feedback for this one, that I'm hoping you'd like to
squash in.  Also, don't forget to drop the squash debris from the
original commit message.

 glamor/glamor_dash.c | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/glamor/glamor_dash.c b/glamor/glamor_dash.c
index 59565be..eefb714 100644
--- a/glamor/glamor_dash.c
+++ b/glamor/glamor_dash.c
@@ -68,7 +68,9 @@ static const glamor_facet glamor_facet_double_dash_lines = {
     .vs_exec = dash_vs_exec,
     .fs_vars = dash_fs_vars,
     .fs_exec = double_fs_exec,
-    .locations = glamor_program_location_dash|glamor_program_location_fg|glamor_program_location_bg,
+    .locations = (glamor_program_location_dash |
+                  glamor_program_location_fg |
+                  glamor_program_location_bg),
 };
 
 static PixmapPtr
@@ -213,7 +215,8 @@ glamor_dash_loop(DrawablePtr drawable, GCPtr gc, glamor_program *prog, int n, GL
         int nbox = RegionNumRects(gc->pCompositeClip);
         BoxPtr box = RegionRects(gc->pCompositeClip);
 
-        glamor_set_destination_drawable(drawable, box_x, box_y, TRUE, TRUE, prog->matrix_uniform, &off_x, &off_y);
+        glamor_set_destination_drawable(drawable, box_x, box_y, TRUE, TRUE,
+                                        prog->matrix_uniform, &off_x, &off_y);
 
         while (nbox--) {
             glScissor(box->x1 + off_x,
@@ -265,11 +268,12 @@ glamor_poly_lines_dash_gl(DrawablePtr drawable, GCPtr gc,
 
     /* Set up the vertex buffers for the points */
 
-    v = glamor_get_vbo_space(drawable->pScreen, (n + add_last) * 4 * sizeof (short), &vbo_offset);
+    v = glamor_get_vbo_space(drawable->pScreen,
+                             (n + add_last) * 3 * sizeof (short), &vbo_offset);
 
     glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
     glVertexAttribPointer(GLAMOR_VERTEX_POS, 3, GL_SHORT, GL_FALSE,
-                          4 * sizeof (short), vbo_offset);
+                          3 * sizeof (short), vbo_offset);
 
     if (mode == CoordModePrevious) {
         DDXPointRec here = { 0, 0 };
@@ -291,12 +295,12 @@ glamor_poly_lines_dash_gl(DrawablePtr drawable, GCPtr gc,
         v[0] = prev_x = points[i].x;
         v[1] = prev_y = points[i].y;
         v[2] = dash_pos;
-        v += 4;
+        v += 3;
     }
 
     if (add_last) {
-        v[0] = v[-4] + 1;
-        v[1] = v[-3];
+        v[0] = prev_x + 1;
+        v[1] = prev_y;
         v[2] = dash_pos + 1;
     }
 
@@ -308,17 +312,17 @@ glamor_poly_lines_dash_gl(DrawablePtr drawable, GCPtr gc,
 }
 
 static short *
-glamor_add_segment(short *v, short x1, short y1, short x2, short y2, int dash_start)
+glamor_add_segment(short *v, short x1, short y1, short x2, short y2,
+                   int dash_start, int dash_end)
 {
     v[0] = x1;
     v[1] = y1;
     v[2] = dash_start;
 
-    v[4] = x2;
-    v[5] = y2;
-    v[6] = dash_start + glamor_line_length(x1, y1,
-                                           x2, y2);
-    return v + 8;
+    v[3] = x2;
+    v[4] = y2;
+    v[5] = dash_end;
+    return v + 6;
 }
 
 Bool
@@ -342,22 +346,27 @@ glamor_poly_segment_dash_gl(DrawablePtr drawable, GCPtr gc,
 
     /* Set up the vertex buffers for the points */
 
-    v = glamor_get_vbo_space(drawable->pScreen, (nseg<<add_last) * 8 * sizeof (short), &vbo_offset);
+    v = glamor_get_vbo_space(drawable->pScreen,
+                             (nseg << add_last) * 6 * sizeof (short),
+                             &vbo_offset);
 
     glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
     glVertexAttribPointer(GLAMOR_VERTEX_POS, 3, GL_SHORT, GL_FALSE,
-                          4 * sizeof (short), vbo_offset);
+                          3 * sizeof (short), vbo_offset);
 
     for (i = 0; i < nseg; i++) {
+        int dash_end = dash_start + glamor_line_length(segs[i].x1, segs[i].y1,
+                                                       segs[i].x2, segs[i].y2);
         v = glamor_add_segment(v,
                                segs[i].x1, segs[i].y1,
                                segs[i].x2, segs[i].y2,
-                               dash_start);
-        if (add_last)
+                               dash_start, dash_end);
+        if (add_last) {
             v = glamor_add_segment(v,
                                    segs[i].x2, segs[i].y2,
                                    segs[i].x2 + 1, segs[i].y2,
-                                   v[-8 + 6]);
+                                   dash_end, dash_end + 1);
+        }
     }
 
     glamor_put_vbo_space(screen);
-- 
1.9.2