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

[Anthony Liguori]

On 05/24/2010 04:40 PM, Michael Tokarev wrote:
> 23.05.2010 12:29, Jan Kiszka wrote:
> > From: Jan Kiszka<address@hidden>
> >
> > 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<address@hidden>
> > ---
> >
> > 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.

I don't think it's worth jumping through hoops to conditionally 
disable/enable screensaver.

More sophisticated screen saver behavior should be implemented in 
upstream SDL.

Regards,

Anthony Liguori

> 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