[cairo] The short road left to 1.6 - EXTEND_PAD
cairo at antoineazar.com
Mon Jan 28 08:14:51 PST 2008
Thanks for the very thorough code review Bertram. My comments inline.
>Looks good to me, code-wise. However, the indentation is messed up
>badly, and there are a few other style nits.
I've had trouble with Visual Studio, hopefully everything should be fine now.
>There's a conflict with commit 1d89bac5a7a5693911d8a74701bd1c0292160478
>-- fix cairo's a1-image-sample test.
I integrated these changes, let me know if there's still any trouble.
> > diff --git a/pixman/pixman-compose.c b/pixman/pixman-compose.c
> > index c3f50e2..e09c809 100644
> > --- a/pixman/pixman-compose.c
> > +++ b/pixman/pixman-compose.c
> > @@ -124,11 +124,6 @@ SourcePictureClassify (source_image_t *pict,
> > int offset1 = stride < 0 ? \
> > offset0 + ((-stride) >> 1) * ((pict->height) >> 1) : \
> > offset0 + (offset0 >> 2)
> > -/* Note n trailing semicolon on the above macro; if it's there, then
> > - * the typical usage of YV12_SETUP(pict); will have an extra trailing ;
> > - * that some compilers will interpret as a statement -- and then
> any further
> > - * variable declarations will cause an error.
> > - */
>Is there any good reason to remove that comment?
I might be missing something but I don't see the usefulness of having
a trailing semicolon. Is the comment only there to let developers
know they should NOT append a semicolon to the define? if so that
should probably be in the coding guidelines rather than in the code.
Anyhow I put it back for now.
>Please use C89 comments - /* ... */.
>The conflict is here - these lines now read
>| - v.vector = pixman_int_to_fixed(x) + pixman_fixed_1 / 2 - 1;
>| - v.vector = pixman_int_to_fixed(y) + pixman_fixed_1 / 2 - 1;
> > + //intialize the two function pointers
>Typo, which is repeated several times in the code.
Good eyes! Fixed.
Here's the updated patch.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 41166 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20080128/a8efc7ee/attachment-0001.obj
More information about the cairo