[PATCH 08/10] glamor: Add glamor_program based fill_spans

[Markus Wick]

Am 2014-03-14 07:30, schrieb Keith Packard:
> diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c
> index 96723c0..8c10c92 100644
> --- a/glamor/glamor_core.c
> +++ b/glamor/glamor_core.c
> @@ -410,7 +410,7 @@ glamor_stipple(PixmapPtr pixmap, PixmapPtr stipple,
>  }
> 
>  GCOps glamor_gc_ops = {
> -    .FillSpans = glamor_fill_spans,
> +    .FillSpans = glamor_fillspans,

This create a lot of dead code. You neither fall back to this old bad
way when glsl130 isn't available nor did you remove it.

>      .SetSpans = glamor_set_spans,
>      .PutImage = glamor_put_image,
>      .CopyArea = glamor_copy_area,
> diff --git a/glamor/glamor_spans.c b/glamor/glamor_spans.c
> new file mode 100644
> index 0000000..d861232
> --- /dev/null
> +++ b/glamor/glamor_spans.c
> @@ -0,0 +1,127 @@
> +/*
> + * Copyright © 2014 Keith Packard
> + *
> + * Permission to use, copy, modify, distribute, and sell this software
> and its
> + * documentation for any purpose is hereby granted without fee,
> provided that
> + * the above copyright notice appear in all copies and that both that
> copyright
> + * notice and this permission notice appear in supporting
> documentation, and
> + * that the name of the copyright holders not be used in advertising
> or
> + * publicity pertaining to distribution of the software without
> specific,
> + * written prior permission.  The copyright holders make no
> representations
> + * about the suitability of this software for any purpose.  It is
> provided "as
> + * is" without express or implied warranty.
> + *
> + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> SOFTWARE,
> + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
> NO
> + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL,
> INDIRECT OR
> + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
> OF USE,
> + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
> OTHER
> + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
> PERFORMANCE
> + * OF THIS SOFTWARE.
> + */
> +
> +#include "glamor_priv.h"
> +#include "glamor_transform.h"
> +
> +glamor_program  fill_spans_progs[4];
> +
> +static const glamor_facet glamor_facet_fillspans = {
> +    .name = "fill_spans",
> +    .version = 130,
> +    .vs_vars =  "attribute vec3 primitive;\n",
> +    .vs_exec = ("       vec2 pos = vec2(primitive.z,1) *
> vec2(gl_VertexID&1, (gl_VertexID&2)>>1);\n"
> +                GLAMOR_POS(gl_Position, (primitive.xy + pos))),
> +};
> +
> +void
> +glamor_fillspans(DrawablePtr drawable,
> +                 GCPtr gc,
> +                 int n, DDXPointPtr points, int *widths, int sorted)
> +{
> +    ScreenPtr screen = drawable->pScreen;
> +    glamor_screen_private *glamor_priv =
> glamor_get_screen_private(screen);
> +    PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
> +    glamor_pixmap_private *pixmap_priv;
> +    glamor_program *prog;
> +    int nbox = RegionNumRects(gc->pCompositeClip);
> +    BoxPtr box = RegionRects(gc->pCompositeClip);
> +    int off_x, off_y;
> +    GLshort *v;
> +    char *vbo_offset;
> +    int c;
> +    int box_x, box_y;
> +
> +    pixmap_priv = glamor_get_pixmap_private(pixmap);
> +    if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
> +        goto bail;
> +
> +    glamor_get_context(glamor_priv);
> +
> +    prog = glamor_use_program_fill(pixmap, gc,
> &glamor_priv->fill_spans_program,
> +                                   &glamor_facet_fillspans);
> +
> +    if (!prog)
> +        goto bail;
> +
> +    /* Set up the vertex buffers for the points */
> +
> +    v = glamor_get_vbo_space(drawable->pScreen, n * (3 * sizeof
> (GLshort)), &vbo_offset);
> +
> +    glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
> +    glVertexAttribDivisor(GLAMOR_VERTEX_POS, 1);

imo we should check for glsl130 support. Just compiling and wait for
errors isn't the way to go.
_but_ this call will crash if ARB_instanced_arrays isn't supported.
As this is a GL3.3 feature, there are likely many plattforms which
doesn't support it.

I don't want to fall back to the old implementation, so I suggest
GL_TRIANGLES + 6 * 2 * sizeof(short) as vertex format.

> +    glVertexAttribPointer(GLAMOR_VERTEX_POS, 3, GL_SHORT, GL_FALSE,
> +                          3 * sizeof (GLshort), vbo_offset);

iirc gpus doesn't like vertex strides which aren't multiple of 4 bytes.
Padding with zero?

> +
> +    for (c = 0; c < n; c++) {
> +        v[0] = points->x;
> +        v[1] = points->y;
> +        v[2] = *widths++;
> +        points++;
> +        v += 3;
> +    }
> +
> +    glamor_put_vbo_space(screen);
> +
> +    glEnable(GL_SCISSOR_TEST);
> +
> +    glamor_pixmap_loop(pixmap_priv, box_x, box_y) {
> +         glamor_set_destination_drawable(drawable, box_x, box_y,
> FALSE, prog->matrix_uniform, &off_x, &off_y);
> +
> +         nbox = RegionNumRects(gc->pCompositeClip);
> +         box = RegionRects(gc->pCompositeClip);
> +
> +         while (nbox--) {
> +             glScissor(box->x1 + off_x,
> +                       box->y1 + off_y,
> +                       box->x2 - box->x1,
> +                       box->y2 - box->y1);
> +             box++;
> +             glDrawArraysInstanced(GL_TRIANGLE_STRIP, 0, 4, n);
> +         }
> +    }
> +
> +    glDisable(GL_SCISSOR_TEST);
> +    glVertexAttribDivisor(GLAMOR_VERTEX_POS, 0);
> +    glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
> +
> +    glamor_put_context(glamor_priv);
> +    return;
> +bail:
> +    glamor_fallback("to %p (%c)\n", drawable,
> +                    glamor_get_drawable_location(drawable));
> +    if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW) &&
> +        glamor_prepare_access_gc(gc)) {
> +        fbFillSpans(drawable, gc, n, points, widths, sorted);
> +    }
> +    glamor_finish_access_gc(gc);
> +    glamor_finish_access(drawable);
> +}
> +
> +void
> +glamor_fini_fillspans_shader(ScreenPtr screen)
> +{
> +    glamor_screen_private *glamor_priv =
> glamor_get_screen_private(screen);
> +
> +    glamor_delete_program_fill(screen,
> &glamor_priv->fill_spans_program);
> +}
> +