After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 342850 - gnome-screensaver doesn't restore gamma value after blanking screen
gnome-screensaver doesn't restore gamma value after blanking screen
Status: RESOLVED FIXED
Product: gnome-screensaver
Classification: Deprecated
Component: general
2.14.x
Other All
: Normal minor
: ---
Assigned To: gnome-screensaver maintainers
gnome-screensaver maintainers
: 469516 527029 549895 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2006-05-24 22:05 UTC by Christian Lohmaier
Modified: 2008-09-02 20:37 UTC
See Also:
GNOME target: ---
GNOME version: 2.13/2.14


Attachments
Obtain gamma info before each fade, release it when the fade is reset (1.47 KB, patch)
2007-10-05 13:38 UTC, John Bryant
none Details | Review
Fix compile error in last patch (1.50 KB, patch)
2007-11-03 21:03 UTC, John Bryant
committed Details | Review

Description Christian Lohmaier 2006-05-24 22:05:28 UTC
Please describe the problem:
After the screen is recovered, the gamma value is reset to 1 and does not
restore the value to the one that was used before blanking the screen

Steps to reproduce:
1. Set the gamma correction using "xgamma -gammma <value>"
2. Wait for the screensaver to blank the screen, then wake it up


Actual results:
The gamma correction is set back to 1

Expected results:
The gamma correction should be set to the same value as before blanking the screen

Does this happen every time?
Yes, happens every time

Other information:
Maybe it doesn't reset it to 1 but to whatever is set in the global x-config,
but you should be able to reproduce anyway.
Comment 1 William Jon McCann 2006-06-02 20:52:06 UTC
We read the gamma value when gnome-screensaver starts and use that as the nominal value.  So if you execute your xgamma call before gnome-screensaver starts this should work as exected.
Comment 2 Christian Lohmaier 2006-06-09 22:23:39 UTC
It is as you wrote. If I kill gnome-screensaver, set the gamma and then launch it, it keeps the gamma.

However, I still think it would be better to always use the value that is in use  just before the screensaver is activated, not to the value it was set to when the process was started.
Comment 3 Martin Andersen 2006-09-13 18:16:40 UTC
I can confirm this problem. There is no gamma set in my xorg.conf (I'm using Ubuntu), and xgamma shows 1.0. If I set it to eg 1.6, it is reset when the screensaver blanks, but the xgamma settings remains at 1.6 though it displays as 1. If I reissue the command xgamma -gamma 1.6, it is set again, though the value hasn't changed.
The same is true for using the nVidia Xconfig. If I set it to 1.6, the screensaver seems to reset it to 1, but simply by opening the nVidia panel (and not touching anything), the Gamma is set right again, as the value hasn't changed.
This didn't happen with the far superior xscreensaver as far as I know.
This is a definite bug.

Expected behavior:
Gnome Screensaver observes the current Gamma, not what was set in xorg.conf when the system started. It shouldn't change the Gamma at all. 
Comment 4 William Jon McCann 2006-09-13 18:33:57 UTC
Yes, it is a bug.  It is a matter of when the baseline gamma value is determined.  Want to try to fix it?
Comment 5 William Jon McCann 2007-08-27 23:13:21 UTC
*** Bug 469516 has been marked as a duplicate of this bug. ***
Comment 6 Ross Burton 2007-08-28 06:32:04 UTC
As I said in bug 469516, this isn't just a matter of saving the gamma value, but also the gamma LUTs.
Comment 7 John Bryant 2007-10-05 13:38:02 UTC
Created attachment 96700 [details] [review]
Obtain gamma info before each fade, release it when the fade is reset

In this patch, gamma_info_init is now called in gs_fade_start, and gamma_info_free is called at the end of gs_fade_reset.  There are some other minor changes to make this work.

It seems to work really well.  The biggest concern I have is that since the gamma info is changed more often, it may be more susceptible to problems if being accessed by more than one thread at a time.  It seems that nothing else in gs-fade.c has been made thread safe, so it must not be necessary, but I can't know for sure.
Comment 8 Ross Burton 2007-10-05 14:00:57 UTC
Everyone who has a calibrated screen would appreciate it if you would save the LUT as well as the RGB gamma values.
Comment 9 John Bryant 2007-10-06 17:02:47 UTC
(In reply to comment #8)
> Everyone who has a calibrated screen would appreciate it if you would save the
> LUT as well as the RGB gamma values.
> 

gamma_info_init already saves them, and gs_fade_reset restores them, provided that VidModeExtension is version 2.1 or greater.  Testing on my system (Ubuntu Gutsy beta) shows that the gamma ramps are being used instead of the RGB values.
Comment 10 Ross Burton 2007-10-07 11:51:16 UTC
Ah, great, thanks. I was going on anecdotal evidence from G2.18 when the LUT was set after gnome-screensaver started.
Comment 11 Tuomo Kohvakka 2007-10-08 15:43:12 UTC
See also

https://bugs.launchpad.net/gnome-screensaver/+bug/33214

It could be nice (while probably out of scope for this bug) if it would be possible to fully disable gamma trickery without recompiling. That would probably mean introducing a preference so that could be set with gconf-editor.

(However, the above patch will probably fix this for me, I haven't yet tried it.)
Comment 12 John Bryant 2007-10-09 18:19:15 UTC
I just tried using the nvidia control panel (nvidia-settings) to adjust the brightness, contrast, and gamma settings.  These were preserved by gnome-screensaver with the patch.  I'd try this with ATI but I don't have a card available at the moment.
Comment 13 Martin Szulecki 2007-10-17 16:48:09 UTC
I am using a 24" screen with calibration using xcalib (which uses the XVidMode extension; mine is 2.2) and noticed this annoying issue. The attached patch fixes it and my calibration stays the same.
Comment 14 Pacho Ramos 2007-11-03 09:35:12 UTC
The attached patch fixed enemy-territory brightness problem under gentoo with gnome-screensaver-2.18.2

Thanks for fixing this
Comment 15 John Bryant 2007-11-03 21:03:14 UTC
Created attachment 98485 [details] [review]
Fix compile error in last patch

I'm glad I could help.

There was a compile error in the last patch if HAVE_XF86VMODE_GAMMA wasn't defined, but the new patch takes care of that.  I still can't determine if this has to be made thread safe, but if someone more knowledgeable than me decides that it does, I can fix it easy enough.
Comment 16 Pacho Ramos 2008-01-10 10:34:06 UTC
It also works with gnome-screensaver-2.20 :-)
Comment 17 Frederic Crozat 2008-01-12 18:05:17 UTC
I can confirm latest patch fixes the issue, I just wrote the exact same patch independently and I only discovered this bug just before submitting a bug with my patch :)
Comment 18 Matti Lindell 2008-02-15 00:43:00 UTC
Any news if the proposed patch will applied to trunk ?
Comment 19 Pacho Ramos 2008-02-21 16:36:39 UTC
Seems that current patch still doesn't retrieve the gamma
value from current setting, but uses the one there was at the launching of
gnome-screensaver, so it resets the gamma if it was changed dynamically in the
running server. At least is much better than current status.

(Thanks to leio for explaining me this in http://bugs.gentoo.org/show_bug.cgi?id=201019#c3)
Comment 20 John Bryant 2008-02-22 18:26:14 UTC
That is incorrect.  Notice the new calls to gamma_info_init and gamma_info_free.  This ensures that the gamma info is obtained immediately before each fade, and freed once it is done.  What leio describes is the behavior of gnome-screensaver without this patch.  Since I wouldn't have submitted a patch that does nothing, maybe something is making it ineffective in Gentoo.  Maybe something has changed in gnome-screensaver; I haven't tested this in months.

The only real concern I still have is thread safety, since I don't know enough about the underlying system to be able to tell if more than one thread will access gamma_info simultaneously.  Does anyone think I should protect gamma_info with a mutex?
Comment 21 Mart Raudsepp 2008-02-23 14:16:08 UTC
Yes, the patch didn't get applied because I failed to inherit our patching functions and the patch applying therefore failed. This is since fixed as pacho pointed it out to me and it works as you, John, describe it should work. It's working much better than before, I'd say this is well worth applying upstream after review of the concerns you cite regarding thread safety and such.
Comment 22 John Bryant 2008-03-08 15:20:58 UTC
Okay, I've learned a few things that make me think this patch is ready.  Even though there are two different classes that try to start and stop the fading (GSMonitor and GSManager both call gs_fade_async and gs_fade_reset), these happen in the same thread, at least on my system.  Also, the fadeout uses g_timeout_add, which causes fade_out_timer to be called repeatedly, but in the same thread.

Since it's all in the same thread, everything should be safe.
Comment 23 Pacho Ramos 2008-04-03 12:31:57 UTC
Please fix this at least for gnome-screensaver-2.24 Thanks
Comment 24 William Jon McCann 2008-05-12 21:31:59 UTC
Committed to trunk.  Thanks!  Sorry for the long delay.
Comment 25 William Jon McCann 2008-05-12 22:14:13 UTC
*** Bug 527029 has been marked as a duplicate of this bug. ***
Comment 26 William Jon McCann 2008-09-02 20:37:58 UTC
*** Bug 549895 has been marked as a duplicate of this bug. ***