fdo-mirrors/cairo
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/cairo/cairo.git synced 2026-05-08 12:38:02 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

[xlib] Correct calculation of XRenderComposite* request size.

Browse source 
show-glyphs-many is triggering an assertion failure within xlib. The cause
appears to be that we are submitting an overlong request.

Reviewing the code and comparing with libXrender/src/Glyph.c I found two
points to address.

1. When encountering the first 2-byte, or 4-byte glyph and thus triggering
the recalculation of the current request_size, we did not check that there
was enough room for the expanded request. In case there is not, we need to
emit the current request (before expansion) and reset.

2. Subtleties in how XRenderCompositeText* constructs the binary protocol
buffer require that xGlyphElts are aligned to 32-bit boundaries and that
it inserts an additional xGlyphElt every 252 glyphs when width==1 or
every 254 glyphs for width==2 || width==4.  Thus we need to explicitly
compute how many bytes would be required to add this glyph in accordance
with the above.

Considering the complexity (and apparent fragility since we require tight
coupling to XRender) of the code, I'm sure there are more bugs to be
found.
This commit is contained in:
Chris Wilson 2008-09-30 13:33:25 +01:00
parent 02a56a4c84
commit 0eb0c26474
1 changed files with 79 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

125
src/cairo-xlib-surface.c
View file

@ -3563,11 +3563,10 @@ _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 *elts;
XGlyphElt8 stack_elts[CAIRO_STACK_ARRAY_LENGTH (XGlyphElt8)];
XGlyphElt8 *elts;
/* Reuse the input glyph array for output char generation */
char *char8 = (char *) glyphs;
@ -3583,22 +3582,18 @@ _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 */
if (num_elts <= ARRAY_LENGTH (stack_elts)) {
elts = stack_elts;
} else {
elts = stack_elts;
if (num_elts > ARRAY_LENGTH (stack_elts)) {
elts = _cairo_malloc_ab (num_elts, sizeof (XGlyphElt8));
if (elts == NULL)
return _cairo_error (CAIRO_STATUS_NO_MEMORY);
@ -3609,16 +3604,14 @@ _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;
nelt++;
elts[nelt++].nchars = n;
n = 0;
}
elts[nelt].chars = char8 + size * j;
elts[nelt].chars = char8 + width * j;
elts[nelt].glyphset = glyphset_info->glyphset;
elts[nelt].xOff = glyphs[i].i.x;
elts[nelt].yOff = glyphs[i].i.y;
@ -3635,11 +3628,9 @@ _cairo_xlib_surface_emit_glyphs_chunk (cairo_xlib_surface_t *dst,
j++;
}
if (n) {
elts[nelt].nchars = n;
nelt++;
n = 0;
}
if (n)
elts[nelt++].nchars = n;
assert (nelt == num_elts);
composite_text_func (dst->dpy,
_render_operator (op),
@ -3657,6 +3648,7 @@ _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,
@ -3676,7 +3668,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_out_glyphs = 0;
int num_elt_glyphs = 0;
int max_request_size = XMaxRequestSize (dst->dpy) * 4
- MAX (sz_xRenderCompositeGlyphs8Req,
@ -3689,7 +3681,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;
int old_width, this_rq_size;
status = _cairo_scaled_glyph_lookup (scaled_font,
glyphs[i].index,
@ -3751,13 +3743,48 @@ _cairo_xlib_surface_emit_glyphs (cairo_xlib_surface_t *dst,
width = 4;
else if (max_index >= 256)
width = 2;
if (width != old_width)
request_size += (width - old_width) * num_out_glyphs;
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;
}
}
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. Note that we account for a
* possible element being added below.
* flush current glyphs.
*
* Also flush if changing glyphsets, as Xrender limits one mask
* format per request, so we can either break up, or use a
@ -3766,44 +3793,50 @@ _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 ||
(this_glyphset_info != glyphset_info)) {
if (request_size + this_rq_size > max_request_size ||
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 != CAIRO_STATUS_SUCCESS)
if (status)
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 = 0;
num_elts = 0;
num_out_glyphs = 0;
x = y = 0;
request_size = width + sz_xGlyphElt;
num_elts = 1;
num_elt_glyphs = 1;
x = this_x;
y = this_y;
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-position */
/* Adjust current-point. */
x = this_x + scaled_glyph->x_advance;
y = this_y + scaled_glyph->y_advance;
num_out_glyphs++;
request_size += width;
}
if (num_elts)