[cairo-commit] 5 commits - src/cairo-xlib-surface.c
Behdad Esfahbod
behdad at kemper.freedesktop.org
Tue Sep 30 14:56:17 PDT 2008
src/cairo-xlib-surface.c | 186 ++++++++++++++++++++++-------------------------
1 file changed, 90 insertions(+), 96 deletions(-)
New commits:
commit 2c58a2c3851afac0386fcf0bf8504a937231185c
Author: Behdad Esfahbod <behdad at behdad.org>
Date: Tue Sep 30 17:55:01 2008 -0400
[xlib] Start a new glyph element every 128 glyphs
Xrender has limits at 252 glyphs. We play safe and fast and limit
elements to 128 glyphs. That's plenty, no measurable performance
hit expected.
diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index d3f0d75..8c222ec 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -3612,7 +3612,17 @@ _emit_glyphs_chunk (cairo_xlib_surface_t *dst,
/* Start a new element for first output glyph, and for glyphs with
* unexpected position */
- if (!j || glyphs[i].i.x || glyphs[i].i.y) {
+ /* Start a new element for the first glyph,
+ * or for any glyph that has unexpected position,
+ * or if current element has too many glyphs
+ * (Xrender limits each element to 252 glyphs, we limit them to 128)
+ *
+ * Note that the first condition is redundant with the introduction
+ * of the third one. Keep for clarity.
+ *
+ * These same conditions are mirrored in _cairo_xlib_surface_emit_glyphs
+ */
+ if (!j || glyphs[i].i.x || glyphs[i].i.y || j % 128 == 0) {
if (j) {
elts[nelt].nchars = n;
nelt++;
@@ -3800,9 +3810,17 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
glyphs[i].i.x = this_x - x;
glyphs[i].i.y = this_y - y;
- /* Start a new element for the first glyph, or for any glyph that
- * has unexpected position */
- if (!num_out_glyphs || glyphs[i].i.x || glyphs[i].i.y) {
+ /* Start a new element for the first glyph,
+ * or for any glyph that has unexpected position,
+ * or if current element has too many glyphs
+ * (Xrender limits each element to 252 glyphs, we limit them to 128)
+ *
+ * Note that the first condition is redundant with the introduction
+ * of the third one. Keep for clarity.
+ *
+ * These same conditions are mirrored in _emit_glyphs_chunk
+ */
+ if (!num_out_glyphs || glyphs[i].i.x || glyphs[i].i.y || num_out_glyphs % 128 == 0) {
num_elts++;
request_size += _cairo_sz_xGlyphElt;
}
commit fd7e09c7e66876b8492424e1c7d1260c12cc17f3
Author: Behdad Esfahbod <behdad at behdad.org>
Date: Tue Sep 30 17:46:06 2008 -0400
[xlib] Allow room for glyph element padding
Xrender pads glyph elements to 4byte boundaries. We didn't consider
that in our request size calculations. We do now.
diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index b5170b7..d3f0d75 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -3657,6 +3657,11 @@ _emit_glyphs_chunk (cairo_xlib_surface_t *dst,
return CAIRO_STATUS_SUCCESS;
}
+
+/* sz_xGlyphtElt required alignment to a 32-bit boundary, so ensure we have
+ * enough room for padding */
+#define _cairo_sz_xGlyphElt (sz_xGlyphElt + 4)
+
static cairo_status_t
_cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
cairo_xlib_glyph_t *glyphs,
@@ -3770,7 +3775,7 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
* to the mask first, and then composes that to final surface,
* though it's not a big deal.
*/
- if (request_size + width > max_request_size - sz_xGlyphElt ||
+ if (request_size + width > max_request_size - _cairo_sz_xGlyphElt ||
(this_glyphset_info != glyphset_info)) {
status = _emit_glyphs_chunk (dst, glyphs, i,
scaled_font, op, src, attributes,
@@ -3799,7 +3804,7 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
* has unexpected position */
if (!num_out_glyphs || glyphs[i].i.x || glyphs[i].i.y) {
num_elts++;
- request_size += sz_xGlyphElt;
+ request_size += _cairo_sz_xGlyphElt;
}
/* adjust current-position */
commit c01fb77abbaf28c03aa6a21ebb997638dbdf950b
Author: Behdad Esfahbod <behdad at behdad.org>
Date: Tue Sep 30 17:41:51 2008 -0400
[xlib] s/_cairo_xlib_surface_emit_glyphs_chunk/_emit_glyphs_chunk/
For readability's sake.
diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index 67dd26c..b5170b7 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -3549,17 +3549,17 @@ typedef union {
COMPILE_TIME_ASSERT (sizeof (cairo_xlib_glyph_t) == sizeof (cairo_glyph_t));
static cairo_status_t
-_cairo_xlib_surface_emit_glyphs_chunk (cairo_xlib_surface_t *dst,
- cairo_xlib_glyph_t *glyphs,
- int num_glyphs,
- cairo_scaled_font_t *scaled_font,
- cairo_operator_t op,
- cairo_xlib_surface_t *src,
- cairo_surface_attributes_t *attributes,
- /* info for this chunk */
- int num_elts,
- int width,
- cairo_xlib_font_glyphset_info_t *glyphset_info)
+_emit_glyphs_chunk (cairo_xlib_surface_t *dst,
+ cairo_xlib_glyph_t *glyphs,
+ int num_glyphs,
+ cairo_scaled_font_t *scaled_font,
+ cairo_operator_t op,
+ cairo_xlib_surface_t *src,
+ cairo_surface_attributes_t *attributes,
+ /* info for this chunk */
+ int num_elts,
+ int width,
+ cairo_xlib_font_glyphset_info_t *glyphset_info)
{
/* Which XRenderCompositeText function to use */
cairo_xrender_composite_text_func_t composite_text_func;
@@ -3772,9 +3772,9 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
*/
if (request_size + width > max_request_size - sz_xGlyphElt ||
(this_glyphset_info != glyphset_info)) {
- status = _cairo_xlib_surface_emit_glyphs_chunk (dst, glyphs, i,
- scaled_font, op, src, attributes,
- num_elts, old_width, glyphset_info);
+ status = _emit_glyphs_chunk (dst, glyphs, i,
+ scaled_font, op, src, attributes,
+ num_elts, old_width, glyphset_info);
if (status != CAIRO_STATUS_SUCCESS)
return status;
@@ -3811,9 +3811,9 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
}
if (num_elts)
- status = _cairo_xlib_surface_emit_glyphs_chunk (dst, glyphs, i,
- scaled_font, op, src, attributes,
- num_elts, width, glyphset_info);
+ status = _emit_glyphs_chunk (dst, glyphs, i,
+ scaled_font, op, src, attributes,
+ num_elts, width, glyphset_info);
*remaining_glyphs = num_glyphs - i;
if (*remaining_glyphs && status == CAIRO_STATUS_SUCCESS)
commit c2ba25df1aec1cebfc4ce85e06a4187950675820
Author: Behdad Esfahbod <behdad at behdad.org>
Date: Tue Sep 30 17:40:56 2008 -0400
[xlib] Add comment about glyph chunk invariant
diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index 3e126b7..67dd26c 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -3742,6 +3742,10 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
if (!glyphset_info)
glyphset_info = this_glyphset_info;
+ /* The invariant here is that we can always flush the glyphs
+ * accumulated before this one, using old_width, and they
+ * would fit in the request.
+ */
old_width = width;
/* Update max glyph index */
commit e983458e1fba15153815430c83619da53929139d
Author: Behdad Esfahbod <behdad at behdad.org>
Date: Tue Sep 30 17:26:46 2008 -0400
Revert "[xlib] Correct calculation of XRenderComposite* request size."
This reverts commit 0eb0c26474a19477554bfd580aa5f8ae77c29779.
The change was too drastic and overlooked some subleties of the old
code, but the main reason for the revert is that it introduced an
ugly duplicated glyph flush block. I'm working on a more incremental
fix.
diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index 9ce4a7c..3e126b7 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -3563,10 +3563,11 @@ _cairo_xlib_surface_emit_glyphs_chunk (cairo_xlib_surface_t *dst,
{
/* Which XRenderCompositeText function to use */
cairo_xrender_composite_text_func_t composite_text_func;
+ int size;
/* Element buffer stuff */
- XGlyphElt8 stack_elts[CAIRO_STACK_ARRAY_LENGTH (XGlyphElt8)];
XGlyphElt8 *elts;
+ XGlyphElt8 stack_elts[CAIRO_STACK_ARRAY_LENGTH (XGlyphElt8)];
/* Reuse the input glyph array for output char generation */
char *char8 = (char *) glyphs;
@@ -3582,18 +3583,22 @@ _cairo_xlib_surface_emit_glyphs_chunk (cairo_xlib_surface_t *dst,
case 1:
/* don't cast the 8-variant, to catch possible mismatches */
composite_text_func = XRenderCompositeText8;
+ size = sizeof (char);
break;
case 2:
composite_text_func = (cairo_xrender_composite_text_func_t) XRenderCompositeText16;
+ size = sizeof (unsigned short);
break;
default:
case 4:
composite_text_func = (cairo_xrender_composite_text_func_t) XRenderCompositeText32;
+ size = sizeof (unsigned int);
}
/* Allocate element array */
- elts = stack_elts;
- if (num_elts > ARRAY_LENGTH (stack_elts)) {
+ if (num_elts <= ARRAY_LENGTH (stack_elts)) {
+ elts = stack_elts;
+ } else {
elts = _cairo_malloc_ab (num_elts, sizeof (XGlyphElt8));
if (elts == NULL)
return _cairo_error (CAIRO_STATUS_NO_MEMORY);
@@ -3604,14 +3609,16 @@ _cairo_xlib_surface_emit_glyphs_chunk (cairo_xlib_surface_t *dst,
n = 0;
j = 0;
for (i = 0; i < num_glyphs; i++) {
+
/* Start a new element for first output glyph, and for glyphs with
* unexpected position */
if (!j || glyphs[i].i.x || glyphs[i].i.y) {
if (j) {
- elts[nelt++].nchars = n;
+ elts[nelt].nchars = n;
+ nelt++;
n = 0;
}
- elts[nelt].chars = char8 + width * j;
+ elts[nelt].chars = char8 + size * j;
elts[nelt].glyphset = glyphset_info->glyphset;
elts[nelt].xOff = glyphs[i].i.x;
elts[nelt].yOff = glyphs[i].i.y;
@@ -3628,9 +3635,11 @@ _cairo_xlib_surface_emit_glyphs_chunk (cairo_xlib_surface_t *dst,
j++;
}
- if (n)
- elts[nelt++].nchars = n;
- assert (nelt == num_elts);
+ if (n) {
+ elts[nelt].nchars = n;
+ nelt++;
+ n = 0;
+ }
composite_text_func (dst->dpy,
_render_operator (op),
@@ -3648,7 +3657,6 @@ _cairo_xlib_surface_emit_glyphs_chunk (cairo_xlib_surface_t *dst,
return CAIRO_STATUS_SUCCESS;
}
-#define PAD(sz, A) ((A)-((sz)&((A)-1)))
static cairo_status_t
_cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
cairo_xlib_glyph_t *glyphs,
@@ -3668,7 +3676,7 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
unsigned long max_index = 0;
int width = 1;
int num_elts = 0;
- int num_elt_glyphs = 0;
+ int num_out_glyphs = 0;
int max_request_size = XMaxRequestSize (dst->dpy) * 4
- MAX (sz_xRenderCompositeGlyphs8Req,
@@ -3681,7 +3689,7 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
for (i = 0; i < num_glyphs; i++) {
int this_x, this_y;
- int old_width, this_rq_size;
+ int old_width;
status = _cairo_scaled_glyph_lookup (scaled_font,
glyphs[i].index,
@@ -3743,48 +3751,13 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
width = 4;
else if (max_index >= 256)
width = 2;
- if (width != old_width) {
- int expand = (width - old_width) * i;
-
- /* Check to see if we have enough room within this request
- * to convert all the current glyphs to the new width.
- */
- if (request_size + expand > max_request_size) {
- status = _cairo_xlib_surface_emit_glyphs_chunk (dst, glyphs, i,
- scaled_font, op, src, attributes,
- num_elts, old_width, glyphset_info);
- if (status)
- return status;
-
- glyphs += i;
- num_glyphs -= i;
- i = 0;
-
- request_size = sz_xGlyphElt + width;
- num_elts = 1;
- num_elt_glyphs = 1;
-
- x = this_x + scaled_glyph->x_advance;
- y = this_y + scaled_glyph->y_advance;
- glyphset_info = this_glyphset_info;
-
- glyphs[i].i.x = 0;
- glyphs[i].i.y = 0;
- continue;
- } else
- request_size += expand;
- }
+ if (width != old_width)
+ request_size += (width - old_width) * num_out_glyphs;
}
- this_rq_size = width;
- /* If the glyph is in an unexpected position, we need a new GlyphElt.
- * And libXrender inserts a new GlyphElt (at most) every 252 glyphs.
- */
- if ((num_elt_glyphs % 252) == 0 || this_x - x || this_y - y)
- this_rq_size += PAD (request_size, 4) + sz_xGlyphElt;
-
/* If we will pass the max request size by adding this glyph,
- * flush current glyphs.
+ * flush current glyphs. Note that we account for a
+ * possible element being added below.
*
* Also flush if changing glyphsets, as Xrender limits one mask
* format per request, so we can either break up, or use a
@@ -3793,50 +3766,44 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
* to the mask first, and then composes that to final surface,
* though it's not a big deal.
*/
- if (request_size + this_rq_size > max_request_size ||
- this_glyphset_info != glyphset_info)
- {
+ if (request_size + width > max_request_size - sz_xGlyphElt ||
+ (this_glyphset_info != glyphset_info)) {
status = _cairo_xlib_surface_emit_glyphs_chunk (dst, glyphs, i,
scaled_font, op, src, attributes,
num_elts, old_width, glyphset_info);
- if (status)
+ if (status != CAIRO_STATUS_SUCCESS)
return status;
glyphs += i;
num_glyphs -= i;
i = 0;
-
max_index = glyphs[i].index;
width = max_index < 256 ? 1 : max_index < 65536 ? 2 : 4;
-
- request_size = width + sz_xGlyphElt;
- num_elts = 1;
- num_elt_glyphs = 1;
-
- x = this_x;
- y = this_y;
+ request_size = 0;
+ num_elts = 0;
+ num_out_glyphs = 0;
+ x = y = 0;
glyphset_info = this_glyphset_info;
+ }
- glyphs[i].i.x = 0;
- glyphs[i].i.y = 0;
- } else {
- request_size += this_rq_size;
-
- /* Convert absolute glyph position to relative-to-current-point. */
- glyphs[i].i.x = this_x - x;
- glyphs[i].i.y = this_y - y;
-
- /* Start a new element for any glyph in an unexpected position. */
- if (num_elt_glyphs == 0 || glyphs[i].i.x || glyphs[i].i.y) {
- num_elts++;
- num_elt_glyphs = 1;
- } else
- num_elt_glyphs++;
+ /* Convert absolute glyph position to relative-to-current-point
+ * position */
+ glyphs[i].i.x = this_x - x;
+ glyphs[i].i.y = this_y - y;
+
+ /* Start a new element for the first glyph, or for any glyph that
+ * has unexpected position */
+ if (!num_out_glyphs || glyphs[i].i.x || glyphs[i].i.y) {
+ num_elts++;
+ request_size += sz_xGlyphElt;
}
- /* Adjust current-point. */
+ /* adjust current-position */
x = this_x + scaled_glyph->x_advance;
y = this_y + scaled_glyph->y_advance;
+
+ num_out_glyphs++;
+ request_size += width;
}
if (num_elts)
More information about the cairo-commit
mailing list