Am 2014-05-06 00:02, schrieb Keith Packard:
+static const char on_off_fs_exec[] =
+ " float pattern = texture2D(dash, vec2(dash_offset, 0.5)).w;\n"
+    "       if (pattern == 0.0)\n"
+    "               discard;\n";

Did you care about the sampler configuration? eg with linear filtering, you don't have to get exactly 0.0. Maybe it's better to check in this way:
if (pattern < 0.5)

+static glamor_program *
+glamor_dash_setup(DrawablePtr drawable, GCPtr gc)
...
+bail_ctx:
+    glDisable(GL_COLOR_LOGIC_OP);

Isn't this already disabled in the caller?

+static int
+glamor_line_length(short x1, short y1, short x2, short y2)
+{
+    int dx = abs(x2 - x1);
+    int dy = abs(y2 - y1);
+    if (dx > dy)
+        return dx;
+    return dy;

max(dx,dy)

+Bool
+glamor_poly_lines_dash_gl(DrawablePtr drawable, GCPtr gc,
...
+    if (mode == CoordModePrevious) {
+        DDXPointRec here = { 0, 0 };
+
+        for (i = 0; i < n; i++) {
+            here.x += points[i].x;
+            points[i].x = here.x;
+            here.y += points[i].y;
+            points[i].y = here.y;

Are you allowed to overwrite "points"? I'd inline this code in the next loop to not iterate twice.

+        }
+    }
+
+    dash_pos = gc->dashOffset;
+    prev_x = prev_y = 0;
+    for (i = 0; i < n; i++) {
+        if (i)
+            dash_pos += glamor_line_length(prev_x, prev_y,
+                                           points[i].x, points[i].y);

Isn't it easier to init prev_xy with points[0]?

+        v[0] = prev_x = points[i].x;
+        v[1] = prev_y = points[i].y;
+        v[2] = dash_pos;
+        v += 4;
+    }
+
+    if (add_last) {
+        v[0] = v[-4] + 1;
+        v[1] = v[-3];
+        v[2] = dash_pos + 1;
+    }

You aren't allowed to access v.
Thx for careing about offsets to be multiple of 4 bytes, but you have to overwrite the buffer completely (v[3]=0). Else write-combining won't like this code.


+static short *
+glamor_add_segment(short *v, short x1, short y1, short x2, short y2,
int dash_start)
+{
+    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;
+}

I don't think it's worth to create such a kind of util functions. It doesn't remove code as you still need 4 lines in the caller. But it hides the vertex format which is set up directly above the caller :/

+Bool
+glamor_poly_segment_dash_gl(DrawablePtr drawable, GCPtr gc,
...
+    for (i = 0; i < nseg; i++) {
+        v = glamor_add_segment(v,
+                               segs[i].x1, segs[i].y1,
+                               segs[i].x2, segs[i].y2,
+                               dash_start);
+        if (add_last)
+            v = glamor_add_segment(v,
+                                   segs[i].x2, segs[i].y2,
+                                   segs[i].x2 + 1, segs[i].y2,
+                                   v[-8 + 6]);

Also accessing v.

--------------------

Of course, the same rasterization issues as in the last patch.
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to