GNOME Bugzilla – Bug 650405
'Ensure we emit the BrightnessChanged DBus signal...' breaks re-brightening of display on un-idle and AC re-connect
Last modified: 2011-10-28 21:35:54 UTC
The commit "Ensure we emit the BrightnessChanged DBus signal when the brightness buttons are pressed" - 48fa50a682afc1c6f2492f9c1c73b7de0fa8906b - breaks handling of the backlight. The function gpm_backlight_brightness_evaluate_and_set operates on the value master_percentage , which until this commit, was either the settings key org/power-manager/brightness-ac (which defaults to 1.0, i.e. 100%), or the value last set by the user manually via the keyboard - the function gpm_backlight_button_pressed_cb was the only one that could otherwise change the master_percentage value. So if we ignore manual changes, the function simply gave you 50% brightness when on battery power, and 30% when on battery and idle, more or less. However, this commit introduces a function gpm_backlight_brightness_changed which sets master_percentage to whatever value the brightness was just changed to, and calls that function from the end of gpm_backlight_brightness_evaluate_and_set . So you wind up with a loop, where the brightness can only ever be decreased - whenever gpm_backlight_brightness_evaluate_and_set is called it winds up operating on the last value it set, not on the default value of 1.0. So if you go from AC to battery, the backlight dims from 100% to 50%. Then when you go to idle, instead of dimming to 30% (0.3*1.0) it dims to 15% (0.3*0.5). Then when you go to un-idle, instead of going to 50% (0.5*1.0) it stays low. When you plug back into an AC outlet, again, it doesn't go back to 100%, it stays at whatever it was at before. Your system is doomed to wind up at its lowest possible brightness after a few unplugs and idles. I don't know what the correct fix for this would be, but a quick fix would be for gpm_backlight_brightness_changed not to set master_percentage.
It looks like the only thing replaced in the commit which *actually* stored master_percentage was gpm_backlight_brightness_changed_cb . -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
The attached patch restores correct behaviour for me. I made gpm_backlight_brightness_changed not set master_percentage, and changed gpm_backlight_brightness_changed_cb to set it before calling gpm_backlight_brightness_changed. I believe this should preserve all intended behaviours from before the problematic commit.
Created attachment 187975 [details] [review] patch to fix the issue
so there's one case my patch doesn't fix: if you adjust the slider in the Screen control panel applet you're immediately stuck in the Descending Spiral Of Brightness. Again, gpm_backlight_brightness_evaluate_and_set starts operating on the current value every time it's called, and so it'll never raise the brightness, even if you just re-connected the power (for instance). At a quick glance I'm not entirely sure why this is, it probably has something to do with gpm_backlight_brightness_changed_cb ? I'll see if this is a regression in my patch, or if it was broken before my patch (I suspect the latter). -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
actually...it's only broken _so long as the screen applet is open_. if you adjust the value, close the applet, then do unplug/plug cycle, it works as expected. -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
I think the issue from comment #4 / comment #5 is probably pretty unrelated to this patch, and could be treated separately; I'm pretty sure my patch is correct, and that's a different bug (probably g-p-m sees an automatic change performed when the Screen applet is open as a manual change). -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
confirmed that #4 and #5 isn't a regression in my patch; without my patch it's more broken (it doesn't work when Screen applet is open *or* when it's closed). -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
There's a Fedora scratch build which incorporates my patch at http://koji.fedoraproject.org/koji/taskinfo?taskID=3077414 . -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
Thanks adam, seems to work for me.
Works here as well.
I'm pretty sure this is already fixed in the new power plugin from gnome-settings-daemon. Richard?
Yes, I think this is much more sane now.
so, let's close it.