[Qemu-devel] [PATCH 4 of 5] sdl shared buffer support

[Qemu-devel] [PATCH 4 of 5] sdl shared buffer support

[Stefano Stabellini]

This patch implements shared buffer support in sdl.c.
It also supports paletted 8 bit colour depths using the
palette functions provided by the SDL library.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

diff --git a/console.h b/console.h
index a054d97..7d252b7 100644
--- a/console.h
+++ b/console.h
@@ -81,6 +81,7 @@ struct DisplayState {
     int width;
     int height;
     void *opaque;
+    uint32_t *palette;
     struct QEMUTimer *gui_timer;
     uint64_t gui_timer_interval;
     int idle; /* there is nothing to update (window invisible), set by vnc/sdl */
diff --git a/hw/vga.c b/hw/vga.c
index 6e40d3d..d5265c0 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2026,6 +2026,7 @@ void vga_common_init(VGAState *s, DisplayState *ds, uint8_t *vga_ram_base,
     s->vram_offset = vga_ram_offset;
     s->vram_size = vga_ram_size;
     s->ds = ds;
+    ds->palette = s->last_palette;
     s->get_bpp = vga_get_bpp;
     s->get_offsets = vga_get_offsets;
     s->get_resolution = vga_get_resolution;
diff --git a/sdl.c b/sdl.c
index 15427c5..4b104b0 100644
--- a/sdl.c
+++ b/sdl.c
@@ -32,6 +32,7 @@
 #endif
 
 static SDL_Surface *screen;
+static SDL_Surface *shared;
 static int gui_grab; /* if true, all keyboard/mouse events are grabbed */
 static int last_vm_running;
 static int gui_saved_grab;
@@ -50,21 +51,83 @@ static int guest_cursor = 0;
 static int guest_x, guest_y;
 static SDL_Cursor *guest_sprite = 0;
 
+static void sdl_colourdepth(DisplayState *ds, int depth);
+
 static void sdl_update(DisplayState *ds, int x, int y, int w, int h)
 {
     //    printf("updating x=%d y=%d w=%d h=%d\n", x, y, w, h);
-    SDL_UpdateRect(screen, x, y, w, h);
+    if (shared) {
+        SDL_Rect rec;
+        rec.x = x;
+        rec.y = y;
+        rec.w = w;
+        rec.h = h;
+        SDL_BlitSurface(shared, &rec, screen, &rec);
+    }
+    SDL_Flip(screen);
 }
 
-static void sdl_resize(DisplayState *ds, int w, int h)
+static void sdl_setdata(DisplayState *ds, void *pixels)
+{
+    uint32_t rmask, gmask, bmask, amask = 0;
+    switch (ds->depth) {
+        case 8:
+            rmask = 0x000000E0;
+            gmask = 0x0000001C;
+            bmask = 0x00000003;
+            break;
+        case 15:
+            rmask = 0x0000F800;
+            gmask = 0x000007C0;
+            bmask = 0x0000003E;
+            break;
+        case 16:
+            rmask = 0x0000F800;
+            gmask = 0x000007E0;
+            bmask = 0x0000001F;
+            break;
+        case 24:
+            rmask = 0x00FF0000;
+            gmask = 0x0000FF00;
+            bmask = 0x000000FF;
+            break;
+        case 32:
+            rmask = 0x00FF0000;
+            gmask = 0x0000FF00;
+            bmask = 0x000000FF;
+            break;
+        default:
+            return;
+    }
+    shared = SDL_CreateRGBSurfaceFrom(pixels, width, height, ds->depth, ds->linesize, rmask , gmask, bmask, amask);
+    if (ds->depth == 8 && ds->palette != NULL) {
+        SDL_Color palette[256];
+        int i;
+        for (i = 0; i < 256; i++) {
+            uint8_t rgb = ds->palette[i] >> 16;
+            palette[i].r = ((rgb & 0xe0) >> 5) * 255 / 7;
+            palette[i].g = ((rgb & 0x1c) >> 2) * 255 / 7;
+            palette[i].b = (rgb & 0x3) * 255 / 3;
+        }
+        SDL_SetColors(shared, palette, 0, 256);
+    }
+    ds->data = pixels;
+}
+
+static void sdl_resize_shared(DisplayState *ds, int w, int h, int depth, int linesize, void *pixels)
 {
     int flags;
 
     //    printf("resizing to %d %d\n", w, h);
 
-    flags = SDL_HWSURFACE|SDL_ASYNCBLIT|SDL_HWACCEL;
-    if (gui_fullscreen)
+    sdl_colourdepth(ds, depth);
+
+    flags = SDL_HWSURFACE|SDL_ASYNCBLIT|SDL_HWACCEL|SDL_DOUBLEBUF|SDL_HWPALETTE;
+
+    if (gui_fullscreen) {
         flags |= SDL_FULLSCREEN;
+        flags &= ~SDL_RESIZABLE;
+    }
     if (gui_noframe)
         flags |= SDL_NOFRAME;
 
@@ -74,7 +137,7 @@ static void sdl_resize(DisplayState *ds, int w, int h)
  again:
     screen = SDL_SetVideoMode(w, h, 0, flags);
     if (!screen) {
-        fprintf(stderr, "Could not open SDL display\n");
+        fprintf(stderr, "Could not open SDL display: %s\n", SDL_GetError());
         exit(1);
     }
     if (!screen->pixels && (flags & SDL_HWSURFACE) && (flags & SDL_FULLSCREEN)) {
@@ -83,30 +146,41 @@ static void sdl_resize(DisplayState *ds, int w, int h)
     }
 
     if (!screen->pixels) {
-        fprintf(stderr, "Could not open SDL display\n");
+        fprintf(stderr, "Could not open SDL display: %s\n", SDL_GetError());
         exit(1);
     }
-    ds->data = screen->pixels;
-    ds->linesize = screen->pitch;
-    ds->depth = screen->format->BitsPerPixel;
-    /* SDL BitsPerPixel never indicates any values other than
-       multiples of 8, so we need to check for strange depths. */
-    if (ds->depth == 16) {
-        uint32_t mask;
-
-        mask = screen->format->Rmask;
-        mask |= screen->format->Gmask;
-        mask |= screen->format->Bmask;
-        if ((mask & 0x8000) == 0)
-            ds->depth = 15;
-    }
-    if (ds->depth == 32 && screen->format->Rshift == 0) {
-        ds->bgr = 1;
-    } else {
-        ds->bgr = 0;
-    }
     ds->width = w;
     ds->height = h;
+    if (!ds->shared_buf) {
+        ds->depth = screen->format->BitsPerPixel;
+	if (screen->format->Bshift > screen->format->Rshift) {
+            ds->bgr = 1;
+        } else {
+            ds->bgr = 0;
+        }
+        shared = NULL;
+        ds->data = screen->pixels;
+        ds->linesize = screen->pitch;
+    } else {
+        ds->linesize = linesize;
+    }
+    if (ds->shared_buf) ds->dpy_setdata(ds, pixels);
+}
+
+static void sdl_resize(DisplayState *ds, int w, int h)
+{
+    sdl_resize_shared(ds, w, h, 0, w * (ds->depth / 8), NULL);
+}
+
+static void sdl_colourdepth(DisplayState *ds, int depth)
+{
+    if (!depth || !ds->depth) {
+	ds->shared_buf = 0;
+	ds->dpy_update = sdl_update;
+	return;
+    }
+    ds->shared_buf = 1;
+    ds->depth = depth;
 }
 
 /* generic keyboard conversion */
@@ -339,7 +413,7 @@ static void sdl_send_mouse_event(int dx, int dy, int dz, int x, int y, int state
 static void toggle_full_screen(DisplayState *ds)
 {
     gui_fullscreen = !gui_fullscreen;
-    sdl_resize(ds, screen->w, screen->h);
+    sdl_resize_shared(ds, ds->width, ds->height, ds->depth, ds->linesize, ds->data);
     if (gui_fullscreen) {
         gui_saved_grab = gui_grab;
         sdl_grab_start();
@@ -639,7 +713,11 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
 
     ds->dpy_update = sdl_update;
     ds->dpy_resize = sdl_resize;
+#if defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
+    ds->dpy_resize_shared = sdl_resize_shared;
+#endif
     ds->dpy_refresh = sdl_refresh;
+    ds->dpy_setdata = sdl_setdata;
     ds->dpy_fill = sdl_fill;
     ds->mouse_set = sdl_mouse_warp;
     ds->cursor_define = sdl_mouse_define;

Re: [Qemu-devel] [PATCH 4 of 5] sdl shared buffer support

[Anthony Liguori]

Stefano Stabellini wrote:
> This patch implements shared buffer support in sdl.c.
> It also supports paletted 8 bit colour depths using the
> palette functions provided by the SDL library.
>   

So the theory goes that it's possible with this patch that both surfaces 
are hardware surfaces so blitting is fast?  In practice, when using X11 
both surfaces are going to be XShmImages so I don't think this is going 
to perform any better than the current code.

Do you have any sort of measurements that show an advantage with this patch?

I sort of think you want the opposite interface.  One where the VGA code 
uses ds->data directly if the guest ds->data is in a supportable format.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4 of 5] sdl shared buffer support

[Stefano Stabellini]

Anthony Liguori wrote:

> Stefano Stabellini wrote:
>> This patch implements shared buffer support in sdl.c.
>> It also supports paletted 8 bit colour depths using the
>> palette functions provided by the SDL library.
>>   
> 
> So the theory goes that it's possible with this patch that both surfaces
> are hardware surfaces so blitting is fast?


The theory goes that SDL blitting and color conversion functions should
be as optimized as you can get, and hardware accelerated if your card
supports it.
Besides I think that offloading blitting and color conversion to a
graphic library is a good idea.

> In practice, when using X11
> both surfaces are going to be XShmImages so I don't think this is going
> to perform any better than the current code.

Only the guest framebuffer derived surface is always a software surface,
the host window can be an hardware surface (as far as I know).

> Do you have any sort of measurements that show an advantage with this
> patch?

I don't have any numbers, but I'll try to get some.

> I sort of think you want the opposite interface.  One where the VGA code
> uses ds->data directly if the guest ds->data is in a supportable format.

Not really, because the frontend can often change his own internal
resolution and pixel format (ex.: vnc).

Re: [Qemu-devel] [PATCH 4 of 5] sdl shared buffer support

[Anthony Liguori]

Stefano Stabellini wrote:
> Anthony Liguori wrote:
>
>   
>> Stefano Stabellini wrote:
>>     
>>> This patch implements shared buffer support in sdl.c.
>>> It also supports paletted 8 bit colour depths using the
>>> palette functions provided by the SDL library.
>>>   
>>>       
>> So the theory goes that it's possible with this patch that both surfaces
>> are hardware surfaces so blitting is fast?
>>     
>
>
> The theory goes that SDL blitting and color conversion functions should
> be as optimized as you can get, and hardware accelerated if your card
> supports it.
> Besides I think that offloading blitting and color conversion to a
> graphic library is a good idea.
>   

It requires two paths in the code to handle the case where the backend 
can't handle conversion itself.  If we're not getting any sort of 
benefit from offloading, then I don't see the need for increased code 
complexity.

>> In practice, when using X11
>> both surfaces are going to be XShmImages so I don't think this is going
>> to perform any better than the current code.
>>     
>
> Only the guest framebuffer derived surface is always a software surface,
> the host window can be an hardware surface (as far as I know).
>   

Not when using SDL on X11.  It will use an XShmImage if possible, 
otherwise falling back to a normal XImage.  There's really no other way 
to do surfaces in X11 without using one of the extensions (like DGA).

Regards,

Anthony Liguori