[cairo-commit] 3 commits - src/cairo-xcb-surface-render.c
Andrea Canciani
ranma42 at kemper.freedesktop.org
Thu Jan 6 04:33:29 PST 2011
src/cairo-xcb-surface-render.c | 135 ++++++++++++++++++++++++++++-------------
1 file changed, 93 insertions(+), 42 deletions(-)
New commits:
commit 588dead005d69c022245ff017f53ff403b50e9db
Author: Andrea Canciani <ranma42 at gmail.com>
Date: Wed Dec 22 11:30:45 2010 +0100
xcb: Handle a wider range of glyph positions
_can_composite_glyphs() checks that the position of each glyph can be
represented as a 16-bit offset from the destination origin.
Fixes https://bugs.freedesktop.org/show_bug.cgi?id=31897
diff --git a/src/cairo-xcb-surface-render.c b/src/cairo-xcb-surface-render.c
index dc9fe15..1a71996 100644
--- a/src/cairo-xcb-surface-render.c
+++ b/src/cairo-xcb-surface-render.c
@@ -4447,25 +4447,8 @@ _composite_glyphs (void *closure,
glyph_cache[cache_index] = scaled_glyph;
}
- /* Glyph skipping:
- *
- * We skip any glyphs that have troublesome coordinates. We want
- * to make sure that (glyph2.x - (glyph1.x + glyph1.width)) fits in
- * a signed 16bit integer, otherwise it will overflow in the render
- * protocol.
- * To ensure this, we'll make sure that (glyph2.x - glyph1.x) fits in
- * a signed 15bit integer. The trivial option would be to allow
- * coordinates -8192..8192, but that's kinda dull. It probably will
- * take a decade or so to get monitors 8192x4096 or something. A
- * negative value of -8192 on the other hand, is absolutely useless.
- * Note that we do want to allow some negative positions. The glyph
- * may start off the screen but part of it make it to the screen.
- * Anyway, we will allow positions in the range -4096..122887. That
- * will buy us a few more years before this stops working.
- */
this_x = _cairo_lround (info->glyphs[i].d.x) - dst_x;
this_y = _cairo_lround (info->glyphs[i].d.y) - dst_y;
- assert (! (((this_x+4096) | (this_y+4096)) & ~0x3fffu));
this_glyphset_info = _cairo_xcb_scaled_glyph_get_glyphset_info (scaled_glyph);
if (glyphset_info == NULL)
@@ -4492,8 +4475,17 @@ _composite_glyphs (void *closure,
* prefer the latter is the fact that Xserver ADDs all glyphs
* to the mask first, and then composes that to final surface,
* though it's not a big deal.
+ *
+ * If the glyph has a coordinate which cannot be represented
+ * as a 16-bit offset from the previous glyph, flush the
+ * current chunk. The current glyph will be the first one in
+ * the next chunk, thus its coordinates will be an offset from
+ * the destination origin. This offset is guaranteed to be
+ * representable as 16-bit offset in _can_composite_glyphs().
*/
if (request_size + width > max_request_size - _cairo_sz_x_glyph_elt_t ||
+ this_x - x > INT16_MAX || this_x - x < INT16_MIN ||
+ this_y - y > INT16_MAX || this_y - y < INT16_MIN ||
this_glyphset_info != glyphset_info)
{
status = _emit_glyphs_chunk (dst, op, src,
commit 10bae9d9ce5ece5bc5b4a929e791d9906a6b24b5
Author: Andrea Canciani <ranma42 at gmail.com>
Date: Wed Dec 22 00:24:59 2010 +0100
xcb: Stricter glyph validation
To ensure that we can correctly issue the glyph operation, glyph size
must fit in an XCB request and its position must be within the
representable range (16-bit offset).
diff --git a/src/cairo-xcb-surface-render.c b/src/cairo-xcb-surface-render.c
index 0aaab2d..dc9fe15 100644
--- a/src/cairo-xcb-surface-render.c
+++ b/src/cairo-xcb-surface-render.c
@@ -3721,14 +3721,18 @@ typedef struct {
static cairo_status_t
_can_composite_glyphs (cairo_xcb_surface_t *dst,
+ cairo_rectangle_int_t *extents,
cairo_scaled_font_t *scaled_font,
cairo_glyph_t *glyphs,
- int num_glyphs)
+ int *num_glyphs)
{
- unsigned long glyph_cache[64];
- cairo_status_t status;
+#define GLYPH_CACHE_SIZE 64
+ cairo_box_t bbox_cache[GLYPH_CACHE_SIZE];
+ unsigned long glyph_cache[GLYPH_CACHE_SIZE];
+#undef GLYPH_CACHE_SIZE
+ cairo_status_t status = CAIRO_STATUS_SUCCESS;
+ cairo_glyph_t *glyphs_end, *valid_glyphs;
const int max_glyph_size = dst->connection->maximum_request_length - 64;
- int i;
/* We must initialize the cache with values that cannot match the
* "hash" to guarantee that when compared for the first time they
@@ -3739,38 +3743,86 @@ _can_composite_glyphs (cairo_xcb_surface_t *dst,
memset (glyph_cache, 0, sizeof (glyph_cache));
glyph_cache[0] = 1;
- /* first scan for oversized glyphs, and fallback in that case */
- for (i = 0; i < num_glyphs; i++) {
+ /* Scan for oversized glyphs or glyphs outside the representable
+ * range and fallback in that case, discard glyphs outside of the
+ * image.
+ */
+ valid_glyphs = glyphs;
+ for (glyphs_end = glyphs + *num_glyphs; glyphs != glyphs_end; glyphs++) {
+ double x1, y1, x2, y2;
cairo_scaled_glyph_t *scaled_glyph;
+ cairo_box_t *bbox;
int width, height, len;
int g;
- g = glyphs[i].index % ARRAY_LENGTH (glyph_cache);
- if (glyph_cache[g] == glyphs[i].index)
- continue;
+ g = glyphs->index % ARRAY_LENGTH (glyph_cache);
+ if (glyph_cache[g] != glyphs->index) {
+ status = _cairo_scaled_glyph_lookup (scaled_font,
+ glyphs->index,
+ CAIRO_SCALED_GLYPH_INFO_METRICS,
+ &scaled_glyph);
+ if (unlikely (status))
+ break;
- status = _cairo_scaled_glyph_lookup (scaled_font,
- glyphs[i].index,
- CAIRO_SCALED_GLYPH_INFO_METRICS,
- &scaled_glyph);
- if (unlikely (status))
- return status;
+ glyph_cache[g] = glyphs->index;
+ bbox_cache[g] = scaled_glyph->bbox;
+ }
+ bbox = &bbox_cache[g];
+
+ /* Drop glyphs outside the clipping */
+ x1 = _cairo_fixed_to_double (bbox->p1.x);
+ y1 = _cairo_fixed_to_double (bbox->p1.y);
+ y2 = _cairo_fixed_to_double (bbox->p2.y);
+ x2 = _cairo_fixed_to_double (bbox->p2.x);
+ if (unlikely (glyphs->x + x2 <= extents->x ||
+ glyphs->y + y2 <= extents->y ||
+ glyphs->x + x1 >= extents->x + extents->width ||
+ glyphs->y + y1 >= extents->y + extents->height))
+ {
+ (*num_glyphs)--;
+ continue;
+ }
/* XRenderAddGlyph does not handle a glyph surface larger than
* the extended maximum XRequest size.
*/
- width =
- _cairo_fixed_integer_ceil (scaled_glyph->bbox.p2.x - scaled_glyph->bbox.p1.x);
- height =
- _cairo_fixed_integer_ceil (scaled_glyph->bbox.p2.y - scaled_glyph->bbox.p1.y);
+ width = _cairo_fixed_integer_ceil (bbox->p2.x - bbox->p1.x);
+ height = _cairo_fixed_integer_ceil (bbox->p2.y - bbox->p1.y);
len = CAIRO_STRIDE_FOR_WIDTH_BPP (width, 32) * height;
- if (len >= max_glyph_size)
- return CAIRO_INT_STATUS_UNSUPPORTED;
+ if (unlikely (len >= max_glyph_size)) {
+ status = CAIRO_INT_STATUS_UNSUPPORTED;
+ break;
+ }
- glyph_cache[g] = glyphs[i].index;
+ /* The glyph coordinates must be representable in an int16_t.
+ * When possible, they will be expressed as an offset from the
+ * previous glyph, otherwise they will be an offset from the
+ * operation extents or from the surface origin. If the last
+ * two options are not valid, fallback.
+ */
+ if (unlikely (glyphs->x > INT16_MAX ||
+ glyphs->y > INT16_MAX ||
+ glyphs->x - extents->x < INT16_MIN ||
+ glyphs->y - extents->y < INT16_MIN))
+ {
+ status = CAIRO_INT_STATUS_UNSUPPORTED;
+ break;
+ }
+
+
+ if (unlikely (valid_glyphs != glyphs))
+ *valid_glyphs = *glyphs;
+ valid_glyphs++;
}
- return CAIRO_STATUS_SUCCESS;
+ if (unlikely (valid_glyphs != glyphs)) {
+ for (; glyphs != glyphs_end; glyphs++) {
+ *valid_glyphs = *glyphs;
+ valid_glyphs++;
+ }
+ }
+
+ return status;
}
/* Start a new element for the first glyph,
@@ -4568,8 +4620,8 @@ _cairo_xcb_surface_render_glyphs (cairo_xcb_surface_t *surface,
_cairo_scaled_font_freeze_cache (scaled_font);
if (_surface_owns_font (surface, scaled_font)) {
- status = _can_composite_glyphs (surface,
- scaled_font, glyphs, num_glyphs);
+ status = _can_composite_glyphs (surface, &extents.bounded,
+ scaled_font, glyphs, &num_glyphs);
if (likely (status == CAIRO_STATUS_SUCCESS)) {
composite_glyphs_info_t info;
commit c3f9a0cf473f3ef9fd89b2a9738e1ce61fd0b6cc
Author: Andrea Canciani <ranma42 at gmail.com>
Date: Wed Dec 22 00:21:19 2010 +0100
xcb: Correct handling of index 0 glyphs
Glyph caches (with direct glyph index matching) cannot be completely
initialized with zeroes, otherwise the code will incorrectly believe
that the lookup for the 0-index glyph has already been performed.
diff --git a/src/cairo-xcb-surface-render.c b/src/cairo-xcb-surface-render.c
index 760c90d..0aaab2d 100644
--- a/src/cairo-xcb-surface-render.c
+++ b/src/cairo-xcb-surface-render.c
@@ -3730,7 +3730,14 @@ _can_composite_glyphs (cairo_xcb_surface_t *dst,
const int max_glyph_size = dst->connection->maximum_request_length - 64;
int i;
+ /* We must initialize the cache with values that cannot match the
+ * "hash" to guarantee that when compared for the first time they
+ * will result in a mismatch. The hash function is simply modulus,
+ * so we cannot use 0 in glyph_cache[0], but we can use it in all
+ * other array cells.
+ */
memset (glyph_cache, 0, sizeof (glyph_cache));
+ glyph_cache[0] = 1;
/* first scan for oversized glyphs, and fallback in that case */
for (i = 0; i < num_glyphs; i++) {
More information about the cairo-commit
mailing list