[cairo] [PATCH] Fix _cairo_region_get_boxes usage

[Chris Wilson]

On Fri, 2009-02-13 at 15:09 -0500, Jeff Muizelaar wrote:
> The attached patch fixes some issues with callers of
> _cairo_region_get_boxes. The new api seems a little bit difficult to
> use and I'm wondering if we can fix it at all? Though I don't really have any
> ideas or know if it's worth it. Thoughts?

It is a poor interface, I agree. The issue at hand is that the clip
regions are set very frequently and typically comprise of just a few
rectangles. In the worst case for xlib we need to allocate a couple of
intermediate buffers for a single set_clip_region() in order to
translate between the 3 different clip box representations. So without
this horrible mitigation strategy, these allocations stand out in malloc
profiles -- from that perspective this is worth doing.

I'm open to suggestions. (Though I'm concerned that if the
initialisation in cairo-paginated-surface.c is indeed required, why
hasn't it triggered an error during testing. Oh I remember,
has_finegrained_fallbacks implies num_boxes != 0, so it either throws an
error or sucessfully initialises boxes.)

A similar strategy is also used during glyph and cluster creation, but
Behdad did a much better job of documenting the API and fixing up the
backends. So perhaps that is the answer. On the other hand, the original
purpose _cairo_region_get_boxes() was to perform the translation between
cairo's 24.8 fixed-point and pixman's 16.16. Without that there seems to
be little point in keeping it around. (Personally I think we might want
to review whether adding some _cairo_fixed_from_pixman24_8() would help
in future.)

My thoughts are to apply your patch, set *boxes=NULL for the num_boxes=0
case in _cairo_region_get_boxes() and we should also add the if(boxes!
=NULL) check to _cairo_region_boxes_fini(). Plus if you add
/* XXX ickle promises to document this function */, then that might just
give me a little impetus to do so. ;-)

Thank you Jeff for finding those potential free() on uninitialised
pointers.
-ickle