[fbdev_backend]: support of DIRECTCOLOR Frame buffer

Marc Chalain marc.chalain at gmail.com
Tue Jun 11 06:15:29 PDT 2013


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.
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;
+
+    cmap.start = 0;
+    cmap.len = cols;
+    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;
-- 
1.7.9.5




2013/6/10 Pekka Paalanen <ppaalanen at gmail.com>

> On Fri, 7 Jun 2013 17:13:59 +0200
> Marc Chalain <marc.chalain at gmail.com> wrote:
>
> > From e30486e0288e7be526fc358a5318b344522e407b Mon Sep 17 00:00:00 2001
> > From: mchalain <marc.chalain at gmail.com>
> > Date: Fri, 7 Jun 2013 17:12:31 +0200
> > Subject: [PATCH] [fbdev_backend]: support of DIRECTCOLOR Frame buffer
> >
>
> A properly formatted title:
>         compositor-fbdev: add direct color support
>
> Commit message is missing.
>
> > ---
> >  src/compositor-fbdev.c |   49
> > +++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 46 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> > index 9c3d17e..d6d911a 100644
> > --- a/src/compositor-fbdev.c
> > +++ b/src/compositor-fbdev.c
> > @@ -203,9 +203,42 @@ finish_frame_handler(void *data)
> >      return 1;
> >  }
> >
> > +static int
> > +build_cmap(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;
> > +
> > +    /* 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;
> > +
> > +    cmap->start = 0;
> > +    cmap->len = cols;
> > +    cmap->red = colors;
> > +    cmap->blue = colors;
> > +    cmap->green = colors;
>
> If there actually is a difference between rcols, gcols, and bcols,
> should you not compute a different a 'colors' array for each different
> _cols?
>
> > +    if (vinfo->transp.length)
> > +        cmap->transp = colors;
> > +    else
> > +        cmap->transp = NULL;
>
> Now, 'colors' is malloc'd here. How does the caller of this function
> know what to free? Freeing each of cmap->{red,blue,green,transp} would
> lead to double-free errors.
>
> Right, you free only 'red'. But that's rather surprising.
>
> > +    return 0;
> > +}
> > +
> >  static pixman_format_code_t
> >  calculate_pixman_format(struct fb_var_screeninfo *vinfo,
> > -                        struct fb_fix_screeninfo *finfo)
> > +                        struct fb_fix_screeninfo *finfo,
> > +                        struct fb_cmap *cmap)
> >  {
> >      /* Calculate the pixman format supported by the frame buffer from
> the
> >       * buffer's metadata. Return 0 if no known pixman format is
> supported
> > @@ -241,7 +274,10 @@ calculate_pixman_format(struct fb_var_screeninfo
> > *vinfo,
> >      if (finfo->type != FB_TYPE_PACKED_PIXELS)
> >          return 0;
> >
> > -    /* We only handle true-colour frame buffers at the moment. */
> > +    if (finfo->visual == FB_VISUAL_DIRECTCOLOR)
> > +        build_cmap(vinfo, cmap);
>
> This is a strange place to call build_cmap(). The purpose of this
> function is to return a pixman format or an error, not to fill a
> fb_cmap struct.
>
> Yes, you probably need to test for FB_VISUAL_DIRECTCOLOR in two places.
>
> > +    else
> > +     /* We only handle true-colour frame buffers at the moment. */
> >      if (finfo->visual != FB_VISUAL_TRUECOLOR || vinfo->grayscale != 0)
> >          return 0;
> >
> > @@ -304,6 +340,9 @@ fbdev_query_screen_info(struct fbdev_output *output,
> > int fd,
> >  {
> >      struct fb_var_screeninfo varinfo;
> >      struct fb_fix_screeninfo fixinfo;
> > +    struct fb_cmap cmap;
> > +
> > +    memset(&cmap, 0, sizeof(cmap));
> >
> >      /* Probe the device for screen information. */
> >      if (ioctl(fd, FBIOGET_FSCREENINFO, &fixinfo) < 0 ||
> > @@ -322,9 +361,13 @@ fbdev_query_screen_info(struct fbdev_output *output,
> > int fd,
> >      info->line_length = fixinfo.line_length;
> >      strncpy(info->id, fixinfo.id, sizeof(info->id) /
> sizeof(*info->id));
> >
> > -    info->pixel_format = calculate_pixman_format(&varinfo, &fixinfo);
> > +    info->pixel_format = calculate_pixman_format(&varinfo, &fixinfo,
> > &cmap);
> >      info->refresh_rate = calculate_refresh_rate(&varinfo);
> >
> > +    if (cmap.red != NULL) {
> > +        ioctl(fd, FBIOPUTCMAP, &cmap);
> > +        free(cmap.red);
> > +    }
> >      if (info->pixel_format == 0) {
> >          weston_log("Frame buffer uses an unsupported format.\n");
> >          return -1;
>
> This patch is indented with 4 spaces, while weston codebase uses hard
> tabs of width 8. Maybe your email client messed up the whitespace?
> I also see a few lines cut unintendedly.
>
>
> Thanks
> pq
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130611/fc6f3c98/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-compositor-fbdev-DirectColor-support.patch
Type: application/octet-stream
Size: 2577 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130611/fc6f3c98/attachment-0001.obj>


More information about the wayland-devel mailing list