[cairo] Use _cairo_rectangle_intersect in more places for intersection checking

[BJörn Lindqvist]

I think this mail got lost. What happened to these patches?


--
mvh Björn

On Sun, Nov 4, 2007 at 3:41 AM, BJörn Lindqvist <bjourne at gmail.com> wrote:
> 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
>



-- 
mvh Björn