[fbdev_backend]: support of DIRECTCOLOR Frame buffer

[Pekka Paalanen]

On Wed, 12 Jun 2013 10:07:00 +0200
Marc Chalain <marc.chalain at gmail.com> wrote:

> It's not broken, it's just 1 value per 65535 unused for red and blue. The
> difference between the red 65534 and the red 65535 is insignificant. The

I have no idea what you are talking about, but lets assume rgb565 and
see what happens.

red.length = 5
green.length = 6

therefore:

rcols = 32
gcols = 64

take the maximum, cols = 64.

Run your loop:

> > > +    for (i = 0; i < cols; i++)
> > > +        colors[i] = (UINT16_MAX / (cols - 1)) * i;

Now:

colors[0] = 0
...
colors[31] = 32240
...
colors[63] = 65520

(There is a rounding error you could probably correct by multiplying
first and dividing last, but that is not the point.)

Green channel can have indices 0..63, which means it covers values
0..65520.

Red channel can have indices 0..31, which means it covers values
0..32240.

Can you explain to me, why this does not make the red (and blue)
channel have only half of the intensity on screen of what it should
have? What have I misunderstood here?

> real fix is more significant when we run on target with 126Mb of RAM or
> less. Don't forget that on PC Desktop you find very big video card which
> runs with A8R8G8B8 pixel format in lot of cases.

Which real fix? What does the RAM amount have to do with how you
compute the color map?

Let's say you have 8 bits and 4 channels. Even if you made separate
arrays for each channel, you would use only 2 kB of RAM, and only
temporarily when setting the color map. That's hardly relevant if your
system can run a Linux kernel to begin with.

- pq

> 2013/6/11 Pekka Paalanen <ppaalanen at gmail.com>
> 
> > On Tue, 11 Jun 2013 16:31:17 +0200
> > Marc Chalain <marc.chalain at gmail.com> wrote:
> >
> > > Theoretically you have right.
> > > But the pixel formats are defined to contains the same amounts of values
> > > for each colors except for 16bpp format (r5g6b5 and b5g6r5 one value more
> > > for green) and the alpha. My code is optimized for a lot of cases.
> > > For the use of rcols instead cols, you have right. It's a mistake.
> >
> > So, you think it's ok to be presumably broken on 565 formats?
> > The fix really is not that hard, and the "optimizations" here are
> > totally insignificant.
> >
> > > Sorry "git send-email" fails on my computer (I think it's come from the
> > > proxy).
> >
> > There are other ways to send email without messing up whitespace,
> > any of them will do. Take a look at other people's patch series on
> > the mailing list, how they look, and try to replicate that.
> >
> > Until then, I think I've reviewed enough here for now.
> >
> > Btw. the idle-time man page patch should be part of the same commit
> > as the idle-time config implementation.
> >
> >
> > Thanks,
> > pq
> >
> > > [PATCH] fbdev: directcolor support
> > > The backend accepts to use TRUECOLOR and DIRECTCOLOR which are similares.
> > > DIRECTCOLOR has better results if the application applies a Colors
> > > map before to use it.
> > > The colors map is just a list of gradients for each colors and alpha.
> > >
> > > ---
> > >  src/compositor-fbdev.c |   43
> > ++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 42 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> > > index 9c3d17e..7a3374e 100644
> > > --- a/src/compositor-fbdev.c
> > > +++ b/src/compositor-fbdev.c
> > > @@ -203,6 +203,43 @@ finish_frame_handler(void *data)
> > >      return 1;
> > >  }
> > >
> > > +static int
> > > +set_cmap(int fd, struct fb_var_screeninfo *vinfo)
> > > +{
> > > +    struct fb_cmap cmap;
> > > +    uint16_t *colors;
> > > +    int i;
> > > +    int rcols = 1 << vinfo->red.length;
> > > +    int gcols = 1 << vinfo->green.length;
> > > +    int bcols = 1 << vinfo->blue.length;
> > > +
> > > +    memset(&cmap, 0, sizeof(cmap));
> > > +
> > > +    /* Make our palette the length of the deepest color */
> > > +    int cols = (rcols > gcols ? rcols : gcols);
> > > +    cols = (cols > bcols ? cols : bcols);
> > > +
> > > +    colors = malloc(sizeof(uint16_t) * cols);
> > > +    if (!colors) return -1;
> > > +
> > > +    for (i = 0; i < cols; i++)
> > > +        colors[i] = (UINT16_MAX / (cols - 1)) * i;
> > > +
> > > +    cmap.start = 0;
> > > +    cmap.len = cols;
> > > +    cmap.red = colors;
> > > +    cmap.blue = colors;
> > > +    cmap.green = colors;