[Qemu-devel] [PATCH] don't die if switching to fullscreen mode fails

[Qemu-devel] [PATCH] don't die if switching to fullscreen mode fails

[Andreas Winkelbauer]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

hi,

the attached patch fixes some glitches when switching to fullscreen mode 
using ctrl+alt+f or when booting using -full-screen.

up to now the VM simply dies if one of the following situations occur:
   * user switches from windowed to fullscreen mode using a resolution 
which is too high (meaning higher than the maximum resolution of the 
display)
   * guest boots in fullscreen mode using a resolution which is too high
   * guest is in fullscreen mode and the user switches to a resolution 
which is too high

IMO this is not what the "normal" user would expect.

This patch changes the behaviour as follows:
   * deny switching to fullscreen mode if the resolution is too high and 
print a message to the console
   * use windowed mode as fallback option if we are already in 
fullscreen mode and the new resolution is too high and print a message 
to the console

cheers,
Andi

[-- Attachment #2: qemu-sdl-fullscreen.patch --]
[-- Type: text/x-patch, Size: 5250 bytes --]

--- qemu/sdl.c	2007-11-17 18:14:38.000000000 +0100
+++ qemu/sdl.c	2008-02-26 13:32:29.000000000 +0100
@@ -56,46 +56,60 @@ static void sdl_update(DisplayState *ds,
     SDL_UpdateRect(screen, x, y, w, h);
 }
 
-static void sdl_resize(DisplayState *ds, int w, int h)
+static int sdl_resize2(DisplayState *ds, int w, int h, int full_screen, int no_frame)
 {
+    SDL_Surface *screen_tmp;
     int flags;
 
-    //    printf("resizing to %d %d\n", w, h);
+    //printf("trying to resize from w=%d h=%d %s to w=%d h=%d %s\n", width, height, gui_fullscreen ? "fullscreen" : "windowed", w, h, full_screen ? "fullscreen" : "windowed");
 
     flags = SDL_HWSURFACE|SDL_ASYNCBLIT|SDL_HWACCEL;
-    if (gui_fullscreen)
+    if (full_screen)
         flags |= SDL_FULLSCREEN;
-    if (gui_noframe)
+    if (no_frame)
         flags |= SDL_NOFRAME;
 
-    width = w;
-    height = h;
-
- again:
-    screen = SDL_SetVideoMode(w, h, 0, flags);
-    if (!screen) {
-        fprintf(stderr, "Could not open SDL display\n");
-        exit(1);
-    }
-    if (!screen->pixels && (flags & SDL_HWSURFACE) && (flags & SDL_FULLSCREEN)) {
-        flags &= ~SDL_HWSURFACE;
-        goto again;
+    if (!(screen_tmp = SDL_SetVideoMode(w, h, 0, flags))) {
+        //fprintf(stderr, "Could not open SDL display (try #1)\n");
+        return -1;
+    } else if (!screen_tmp->pixels && (flags & SDL_HWSURFACE) && (flags & SDL_FULLSCREEN)) {
+        screen_tmp = SDL_SetVideoMode(w, h, 0, flags & ~SDL_HWSURFACE);
     }
 
-    if (!screen->pixels) {
-        fprintf(stderr, "Could not open SDL display\n");
-        exit(1);
-    }
-    ds->data = screen->pixels;
-    ds->linesize = screen->pitch;
-    ds->depth = screen->format->BitsPerPixel;
-    if (screen->format->Bshift > screen->format->Rshift) {
-        ds->bgr = 1;
+    if (!screen_tmp || !screen_tmp->pixels) {
+        //fprintf(stderr, "Could not open SDL display (try #2)\n");
+        return -1;
     } else {
-        ds->bgr = 0;
+        screen = screen_tmp;
+        gui_fullscreen = full_screen;
+        gui_noframe = no_frame;
+
+        ds->data = screen->pixels;
+        ds->linesize = screen->pitch;
+        ds->depth = screen->format->BitsPerPixel;
+        if (screen->format->Bshift > screen->format->Rshift) {
+            ds->bgr = 1;
+        } else {
+            ds->bgr = 0;
+        }
+        ds->width = width = w;
+        ds->height = height = h;
+    }
+}
+
+static void sdl_resize(DisplayState *ds, int w, int h)
+{
+    if (sdl_resize2(ds, w, h, gui_fullscreen, gui_noframe) == -1) {
+        fprintf(stderr, "Could not resize display to %d x %d (%s)\n",
+						w, h, gui_fullscreen ? "fullscreen" : "windowed");
+        
+        /* if we are in fullscreen mode use windowed mode as fallback */
+        if (!gui_fullscreen || sdl_resize2(ds, w, h, 0, gui_noframe) == -1) {
+            exit(1);
+        } else {
+            fprintf(stderr, "Using windowed mode as fallback\n");
+        }
     }
-    ds->width = w;
-    ds->height = h;
 }
 
 /* generic keyboard conversion */
@@ -330,17 +344,21 @@ static void sdl_send_mouse_event(int dz)
 
 static void toggle_full_screen(DisplayState *ds)
 {
-    gui_fullscreen = !gui_fullscreen;
-    sdl_resize(ds, screen->w, screen->h);
-    if (gui_fullscreen) {
-        gui_saved_grab = gui_grab;
-        sdl_grab_start();
-    } else {
-        if (!gui_saved_grab)
-            sdl_grab_end();
+    if (sdl_resize2(ds, screen->w, screen->h, !gui_fullscreen, gui_noframe) != -1) {
+        if (gui_fullscreen) {
+            gui_saved_grab = gui_grab;
+            sdl_grab_start();
+        } else {
+            if (!gui_saved_grab)
+                sdl_grab_end();
+        }
+        vga_hw_invalidate();
+        vga_hw_update();
+    }
+    else {
+        fprintf(stderr, "Could not switch to %s mode\n",
+						!gui_fullscreen ? "fullscreen" : "windowed");
     }
-    vga_hw_invalidate();
-    vga_hw_update();
 }
 
 static void sdl_refresh(DisplayState *ds)
@@ -603,14 +621,14 @@ void sdl_display_init(DisplayState *ds, 
             exit(1);
     }
 
-    if (no_frame)
-        gui_noframe = 1;
-
     flags = SDL_INIT_VIDEO | SDL_INIT_NOPARACHUTE;
     if (SDL_Init (flags)) {
         fprintf(stderr, "Could not initialize SDL - exiting\n");
         exit(1);
     }
+
+    atexit(sdl_cleanup);
+
 #ifndef _WIN32
     /* NOTE: we still want Ctrl-C to work, so we undo the SDL redirections */
     signal(SIGINT, SIG_DFL);
@@ -624,7 +642,10 @@ void sdl_display_init(DisplayState *ds, 
     ds->mouse_set = sdl_mouse_warp;
     ds->cursor_define = sdl_mouse_define;
 
-    sdl_resize(ds, 640, 400);
+    if (sdl_resize2(ds, 640, 400, full_screen, no_frame) == -1) {
+        fprintf(stderr, "Could not resize display in sdl_display_init()\n");
+        exit(1);
+    }
     sdl_update_caption();
     SDL_EnableKeyRepeat(250, 50);
     SDL_EnableUNICODE(1);
@@ -633,9 +654,7 @@ void sdl_display_init(DisplayState *ds, 
     sdl_cursor_hidden = SDL_CreateCursor(&data, &data, 8, 1, 0, 0);
     sdl_cursor_normal = SDL_GetCursor();
 
-    atexit(sdl_cleanup);
-    if (full_screen) {
-        gui_fullscreen = 1;
+    if (gui_fullscreen) {
         gui_fullscreen_initial_grab = 1;
         sdl_grab_start();
     }

Re: [Qemu-devel] [PATCH] don't die if switching to fullscreen mode fails

[Johannes Schindelin]

Hi,

On Tue, 26 Feb 2008, Andreas Winkelbauer wrote:

> the attached patch fixes some glitches when switching to fullscreen mode 
> using ctrl+alt+f or when booting using -full-screen.

Wow.  This patch is messy, if I may say so.  There must be a more elegant 
way to do this, especially given the fact that you remove 
atexit(sdl_cleanup)s without explanation.

Ciao,
Dscho

Re: [Qemu-devel] [PATCH] don't die if switching to fullscreen mode fails

[Markus Hitter]

Am 26.02.2008 um 13:48 schrieb Andreas Winkelbauer:

> This patch changes the behaviour as follows:
>   * deny switching to fullscreen mode if the resolution is too high  
> and print a message to the console

Very good idea.

>   * use windowed mode as fallback option if we are already in  
> fullscreen mode and the new resolution is too high and print a  
> message to the console

Do you end up with a window bigger than the screen, then? Is there a  
chance the user can escape from this situation, i.e. reach all parts  
of the virtual screen to find the switch for setting the resolution?

Another option would be to simply display an "Out of range error"  
across the screen, like a real monitor would do. Usually, operations  
systems feature a protection against setting a resolution higher than  
supported by hardware already (set back to a lower reolution after  
some delay).


Markus

- - - - - - - - - - - - - - - - - - -
Dipl. Ing. Markus Hitter
http://www.jump-ing.de/

Re: [Qemu-devel] [PATCH] don't die if switching to fullscreen mode fails

[Andreas Winkelbauer]

hi,

 > Wow.  This patch is messy, if I may say so.  There must be a more
 > elegant way to do this, especially given the fact that you remove
 > atexit(sdl_cleanup)s without explanation.

well, the diff output is messy, because I added sdl_resize2() above 
sdl_resize() to avoid adding a forward declaration.

the patch itself might be messy as well, but that depends on your point 
of view. I don't think it is too messy. I did it that way because I 
wanted the patch to be as less invasively as possible. I did not change 
any existing function prototypes and all the changes are restricted to 
one source file.

just as an example: changing the function prototype of sdl_resize() 
would lead to much more changes at other places as well to make things 
consistent again. I don't think this would make the patch less messy.

I didn't remove the atexit() call, it is just moved some lines upwards 
(look carefully) because now there is a check if the initial resize to 
640x400 fails. If it fails we call exit() and that means the call to 
atexit() must be done before a possible call to exit().

However, I agree with you that there should exist another solution.

Does qemu have access to the maximum resolution of the host display?

cheers,
Andi

Re: [Qemu-devel] [PATCH] don't die if switching to fullscreen mode fails

[Andreas Winkelbauer]

hi,

 >> use windowed mode as fallback option if we are already in fullscreen
 >> mode and the new resolution is too high and print a message to the
 >> console
 >
 > Do you end up with a window bigger than the screen, then? Is there a
 > chance the user can escape from this situation, i.e. reach all parts
 > of the virtual screen to find the switch for setting the resolution?

yes, that's true, but it is no problem at all. First of all you can move 
the window around so it is possible to reach everything inside the 
window. in addition you can always use ctrl+alt+del under windows to 
shutdown. under X11 you can always escape using ctrl+alt+backspace.

 > Another option would be to simply display an "Out of range error"
 > across the screen, like a real monitor would do. Usually, operations
 > systems feature a protection against setting a resolution higher than
 > supported by hardware already (set back to a lower reolution after
 > some delay).

to make that working we would need to know the maximum resolution of the 
host display. does qemu store this information somewhere?

but I think this would not work anyway because the guest os might (at 
least windows does) crash if qemu resizes the display to an other 
resolution than the requested one. so testing if the new resolution is 
too high and in case using a lower resolution would crash windows xp, I 
have tried that already.

I've extensively tested this patch using windows as guest os and it 
works perfectly for me. Maybe somebody could test the patch using other 
guest os as well.

cheers,
Andi