From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MatVI-0000Eq-3u for qemu-devel@nongnu.org; Tue, 11 Aug 2009 11:43:12 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MatVC-0000Cv-Qw for qemu-devel@nongnu.org; Tue, 11 Aug 2009 11:43:11 -0400 Received: from [199.232.76.173] (port=44899 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MatVC-0000Cm-Fo for qemu-devel@nongnu.org; Tue, 11 Aug 2009 11:43:06 -0400 Received: from mail-yx0-f188.google.com ([209.85.210.188]:39443) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MatVB-0001o3-Cp for qemu-devel@nongnu.org; Tue, 11 Aug 2009 11:43:06 -0400 Received: by yxe26 with SMTP id 26so4870327yxe.4 for ; Tue, 11 Aug 2009 08:42:56 -0700 (PDT) Message-ID: <4A81917C.3070706@codemonkey.ws> Date: Tue, 11 Aug 2009 10:42:52 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] make vga screen_dump use DisplayState properly References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: qemu-devel@nongnu.org, Avi Kivity Stefano Stabellini wrote: > Hi all, > currently the vga screen_dump code doesn't use the DisplayState > interface properly and tries to replace it temporarily while taking the > screenshot. > A better approach is to register a DisplayChangeListener, call > vga_hw_update, and finally write the ppm in the next call from dpy_update. > > Testing is appreciated. > Does this fix kvm-autotest Avi? > Signed-off-by: Stefano Stabellini > > --- > > diff --git a/hw/vga.c b/hw/vga.c > index 4a0f197..3882f20 100644 > --- a/hw/vga.c > +++ b/hw/vga.c > @@ -150,6 +150,8 @@ static uint16_t expand2[256]; > static uint8_t expand4to8[16]; > > static void vga_screen_dump(void *opaque, const char *filename); > +static char *screen_dump_filename; > +static DisplayChangeListener *screen_dump_dcl; > > static void vga_dumb_update_retrace_info(VGAState *s) > { > @@ -2548,9 +2550,13 @@ device_init(vga_register); > /********************************************************/ > /* vga screen dump */ > > -static void vga_save_dpy_update(DisplayState *s, > +static void vga_save_dpy_update(DisplayState *ds, > int x, int y, int w, int h) > { > + if (screen_dump_filename) { > + ppm_save(screen_dump_filename, ds->surface); > + screen_dump_filename = NULL; > + } > } > > static void vga_save_dpy_resize(DisplayState *s) > @@ -2599,70 +2605,16 @@ int ppm_save(const char *filename, struct DisplaySurface *ds) > return 0; > } > > -static void vga_screen_dump_blank(VGAState *s, const char *filename) > -{ > - FILE *f; > - unsigned int y, x, w, h; > - unsigned char blank_sample[3] = { 0, 0, 0 }; > - > - w = s->last_scr_width; > - h = s->last_scr_height; > - > - f = fopen(filename, "wb"); > - if (!f) > - return; > - fprintf(f, "P6\n%d %d\n%d\n", w, h, 255); > - for (y = 0; y < h; y++) { > - for (x = 0; x < w; x++) { > - fwrite(blank_sample, 3, 1, f); > - } > - } > - fclose(f); > -} > - > -static void vga_screen_dump_common(VGAState *s, const char *filename, > - int w, int h) > -{ > - DisplayState *saved_ds, ds1, *ds = &ds1; > - DisplayChangeListener dcl; > - > - /* XXX: this is a little hackish */ > - vga_invalidate_display(s); > - saved_ds = s->ds; > - > - memset(ds, 0, sizeof(DisplayState)); > - memset(&dcl, 0, sizeof(DisplayChangeListener)); > - dcl.dpy_update = vga_save_dpy_update; > - dcl.dpy_resize = vga_save_dpy_resize; > - dcl.dpy_refresh = vga_save_dpy_refresh; > - register_displaychangelistener(ds, &dcl); > - ds->allocator = &default_allocator; > - ds->surface = qemu_create_displaysurface(ds, w, h); > - > - s->ds = ds; > - s->graphic_mode = -1; > - vga_update_display(s); > - > - ppm_save(filename, ds->surface); > - > - qemu_free_displaysurface(ds); > - s->ds = saved_ds; > -} > - > -static void vga_screen_dump_graphic(VGAState *s, const char *filename) > +static DisplayChangeListener* vga_screen_dump_init(DisplayState *ds) > { > - int w, h; > + DisplayChangeListener *dcl; > > - s->get_resolution(s, &w, &h); > - vga_screen_dump_common(s, filename, w, h); > -} > - > -static void vga_screen_dump_text(VGAState *s, const char *filename) > -{ > - int w, h, cwidth, cheight; > - > - vga_get_text_resolution(s, &w, &h, &cwidth, &cheight); > - vga_screen_dump_common(s, filename, w * cwidth, h * cheight); > + dcl = qemu_mallocz(sizeof(DisplayChangeListener)); > + dcl->dpy_update = vga_save_dpy_update; > + dcl->dpy_resize = vga_save_dpy_resize; > + dcl->dpy_refresh = vga_save_dpy_refresh; > + register_displaychangelistener(ds, dcl); > + return dcl; > } > > /* save the vga display in a PPM image even if no display is > @@ -2671,11 +2623,11 @@ static void vga_screen_dump(void *opaque, const char *filename) > { > VGAState *s = (VGAState *)opaque; > > - if (!(s->ar_index & 0x20)) > - vga_screen_dump_blank(s, filename); > - else if (s->gr[6] & 1) > - vga_screen_dump_graphic(s, filename); > - else > - vga_screen_dump_text(s, filename); > + if (!screen_dump_dcl) > + screen_dump_dcl = vga_screen_dump_init(s->ds); > + > + screen_dump_filename = (char *)filename; > vga_invalidate_display(s); > + vga_hw_update(); > } > + > > >