From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=32811 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OGfaA-0005bI-Gu for qemu-devel@nongnu.org; Mon, 24 May 2010 17:53:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OGfa2-0005Dh-Dg for qemu-devel@nongnu.org; Mon, 24 May 2010 17:53:10 -0400 Received: from mail-yw0-f185.google.com ([209.85.211.185]:59873) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OGfa2-0005DZ-83 for qemu-devel@nongnu.org; Mon, 24 May 2010 17:53:02 -0400 Received: by ywh15 with SMTP id 15so1759087ywh.14 for ; Mon, 24 May 2010 14:53:01 -0700 (PDT) Message-ID: <4BFAF53B.30102@codemonkey.ws> Date: Mon, 24 May 2010 16:52:59 -0500 From: Anthony Liguori 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> <4BFAF249.9050502@msgid.tls.msk.ru> In-Reply-To: <4BFAF249.9050502@msgid.tls.msk.ru> 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: Michael Tokarev Cc: j.r.versteegh@gmail.com, Jan Kiszka , qemu-devel@nongnu.org On 05/24/2010 04:40 PM, Michael Tokarev wrote: > 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. 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