Eric Anholt <[email protected]> writes: > Fixes regressions since my "don't make an FBO for the glyph atlas" > change. The a1 upload was a fallback, as I expected. However, > fallback reads use glReadPixels() because there's no > glGetTexSubImage2D() to match glTexSubImage2D(). We were just binding > the 0 FBO value, so the glReadPixels() would throw a GL error instead > of getting any data. After the fallback was done we'd write back the > undefined data to the atlas, blowing away the entire rest of the atlas > because we didn't specify any bounds on our prepare. > > To fix the fallbacks to actually work, we'd need a prepare path that > allocates some memory memory do a full glGetTexImage() into, then > memcpy out of that. Instead, just dodge the general fallback by > implementing the specific upload we need to do here, which should also > be *much* faster at uploading a1 glyphs since it's not > readpixels/texsubimaging back and forth. > > Signed-off-by: Eric Anholt <[email protected]>
Nice fix. However, it's doing a CopyPlane operation by hand, making it LSB-specific. A shorter version just creates a temporary 8bpp CPU pixmap, uses CopyPlane to get the glyph into that and uploads from there. This works for me on ephyr
From 7b0c36d72221ab60b8533806c7bc0e23d9dc72e0 Mon Sep 17 00:00:00 2001 From: Keith Packard <[email protected]> Date: Thu, 2 Jul 2015 17:06:27 -0600 Subject: [PATCH] glamor: Fix bad rendering of glyphs after an a1 glyph upload. (v3) Fixes regressions since my "don't make an FBO for the glyph atlas" change. The a1 upload was a fallback, as I expected. However, fallback reads use glReadPixels() because there's no glGetTexSubImage2D() to match glTexSubImage2D(). We were just binding the 0 FBO value, so the glReadPixels() would throw a GL error instead of getting any data. After the fallback was done we'd write back the undefined data to the atlas, blowing away the entire rest of the atlas because we didn't specify any bounds on our prepare. To fix the fallbacks to actually work, we'd need a prepare path that allocates some memory memory do a full glGetTexImage() into, then memcpy out of that. Instead, just dodge the general fallback by implementing the specific upload we need to do here, which should also be *much* faster at uploading a1 glyphs since it's not readpixels/texsubimaging back and forth. v3: Use CopyPlane to a temp pixmap for the upload Signed-off-by: Keith Packard <[email protected]> --- glamor/glamor_composite_glyphs.c | 63 ++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/glamor/glamor_composite_glyphs.c b/glamor/glamor_composite_glyphs.c index e92f1d9..cd88524 100644 --- a/glamor/glamor_composite_glyphs.c +++ b/glamor/glamor_composite_glyphs.c @@ -64,45 +64,58 @@ glamor_copy_glyph(PixmapPtr glyph_pixmap, .x2 = glyph_draw->width, .y2 = glyph_draw->height, }; + PixmapPtr upload_pixmap = glyph_pixmap; - 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 { - GCPtr scratch_gc = GetScratchGC(atlas_draw->depth, atlas_draw->pScreen); - ChangeGCVal changes[2]; - if (!scratch_gc) - return; + if (glyph_pixmap->drawable.bitsPerPixel != atlas_draw->bitsPerPixel) { - /* 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. + /* If we're dealing with 1-bit glyphs, we copy them to a + * temporary 8-bit pixmap and upload them from there, since + * that's what GL can handle. */ - assert(glyph_draw->depth == 1); - assert(atlas_draw->depth == 8); + ScreenPtr screen = atlas_draw->pScreen; + GCPtr scratch_gc; + ChangeGCVal changes[2]; + + upload_pixmap = glamor_create_pixmap(screen, + glyph_draw->width, + glyph_draw->height, + atlas_draw->depth, + GLAMOR_CREATE_PIXMAP_CPU); + if (!upload_pixmap) + return; + scratch_gc = GetScratchGC(upload_pixmap->drawable.depth, screen); + if (!scratch_gc) { + glamor_destroy_pixmap(upload_pixmap); + 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); + GCForeground|GCBackground, changes) != Success) { + glamor_destroy_pixmap(upload_pixmap); + FreeScratchGC(scratch_gc); + return; + } + ValidateGC(&upload_pixmap->drawable, scratch_gc); (*scratch_gc->ops->CopyPlane)(glyph_draw, - atlas_draw, + &upload_pixmap->drawable, scratch_gc, 0, 0, glyph_draw->width, glyph_draw->height, - x, y, 0x1); - - bail_gc: - FreeScratchGC(scratch_gc); + 0, 0, 0x1); } + glamor_upload_boxes((PixmapPtr) atlas_draw, + &box, 1, + 0, 0, + x, y, + upload_pixmap->devPrivate.ptr, + upload_pixmap->devKind); + + if (upload_pixmap != glyph_pixmap) + glamor_destroy_pixmap(upload_pixmap); } static Bool -- 2.1.4
-- -keith
signature.asc
Description: PGP signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
