[Pixman] [PATCH] Add support for 18bpp X14R6G6B6 format.

[Marek Vasut]

Dne So 31. července 2010 12:38:53 Soeren Sandmann napsal(a):
> Marek Vasut <marek.vasut at gmail.com> writes:
> > This format is used on PXA framebuffer with some boards.
> 
> Looks good in general; just a couple of comments, the main one being
> that the format needs to be added to pixman_format_supported_source().
> 
> The format is 32bpp, not 18bpp as you say in the subject line.
> 
> See below for some style issues.
> 
> > Signed-off-by: Marek Vasut <marek.vasut at gmail.com>
> > ---
> > 
> >  pixman/pixman-access.c |   68
> >  +++++++++++++++++++++++++++++++++++++++++++++++- pixman/pixman.h       
> >  |    1 +
> >  2 files changed, 68 insertions(+), 1 deletions(-)
> > 
> > diff --git a/pixman/pixman-access.c b/pixman/pixman-access.c
> > index 80fa9e8..82a28de 100644
> > --- a/pixman/pixman-access.c
> > +++ b/pixman/pixman-access.c
> > @@ -397,6 +397,31 @@ fetch_scanline_b8g8r8 (pixman_image_t *image,
> > 
> >  }
> >  
> >  static void
> > 
> > +fetch_scanline_x14r6g6b6 (pixman_image_t *image,
> > +                       int             x,
> > +                       int             y,
> > +                       int             width,
> > +                       uint32_t *      buffer,
> > +                       const uint32_t *mask,
> > +                       uint32_t        mask_bits)
> 
> Please make sure all the arguments are aligned, instead of just
> leaving the indentation in place from where cutted-and-pasted. This
> applies to the other functions as well.

Hey,

this would better apply to the whole file. Maybe some coding-style cleanup wont 
hurt there. The reason I didn't use tabs for indent is exactly this -- to 
maintain consistency with the other function. I'll send an updated patch.
> 
> > +{
> > +    const uint32_t *bits = image->bits.bits + y * image->bits.rowstride;
> > +    const uint32_t *pixel = (const uint32_t *)bits + x;
> > +    const uint32_t *end = pixel + width;
> > +
> > +    while (pixel < end) {
> 
> Braces go on their own line.
> 
> Finally, the functions in pixman-access.c are generally in the same
> order as the formats are listed in pixman.h.

All right.

From your other mail:

> Also, 'mask_bits' is not used anymore. Please make sure your
> patch compiles without warnings and works with the pixman master.

That's a bit of a problem, it takes many hours on my machine (armel device). 
It's rebuilding mesa now and that will likely take a few more days, sorry for 
causing you trouble. I'll try rebuilding it asap.

Cheers
> 
> 
> Thanks,
> Soren