From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=53304 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OGfO1-0005u8-0i for qemu-devel@nongnu.org; Mon, 24 May 2010 17:40:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OGfNv-0003sx-UI for qemu-devel@nongnu.org; Mon, 24 May 2010 17:40:36 -0400 Received: from isrv.corpit.ru ([81.13.33.159]:53139) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OGfNv-0003sc-NA for qemu-devel@nongnu.org; Mon, 24 May 2010 17:40:31 -0400 Message-ID: <4BFAF249.9050502@msgid.tls.msk.ru> Date: Tue, 25 May 2010 01:40:25 +0400 From: Michael Tokarev MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] sdl: Do not disable screensaver by default References: <20100520181304.10437.90521.malonedeb@potassium.ubuntu.com> <20100522234727.1683.30745.malone@palladium.canonical.com> <4BF8E76E.7040601@web.de> In-Reply-To: <4BF8E76E.7040601@web.de> 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: Jan Kiszka Cc: j.r.versteegh@gmail.com, qemu-devel@nongnu.org 23.05.2010 12:29, Jan Kiszka wrote: > From: Jan Kiszka > > 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 > --- > > 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