[cairo] The short road left to 1.6 - EXTEND_PAD

Antoine Azar 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 - /* ... */.

Done.

>The conflict is here - these lines now read
>
>| -    v.vector[0] = pixman_int_to_fixed(x) + pixman_fixed_1 / 2 - 1;
>| -    v.vector[1] = pixman_int_to_fixed(y) + pixman_fixed_1 / 2 - 1;

Done.

> > +    //intialize the two function pointers
>
>Typo, which is repeated several times in the code.

Good eyes! Fixed.

Here's the updated patch.
Thanks,
Antoine 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pixman-compose_refactor.patch
Type: application/octet-stream
Size: 41166 bytes
Desc: not available
Url : http://lists.cairographics.org/archives/cairo/attachments/20080128/a8efc7ee/attachment-0001.obj 


More information about the cairo mailing list