GNOME Bugzilla – Bug 301528
Screen saver support patch in gdmlogin
Last modified: 2005-04-25 19:39:46 UTC
Version details: 2.6.0.9 Distribution/Version: Gentoo I patched GDM to add support for screen savers. Currently in GDM one can specify a program using greeter/BackgroundProgram. There are two shortcomings, however: - The program is run immediately after GDM is started. - The program is not restarted after a while when it has exited. I patched gdmlogin to implement the following features: - You can specify greeter/BackgroundProgramInitialDelay which controls when the program is run after GDM is started. - You can specify greeter/RestartBackgroundProgram and greeter/BackgroundProgramRestartDelay which controls if and when the program is restarted after it has exited. Essentially, I use a timeout to start the program when it is time, a watcher to detect when the program has exited, and event catchers to avoid spawning the program when the user is doing something. I replaced calls to gdm_run_command(), run_backgrounds(), and gdm_kill_thingies() with their equivalents in the new API. The logic of the program did not change much. Currently it only works with gdmlogin. If my patch is accepted and receive positive input I could update gdmgreeter as well. Brian Cameron replied to my original post about the patch with questions, here are the answers (note that some info mentionned in the original post is outdated). >> I'd like to use GDM to start a screen saver-like program. >> This would allow the machine to join a distributed computing >> system when the machine is idle. >> >> Currently in GDM one can specify a program using >> greeter/BackgroundProgram. There are two shortcomings, however: >> >> - The program is run immediately after GDM is started. >> >> - The program is not restarted after a while when it has exited. >> >> I patched gdmlogin to implement the following features: >> >> - You can specify greeter/BackgroundProgramInitialDelay >> which controls when the program is run after GDM is started. >> >> - You can specify greeter/BackgroundProgramNextDelay >> which controls when the program is restarted after it has >> exited. >> >> Essentially, I use a timeout to start the program when it is >> time, and a watcher to detect when the program has exited. >> I replaced calls to gdm_run_command(), run_backgrounds(), and >> gdm_kill_thingies() with their equivalents in the new API. >> The logic of the program did not change much. > The above seems reasonable. >> I'm sending diffs against version 2.6.0.8 of gdm. The >> integration is not complete. In particular, >> >> - Need to remove backgroundpid, gdm_run_command(), >> run_backgrounds() and gdm_kill_thingies() (commented out). > Please clean up your patches. Also submit a bug under category > gdm in http://bugzilla.gnome.org and attach your patch there. Done. > Why is it necessary to remove gdm_kill_thingies() I missed the point of gdm_kill_thingies(). I looked at it some more and I saw that you use it as a general function to do cleanup (although currently it is only used to stop the background program). I modified gdm_kill_thingies() to call my function to stop the background program. >> - Must no longer ignore SIGCHLD (commented out). > Why is this necessary? I use g_child_watch_add() to catch the event of the background program exiting. The SIGCHLD signal wakes up the the GTK main loop when the program exits, so it must not be ignored. >> - Maybe let user specify -1 to disallow restarting? > No, there should be an extra configuration option to turn this > feature on/off. Much like TimedLoginEnable. Done. >> - Documentation for the user. >> >> The patched version works almost correctly on my system, but >> there is still one problem. While the program always get >> mouse focus, it does not always receive keyboard focus. >> Sometimes you can login while the screen saver is running >> (the screen saver gets killed, but it should go when the >> user is typing). > This problem will need to be corrected before we should apply > the patch, I think. I'm not sure how to fix this, but perhaps > someone else might have a comment. You might take a look at > the gdmwm.c file. GDM uses a light window manager. It might > be necessary to add some code to tell the window manager to > deal with focus issues. Perhaps something like > gdm_wm_focus_window is what you need? Indeed, gdm_wm_focus_new_windows (TRUE) does the trick. I also added support to detect interactions to avoid spawning the background program when the user is doing something. > Also, you didn't update docs/C/gdm.xml with information about the > new feature, which is necessary before a patch would be accepted. > The documentation should highlight that this is a feature for > the GTK+ Greeter. Done. Also patched config/gdm.conf.in.
Created attachment 45528 [details] [review] The patch (patch -p1 < gdm-2.6.0.9.patch)
Fixed in CVS head. Laurent, it would also be cool to support this in gdmsetup if you have time.