[fbdev_backend]: support of DIRECTCOLOR Frame buffer

[Pekka Paalanen]

On Tue, 11 Jun 2013 15:15:29 +0200
Marc Chalain <marc.chalain at gmail.com> wrote:

> Hello,
> OK this patch now checks FB_VISUAL_DIRECTCOLOR twice but set the cmap
> inside the fbdev_query_screen_info function.
> I use "malloc" instead "table" declaration because it takes less memory and
> it's used only once, it's useless to be faster.
> After I set the same values for each colors, because it's a default map. To
> set more finely we have to know the screen and it's capabilities.

What I meant is, if rcols = 32 and gcols = 64, you allocate a 64-long
array, and fill it with 64 step values. Does that not mean, that the
maximum red color will be only 50% intensity, because it goes only up to
32 steps, and the remaining 32 steps will never be used?

Or am I misunderstanding how fbdev colormap works?

> I don't set the alpha when it's required (no alpha => no alpha map)
> 
> [PATCH] [compositor-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..c966d5a 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 < rcols; i++)
> +        colors[i] = (UINT16_MAX / (rcols - 1)) * i;

IOW, should this divisor not be per channel, instead of using the same
for all channels?

In fact, you are using rcols in the divisor here, so if cols > rcols...
no wait...

> +
> +    cmap.start = 0;
> +    cmap.len = cols;

Here you use cols, but the loop uses rcols. How does this make sense?

> +    cmap.red = colors;
> +    cmap.blue = colors;
> +    cmap.green = colors;
> +    if (vinfo->transp.length)
> +        cmap.transp = colors;
> +    else
> +        cmap.transp = NULL;
> +    ioctl(fd, FBIOPUTCMAP, &cmap);
> +
> +    free(colors);
> +    return 0;
> +}
> +
>  static pixman_format_code_t
>  calculate_pixman_format(struct fb_var_screeninfo *vinfo,
>                          struct fb_fix_screeninfo *finfo)
> @@ -242,7 +279,8 @@ calculate_pixman_format(struct fb_var_screeninfo *vinfo,
>          return 0;
> 
>      /* We only handle true-colour frame buffers at the moment. */
> -    if (finfo->visual != FB_VISUAL_TRUECOLOR || vinfo->grayscale != 0)
> +    if (!(finfo->visual == FB_VISUAL_TRUECOLOR || finfo->visual ==
> FB_VISUAL_DIRECTCOLOR)
> +        || vinfo->grayscale != 0)
>          return 0;
> 
>      /* We only support formats with MSBs on the left. */
> @@ -325,6 +363,9 @@ fbdev_query_screen_info(struct fbdev_output *output,
> int fd,
>      info->pixel_format = calculate_pixman_format(&varinfo, &fixinfo);
>      info->refresh_rate = calculate_refresh_rate(&varinfo);
> 
> +    if (fixinfo.visual == FB_VISUAL_DIRECTCOLOR)
> +        set_cmap(fd, &varinfo);
> +
>      if (info->pixel_format == 0) {
>          weston_log("Frame buffer uses an unsupported format.\n");
>          return -1;

The last two hunks look good now.


Thanks,
pq