[cairo] Use _cairo_rectangle_intersect in more places for intersection checking

[BJörn Lindqvist]

On 10/29/07, Carl Worth <cworth at cworth.org> wrote:
> > Alright. Here are the two patches again with all indentation redone to
> > tabs. Note that it will still look inconsistent in some places when
> > viewing the patch. This is not my fault, the first tab character is
> > always only 7 space long when printed in a diff.
>
> Well, there is the fact that we do the first-level of indentation with
> 4 spaces, so those don't hide the extra diff-added space like an
> initial tab. But that won't actually result in anything that looks
> obviously inconsistent in the diff output. (It won't lead to a column
> that wanders left and right for example.)

Yes it will. First indentation looks indented four spaces, the next
one seven spaces because of the shorter tab character.

> PS. A couple of notes follow on indentation, (which I really wouldn't
> be mentioning here except that it was already raised as a topic in
> this thread).
>
> > +    if (interest) {
> > +     if (!_cairo_rectangle_intersect (&read_rect, interest)) {
>
> This is an example of a case you could have been referring to when you
> said the patch would have inconsistent indentation that is not your
> fault.

Yes.

> Notice that in my reply here, the problem has gotten worse, since my
> email composer has added two additional characters so the apparent
> indentation is now only 1 column. And that does look quite nasty, I
> admit.

Tabs are evil. :)

> >      if (rect_out)
> > -    {
> > -     rect_out->x = x1;
> > -     rect_out->y = y1;
> > -     rect_out->width = width;
> > -     rect_out->height = height;
> > -    }
> > +        *rect_out = read_rect;
>
> Here you are introducing a new line of code that is indented with
> spaces instead of a tab, and this one is noticeable, (the code being
> removed is clearly indented differently than the code being added), so
> this should be fixed.

Ok. Then it should also be written down in CODING_STYLE that the
_only_ form of indentation allowed is with tabs.

> >       imagerep = xcb_get_image_reply(surface->dpy,
> >                                   xcb_get_image(surface->dpy, XCB_IMAGE_FORMAT_Z_PIXMAP,
> > -                                             surface->drawable,
> > -                                             x1, y1,
> > -                                             x2 - x1, y2 - y1,
> > -                                             AllPlanes), &error);
> > +                                                surface->drawable,
> > +                                                read_rect.x,
> > +                                                read_rect.y,
> > +                                                read_rect.width,
> > +                                                read_rect.height,
> > +                                                AllPlanes), &error);
>
> I made a point above about separating commits with logically
> independent pieces. To be honest though, if, when making a small
> change you notice an indentation problem or some missing
> documentation, then that's generally not a bad thing to just fix on
> the spot. So I wouldn't really object to a single commit for this
> change here. (But the general rule of splitting commits _is_ still
> important.)

Maybe I'm misusing the git-tools but manifacturing a new commit for
every set of spaces I write would be way to tedious.

> > -                  x1, y1, 0, 0, x2 - x1, y2 - y1);
> > +                     read_rect.x, read_rect.y,
> > +                     0, 0,
> > +                     read_rect.width, read_rect.height);
>
> Clearly cairo-xcb-surface.c is a total mess with regard to alignment
> of these multi-line function calls. It's definitely worth a separate
> commit first to fix this up since presumably there are many similar
> problems that your work doesn't happen to hit.
>
> I could do that easily with a quick "M-x indent-region" and commit,
> but I'll hold off since I don't want to make things more difficult for
> you by introducing conflicts.

I think I have fixed all the new indentation inconsistensies my patch
introduced. Below are the three final patches attached again.


-- 
mvh Björn
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Let-cairo_rectangle_intersect-return-TRUE-or-FALSE.patch
Type: text/x-patch
Size: 2199 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20071104/68c7b3b1/attachment-0003.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-docstrings-for-_get_image_surface-in-cairo-xli.patch
Type: text/x-patch
Size: 3813 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20071104/68c7b3b1/attachment-0004.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Modify-_get_image_surface-in-cairo-xlib-surface.c.patch
Type: text/x-patch
Size: 12589 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20071104/68c7b3b1/attachment-0005.bin