On Wed, 19 Sep 2012, Gerd Hoffmann wrote:
> Stop reinventing the wheel, use the pixman library for raster ops.
> Also add setdata callback.
>
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> ---
> configure | 12 ++++
> ui/fbdev.c | 182
> +++++++++++++++++++++++++++++++++++++-----------------------
> 2 files changed, 124 insertions(+), 70 deletions(-)
>
> diff --git a/configure b/configure
> index c4ba338..d10ff78 100755
> --- a/configure
> +++ b/configure
> @@ -148,6 +148,7 @@ docs=""
> fdt=""
> nptl=""
> sdl=""
> +pixman=""
> fbdev="no"
> virtfs=""
> vnc="yes"
> @@ -2153,6 +2154,17 @@ else
> exit 1
> fi
>
> +if $pkg_config pixman-1 > /dev/null 2>&1
> +then
> + pixman="yes"
> + pixman_cflags=`$pkg_config --cflags pixman-1 2>/dev/null`
> + pixman_libs=`$pkg_config --libs pixman-1 2>/dev/null`
> + QEMU_CFLAGS="$QEMU_CFLAGS $pixman_cflags"
> + libs_softmmu="$libs_softmmu $pixman_libs"
> +else
> + fbdev="no"
> +fi
> +
> ##########################################
> # libcap probe
>
> diff --git a/ui/fbdev.c b/ui/fbdev.c
> index 6dfbacc..d55850e 100644
> --- a/ui/fbdev.c
> +++ b/ui/fbdev.c
> @@ -23,11 +23,12 @@
> #include <linux/vt.h>
> #include <linux/fb.h>
>
> +#include <pixman.h>
> +
> #include "qemu-common.h"
> #include "console.h"
> #include "keymaps.h"
> #include "sysemu.h"
> -#include "pflib.h"
>
> /*
> * must be last so we get the linux input layer
> @@ -70,19 +71,82 @@ static bool key_down[KEY_CNT];
> #define FB_ACQ_REQ 3
> static int fb_switch_state;
>
> -/* qdev windup */
> +/* qemu windup */
> static DisplayChangeListener *dcl;
> -static QemuPfConv *conv;
> -static PixelFormat fbpf;
> static int resize_screen;
> static int redraw_screen;
> static int cx, cy, cw, ch;
> static Notifier exit_notifier;
> +static pixman_image_t *surface;
> +static pixman_image_t *framebuffer;
> +static pixman_transform_t transform;
> +static pixman_region16_t dirty;
>
> /* fwd decls */
> static int fbdev_activate_vt(int tty, int vtno, bool wait);
>
> /* -------------------------------------------------------------------- */
> +/* pixman helpers */
> +
> +static int pixman_shifts_to_type(int rshift, int gshift, int bshift)
> +{
> + int type = PIXMAN_TYPE_OTHER;
> +
> + if (rshift > gshift && gshift > bshift) {
> + if (bshift == 0) {
> + type = PIXMAN_TYPE_ARGB;
> + } else {
> +#if PIXMAN_VERSION >= PIXMAN_VERSION_ENCODE(0, 21, 8)
> + type = PIXMAN_TYPE_RGBA;
> +#endif
> + }
> + } else if (rshift < gshift && gshift < bshift) {
> + if (rshift == 0) {
> + type = PIXMAN_TYPE_ABGR;
> + } else {
> + type = PIXMAN_TYPE_BGRA;
> + }
> + }
> + return type;
> +}
> +
> +static pixman_image_t *pixman_from_displaystate(DisplayState *ds)
> +{
> + PixelFormat *pf = &ds->surface->pf;
> + pixman_format_code_t format;
> + pixman_image_t *image;
> + int type;
> +
> + type = pixman_shifts_to_type(pf->rshift, pf->gshift, pf->bshift);
> + format = PIXMAN_FORMAT(pf->bits_per_pixel, type,
> + pf->abits, pf->rbits, pf->gbits, pf->bbits);
> + image = pixman_image_create_bits(format, ds_get_width(ds),
> + ds_get_height(ds),
> + (void *)ds_get_data(ds),
> + ds_get_linesize(ds));
> + return image;
> +}
> +
> +static pixman_image_t *pixman_from_framebuffer(void)
> +{
> + pixman_format_code_t format;
> + pixman_image_t *image;
> + int type;
> +
> + type = pixman_shifts_to_type(fb_var.red.offset,
> + fb_var.green.offset,
> + fb_var.blue.offset);
> + format = PIXMAN_FORMAT(fb_var.bits_per_pixel, type,
> + fb_var.transp.length,
> + fb_var.red.length,
> + fb_var.green.length,
> + fb_var.blue.length);
> + image = pixman_image_create_bits(format, fb_var.xres, fb_var.yres,
> + (void *)fb_mem, fb_fix.line_length);
> + return image;
> +}
> +
> +/* -------------------------------------------------------------------- */
> /* mouse */
>
> static void read_mouse(void *opaque)
> @@ -529,6 +593,17 @@ static void fbdev_cleanup(void)
> {
> trace_fbdev_cleanup();
>
> + /* release pixman stuff */
> + pixman_region_fini(&dirty);
> + if (framebuffer) {
> + pixman_image_unref(framebuffer);
> + framebuffer = NULL;
> + }
> + if (surface) {
> + pixman_image_unref(surface);
> + surface = NULL;
> + }
> +
> /* restore console */
> if (fb_mem != NULL) {
> munmap(fb_mem, fb_fix.smem_len+fb_mem_offset);
> @@ -682,36 +757,8 @@ static int fbdev_init(const char *device, Error **err)
> start_mediumraw(tty);
> qemu_set_fd_handler(tty, read_mediumraw, NULL, NULL);
>
> - /* create PixelFormat from fbdev structs */
> - fbpf.bits_per_pixel = fb_var.bits_per_pixel;
> - fbpf.bytes_per_pixel = (fb_var.bits_per_pixel+7)/8;
> - fbpf.depth = fb_var.bits_per_pixel == 32
> - ? 24 : fb_var.bits_per_pixel;
> - fbpf.rshift = fb_var.red.offset;
> - fbpf.rbits = fb_var.red.length;
> - fbpf.gshift = fb_var.green.offset;
> - fbpf.gbits = fb_var.green.length;
> - fbpf.bshift = fb_var.blue.offset;
> - fbpf.bbits = fb_var.blue.length;
> - fbpf.ashift = fb_var.transp.offset;
> - fbpf.abits = fb_var.transp.length;
> -
> - if (fbpf.rbits) {
> - fbpf.rmax = (1 << fbpf.rbits) - 1;
> - fbpf.rmask = fbpf.rmax << fbpf.rshift;
> - }
> - if (fbpf.gbits) {
> - fbpf.gmax = (1 << fbpf.gbits) - 1;
> - fbpf.gmask = fbpf.gmax << fbpf.gshift;
> - }
> - if (fbpf.bbits) {
> - fbpf.bmax = (1 << fbpf.bbits) - 1;
> - fbpf.bmask = fbpf.bmax << fbpf.bshift;
> - }
> - if (fbpf.abits) {
> - fbpf.amax = (1 << fbpf.abits) - 1;
> - fbpf.amask = fbpf.amax << fbpf.ashift;
> - }
> + framebuffer = pixman_from_framebuffer();
> + pixman_region_init(&dirty);
> return 0;
>
> err_early:
> @@ -819,36 +866,15 @@ static int fbdev_switch_init(void)
> /* -------------------------------------------------------------------- */
> /* rendering */
>
> -static void fbdev_render(DisplayState *ds, int x, int y, int w, int h)
> +static void fbdev_render(DisplayState *ds)
> {
> - uint8_t *dst;
> - uint8_t *src;
> - int line;
> + assert(surface);
>
> - if (!conv) {
> - return;
> - }
> -
> - src = ds_get_data(ds) + y * ds_get_linesize(ds)
> - + x * ds_get_bytes_per_pixel(ds);
> - dst = fb_mem + y * fb_fix.line_length
> - + x * fbpf.bytes_per_pixel;
> -
> - dst += cy * fb_fix.line_length;
> - dst += cx * fbpf.bytes_per_pixel;
> -
> - if (h > fb_var.yres - y) {
> - h = fb_var.yres - y;
> - }
> - if (w > fb_var.xres - x) {
> - w = fb_var.xres - x;
> - }
> -
> - for (line = y; line < y+h; line++) {
> - qemu_pf_conv_run(conv, dst, src, w);
> - dst += fb_fix.line_length;
> - src += ds_get_linesize(ds);
> - }
> + pixman_image_set_clip_region(surface, &dirty);
> + pixman_image_composite(PIXMAN_OP_SRC, surface, NULL, framebuffer,
> + 0, 0, 0, 0, 0, 0, fb_var.xres, fb_var.yres);
> + pixman_region_fini(&dirty);
> + pixman_region_init(&dirty);
> }
>
> /* -------------------------------------------------------------------- */
> @@ -872,14 +898,16 @@ static void fbdev_update(DisplayState *ds, int x, int
> y, int w, int h)
> if (ds_get_height(ds) < fb_var.yres) {
> cy = (fb_var.yres - ds_get_height(ds)) / 2;
> }
> -
> - if (conv) {
> - qemu_pf_conv_put(conv);
> - }
> - conv = qemu_pf_conv_get(&fbpf, &ds->surface->pf);
> - if (conv == NULL) {
> - fprintf(stderr, "fbdev: unsupported PixelFormat conversion\n");
> + if (surface) {
> + pixman_image_unref(surface);
> }
> + surface = pixman_from_displaystate(ds);
Am I reading this right? Are you creating a new pixman surface in every
call to fbdev_update?
> + pixman_transform_init_identity(&transform);
> + pixman_transform_translate(&transform, NULL,
> + pixman_int_to_fixed(-cx),
> + pixman_int_to_fixed(-cy));
> + pixman_image_set_transform(surface, &transform);
> }
>
> if (redraw_screen) {
> @@ -889,7 +917,7 @@ static void fbdev_update(DisplayState *ds, int x, int y,
> int w, int h)
> x = 0; y = 0; w = ds_get_width(ds); h = ds_get_height(ds);
> }
>
> - fbdev_render(ds, x, y, w, h);
> + pixman_region_union_rect(&dirty, &dirty, x, y, w, h);
> }
>
> static void fbdev_resize(DisplayState *ds)
> @@ -898,6 +926,15 @@ static void fbdev_resize(DisplayState *ds)
> redraw_screen++;
> }
>
> +static void fbdev_setdata(DisplayState *ds)
> +{
> + if (surface) {
> + pixman_image_unref(surface);
> + }
> + surface = pixman_from_displaystate(ds);
> + redraw_screen++;
> +}
> +
> static void fbdev_refresh(DisplayState *ds)
> {
> switch (fb_switch_state) {
> @@ -916,6 +953,10 @@ static void fbdev_refresh(DisplayState *ds)
> if (redraw_screen) {
> fbdev_update(ds, 0, 0, 0, 0);
> }
> +
> + if (pixman_region_not_empty(&dirty)) {
> + fbdev_render(ds);
> + }
Why are you using fbdev_refresh for rendering instead of fbdev_update?
>From consistency with sdl and vnc as well as the semantics of these
callbacks I think it would be better to do the rendering from
fbdev_update and just call vga_hw_update here.
> }
>
> static void fbdev_exit_notifier(Notifier *notifier, void *data)
> @@ -942,6 +983,7 @@ int fbdev_display_init(DisplayState *ds, const char
> *device, Error **err)
> dcl = g_new0(DisplayChangeListener, 1);
> dcl->dpy_update = fbdev_update;
> dcl->dpy_resize = fbdev_resize;
> + dcl->dpy_setdata = fbdev_setdata;
> dcl->dpy_refresh = fbdev_refresh;
> register_displaychangelistener(ds, dcl);
Pixman or non-pixman, I still think that this could benefit from
implementing a DisplayAllocator interface: it would avoid a memcpy
whenever there is no need for scaling and pixel conversions.
It should be able to make visible difference from the usability
perspective.
