[cairo] [API] Export region data type

Soeren Sandmann sandmann at daimi.au.dk
Fri Mar 27 18:44:48 PDT 2009


Behdad Esfahbod <behdad at behdad.org> writes:

> >>>>    typedef struct _cairo_rectangle_int {
> >>>>        int x, y;
> >>>>        unsigned int width, height;
> >>>>    } cairo_rectangle_int_t;
> >>> If we end up exporting this, I'd suggest making width and height
> >>> signed ints to avoid overflow pitfalls such as
> >> With signed width/height you get overflows when doing:
> >>
> >>  rect->width = x2 - x1;
> > 
> > How so?  Getting a negative width is not an overflow situation.  It's
> > potentially unexepected, but certainly not worse than having x2=100
> > and x1=150 and then have width be 2^32 - 50.
> 
> What's a negative width good for?  It doesn't mean anything.  It's undefined.
>  It's a hassle to handle (you have to check for width >= 0 all over the code).
>  The 2^32 - 50 on the other hand will be automatically detected and handled by
> whatever check you already have in place for handling large sizes.

I tend to agree with Kristian on this. The negative width may not be
good for anything, but at least it is easily understood that if you
compute with buggy input, you end up with buggy output.

With unsigned integers, on the other hand, the output will not just be
buggy, it will be *incomprensible*. If you don't know the C type
promotion rules, or if you have repressed them, or if you have
forgotten that width is unsigned, it will look like the computer has
lost its ability to do arithmetic when

        if (x2 - x1 > rect->width)
                ...

makes it seem like -50 is greater than 100. I believe GdkRectangle
uses signed integers for precisely this reason.

I don't feel that strongly about it, though, so if you still want the
unsigned integers, I'm fine with that.

I haven't seen any other objections to the region API, so unless I
hear otherwise, I'll go ahead and push what I have in

        http://cgit.freedesktop.org/~sandmann/cairo-damage/log/?h=public-region

Since last time I have added:
        
        cairo_status_t
        cairo_region_intersect_rectangle (cairo_region_t *region,
                                          const cairo_rectangle_int_t *rectangle);

This should be uncontroversial; there was already union_with_rectangle(), 
and intersecting with a rectangle is a pretty common operation
(typically it is used to confine a region to its underlying drawable).


Soren


More information about the cairo mailing list