Re: [Qemu-devel] [PATCH] sdl: Do not disable screensaver by default

[Michael Tokarev <mjt@tls.msk.ru>]

23.05.2010 12:29, Jan Kiszka wrote:
> From: Jan Kiszka<jan.kiszka@siemens.com>
>
> Unless we are running in full-screen mode, QEMU's SDL window should not
> disable the host's screensaver. The user can still change this behaviour
> by setting the environment variable SDL_VIDEO_ALLOW_SCREENSAVER as
> desired.
>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> ---
>
> Cool, thanks for digging out SDL_VIDEO_ALLOW_SCREENSAVER. I came across
> by this issue as well but I was too lazy to analyze to reason. This
> patch solves it for me.
>
>   sdl.c |    4 ++++
>   1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/sdl.c b/sdl.c
> index 16a48e9..3bdd518 100644
> --- a/sdl.c
> +++ b/sdl.c
> @@ -855,6 +855,10 @@ void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
>       if (no_frame)
>           gui_noframe = 1;
>
> +    if (!full_screen) {
> +        setenv("SDL_VIDEO_ALLOW_SCREENSAVER", "1", 0);
> +    }
> +

I think it's conceptually wrong.

It's trivial to toggle full-screen mode by hitting Ctrl+Alt+F.
Following this logic, on each toggle we should toggle the
environment variable (which wont work).  Or else the next bug
report to be filed is that screen saver isn't re-enabled when
switching from fullscreen to window and it isn't re-disabled
when switching to fullscreen.

How about not poking at screensaver by default unconditionally?
To me it sound more correct.  I.e., the above setenv(), but
without the if part...

Thanks!

/mjt