[PATCH v3] fbdev: Add an fbdev compositor backend using pixman and evdev
Kristian Høgsberg
hoegsberg at gmail.com
Thu Jan 24 13:53:27 PST 2013
On Wed, Jan 23, 2013 at 10:33:28AM +0200, Pekka Paalanen wrote:
> Hi Philip,
>
> I still didn't test this, but it looks good. I have some minor comments
> inline below.
If we can address Pekkas comments below, I think we're ready to commit
this. I second the nitpick about using $(NULL), we've not used those
before. The occasional extra diff-line is a small problem, but the
$(NULL) terminators look weird in the Makefile.am every day.
Great work Philip and thanks to everybody who helped review this patch.
Kristian
> On Tue, 22 Jan 2013 20:59:44 +0000
> Philip Withnall <philip at tecnocode.co.uk> wrote:
>
> > Here's the third version of the fbdev backend. This includes all the
> > fixes from the second version, plus a fix to use weston_launcher_open()
> > instead of open() for evdev devices, as pointed out by MoD.
> >
> > This patch is a squashed and tidied up version of various commits on my
> > branch:
> > https://gitorious.org/~pwithnall/weston/pwithnalls-weston/commits/compositor-fbdev.
> > As far as I'm concerned, it's now ready to be merged, pending a final
> > round of review (since there weren't any replies to v2 of the patch).
> >
>
> The above probably is not meant to end up in the git history, is it? If
> so, you should put it after the "---" line, where the diffstat is too.
> That way it gets discarded automatically on committing.
>
> >
> > Add a frame buffer backend using pixman to render to fbdev.
> > This has been tested against nouveaufb but nothing else. Much of the code
> > came straight from the rpi backend (and copyright has been attributed
> > accordingly).
> >
> > The behaviour of this backend on less modern frame buffers has yet to be
> > tested.
> >
> > The refresh rate is calculated from the frame buffer's metadata. Every frame
> > is finished in synchrony with the refresh rate.
> >
> > Frame buffer devices are currently specified on the command line (or using
> > the default of /dev/fb0); udev could be used in future to enumerate them.
> >
> > pixman is used for compositing, and a suitable pixman format is built from
> > the frame buffer's metadata. This doesn't support the full range of
> > frame buffer formats, but does support varying BPPs of RGBA and ARGB. That
> > should be enough for now.
> >
> > The following are not currently supported:
> > • FOURCC
> > • Non-packed formats (interleaved, planes, etc.)
> > • Non-true-colour formats (monochrome, greyscale, etc.)
> > • Big-endian formats (with component MSBs on the right)
> > • Non-RGBA and non-ARGB formats
> >
> > Signed-off-by: Philip Withnall <philip at tecnocode.co.uk>
> > ---
> > configure.ac | 10 +
> > src/Makefile.am | 27 +-
> > src/compositor-fbdev.c | 960 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 996 insertions(+), 1 deletion(-)
> > create mode 100644 src/compositor-fbdev.c
> >
> > diff --git a/configure.ac b/configure.ac
> > index 64505dd..57a444b 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -154,6 +154,16 @@ fi
> > AM_CONDITIONAL(INSTALL_RPI_COMPOSITOR, test "x$have_bcm_host" = "xyes")
> >
> >
> > +AC_ARG_ENABLE([fbdev-compositor], [ --enable-fbdev-compositor],,
> > + enable_fbdev_compositor=yes)
> > +AM_CONDITIONAL([ENABLE_FBDEV_COMPOSITOR],
> > + [test x$enable_fbdev_compositor = xyes])
> > +AS_IF([test x$enable_fbdev_compositor = xyes], [
> > + AC_DEFINE([BUILD_FBDEV_COMPOSITOR], [1], [Build the fbdev compositor])
> > + PKG_CHECK_MODULES([FBDEV_COMPOSITOR], [libudev >= 136 mtdev >= 1.1.0])
> > +])
> > +
> > +
> > AC_ARG_WITH(cairo-glesv2,
> > AS_HELP_STRING([--with-cairo-glesv2],
> > [Use GLESv2 cairo instead of full GL]))
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index cbfa911..ccbb4bd 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -84,7 +84,8 @@ module_LTLIBRARIES = \
> > $(x11_backend) \
> > $(drm_backend) \
> > $(wayland_backend) \
> > - $(headless_backend)
> > + $(headless_backend) \
> > + $(fbdev_backend)
> >
> > # Do not install, since the binary produced via autotools is unusable.
> > # The real backend is built by the Android build system.
> > @@ -195,6 +196,30 @@ headless_backend_la_CFLAGS = \
> > headless_backend_la_SOURCES = compositor-headless.c
> > endif
> >
> > +if ENABLE_FBDEV_COMPOSITOR
> > +fbdev_backend = fbdev-backend.la
> > +fbdev_backend_la_LDFLAGS = -module -avoid-version
> > +fbdev_backend_la_LIBADD = \
> > + $(COMPOSITOR_LIBS) \
> > + $(FBDEV_COMPOSITOR_LIBS) \
> > + ../shared/libshared.la \
> > + $(NULL)
> > +fbdev_backend_la_CFLAGS = \
> > + $(COMPOSITOR_CFLAGS) \
> > + $(FBDEV_COMPOSITOR_CFLAGS) \
> > + $(PIXMAN_CFLAGS) \
> > + $(GCC_CFLAGS) \
> > + $(NULL)
> > +fbdev_backend_la_SOURCES = \
> > + compositor-fbdev.c \
> > + tty.c \
> > + evdev.c \
> > + evdev.h \
> > + evdev-touchpad.c \
> > + launcher-util.c \
> > + $(NULL)
> > +endif
>
> This will be first time using $(NULL), and I believe it relies on
> nothing ever assigning a value to variable NULL (grep finds no NULL at
> all in currently generated Makefiles). *shrug*
>
> > +
> > if ENABLE_DESKTOP_SHELL
> > desktop_shell = desktop-shell.la
> > desktop_shell_la_LDFLAGS = -module -avoid-version
> > diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> > new file mode 100644
> > index 0000000..9674017
> > --- /dev/null
> > +++ b/src/compositor-fbdev.c
> > @@ -0,0 +1,960 @@
> > +/*
> > + * Copyright © 2008-2011 Kristian Høgsberg
> > + * Copyright © 2011 Intel Corporation
> > + * Copyright © 2012 Raspberry Pi Foundation
> > + * Copyright © 2013 Philip Withnall
> > + *
> > + * Permission to use, copy, modify, distribute, and sell this software and
> > + * its documentation for any purpose is hereby granted without fee, provided
> > + * that the above copyright notice appear in all copies and that both that
> > + * copyright notice and this permission notice appear in supporting
> > + * documentation, and that the name of the copyright holders not be used in
> > + * advertising or publicity pertaining to distribution of the software
> > + * without specific, written prior permission. The copyright holders make
> > + * no representations about the suitability of this software for any
> > + * purpose. It is provided "as is" without express or implied warranty.
> > + *
> > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> > + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
> > + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
> > + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> > + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + */
> > +
> > +#define _GNU_SOURCE
> > +
> > +#include <errno.h>
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <math.h>
> > +#include <sys/mman.h>
> > +#include <sys/types.h>
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <linux/fb.h>
> > +
> > +#include <libudev.h>
> > +
> > +#include "config.h"
> > +
> > +#include "compositor.h"
> > +#include "launcher-util.h"
> > +#include "pixman-renderer.h"
> > +#include "evdev.h"
> > +
> > +struct fbdev_compositor {
> > + struct weston_compositor base;
> > + uint32_t prev_state;
> > +
> > + struct udev *udev;
> > + struct tty *tty;
> > +};
> > +
> > +struct fbdev_output {
> > + struct fbdev_compositor *compositor;
> > + struct weston_output base;
> > +
> > + struct weston_mode mode;
> > + struct wl_event_source *finish_frame_timer;
> > +
> > + /* Frame buffer details. */
> > + struct fb_fix_screeninfo fixed_info;
> > + struct fb_var_screeninfo variable_info;
> > + void *fb;
> > + size_t fb_len;
> > +
> > + /* pixman details. */
> > + pixman_image_t *hw_surface;
> > + pixman_image_t *shadow_surface;
> > + void *shadow_buf;
> > + uint8_t depth;
> > +};
> > +
> > +struct fbdev_seat {
> > + struct weston_seat base;
> > + struct wl_list devices_list;
> > +
> > + struct udev_monitor *udev_monitor;
> > + struct wl_event_source *udev_monitor_source;
> > + char *seat_id;
> > +};
> > +
> > +struct fbdev_parameters {
> > + int tty;
> > + char *device;
> > +};
> > +
> > +static inline struct fbdev_output *
> > +to_fbdev_output(struct weston_output *base)
> > +{
> > + return container_of(base, struct fbdev_output, base);
> > +}
> > +
> > +static inline struct fbdev_seat *
> > +to_fbdev_seat(struct weston_seat *base)
> > +{
> > + return container_of(base, struct fbdev_seat, base);
> > +}
> > +
> > +static inline struct fbdev_compositor *
> > +to_fbdev_compositor(struct weston_compositor *base)
> > +{
> > + return container_of(base, struct fbdev_compositor, base);
> > +}
> > +
> > +static void
> > +fbdev_output_repaint(struct weston_output *base, pixman_region32_t *damage)
> > +{
> > + struct fbdev_output *output = to_fbdev_output(base);
> > + struct weston_compositor *ec = output->base.compositor;
> > + pixman_box32_t *rects;
> > + int nrects, i, src_x, src_y, x1, y1, x2, y2, width, height;
> > +
> > + /* Repaint the damaged region onto the back buffer. */
> > + pixman_renderer_output_set_buffer(base, output->shadow_surface);
> > + ec->renderer->repaint_output(base, damage);
> > +
> > + /* Transform and composite onto the frame buffer. */
> > + width = pixman_image_get_width(output->shadow_surface);
> > + height = pixman_image_get_height(output->shadow_surface);
> > + rects = pixman_region32_rectangles(damage, &nrects);
> > +
> > + for (i = 0; i < nrects; i++) {
> > + switch (base->transform) {
> > + default:
> > + case WL_OUTPUT_TRANSFORM_NORMAL:
> > + x1 = rects[i].x1;
> > + x2 = rects[i].x2;
> > + y1 = rects[i].y1;
> > + y2 = rects[i].y2;
> > + break;
> > + case WL_OUTPUT_TRANSFORM_180:
> > + x1 = width - rects[i].x2;
> > + x2 = width - rects[i].x1;
> > + y1 = height - rects[i].y2;
> > + y2 = height - rects[i].y1;
> > + break;
> > + case WL_OUTPUT_TRANSFORM_90:
> > + x1 = height - rects[i].y2;
> > + x2 = height - rects[i].y1;
> > + y1 = rects[i].x1;
> > + y2 = rects[i].x2;
> > + break;
> > + case WL_OUTPUT_TRANSFORM_270:
> > + x1 = rects[i].y1;
> > + x2 = rects[i].y2;
> > + y1 = width - rects[i].x2;
> > + y2 = width - rects[i].x1;
> > + break;
> > + }
> > + src_x = x1;
> > + src_y = y1;
> > +
> > + pixman_image_composite32(PIXMAN_OP_SRC,
> > + output->shadow_surface, /* src */
> > + NULL /* mask */,
> > + output->hw_surface, /* dest */
> > + src_x, src_y, /* src_x, src_y */
> > + 0, 0, /* mask_x, mask_y */
> > + x1, y1, /* dest_x, dest_y */
> > + x2 - x1, /* width */
> > + y2 - y1 /* height */);
> > + }
> > +
> > + /* Update the damage region. */
> > + pixman_region32_subtract(&ec->primary_plane.damage,
> > + &ec->primary_plane.damage, damage);
> > +
> > + /* Schedule the end of the frame. We do not sync this to the frame
> > + * buffer clock because users who want that should be using the DRM
> > + * compositor. FBIO_WAITFORVSYNC blocks and FB_ACTIVATE_VBL requires
> > + * panning, which is broken in most kernel drivers.
> > + *
> > + * Finish the frame synchronised to the specified refresh rate. The
> > + * refresh rate is given in mHz and the interval in ms. */
> > + wl_event_source_timer_update(output->finish_frame_timer,
> > + 1000000 / output->mode.refresh);
> > +}
> > +
> > +static int
> > +finish_frame_handler(void *data)
> > +{
> > + struct fbdev_output *output = data;
> > + uint32_t msec;
> > + struct timeval tv;
> > +
> > + gettimeofday(&tv, NULL);
> > + msec = tv.tv_sec * 1000 + tv.tv_usec / 1000;
> > + weston_output_finish_frame(&output->base, msec);
> > +
> > + return 1;
> > +}
> > +
> > +static int
> > +fbdev_frame_buffer_create(struct fbdev_output *output, const char *fb_dev)
> > +{
> > + int retval = -1;
> > + int fd = -1;
> > +
> > + /* Open the frame buffer device. */
> > + fd = open (fb_dev, O_RDWR | O_CLOEXEC);
>
> Extra space.
>
> > + if (fd < 0) {
> > + weston_log("Failed to open frame buffer device ‘%s’: %s\n",
> > + fb_dev, strerror(errno));
> > + goto err;
> > + }
> > +
> > + /* Grab the screen info. */
> > + if (ioctl(fd, FBIOGET_FSCREENINFO, &output->fixed_info) < 0 ||
> > + ioctl(fd, FBIOGET_VSCREENINFO, &output->variable_info) < 0) {
> > + weston_log("Failed to get frame buffer info: %s\n",
> > + strerror(errno));
> > + goto err;
> > + }
> > +
> > + /* Map the frame buffer. Write-only mode, since we don't want to read
> > + * anything back (because it's slow). */
> > + output->fb = mmap (NULL, output->fixed_info.smem_len,
>
> Extra space.
>
> > + PROT_WRITE, MAP_SHARED, fd, 0);
> > + if (output->fb == MAP_FAILED) {
> > + weston_log("Failed to mmap frame buffer: %s\n",
> > + strerror(errno));
> > + goto err;
> > + }
> > +
> > + /* Success! */
> > + output->fb_len = output->fixed_info.smem_len;
> > + retval = 0;
> > +
> > +err:
> > + if (fd >= 0)
> > + close (fd);
>
> Extra space.
>
> > +
> > + return retval;
> > +}
> > +
> > +static void
> > +fbdev_frame_buffer_destroy(struct fbdev_output *output)
> > +{
> > + if (munmap(output->fb, output->fb_len) < 0)
> > + weston_log("Failed to munmap frame buffer: %s\n",
> > + strerror(errno));
> > +
> > + output->fb = NULL;
> > + output->fb_len = 0;
> > +}
> > +
> > +static pixman_format_code_t
> > +calculate_pixman_format(struct fb_var_screeninfo *vinfo,
> > + struct fb_fix_screeninfo *finfo)
> > +{
> > + /* Calculate the pixman format supported by the frame buffer from the
> > + * buffer's metadata. Return 0 if no known pixman format is supported
> > + * (since this has depth 0 it's guaranteed to not conflict with any
> > + * actual pixman format).
> > + *
> > + * Documentation on the vinfo and finfo structures:
> > + * http://www.mjmwired.net/kernel/Documentation/fb/api.txt
> > + *
> > + * TODO: Try a bit harder to support other formats, including setting
> > + * the preferred format in the hardware. */
> > + int type;
> > +
> > + weston_log("Calculating pixman format from:\n"
> > + STAMP_SPACE " - type: %i (aux: %i)\n"
> > + STAMP_SPACE " - visual: %i\n"
> > + STAMP_SPACE " - bpp: %i (grayscale: %i)\n"
> > + STAMP_SPACE " - red: offset: %i, length: %i, MSB: %i\n"
> > + STAMP_SPACE " - green: offset: %i, length: %i, MSB: %i\n"
> > + STAMP_SPACE " - blue: offset: %i, length: %i, MSB: %i\n"
> > + STAMP_SPACE " - transp: offset: %i, length: %i, MSB: %i\n",
> > + finfo->type, finfo->type_aux, finfo->visual,
> > + vinfo->bits_per_pixel, vinfo->grayscale,
> > + vinfo->red.offset, vinfo->red.length, vinfo->red.msb_right,
> > + vinfo->green.offset, vinfo->green.length,
> > + vinfo->green.msb_right,
> > + vinfo->blue.offset, vinfo->blue.length,
> > + vinfo->blue.msb_right,
> > + vinfo->transp.offset, vinfo->transp.length,
> > + vinfo->transp.msb_right);
> > +
> > + /* We only handle packed formats at the moment. */
> > + if (finfo->type != FB_TYPE_PACKED_PIXELS)
> > + return 0;
> > +
> > + /* We only handle true-colour frame buffers at the moment. */
> > + if (finfo->visual != FB_VISUAL_TRUECOLOR || vinfo->grayscale != 0)
> > + return 0;
> > +
> > + /* We only support formats with MSBs on the left. */
> > + if (vinfo->red.msb_right != 0 || vinfo->green.msb_right != 0 ||
> > + vinfo->blue.msb_right != 0)
> > + return 0;
> > +
> > + /* Work out the format type from the offsets. We only support RGBA and
> > + * ARGB at the moment. */
> > + type = PIXMAN_TYPE_OTHER;
> > +
> > + if ((vinfo->transp.offset >= vinfo->red.offset ||
> > + vinfo->transp.length == 0) &&
> > + vinfo->red.offset >= vinfo->green.offset &&
> > + vinfo->green.offset >= vinfo->blue.offset)
> > + type = PIXMAN_TYPE_ARGB;
> > + else if (vinfo->red.offset >= vinfo->green.offset &&
> > + vinfo->green.offset >= vinfo->blue.offset &&
> > + vinfo->blue.offset >= vinfo->transp.offset)
> > + type = PIXMAN_TYPE_RGBA;
> > +
> > + if (type == PIXMAN_TYPE_OTHER)
> > + return 0;
> > +
> > + /* Build the format. */
> > + return PIXMAN_FORMAT(vinfo->bits_per_pixel, type,
> > + vinfo->transp.length,
> > + vinfo->red.length,
> > + vinfo->green.length,
> > + vinfo->blue.length);
> > +}
> > +
> > +static int
> > +calculate_refresh_rate(struct fb_var_screeninfo *vinfo)
> > +{
> > + uint64_t quot;
> > +
> > + /* Calculate monitor refresh rate. Default is 60 Hz. Units are mHz. */
> > + quot = (vinfo->upper_margin + vinfo->lower_margin + vinfo->yres);
> > + quot *= (vinfo->left_margin + vinfo->right_margin + vinfo->xres);
> > + quot *= vinfo->pixclock;
> > +
> > + if (quot > 0) {
> > + uint64_t refresh_rate;
> > +
> > + refresh_rate = 1000000000000000LLU / quot;
> > + if (refresh_rate > 200000)
> > + refresh_rate = 200000; /* cap at 200 Hz */
> > +
> > + return refresh_rate;
> > + }
> > +
> > + return 60 * 1000; /* default to 60 Hz */
> > +}
> > +
> > +static void fbdev_output_destroy(struct weston_output *base);
> > +
> > +static int
> > +fbdev_output_create(struct fbdev_compositor *compositor,
> > + struct fbdev_parameters *param)
> > +{
> > + struct fbdev_output *output;
> > + pixman_transform_t transform;
> > + int shadow_width, shadow_height;
> > + int width, height;
> > + pixman_format_code_t pixman_format;
> > + struct wl_event_loop *loop;
> > +
> > + output = calloc(1, sizeof *output);
> > + if (!output)
> > + return -1;
> > +
> > + output->compositor = compositor;
> > +
> > + /* Create the frame buffer. */
> > + if (fbdev_frame_buffer_create(output, param->device) < 0) {
> > + weston_log("Creating frame buffer failed.\n");
> > + goto out_free;
> > + }
> > +
> > + pixman_format = calculate_pixman_format(&output->variable_info, &output->fixed_info);
> > + if (pixman_format == 0) {
> > + weston_log("Frame buffer uses an unsupported format.\n");
> > + goto out_fb;
> > + }
> > +
> > + output->base.repaint = fbdev_output_repaint;
> > + output->base.destroy = fbdev_output_destroy;
> > + output->base.assign_planes = NULL;
> > + output->base.set_backlight = NULL;
> > + output->base.set_dpms = NULL;
> > + output->base.switch_mode = NULL;
> > +
> > + /* only one static mode in list */
> > + output->mode.flags =
> > + WL_OUTPUT_MODE_CURRENT | WL_OUTPUT_MODE_PREFERRED;
> > + output->mode.width = output->variable_info.xres;
> > + output->mode.height = output->variable_info.yres;
> > + output->mode.refresh = calculate_refresh_rate(&output->variable_info);
> > + wl_list_init(&output->base.mode_list);
> > + wl_list_insert(&output->base.mode_list, &output->mode.link);
> > +
> > + output->base.current = &output->mode;
> > + output->base.origin = &output->mode;
> > + output->base.subpixel = WL_OUTPUT_SUBPIXEL_UNKNOWN;
>
> > + output->base.make = "fbdev";
> > + output->base.model = output->fixed_info.id;
>
> These are meant for the monitor make and model, and are transmitted to
> clients. I think "fbdev" is not right, since there actually is a real
> monitor, so make could be "unknown".
>
> > +
> > + weston_output_init(&output->base, &compositor->base,
> > + 0, 0, output->variable_info.width,
> > + output->variable_info.height,
> > + WL_OUTPUT_TRANSFORM_NORMAL);
> > +
> > + /* Create a pixman image to wrap the memory mapped frame buffer. */
> > + width = output->variable_info.xres;
> > + height = output->variable_info.yres;
> > +
> > + output->hw_surface =
> > + pixman_image_create_bits(pixman_format, width, height,
> > + output->fb,
> > + output->fixed_info.line_length);
> > + if (output->hw_surface == NULL) {
> > + weston_log("Failed to create surface for frame buffer.\n");
> > + goto out_output;
> > + }
> > +
> > + pixman_transform_init_identity(&transform);
> > + switch (output->base.transform) {
> > + default:
> > + case WL_OUTPUT_TRANSFORM_NORMAL:
> > + shadow_width = width;
> > + shadow_height = height;
> > + pixman_transform_rotate(&transform,
> > + NULL, 0, 0);
> > + pixman_transform_translate(&transform, NULL,
> > + 0, 0);
> > + break;
> > + case WL_OUTPUT_TRANSFORM_180:
> > + shadow_width = width;
> > + shadow_height = height;
> > + pixman_transform_rotate(&transform,
> > + NULL, -pixman_fixed_1, 0);
> > + pixman_transform_translate(NULL, &transform,
> > + pixman_int_to_fixed(shadow_width),
> > + pixman_int_to_fixed(shadow_height));
> > + break;
> > + case WL_OUTPUT_TRANSFORM_270:
> > + shadow_width = height;
> > + shadow_height = width;
> > + pixman_transform_rotate(&transform,
> > + NULL, 0, pixman_fixed_1);
> > + pixman_transform_translate(&transform,
> > + NULL,
> > + pixman_int_to_fixed(shadow_width),
> > + 0);
> > + break;
> > + case WL_OUTPUT_TRANSFORM_90:
> > + shadow_width = height;
> > + shadow_height = width;
> > + pixman_transform_rotate(&transform,
> > + NULL, 0, -pixman_fixed_1);
> > + pixman_transform_translate(&transform,
> > + NULL,
> > + 0,
> > + pixman_int_to_fixed(shadow_height));
> > + break;
> > + }
> > +
> > + output->shadow_buf = malloc(width * height * (output->variable_info.bits_per_pixel / 8));
> > + output->shadow_surface =
> > + pixman_image_create_bits(pixman_format, shadow_width,
> > + shadow_height, output->shadow_buf,
> > + shadow_width * (output->variable_info.bits_per_pixel / 8));
>
> Way long lines, maybe a helper variable for bytes per pixel?
>
> Hmm, so the shadow buffer is in the same format as the framebuffer,
> right? Would it be better for the renderer to have the shadow buffer in
> a default format, and convert only on blit? I think that way the
> renderer could work with its most performant format for blending etc.
> and do the fb-format conversion just once.
>
> However, I see that anderco's patch moves the shadow buffer to the
> renderer, so this issue should resolve naturally, once the two patch
> sets get merged.
>
> > + if (output->shadow_buf == NULL || output->shadow_surface == NULL) {
> > + weston_log("Failed to create surface for frame buffer.\n");
> > + goto out_hw_surface;
> > + }
> > +
> > + /* No need in transform for normal output */
> > + if (output->base.transform != WL_OUTPUT_TRANSFORM_NORMAL)
> > + pixman_image_set_transform(output->shadow_surface, &transform);
> > +
> > + if (pixman_renderer_output_create(&output->base) < 0)
> > + goto out_shadow_surface;
> > +
> > + loop = wl_display_get_event_loop(compositor->base.wl_display);
> > + output->finish_frame_timer =
> > + wl_event_loop_add_timer(loop, finish_frame_handler, output);
> > +
> > + wl_list_insert(compositor->base.output_list.prev, &output->base.link);
> > +
> > + weston_log("fbdev output %d×%d px\n",
>
> That looks like an utf-8 cross-product character, is it? Is that
> portable?
>
> > + output->mode.width, output->mode.height);
> > + weston_log_continue(STAMP_SPACE "guessing %d Hz and 96 dpi\n",
> > + output->mode.refresh / 1000);
> > +
> > + return 0;
> > +
> > +out_shadow_surface:
> > + pixman_image_unref(output->shadow_surface);
> > +out_hw_surface:
> > + free(output->shadow_buf);
> > + pixman_image_unref(output->hw_surface);
> > +out_output:
> > + weston_output_destroy(&output->base);
> > +out_fb:
> > + fbdev_frame_buffer_destroy(output);
> > +out_free:
> > + free(output);
> > +
> > + return -1;
> > +}
> > +
> > +static void
> > +fbdev_output_destroy(struct weston_output *base)
> > +{
> > + struct fbdev_output *output = to_fbdev_output(base);
> > +
> > + weston_log("%s\n", __func__);
>
> Forgotten debug print?
>
>
> Thanks,
> pq
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list