[PATCH 7/7] glamor: Replace CompositeGlyphs code
Eric Anholt
eric at anholt.net
Tue May 12 14:29:40 PDT 2015
Keith Packard <keithp at keithp.com> writes:
> New composite glyphs code uses the updated glamor program
> infrastructure to create efficient shaders for drawing render text.
>
> Glyphs are cached in two atlases (one 8-bit, one 32-bit) in a simple
> linear fashion. When the atlas fills, it is discarded and a new one
> constructed.
> diff --git a/glamor/glamor_composite_glyphs.c b/glamor/glamor_composite_glyphs.c
> new file mode 100644
> index 0000000..e609acf
> --- /dev/null
> +++ b/glamor/glamor_composite_glyphs.c
> @@ -0,0 +1,553 @@
> +/*
> + * 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 <stdlib.h>
> +#include "Xprintf.h"
> +
> +#include "glamor_priv.h"
> +#include "glamor_transform.h"
> +#include "glamor_transfer.h"
> +
> +#include <mipict.h>
> +
> +#define DEFAULT_ATLAS_DIM 1024
> +
> +static DevPrivateKeyRec glamor_glyph_private_key;
> +
> +struct glamor_glyph_private {
> + int16_t x;
> + int16_t y;
> + uint32_t serial;
> +};
> +
> +struct glamor_glyph_atlas {
> + PicturePtr atlas;
> + PictFormatPtr format;
> + int dim;
> + int max_glyph_dim;
> + int x, y;
> + int row_height;
> + int nglyph;
> + uint32_t serial;
> +};
> +
> +static inline struct glamor_glyph_private *glamor_get_glyph_private(PixmapPtr pixmap) {
> + return dixLookupPrivate(&pixmap->devPrivates, &glamor_glyph_private_key);
> +}
> +
> +static inline void
> +glamor_copy_glyph(PixmapPtr glyph_pixmap,
> + DrawablePtr atlas_draw,
> + int16_t x,
> + int16_t y)
> +{
> + DrawablePtr glyph_draw = &glyph_pixmap->drawable;
> + BoxRec box = {
> + .x1 = 0,
> + .y1 = 0,
> + .x2 = glyph_draw->width,
> + .y2 = glyph_draw->height,
> + };
> +
> + if (glyph_pixmap->drawable.bitsPerPixel == atlas_draw->bitsPerPixel) {
> + glamor_upload_boxes((PixmapPtr) atlas_draw,
> + &box, 1,
> + 0, 0,
> + x, y,
> + glyph_pixmap->devPrivate.ptr,
> + glyph_pixmap->devKind);
> + } else {
I think this explanation is what I wanted in this block:
/* If we're dealing with 1-bit glyphs, we upload them to
* the cache as normal 8-bit alpha, since that's what GL
* can handle.
*/
assert(glyph_draw->bitsPerPixel == 1);
assert(atlas_draw->bitsPerPixel == 8);
> + GCPtr scratch_gc = GetScratchGC(atlas_draw->depth, atlas_draw->pScreen);
> + ChangeGCVal changes[2];
> + if (!scratch_gc)
> + return;
> +
> + changes[0].val = 0xff;
> + changes[1].val = 0x00;
> + if (ChangeGC(NullClient, scratch_gc,
> + GCForeground|GCBackground, changes) != Success)
> + goto bail_gc;
> + ValidateGC(atlas_draw, scratch_gc);
> +
> + (*scratch_gc->ops->CopyPlane)(glyph_draw,
> + atlas_draw,
> + scratch_gc,
> + 0, 0,
> + glyph_draw->width,
> + glyph_draw->height,
> + x, y, 0x1);
> +
> + bail_gc:
> + FreeScratchGC(scratch_gc);
> + }
> +}
> +
> +static Bool
> +glamor_glyph_atlas_init(ScreenPtr screen, struct glamor_glyph_atlas *atlas)
> +{
> + PixmapPtr pixmap;
> + int error;
> + PictFormatPtr format = atlas->format;
> +
> + pixmap = glamor_create_pixmap(screen, atlas->dim, atlas->dim, format->depth, 0);
> + if (!pixmap)
> + return FALSE;
> + atlas->atlas = CreatePicture(0, &pixmap->drawable, format, 0, NULL, serverClient, &error);
It looks like this picture is never used, just the pixmap it's
wrapping. Store the pixmap, instead?
> + atlas->x = 0;
> + atlas->y = 0;
> + atlas->row_height = 0;
> + atlas->serial++;
> + ErrorF("Create new atlas depth %d serial %d nglyph %d\n", format->depth, atlas->serial, atlas->nglyph);
Debugging errorf left over
> + atlas->nglyph = 0;
> + glamor_destroy_pixmap(pixmap);
> + return TRUE;
> +}
> +
> +static Bool
> +glamor_glyph_can_add(struct glamor_glyph_atlas *atlas, PicturePtr glyph)
> +{
> + DrawablePtr glyph_draw = glyph->pDrawable;
> +
> + /* Step down */
> + if (atlas->x + glyph_draw->width > atlas->dim) {
> + atlas->x = 0;
> + atlas->y += atlas->row_height;
> + atlas->row_height =0;
missing space
> + }
> +
> + /* Check for overfull */
> + if (atlas->y + glyph_draw->height > atlas->dim)
> + return FALSE;
> +
> + return TRUE;
> +}
> +static const glamor_facet glamor_facet_composite_glyphs_130 = {
> + .name = "composite_glyphs",
> + .version = 130,
> + .vs_vars = ("attribute vec4 primitive;\n"
> + "attribute vec2 source;\n"
> + "varying vec2 glyph_pos;\n"),
> + .vs_exec = (" vec2 pos = primitive.zw * vec2(gl_VertexID&1, (gl_VertexID&2)>>1);\n"
> + GLAMOR_POS(gl_Position, (primitive.xy + pos))
> + " glyph_pos = (source + pos) / ATLAS_DIM;\n"),
> + .fs_vars = ("varying vec2 glyph_pos;\n"),
> + .fs_exec = (" vec4 mask = texture2D(atlas, glyph_pos);\n"),
> + .source_name = "source",
> + .locations = glamor_program_location_atlas,
> +};
> +
> +static const glamor_facet glamor_facet_composite_glyphs_120 = {
> + .name = "composite_glyphs",
> + .vs_vars = ("attribute vec2 primitive;\n"
> + "attribute vec2 source;\n"
> + "varying vec2 glyph_pos;\n"),
> + .vs_exec = (" vec2 pos = vec2(0.0,0.0);\n"
Isn't vec2 pos unused here? Drop this line of source?
> + GLAMOR_POS(gl_Position, primitive)
> + " glyph_pos = source.xy / ATLAS_DIM.0;\n"),
> + .fs_vars = ("varying vec2 glyph_pos;\n"),
> + .fs_exec = (" vec4 mask = texture2D(atlas, glyph_pos);\n"),
> + .fs_exec = (" vec4 mask = texture2D(atlas, glyph_pos);\n"),
duplicated line here
> + .source_name = "source",
> + .locations = glamor_program_location_atlas,
> +};
> +static void
> +glamor_glyphs_flush(CARD8 op, PicturePtr src, PicturePtr dst,
> + glamor_program *prog,
> + struct glamor_glyph_atlas *atlas, int nglyph)
> +{
> + DrawablePtr drawable = dst->pDrawable;
> + glamor_screen_private *glamor_priv = glamor_get_screen_private(drawable->pScreen);
> + PicturePtr atlas_pict = atlas->atlas;
> + PixmapPtr atlas_pixmap = glamor_get_drawable_pixmap(atlas_pict->pDrawable);
> + glamor_pixmap_private *atlas_priv = glamor_get_pixmap_private(atlas_pixmap);
> + glamor_pixmap_fbo *atlas_fbo = glamor_pixmap_fbo_at(atlas_priv, 0, 0);
> + PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
> + glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap);
> + int box_x, box_y;
> + int off_x, off_y;
> +
> + glamor_put_vbo_space(drawable->pScreen);
> +
> + glEnable(GL_SCISSOR_TEST);
> + glActiveTexture(GL_TEXTURE1);
> + glBindTexture(GL_TEXTURE_2D, atlas_fbo->tex);
> +
> + for (;;) {
> + if (!glamor_use_program_render(prog, op, src, dst))
> + break;
> +
> + glUniform1i(prog->atlas_uniform, 1);
> +
> + glamor_pixmap_loop(pixmap_priv, box_x, box_y) {
> + BoxPtr box = RegionRects(dst->pCompositeClip);
> + int nbox = RegionNumRects(dst->pCompositeClip);
> +
> + glamor_set_destination_drawable(drawable, box_x, box_y, TRUE, FALSE, prog->matrix_uniform, &off_x, &off_y);
> +
> + /* Run over the clip list, drawing the glyphs
> + * in each box
> + */
> +
> + while (nbox--) {
> + glScissor(box->x1 + off_x,
> + box->y1 + off_y,
> + box->x2 - box->x1,
> + box->y2 - box->y1);
> + box++;
> +
> + if (glamor_glyph_use_130(glamor_priv))
> + glDrawArraysInstanced(GL_TRIANGLE_STRIP, 0, 4, nglyph);
> + else
> + glDrawArrays(GL_TRIANGLES, 0, nglyph * 6);
> + }
> + }
> + if (prog->alpha != glamor_program_alpha_ca_first)
> + break;
> + prog++;
> + }
OK, this is the second copy of this loop, and it's still weird but I
don't really see a better way to do this.
> +
> + glDisable(GL_SCISSOR_TEST);
> +
> + glVertexAttribDivisor(GLAMOR_VERTEX_SOURCE, 0);
> + glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
> + glVertexAttribDivisor(GLAMOR_VERTEX_POS, 0);
> + glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
> + glDisable(GL_BLEND);
> +}
> +void
> +glamor_composite_glyphs(CARD8 op,
> + PicturePtr src,
> + PicturePtr dst,
> + PictFormatPtr glyph_format,
> + INT16 x_src,
> + INT16 y_src, int nlist, GlyphListPtr list,
> + GlyphPtr *glyphs)
> +{
> + int this_time;
> + GLshort *v = NULL;
> + DrawablePtr drawable = dst->pDrawable;
> + ScreenPtr screen = drawable->pScreen;
> + glamor_screen_private *glamor_priv = glamor_get_screen_private(screen);
> + glamor_program *prog = NULL;
> + PicturePtr glyph_pict = NULL;
> + DrawablePtr glyph_draw;
> + glamor_program_render *glyphs_program = &glamor_priv->glyphs_program;
> + struct glamor_glyph_atlas *glyph_atlas = NULL;
> + int x = 0, y = 0;
> + int n;
> + int nglyph = 0;
> +
> + for (n = 0; n < nlist; n++)
> + nglyph += list[n].len;
> +
> + glamor_make_current(glamor_priv);
> +
> + this_time = 0;
I think an appropriate name for this variable would be "glyphs_queued"
(or glyphs_in_vbo).
> +
> + while (nlist--) {
> + x += list->xOff;
> + y += list->yOff;
> + n = list->len;
> + list++;
> + while (n--) {
<snip>
(Wow, the contents of this loop look good, and that's impressive because
this is rather complicated).
> + }
> + }
> +
> + if (this_time)
> + glamor_glyphs_flush(op, src, dst, prog, glyph_atlas, this_time);
> +
> + return;
> +}
My feedback all seems minor to me. With however much of it you want to
take,
Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20150512/0a1f951b/attachment-0001.sig>
More information about the xorg-devel
mailing list