From dc8a528cd6b9a4da3e60fa31428c37f5b34a897f Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Wed, 25 Jul 2007 14:57:13 -0700 Subject: ProcRenderAddGlyphs: Convert while loops to for loops where more natural --- render/render.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'render') diff --git a/render/render.c b/render/render.c index caaa2781c..d311fb383 100644 --- a/render/render.c +++ b/render/render.c @@ -1098,6 +1098,7 @@ ProcRenderAddGlyphs (ClientPtr client) CARD8 *bits; int size; int err = BadAlloc; + int i; REQUEST_AT_LEAST_SIZE(xRenderAddGlyphsReq); glyphSet = (GlyphSetPtr) SecurityLookupIDByType (client, @@ -1131,7 +1132,7 @@ ProcRenderAddGlyphs (ClientPtr client) gi = (xGlyphInfo *) (gids + nglyphs); bits = (CARD8 *) (gi + nglyphs); remain -= (sizeof (CARD32) + sizeof (xGlyphInfo)) * nglyphs; - while (remain >= 0 && nglyphs) + for (i = 0; i < nglyphs; i++) { glyph = AllocateGlyph (gi, glyphSet->fdepth); if (!glyph) @@ -1155,21 +1156,19 @@ ProcRenderAddGlyphs (ClientPtr client) gi++; gids++; glyphs++; - nglyphs--; } - if (nglyphs || remain) + if (remain || i < nglyphs) { err = BadLength; goto bail; } - nglyphs = stuff->nglyphs; if (!ResizeGlyphSet (glyphSet, nglyphs)) { err = BadAlloc; goto bail; } glyphs = glyphsBase; - while (nglyphs--) { + for (i = 0; i < nglyphs; i++) { AddGlyph (glyphSet, glyphs->glyph, glyphs->id); glyphs++; } -- cgit v1.2.3 From 363d764ea32b938f3dff35df7cf3370363c04d5c Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Mon, 30 Jul 2007 15:10:11 -0700 Subject: ProcRenderAddGlyphs: Take advantage of the for loops to simplify the code a bit --- render/render.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'render') diff --git a/render/render.c b/render/render.c index d311fb383..10a5b805c 100644 --- a/render/render.c +++ b/render/render.c @@ -1134,7 +1134,7 @@ ProcRenderAddGlyphs (ClientPtr client) remain -= (sizeof (CARD32) + sizeof (xGlyphInfo)) * nglyphs; for (i = 0; i < nglyphs; i++) { - glyph = AllocateGlyph (gi, glyphSet->fdepth); + glyph = AllocateGlyph (&gi[i], glyphSet->fdepth); if (!glyph) { err = BadAlloc; @@ -1142,7 +1142,7 @@ ProcRenderAddGlyphs (ClientPtr client) } glyphs->glyph = glyph; - glyphs->id = *gids; + glyphs->id = gids[i]; size = glyph->size - sizeof (xGlyphInfo); if (remain < size) @@ -1153,8 +1153,6 @@ ProcRenderAddGlyphs (ClientPtr client) size += 4 - (size & 3); bits += size; remain -= size; - gi++; - gids++; glyphs++; } if (remain || i < nglyphs) @@ -1168,10 +1166,8 @@ ProcRenderAddGlyphs (ClientPtr client) goto bail; } glyphs = glyphsBase; - for (i = 0; i < nglyphs; i++) { - AddGlyph (glyphSet, glyphs->glyph, glyphs->id); - glyphs++; - } + for (i = 0; i < nglyphs; i++) + AddGlyph (glyphSet, glyphs[i].glyph, glyphs[i].id); if (glyphsBase != glyphsLocal) Xfree (glyphsBase); -- cgit v1.2.3 From 4c6abe1c7c8abcf203572bbf86b21d97ea4e756f Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Mon, 30 Jul 2007 21:43:20 -0700 Subject: Split HashGlyph functionality out into HashGlyphInfoAndBits This is in preparation for a future change that will take advantage of being able to compute a hash for a separate xGlyphInfo and chunk of bits without a combined Glyph object. --- render/glyph.c | 23 ++++++++++++++++++++--- render/glyphstr.h | 3 +++ 2 files changed, 23 insertions(+), 3 deletions(-) (limited to 'render') diff --git a/render/glyph.c b/render/glyph.c index 583a52ba3..53c00b326 100644 --- a/render/glyph.c +++ b/render/glyph.c @@ -461,18 +461,35 @@ FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare) } CARD32 -HashGlyph (GlyphPtr glyph) +HashGlyphInfoAndBits (xGlyphInfo *gi, CARD8 *data, unsigned int size) { - CARD32 *bits = (CARD32 *) &(glyph->info); + CARD32 *bits; CARD32 hash; - int n = glyph->size / sizeof (CARD32); + int n; hash = 0; + + bits = (CARD32 *) gi; + n = sizeof (xGlyphInfo) / sizeof (CARD32); + while (n--) + hash ^= *bits++; + + bits = (CARD32 *) data; + n = size / sizeof (CARD32); while (n--) hash ^= *bits++; + return hash; } +CARD32 +HashGlyph (GlyphPtr glyph) +{ + return HashGlyphInfoAndBits (&glyph->info, + (CARD8 *) (&glyph->info + 1), + glyph->size - sizeof (xGlyphInfo)); +} + #ifdef CHECK_DUPLICATES void DuplicateRef (GlyphPtr glyph, char *where) diff --git a/render/glyphstr.h b/render/glyphstr.h index 22150deee..b941dabaf 100644 --- a/render/glyphstr.h +++ b/render/glyphstr.h @@ -129,6 +129,9 @@ FindGlyphHashSet (CARD32 filled); GlyphRefPtr FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare); +CARD32 +HashGlyphInfoAndBits (xGlyphInfo *gi, CARD8 *data, unsigned int size); + CARD32 HashGlyph (GlyphPtr glyph); -- cgit v1.2.3 From 516b96387b0e57b524a37a96da22dbeeeb041712 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Mon, 30 Jul 2007 17:31:47 -0700 Subject: ProcRenderAddGlyphs: Avoid allocating a glyph just to find it cached This is a cleanup without any real savings (yet). Previously, the implementation would allocate a new glyph, then (often) find it in the cache, and immediately discard the allocated object. This re-organization first uses a new FindGlyphByHash function and only allocates the glyph if nothing is found. This isn't a real savings yet, since FindGlyphByHash currently still does a temporary glyph allocation, but this is expected to be replaced immediately as we switch to an alternate hashing mechanism (SHA1). --- render/glyph.c | 29 ++++++++++++++++++++++++-- render/glyphstr.h | 6 ++++++ render/render.c | 61 ++++++++++++++++++++++++++++++++++--------------------- 3 files changed, 71 insertions(+), 25 deletions(-) (limited to 'render') diff --git a/render/glyph.c b/render/glyph.c index 53c00b326..1204c3b79 100644 --- a/render/glyph.c +++ b/render/glyph.c @@ -490,6 +490,31 @@ HashGlyph (GlyphPtr glyph) glyph->size - sizeof (xGlyphInfo)); } +GlyphPtr +FindGlyphByHash (CARD32 hash, + xGlyphInfo *gi, + CARD8 *bits, + int format) +{ + GlyphRefPtr gr; + GlyphPtr template; + + /* XXX: Should handle out-of-memory here */ + template = AllocateGlyph (gi, format); + memcpy ((CARD8 *) (template + 1), bits, + template->size - sizeof (xGlyphInfo)); + + gr = FindGlyphRef (&globalGlyphs[format], + hash, TRUE, template); + + xfree (template); + + if (gr->glyph && gr->glyph != DeletedGlyph) + return gr->glyph; + else + return NULL; +} + #ifdef CHECK_DUPLICATES void DuplicateRef (GlyphPtr glyph, char *where) @@ -572,7 +597,7 @@ AddGlyph (GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id) /* Locate existing matching glyph */ hash = HashGlyph (glyph); gr = FindGlyphRef (&globalGlyphs[glyphSet->fdepth], hash, TRUE, glyph); - if (gr->glyph && gr->glyph != DeletedGlyph) + if (gr->glyph && gr->glyph != DeletedGlyph && gr->glyph != glyph) { PictureScreenPtr ps; int i; @@ -588,7 +613,7 @@ AddGlyph (GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id) xfree (glyph); glyph = gr->glyph; } - else + else if (gr->glyph != glyph) { gr->glyph = glyph; gr->signature = hash; diff --git a/render/glyphstr.h b/render/glyphstr.h index b941dabaf..37462f744 100644 --- a/render/glyphstr.h +++ b/render/glyphstr.h @@ -129,6 +129,12 @@ FindGlyphHashSet (CARD32 filled); GlyphRefPtr FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare); +GlyphPtr +FindGlyphByHash (CARD32 hash, + xGlyphInfo *gi, + CARD8 *bits, + int format); + CARD32 HashGlyphInfoAndBits (xGlyphInfo *gi, CARD8 *data, unsigned int size); diff --git a/render/render.c b/render/render.c index 10a5b805c..831c98417 100644 --- a/render/render.c +++ b/render/render.c @@ -1082,6 +1082,8 @@ ProcRenderFreeGlyphSet (ClientPtr client) typedef struct _GlyphNew { Glyph id; GlyphPtr glyph; + Bool found; + CARD32 hash; } GlyphNewRec, *GlyphNewPtr; static int @@ -1090,8 +1092,7 @@ ProcRenderAddGlyphs (ClientPtr client) GlyphSetPtr glyphSet; REQUEST(xRenderAddGlyphsReq); GlyphNewRec glyphsLocal[NLOCALGLYPH]; - GlyphNewPtr glyphsBase, glyphs; - GlyphPtr glyph; + GlyphNewPtr glyphsBase, glyphs, glyph_new; int remain, nglyphs; CARD32 *gids; xGlyphInfo *gi; @@ -1115,11 +1116,13 @@ ProcRenderAddGlyphs (ClientPtr client) if (nglyphs > UINT32_MAX / sizeof(GlyphNewRec)) return BadAlloc; - if (nglyphs <= NLOCALGLYPH) + if (nglyphs <= NLOCALGLYPH) { + memset (glyphsLocal, 0, sizeof (glyphsLocal)); glyphsBase = glyphsLocal; + } else { - glyphsBase = (GlyphNewPtr) Xalloc (nglyphs * sizeof (GlyphNewRec)); + glyphsBase = (GlyphNewPtr) Xcalloc (nglyphs * sizeof (GlyphNewRec)); if (!glyphsBase) return BadAlloc; } @@ -1134,26 +1137,41 @@ ProcRenderAddGlyphs (ClientPtr client) remain -= (sizeof (CARD32) + sizeof (xGlyphInfo)) * nglyphs; for (i = 0; i < nglyphs; i++) { - glyph = AllocateGlyph (&gi[i], glyphSet->fdepth); - if (!glyph) - { - err = BadAlloc; - goto bail; - } - - glyphs->glyph = glyph; - glyphs->id = gids[i]; - - size = glyph->size - sizeof (xGlyphInfo); + glyph_new = &glyphs[i]; + size = gi[i].height * PixmapBytePad (gi[i].width, + glyphSet->format->depth); if (remain < size) break; - memcpy ((CARD8 *) (glyph + 1), bits, size); + + glyph_new->hash = HashGlyphInfoAndBits (&gi[i], bits, size); + + glyph_new->glyph = FindGlyphByHash (glyph_new->hash, + &gi[i], bits, + glyphSet->fdepth); + + if (glyph_new->glyph && glyph_new->glyph != DeletedGlyph) + { + glyph_new->found = TRUE; + } + else + { + glyph_new->found = FALSE; + glyph_new->glyph = AllocateGlyph (&gi[i], glyphSet->fdepth); + if (! glyph_new->glyph) + { + err = BadAlloc; + goto bail; + } + + memcpy ((CARD8 *) (glyph_new->glyph + 1), bits, size); + } + + glyph_new->id = gids[i]; if (size & 3) size += 4 - (size & 3); bits += size; remain -= size; - glyphs++; } if (remain || i < nglyphs) { @@ -1165,7 +1183,6 @@ ProcRenderAddGlyphs (ClientPtr client) err = BadAlloc; goto bail; } - glyphs = glyphsBase; for (i = 0; i < nglyphs; i++) AddGlyph (glyphSet, glyphs[i].glyph, glyphs[i].id); @@ -1173,11 +1190,9 @@ ProcRenderAddGlyphs (ClientPtr client) Xfree (glyphsBase); return client->noClientException; bail: - while (glyphs != glyphsBase) - { - --glyphs; - xfree (glyphs->glyph); - } + for (i = 0; i < nglyphs; i++) + if (glyphs[i].glyph && ! glyphs[i].found) + xfree (glyphs[i].glyph); if (glyphsBase != glyphsLocal) Xfree (glyphsBase); return err; -- cgit v1.2.3 From 19b3b1fd8feb343a690331cafe88ef10b34b9d98 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Tue, 31 Jul 2007 17:04:13 -0700 Subject: Use strong hash (SHA1) for glyphs Using a cryptographically strong hash means that comparing the hash alone is sufficient for determining glyph equality (no need to compare the glyph bits directly). This will allow us to replace system-memory copies of the glyph bits, (which we've only been holding onto for comparisons), with Pixmaps. --- configure.ac | 2 +- render/glyph.c | 84 ++++++++++++++++++++++++++----------------------------- render/glyphstr.h | 29 +++++++++---------- render/render.c | 16 ++++++----- 4 files changed, 65 insertions(+), 66 deletions(-) (limited to 'render') diff --git a/configure.ac b/configure.ac index 518f332a8..89c6b55b1 100644 --- a/configure.ac +++ b/configure.ac @@ -621,7 +621,7 @@ PIXMAN="[pixman >= 0.9.2]" dnl Core modules for most extensions, et al. REQUIRED_MODULES="[randrproto >= 1.2] renderproto [fixesproto >= 4.0] [damageproto >= 1.1] xcmiscproto xextproto [xproto >= 7.0.9] xtrans [scrnsaverproto >= 1.1] bigreqsproto resourceproto fontsproto [inputproto >= 1.4.2] [kbproto >= 1.0.3]" -REQUIRED_LIBS="xfont xau fontenc $PIXMAN" +REQUIRED_LIBS="xfont xau fontenc $PIXMAN openssl" dnl HAVE_DBUS is true if we actually have the D-Bus library, whereas dnl CONFIG_DBUS_API is true if we want to enable the D-Bus config diff --git a/render/glyph.c b/render/glyph.c index 1204c3b79..7dbdda2d9 100644 --- a/render/glyph.c +++ b/render/glyph.c @@ -26,6 +26,8 @@ #include #endif +#include + #include "misc.h" #include "scrnintstr.h" #include "os.h" @@ -412,7 +414,10 @@ _GlyphSetSetNewPrivate (GlyphSetPtr glyphSet, int n, pointer ptr) } GlyphRefPtr -FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare) +FindGlyphRef (GlyphHashPtr hash, + CARD32 signature, + Bool match, + unsigned char sha1[20]) { CARD32 elt, step, s; GlyphPtr glyph; @@ -443,7 +448,7 @@ FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare) } else if (s == signature && (!match || - memcmp (&compare->info, &glyph->info, compare->size) == 0)) + memcmp (glyph->sha1, sha1, 20) == 0)) { break; } @@ -460,54 +465,42 @@ FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare) return gr; } -CARD32 -HashGlyphInfoAndBits (xGlyphInfo *gi, CARD8 *data, unsigned int size) +int +HashGlyph (xGlyphInfo *gi, + CARD8 *bits, + unsigned long size, + unsigned char sha1[20]) { - CARD32 *bits; - CARD32 hash; - int n; + SHA_CTX ctx; + int success; - hash = 0; + success = SHA1_Init (&ctx); + if (! success) + return BadAlloc; - bits = (CARD32 *) gi; - n = sizeof (xGlyphInfo) / sizeof (CARD32); - while (n--) - hash ^= *bits++; + success = SHA1_Update (&ctx, gi, sizeof (xGlyphInfo)); + if (! success) + return BadAlloc; - bits = (CARD32 *) data; - n = size / sizeof (CARD32); - while (n--) - hash ^= *bits++; + success = SHA1_Update (&ctx, bits, size); + if (! success) + return BadAlloc; - return hash; -} + success = SHA1_Final (sha1, &ctx); + if (! success) + return BadAlloc; -CARD32 -HashGlyph (GlyphPtr glyph) -{ - return HashGlyphInfoAndBits (&glyph->info, - (CARD8 *) (&glyph->info + 1), - glyph->size - sizeof (xGlyphInfo)); + return Success; } GlyphPtr -FindGlyphByHash (CARD32 hash, - xGlyphInfo *gi, - CARD8 *bits, - int format) +FindGlyphByHash (unsigned char sha1[20], int format) { GlyphRefPtr gr; - GlyphPtr template; - - /* XXX: Should handle out-of-memory here */ - template = AllocateGlyph (gi, format); - memcpy ((CARD8 *) (template + 1), bits, - template->size - sizeof (xGlyphInfo)); + CARD32 signature = *(CARD32 *) sha1; gr = FindGlyphRef (&globalGlyphs[format], - hash, TRUE, template); - - xfree (template); + signature, TRUE, sha1); if (gr->glyph && gr->glyph != DeletedGlyph) return gr->glyph; @@ -553,6 +546,7 @@ FreeGlyph (GlyphPtr glyph, int format) GlyphRefPtr gr; int i; int first; + CARD32 signature; first = -1; for (i = 0; i < globalGlyphs[format].hashSet->size; i++) @@ -563,8 +557,9 @@ FreeGlyph (GlyphPtr glyph, int format) first = i; } - gr = FindGlyphRef (&globalGlyphs[format], - HashGlyph (glyph), TRUE, glyph); + signature = *(CARD32 *) glyph->sha1; + gr = FindGlyphRef (&globalGlyphs[format], signature, + TRUE, glyph->sha1); if (gr - globalGlyphs[format].table != first) DuplicateRef (glyph, "Found wrong one"); if (gr->glyph && gr->glyph != DeletedGlyph) @@ -591,12 +586,13 @@ void AddGlyph (GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id) { GlyphRefPtr gr; - CARD32 hash; + CARD32 signature; CheckDuplicates (&globalGlyphs[glyphSet->fdepth], "AddGlyph top global"); /* Locate existing matching glyph */ - hash = HashGlyph (glyph); - gr = FindGlyphRef (&globalGlyphs[glyphSet->fdepth], hash, TRUE, glyph); + signature = *(CARD32 *) glyph->sha1; + gr = FindGlyphRef (&globalGlyphs[glyphSet->fdepth], signature, + TRUE, glyph->sha1); if (gr->glyph && gr->glyph != DeletedGlyph && gr->glyph != glyph) { PictureScreenPtr ps; @@ -616,7 +612,7 @@ AddGlyph (GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id) else if (gr->glyph != glyph) { gr->glyph = glyph; - gr->signature = hash; + gr->signature = signature; globalGlyphs[glyphSet->fdepth].tableEntries++; } @@ -753,7 +749,7 @@ ResizeGlyphHash (GlyphHashPtr hash, CARD32 change, Bool global) if (glyph && glyph != DeletedGlyph) { s = hash->table[i].signature; - gr = FindGlyphRef (&newHash, s, global, glyph); + gr = FindGlyphRef (&newHash, s, global, glyph->sha1); gr->signature = s; gr->glyph = glyph; ++newHash.tableEntries; diff --git a/render/glyphstr.h b/render/glyphstr.h index 37462f744..d47dfecfc 100644 --- a/render/glyphstr.h +++ b/render/glyphstr.h @@ -39,10 +39,11 @@ #define GlyphFormatNum 5 typedef struct _Glyph { - CARD32 refcnt; - DevUnion *devPrivates; - CARD32 size; /* info + bitmap */ - xGlyphInfo info; + CARD32 refcnt; + DevUnion *devPrivates; + unsigned char sha1[20]; + CARD32 size; /* info + bitmap */ + xGlyphInfo info; /* bits follow */ } GlyphRec, *GlyphPtr; @@ -127,19 +128,19 @@ GlyphHashSetPtr FindGlyphHashSet (CARD32 filled); GlyphRefPtr -FindGlyphRef (GlyphHashPtr hash, CARD32 signature, Bool match, GlyphPtr compare); +FindGlyphRef (GlyphHashPtr hash, + CARD32 signature, + Bool match, + unsigned char sha1[20]); GlyphPtr -FindGlyphByHash (CARD32 hash, - xGlyphInfo *gi, - CARD8 *bits, - int format); - -CARD32 -HashGlyphInfoAndBits (xGlyphInfo *gi, CARD8 *data, unsigned int size); +FindGlyphByHash (unsigned char sha1[20], int format); -CARD32 -HashGlyph (GlyphPtr glyph); +int +HashGlyph (xGlyphInfo *gi, + CARD8 *bits, + unsigned long size, + unsigned char sha1[20]); void FreeGlyph (GlyphPtr glyph, int format); diff --git a/render/render.c b/render/render.c index 831c98417..c7a6dcb44 100644 --- a/render/render.c +++ b/render/render.c @@ -1080,10 +1080,10 @@ ProcRenderFreeGlyphSet (ClientPtr client) } typedef struct _GlyphNew { - Glyph id; - GlyphPtr glyph; - Bool found; - CARD32 hash; + Glyph id; + GlyphPtr glyph; + Bool found; + unsigned char sha1[20]; } GlyphNewRec, *GlyphNewPtr; static int @@ -1143,10 +1143,11 @@ ProcRenderAddGlyphs (ClientPtr client) if (remain < size) break; - glyph_new->hash = HashGlyphInfoAndBits (&gi[i], bits, size); + err = HashGlyph (&gi[i], bits, size, glyph_new->sha1); + if (err) + goto bail; - glyph_new->glyph = FindGlyphByHash (glyph_new->hash, - &gi[i], bits, + glyph_new->glyph = FindGlyphByHash (glyph_new->sha1, glyphSet->fdepth); if (glyph_new->glyph && glyph_new->glyph != DeletedGlyph) @@ -1164,6 +1165,7 @@ ProcRenderAddGlyphs (ClientPtr client) } memcpy ((CARD8 *) (glyph_new->glyph + 1), bits, size); + memcpy (glyph_new->glyph->sha1, glyph_new->sha1, 20); } glyph_new->id = gids[i]; -- cgit v1.2.3 From a2af34d5a861982a03afad8e586bb0181b72bbd0 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Wed, 1 Aug 2007 15:48:30 -0700 Subject: Use per-screen Pixmaps for glyphs Instead of system-memory data which prevents accelerated compositing of glyphs, (at least without forcing an upload of the glyph data before compositing). --- exa/exa_priv.h | 1 + exa/exa_render.c | 99 ++++++++----------------------------------------------- render/glyph.c | 55 ++++++++++++++++++++----------- render/glyphstr.h | 4 ++- render/miglyph.c | 40 ++++++---------------- render/render.c | 67 +++++++++++++++++++++++++++++++++++-- 6 files changed, 127 insertions(+), 139 deletions(-) (limited to 'render') diff --git a/exa/exa_priv.h b/exa/exa_priv.h index a6d98cd2d..a80704a21 100644 --- a/exa/exa_priv.h +++ b/exa/exa_priv.h @@ -50,6 +50,7 @@ #include "fboverlay.h" #ifdef RENDER #include "fbpict.h" +#include "glyphstr.h" #endif #include "damage.h" diff --git a/exa/exa_render.c b/exa/exa_render.c index 5e7c67feb..332683949 100644 --- a/exa/exa_render.c +++ b/exa/exa_render.c @@ -996,8 +996,6 @@ exaGlyphs (CARD8 op, { GCPtr pGC = NULL; int maxwidth = 0, maxheight = 0, i; - ExaMigrationRec pixmaps[1]; - PixmapPtr pScratchPixmap = NULL; x += list->xOff; y += list->yOff; @@ -1021,37 +1019,9 @@ exaGlyphs (CARD8 op, continue; } - /* Create the (real) temporary pixmap to store the current glyph in */ - pPixmap = (*pScreen->CreatePixmap) (pScreen, maxwidth, maxheight, - list->format->depth); - if (!pPixmap) - return; - - /* Create a temporary picture to wrap the temporary pixmap, so it can be - * used as a source for Composite. - */ - component_alpha = NeedsComponent(list->format->format); - pPicture = CreatePicture (0, &pPixmap->drawable, list->format, - CPComponentAlpha, &component_alpha, - serverClient, &error); - if (!pPicture) { - (*pScreen->DestroyPixmap) (pPixmap); - return; - } - ValidatePicture(pPicture); - - /* Give the temporary pixmap an initial kick towards the screen, so - * it'll stick there. - */ - pixmaps[0].as_dst = TRUE; - pixmaps[0].as_src = TRUE; - pixmaps[0].pPix = pPixmap; - exaDoMigration (pixmaps, 1, pExaScr->info->PrepareComposite != NULL); - while (n--) { GlyphPtr glyph = *glyphs++; - pointer glyphdata = (pointer) (glyph + 1); DrawablePtr pCmpDrw = (maskFormat ? pMask : pDst)->pDrawable; x1 = x - glyph->info.x; @@ -1061,60 +1031,19 @@ exaGlyphs (CARD8 op, (x1 + glyph->info.width) <= 0 || (y1 + glyph->info.height) <= 0) goto nextglyph; - (*pScreen->ModifyPixmapHeader) (pScratchPixmap, - glyph->info.width, - glyph->info.height, - 0, 0, -1, glyphdata); + /* The glyph already has a pixmap waiting for us to use. */ + pPixmap = GlyphPixmap (glyph)[pScreen->myNum]; - /* Copy the glyph data into the proper pixmap instead of a fake. - * First we try to use UploadToScreen, if we can, then we fall back - * to a plain exaCopyArea in case of failure. + /* Create a temporary picture to wrap the pixmap, so it can be + * used as a source for Composite. */ - if (pExaScr->info->UploadToScreen && - exaPixmapIsOffscreen(pPixmap) && - (*pExaScr->info->UploadToScreen) (pPixmap, 0, 0, - glyph->info.width, - glyph->info.height, - glyphdata, - PixmapBytePad(glyph->info.width, - list->format->depth))) - { - exaMarkSync (pScreen); - } else { - /* Set up the scratch pixmap/GC for doing a CopyArea. */ - if (pScratchPixmap == NULL) { - /* Get a scratch pixmap to wrap the original glyph data */ - pScratchPixmap = GetScratchPixmapHeader (pScreen, - glyph->info.width, - glyph->info.height, - list->format->depth, - list->format->depth, - -1, glyphdata); - if (!pScratchPixmap) { - FreePicture(pPicture, 0); - (*pScreen->DestroyPixmap) (pPixmap); - return; - } - - /* Get a scratch GC with which to copy the glyph data from - * scratch to temporary - */ - pGC = GetScratchGC (list->format->depth, pScreen); - ValidateGC (&pPixmap->drawable, pGC); - } else { - (*pScreen->ModifyPixmapHeader) (pScratchPixmap, - glyph->info.width, - glyph->info.height, - 0, 0, -1, glyphdata); - pScratchPixmap->drawable.serialNumber = NEXT_SERIAL_NUMBER; - } - - exaCopyArea (&pScratchPixmap->drawable, &pPixmap->drawable, pGC, - 0, 0, glyph->info.width, glyph->info.height, 0, 0); - } - - exaPixmapDirty (pPixmap, 0, 0, - glyph->info.width, glyph->info.height); + component_alpha = NeedsComponent(list->format->format); + pPicture = CreatePicture (0, &pPixmap->drawable, list->format, + CPComponentAlpha, &component_alpha, + serverClient, &error); + if (!pPicture) + return; + ValidatePicture(pPicture); if (maskFormat) { @@ -1134,6 +1063,8 @@ exaGlyphs (CARD8 op, exaPixmapDirty(pDstPixmap, x1, y1, x1 + glyph->info.width, y1 + glyph->info.height); } + FreePicture ((pointer) pPicture, 0); + nextglyph: x += glyph->info.xOff; y += glyph->info.yOff; @@ -1141,10 +1072,6 @@ nextglyph: list++; if (pGC != NULL) FreeScratchGC (pGC); - FreePicture ((pointer) pPicture, 0); - (*pScreen->DestroyPixmap) (pPixmap); - if (pScratchPixmap != NULL) - FreeScratchPixmapHeader (pScratchPixmap); } if (maskFormat) { diff --git a/render/glyph.c b/render/glyph.c index 7dbdda2d9..7fd3705df 100644 --- a/render/glyph.c +++ b/render/glyph.c @@ -571,9 +571,13 @@ FreeGlyph (GlyphPtr glyph, int format) for (i = 0; i < screenInfo.numScreens; i++) { - ps = GetPictureScreenIfSet (screenInfo.screens[i]); + ScreenPtr pScreen = screenInfo.screens[i]; + + (pScreen->DestroyPixmap) (GlyphPixmap (glyph)[i]); + + ps = GetPictureScreenIfSet (pScreen); if (ps) - (*ps->UnrealizeGlyph) (screenInfo.screens[i], glyph); + (*ps->UnrealizeGlyph) (pScreen, glyph); } if (glyph->devPrivates) @@ -665,7 +669,7 @@ AllocateGlyph (xGlyphInfo *gi, int fdepth) GlyphPtr glyph; int i; - size = gi->height * PixmapBytePad (gi->width, glyphDepths[fdepth]); + size = screenInfo.numScreens * sizeof (PixmapPtr); glyph = (GlyphPtr) xalloc (size + sizeof (GlyphRec)); if (!glyph) return 0; @@ -685,27 +689,38 @@ AllocateGlyph (xGlyphInfo *gi, int fdepth) for (i = 0; i < screenInfo.numScreens; i++) { - ps = GetPictureScreenIfSet (screenInfo.screens[i]); - if (ps) - { - if (!(*ps->RealizeGlyph) (screenInfo.screens[i], glyph)) - { - while (i--) - { - ps = GetPictureScreenIfSet (screenInfo.screens[i]); - if (ps) - (*ps->UnrealizeGlyph) (screenInfo.screens[i], glyph); - } - - if (glyph->devPrivates) - xfree (glyph->devPrivates); - xfree (glyph); - return 0; - } + ScreenPtr pScreen = screenInfo.screens[i]; + + GlyphPixmap (glyph)[i] = (pScreen->CreatePixmap) (pScreen, + gi->width, gi->height, + glyphDepths[fdepth]); + if (! GlyphPixmap (glyph)[i]) + goto bail; + + ps = GetPictureScreenIfSet (pScreen); + if (! ps) + continue; + + if (!(*ps->RealizeGlyph) (pScreen, glyph)) { + (pScreen->DestroyPixmap) (GlyphPixmap (glyph)[i]); + goto bail; } } return glyph; + +bail: + while (i--) + { + ps = GetPictureScreenIfSet (screenInfo.screens[i]); + if (ps) + (*ps->UnrealizeGlyph) (screenInfo.screens[i], glyph); + } + + if (glyph->devPrivates) + xfree (glyph->devPrivates); + xfree (glyph); + return 0; } Bool diff --git a/render/glyphstr.h b/render/glyphstr.h index d47dfecfc..4f8746003 100644 --- a/render/glyphstr.h +++ b/render/glyphstr.h @@ -44,9 +44,11 @@ typedef struct _Glyph { unsigned char sha1[20]; CARD32 size; /* info + bitmap */ xGlyphInfo info; - /* bits follow */ + /* per-screen pixmaps follow */ } GlyphRec, *GlyphPtr; +#define GlyphPixmap(glyph) ((PixmapPtr *) ((glyph) + 1)) + typedef struct _GlyphRef { CARD32 signature; GlyphPtr glyph; diff --git a/render/miglyph.c b/render/miglyph.c index 7968c90ea..2aa94bd09 100644 --- a/render/miglyph.c +++ b/render/miglyph.c @@ -112,7 +112,7 @@ miGlyphs (CARD8 op, GlyphListPtr list, GlyphPtr *glyphs) { - PixmapPtr pPixmap = 0; + PixmapPtr pPixmap; PicturePtr pPicture; PixmapPtr pMaskPixmap = 0; PicturePtr pMask; @@ -166,7 +166,6 @@ miGlyphs (CARD8 op, x = 0; y = 0; } - pPicture = 0; while (nlist--) { x += list->xOff; @@ -175,28 +174,14 @@ miGlyphs (CARD8 op, while (n--) { glyph = *glyphs++; + pPixmap = GlyphPixmap (glyph)[pScreen->myNum]; + component_alpha = NeedsComponent(list->format->format); + pPicture = CreatePicture (0, &pPixmap->drawable, list->format, + CPComponentAlpha, &component_alpha, + serverClient, &error); if (!pPicture) - { - pPixmap = GetScratchPixmapHeader (pScreen, glyph->info.width, glyph->info.height, - list->format->depth, - list->format->depth, - 0, (pointer) (glyph + 1)); - if (!pPixmap) - return; - component_alpha = NeedsComponent(list->format->format); - pPicture = CreatePicture (0, &pPixmap->drawable, list->format, - CPComponentAlpha, &component_alpha, - serverClient, &error); - if (!pPicture) - { - FreeScratchPixmapHeader (pPixmap); - return; - } - } - (*pScreen->ModifyPixmapHeader) (pPixmap, - glyph->info.width, glyph->info.height, - 0, 0, -1, (pointer) (glyph + 1)); - pPixmap->drawable.serialNumber = NEXT_SERIAL_NUMBER; + return; + if (maskFormat) { CompositePicture (PictOpAdd, @@ -224,17 +209,12 @@ miGlyphs (CARD8 op, glyph->info.width, glyph->info.height); } + FreePicture ((pointer) pPicture, 0); + x += glyph->info.xOff; y += glyph->info.yOff; } list++; - if (pPicture) - { - FreeScratchPixmapHeader (pPixmap); - FreePicture ((pointer) pPicture, 0); - pPicture = 0; - pPixmap = 0; - } } if (maskFormat) { diff --git a/render/render.c b/render/render.c index c7a6dcb44..4bad379f6 100644 --- a/render/render.c +++ b/render/render.c @@ -1099,7 +1099,9 @@ ProcRenderAddGlyphs (ClientPtr client) CARD8 *bits; int size; int err = BadAlloc; - int i; + int i, screen; + PicturePtr pSrc = NULL, pDst = NULL; + PixmapPtr pSrcPix = NULL, pDstPix = NULL; REQUEST_AT_LEAST_SIZE(xRenderAddGlyphsReq); glyphSet = (GlyphSetPtr) SecurityLookupIDByType (client, @@ -1164,7 +1166,62 @@ ProcRenderAddGlyphs (ClientPtr client) goto bail; } - memcpy ((CARD8 *) (glyph_new->glyph + 1), bits, size); + for (screen = 0; screen < screenInfo.numScreens; screen++) + { + int width = gi[i].width; + int height = gi[i].height; + int depth = glyphSet->format->depth; + ScreenPtr pScreen; + int error; + + pScreen = screenInfo.screens[screen]; + pSrcPix = GetScratchPixmapHeader (pScreen, + width, height, + depth, depth, + -1, bits); + if (! pSrcPix) + { + err = BadAlloc; + goto bail; + } + + pSrc = CreatePicture (0, &pSrcPix->drawable, + glyphSet->format, 0, NULL, + serverClient, &error); + if (! pSrc) + { + err = BadAlloc; + goto bail; + } + + pDstPix = GlyphPixmap (glyph_new->glyph)[screen]; + + pDst = CreatePicture (0, &pDstPix->drawable, + glyphSet->format, 0, NULL, + serverClient, &error); + if (! pDst) + { + err = BadAlloc; + goto bail; + } + + CompositePicture (PictOpSrc, + pSrc, + None, + pDst, + 0, 0, + 0, 0, + 0, 0, + width, height); + + FreePicture ((pointer) pSrc, 0); + pSrc = NULL; + FreePicture ((pointer) pDst, 0); + pDst = NULL; + FreeScratchPixmapHeader (pSrcPix); + pSrcPix = NULL; + } + memcpy (glyph_new->glyph->sha1, glyph_new->sha1, 20); } @@ -1192,6 +1249,12 @@ ProcRenderAddGlyphs (ClientPtr client) Xfree (glyphsBase); return client->noClientException; bail: + if (pSrc) + FreePicture ((pointer) pSrc, 0); + if (pDst) + FreePicture ((pointer) pDst, 0); + if (pSrcPix) + FreeScratchPixmapHeader (pSrcPix); for (i = 0; i < nglyphs; i++) if (glyphs[i].glyph && ! glyphs[i].found) xfree (glyphs[i].glyph); -- cgit v1.2.3 From 0a71e1542a07abc5e32501973a7cf6de3f641317 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Thu, 2 Aug 2007 22:48:32 -0700 Subject: Create a Picture as well as a Pixmap at the time of AllocateGlyph This avoids some inefficiency in creating a temporary Picture for every glyph at rendering time. My measurements with an i965 showed the previous patch causing a 10-15% slowdown for NoAccel and XAA cases, (while providing an 18% speedup for EXA). With this change, the NoAccel and XAA performance regression is eliminated, and the overall EXA speedup, (before any of the glyphs-as-pixmaps work), is now 32%. --- exa/exa_render.c | 16 ++-------------- render/glyph.c | 23 +++++++---------------- render/glyphstr.h | 2 +- render/miglyph.c | 9 +-------- render/render.c | 26 ++++++++++++++++---------- 5 files changed, 27 insertions(+), 49 deletions(-) (limited to 'render') diff --git a/exa/exa_render.c b/exa/exa_render.c index 332683949..24411dda7 100644 --- a/exa/exa_render.c +++ b/exa/exa_render.c @@ -896,7 +896,6 @@ exaGlyphs (CARD8 op, GlyphPtr *glyphs) { ExaScreenPriv (pDst->pDrawable->pScreen); - PixmapPtr pPixmap = NULL; PicturePtr pPicture; PixmapPtr pMaskPixmap = NULL; PixmapPtr pDstPixmap = exaGetDrawablePixmap(pDst->pDrawable); @@ -1031,18 +1030,8 @@ exaGlyphs (CARD8 op, (x1 + glyph->info.width) <= 0 || (y1 + glyph->info.height) <= 0) goto nextglyph; - /* The glyph already has a pixmap waiting for us to use. */ - pPixmap = GlyphPixmap (glyph)[pScreen->myNum]; - - /* Create a temporary picture to wrap the pixmap, so it can be - * used as a source for Composite. - */ - component_alpha = NeedsComponent(list->format->format); - pPicture = CreatePicture (0, &pPixmap->drawable, list->format, - CPComponentAlpha, &component_alpha, - serverClient, &error); - if (!pPicture) - return; + /* The glyph already has a Picture ready for us to use. */ + pPicture = GlyphPicture (glyph)[pScreen->myNum]; ValidatePicture(pPicture); if (maskFormat) @@ -1063,7 +1052,6 @@ exaGlyphs (CARD8 op, exaPixmapDirty(pDstPixmap, x1, y1, x1 + glyph->info.width, y1 + glyph->info.height); } - FreePicture ((pointer) pPicture, 0); nextglyph: x += glyph->info.xOff; diff --git a/render/glyph.c b/render/glyph.c index 7fd3705df..975c62b77 100644 --- a/render/glyph.c +++ b/render/glyph.c @@ -573,7 +573,7 @@ FreeGlyph (GlyphPtr glyph, int format) { ScreenPtr pScreen = screenInfo.screens[i]; - (pScreen->DestroyPixmap) (GlyphPixmap (glyph)[i]); + FreePicture ((pointer) GlyphPicture (glyph)[i], 0); ps = GetPictureScreenIfSet (pScreen); if (ps) @@ -669,7 +669,7 @@ AllocateGlyph (xGlyphInfo *gi, int fdepth) GlyphPtr glyph; int i; - size = screenInfo.numScreens * sizeof (PixmapPtr); + size = screenInfo.numScreens * sizeof (PicturePtr); glyph = (GlyphPtr) xalloc (size + sizeof (GlyphRec)); if (!glyph) return 0; @@ -689,21 +689,12 @@ AllocateGlyph (xGlyphInfo *gi, int fdepth) for (i = 0; i < screenInfo.numScreens; i++) { - ScreenPtr pScreen = screenInfo.screens[i]; - - GlyphPixmap (glyph)[i] = (pScreen->CreatePixmap) (pScreen, - gi->width, gi->height, - glyphDepths[fdepth]); - if (! GlyphPixmap (glyph)[i]) - goto bail; - - ps = GetPictureScreenIfSet (pScreen); - if (! ps) - continue; + ps = GetPictureScreenIfSet (screenInfo.screens[i]); - if (!(*ps->RealizeGlyph) (pScreen, glyph)) { - (pScreen->DestroyPixmap) (GlyphPixmap (glyph)[i]); - goto bail; + if (ps) + { + if (!(*ps->RealizeGlyph) (screenInfo.screens[i], glyph)) + goto bail; } } diff --git a/render/glyphstr.h b/render/glyphstr.h index 4f8746003..c6ab5aa11 100644 --- a/render/glyphstr.h +++ b/render/glyphstr.h @@ -47,7 +47,7 @@ typedef struct _Glyph { /* per-screen pixmaps follow */ } GlyphRec, *GlyphPtr; -#define GlyphPixmap(glyph) ((PixmapPtr *) ((glyph) + 1)) +#define GlyphPicture(glyph) ((PicturePtr *) ((glyph) + 1)) typedef struct _GlyphRef { CARD32 signature; diff --git a/render/miglyph.c b/render/miglyph.c index 2aa94bd09..a52ea49d6 100644 --- a/render/miglyph.c +++ b/render/miglyph.c @@ -174,13 +174,7 @@ miGlyphs (CARD8 op, while (n--) { glyph = *glyphs++; - pPixmap = GlyphPixmap (glyph)[pScreen->myNum]; - component_alpha = NeedsComponent(list->format->format); - pPicture = CreatePicture (0, &pPixmap->drawable, list->format, - CPComponentAlpha, &component_alpha, - serverClient, &error); - if (!pPicture) - return; + pPicture = GlyphPicture (glyph)[pScreen->myNum]; if (maskFormat) { @@ -209,7 +203,6 @@ miGlyphs (CARD8 op, glyph->info.width, glyph->info.height); } - FreePicture ((pointer) pPicture, 0); x += glyph->info.xOff; y += glyph->info.yOff; diff --git a/render/render.c b/render/render.c index 4bad379f6..300b78488 100644 --- a/render/render.c +++ b/render/render.c @@ -1086,6 +1086,8 @@ typedef struct _GlyphNew { unsigned char sha1[20]; } GlyphNewRec, *GlyphNewPtr; +#define NeedsComponent(f) (PICT_FORMAT_A(f) != 0 && PICT_FORMAT_RGB(f) != 0) + static int ProcRenderAddGlyphs (ClientPtr client) { @@ -1102,6 +1104,7 @@ ProcRenderAddGlyphs (ClientPtr client) int i, screen; PicturePtr pSrc = NULL, pDst = NULL; PixmapPtr pSrcPix = NULL, pDstPix = NULL; + CARD32 component_alpha; REQUEST_AT_LEAST_SIZE(xRenderAddGlyphsReq); glyphSet = (GlyphSetPtr) SecurityLookupIDByType (client, @@ -1118,6 +1121,8 @@ ProcRenderAddGlyphs (ClientPtr client) if (nglyphs > UINT32_MAX / sizeof(GlyphNewRec)) return BadAlloc; + component_alpha = NeedsComponent (glyphSet->format->format); + if (nglyphs <= NLOCALGLYPH) { memset (glyphsLocal, 0, sizeof (glyphsLocal)); glyphsBase = glyphsLocal; @@ -1158,9 +1163,11 @@ ProcRenderAddGlyphs (ClientPtr client) } else { + GlyphPtr glyph; + glyph_new->found = FALSE; - glyph_new->glyph = AllocateGlyph (&gi[i], glyphSet->fdepth); - if (! glyph_new->glyph) + glyph_new->glyph = glyph = AllocateGlyph (&gi[i], glyphSet->fdepth); + if (! glyph) { err = BadAlloc; goto bail; @@ -1194,11 +1201,14 @@ ProcRenderAddGlyphs (ClientPtr client) goto bail; } - pDstPix = GlyphPixmap (glyph_new->glyph)[screen]; + pDstPix = (pScreen->CreatePixmap) (pScreen, + width, height, depth); - pDst = CreatePicture (0, &pDstPix->drawable, - glyphSet->format, 0, NULL, - serverClient, &error); + GlyphPicture (glyph)[screen] = pDst = + CreatePicture (0, &pDstPix->drawable, + glyphSet->format, + CPComponentAlpha, &component_alpha, + serverClient, &error); if (! pDst) { err = BadAlloc; @@ -1216,8 +1226,6 @@ ProcRenderAddGlyphs (ClientPtr client) FreePicture ((pointer) pSrc, 0); pSrc = NULL; - FreePicture ((pointer) pDst, 0); - pDst = NULL; FreeScratchPixmapHeader (pSrcPix); pSrcPix = NULL; } @@ -1251,8 +1259,6 @@ ProcRenderAddGlyphs (ClientPtr client) bail: if (pSrc) FreePicture ((pointer) pSrc, 0); - if (pDst) - FreePicture ((pointer) pDst, 0); if (pSrcPix) FreeScratchPixmapHeader (pSrcPix); for (i = 0; i < nglyphs; i++) -- cgit v1.2.3 From 93ae6fe18c417a22f1fccb22add4890a20cae713 Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Thu, 23 Aug 2007 16:33:05 -0700 Subject: Avoid leaking a Pixmap for every glyph --- render/render.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'render') diff --git a/render/render.c b/render/render.c index 300b78488..1a1cd7a0e 100644 --- a/render/render.c +++ b/render/render.c @@ -1209,6 +1209,11 @@ ProcRenderAddGlyphs (ClientPtr client) glyphSet->format, CPComponentAlpha, &component_alpha, serverClient, &error); + + /* The picture takes a reference to the pixmap, so we + drop ours. */ + (pScreen->DestroyPixmap) (pDstPix); + if (! pDst) { err = BadAlloc; -- cgit v1.2.3