So, would calls to:

gpm_brightness_level_save (manager->priv->brightness);
ret = gpm_hal_suspend (0);
gpm_brightness_level_resume (manager->priv->brightness);

fix the issue, or have I oversimplified the problem?

I don't think that g-p-m sets the brightness on resume, I think the acpi bios default is used.

Seems plausible. Should be done for hibernate too I guess?

Or, another option would be to change the policy brightness when the user manually changes the brightness, i.e. so that pressing brightness up when on AC would change the policy value for the brightness on AC.
This is probably the more correct solution, but what do you think?

I think that would confuse me.

Indeed I think of the "Brightness on AC" setting as "Brightness to switch to when I transition from battery to AC" and the converse for "Brightness on battery"

Often I will adjust brightness depending on whether I'm watching a film, or coding, or on a train, or whatever.

I'd be tempted to rename the brightness sliders to "Brightness to set when switching to AC power" and "Brightness to set when switching to battery power" or something

[Note that I didn't look at g-p-m's source code so far.]

> So, would calls to:
> 
> gpm_brightness_level_save (manager->priv->brightness);
> ret = gpm_hal_suspend (0);
> gpm_brightness_level_resume (manager->priv->brightness);
> 
> fix the issue, or have I oversimplified the problem?

Will this also work if I use the Fn-F4 and Fn-F12 key combinations to suspend resp. hibernate the system?

> I don't think that g-p-m sets the brightness on resume

It does, I just verified that by invoking the screen saver with and without gnome-power-manager running.  If it's not running, the display brightness is not changed when returning from screen saving, if it's running, the display brightness is set to the value specified in g-p-m's settings.

> Or, another option would be to change the policy brightness when the user
> manually changes the brightness, i.e. so that pressing brightness up when on AC
> would change the policy value for the brightness on AC.
> This is probably the more correct solution, but what do you think?

If ``policy brightness'' is the brightness setting in g-p-m's configuration, then that's exactly what I'd expect and also explained in <URL:http://launchpad.net/bugs/34743>.

> Indeed I think of the "Brightness on AC" setting as "Brightness to switch to
> when I transition from battery to AC" and the converse for "Brightness on
> battery"

That's not how I understand it.  This setting should just reflect the currently set brightness imo.

As I explained in <URL:http://launchpad.net/bugs/34743>, there isn't even an universal ``brightness on AC'' for me that I could set and I'd always be happy with.

> Often I will adjust brightness depending on whether I'm watching a film, or
> coding, or on a train, or whatever.

Yes, and then I'd expect that setting to be remembered when returning from screen saving mode, etc.

> I'd be tempted to rename the brightness sliders to "Brightness to set when
> switching to AC power" and "Brightness to set when switching to battery power"
> or something

Then I'd request them to be disengagable.

> Or, another option would be to change the policy brightness when the user
> manually changes the brightness, i.e. so that pressing brightness up when on AC
> would change the policy value for the brightness on AC.
> This is probably the more correct solution, but what do you think?

That's by the way also what I'd expect for all the other possibilities where the actual system (hardware) state could diverge from the OS's state, like volume settings (including the mute setting), ThinkLight setting (if that is modifiable via Gnome; Fn-F12 on my Thinkpad), Wireless on/off setting (Fn-F5), external video port on/off (Fn-F7).  I have not yet examined how these are handled withing Gnome (or elsewhere).

So what's the plan? Jaap, what do you think of the suggestions?

I'm thinking, when the user changes the brightness using keys on the keyboard, to change the policy brightness for that state.
i.e. The preferences slider should change when I alter brightness manually.

This way we only have a certain number of switching permutations and the poor user who's switching in and out his ac_adapter isn't so confused.

I can't see a use case when i think "I like brightness a on battery", "I like brightness b on AC", "I like brightness c just for now" -- i figure if the user changes the brightness manually then thats typically how he wants to use it in this state.

For laptops with brightness sensors this is different, but as we have no support for these in HAL (yet) I don't think it matters. Even if we did, the brightness sliders would just scale the brightness of the current calculated brightness.

I think this is most sane, having two states that we "remember".

Think -- if we try to get too clever, then little old me gets bugzillas because it's doing "non-logical" things... :-)

Comments?

Richard.

Daniel, what do you use to "change it with the keyboard".
The next version of g-p-m will have support for changing the brightness with the keyboard itself, so whatever else is being used (gnome-settings-daemon for example on Apple laptops) should be killed.

Bastien,

The toshiba laptops have a 'Fn' key which when combined with certain of the function keys and the right kernel module (toshiba_acpi) give events via the acpi socket which get transformed into brightness changes by the Ubuntu acpi-support package (or via fnfxd or similar in other distributions)

I imagine that when g-p-m responds to the allocated keypresses for brightness adjustment then the acpi-support scripts in Ubuntu will be altered to emit those keypresses instead of making the adjustment themselves.

Daniel, the /usr/libexec/hald-addon-acpi-buttons-toshiba addon takes care of abstracting out the toshiba kernel interface and presenting then as buttonpressed events that g-p-m can pick up.
I don't think the acpi-support should touch toshiba buttons, but that probably depends on which version of hal you are shipping for dapper.

It's all pretty locked down now I think.

Hal 0.5.7, g-p-m 2.14.x

I imagine we'll switch to hal's toshiba addon during edgy.

*** Bug 361273 has been marked as a duplicate of this bug. ***

I think the whole concept of setting 'absolute' brightness levels on given states should be revised. Instead, there should be a master or 'absolute' level adjusted by the user, and changing power mode would just dim or brighten by a certain amount. Think about html: the font sizes are relative to a base value, which is a user preference...
Perhaps the comparison to the audio volume could be explored further: a 'master' volume which affects the volume set for multiple sources - except that I can't think of other sources for controlling brightness; perhaps the integrated light sensor?

hardware or absolute brightness=Master Brightness * User-prefered + ( p-m * 'p-m-weight' ) * Sensor weight

or something....

(It's 4 am, this is just a hint, I didn't give much thinking to that formula...)

A similar bug report for this issue : http://bugs.debian.org/437997

Related bug:

https://bugs.launchpad.net/gnome-power/+bug/35223

"You mean save a new policy value for that state on every adjustment... that's probably a good idea."  -Richard Hughes

...

My take is that the simplest might just be to have two values saved:

  on-ac value
  off-ac value

whenever changes are made, make those change take effect and save the value.  Only ever read the saved values when the state changes.

Only update saved values:

  1. one-time initial package installation
  2. on user change

Only load (apply) saved values:

  1. on boot (login)
  2. on power-state change (on-ac, off-ac)
  3. on resume from suspend
  4. on resume from hibernate

(adjust to take account of multi-user systems...).

I think I have the very same problem on my hp 6710s laptop. After every lid opening the brightness backs to the lowest level first and to the brightest after few seconds. Really annoying. 

I confirm this, and on Gnome 2.20.1 (Gutsy), the options are indeed "On AC Power: Set display brightness to:" and "On Battery Power: Dim display brightness by:"

However, when the brightness setting is manually changed with the keyboard, the LCD brightness changes without updating the policy (slider). This causes the setting to be lost quite frequently (not only on suspend/resume). As Miguel states, this should be handled in the same manner as the Master Volume Control; If the brightness is manually changed using the keyboard, update the On AC Power (Master) brightness, it can then be dimmed the normal amount using the On Battery Setting. It would then become apparent to the user that there was a correlation between the adjustment and the GPM setting. The "On AC Power" title could be modified to more clearly explain the "master" status. This covers the majority of use cases I can think of since the user is generally responding to a change in ambient light level or generally desired brightness. Switching back and forth between AC and battery should dim the display the same % regardless of the master level.

The only limitation to this solution is when the user is on battery power, and they adjust the master brightness to maximum, however the battery dim % is set too high for their preference. However, the users experience with the volume control would assist in understanding the limitation.

Has something happened on this bug? Is there a patch available?

The most frequent scenario where this is evident is in Ubuntu Gutsy when the screen is dimmed due to inactive and then set back to the configured level when I return. Sometimes this happens if I just sit still and read a web page.

Scenario:

1. Push up brightness (due to various reasons, bright outside etc)
2. Read something without touching the computer
3. The brightness is lowered to save battery (proper behavior)
4. I touch the mouse to be able to see
5. The brightness is restored to whatever is configured (i.e. NOT the level I set manually just before)

The proper thing to to is to save the default brightness level as the one set manually. Whether to do that in the same setting as the one configurable in Power Settings I don't know, but it should be saved!

Right, I've merged in a ton of patches into trunk. Could you guys test it and tell me what you think please? Thanks.

Hello,

1) I hate all these brightness related bugs, especially since they represent a regression.
2) There are several bugs that overlap(#478128, #483134, #483143, #509211)  ; IMHO, there should be ONLY ONE

I have reported this in Debian bug #467264 and I explained why the current (and former) behaviour is(was) wrong and how they should actually behave.


http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=467264


To summarize:
- AC and battery settings MUST not interact with each other
- the battery *brightness*slider* should be *brought*back* (one should be able to set the default battery brightness level, no matter what automatic dimming effect is added)
- dimming on idle should *really*dim*or*do*nothing*, if dimming is not possible; is should NEVER bright up the display
- using the *brightness* *keys* should be a clear adjustment of the default brightness level and it should *always* have precedence over anything automatically computed or commanded
- DO NOT DIM WHILE ACTIVE


If anyone cares, here is the summary I made in the debian bug report: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=467264#33

Sorry, the entire list of bug was:
bug #478128,
bug #483134,
bug #483143,
bug #509211,
bug #530346,
bug #335673 and
bug #483144

Eddy, you've made 2 identical comments on 7 bugs, causing my inbox to be filled with bugzilla spam this morning. Please don't do that in the future, it's not polite. I would also appreciate you not telling me what I MUST do.

>- AC and battery settings MUST not interact with each other

Wrong. You're never doing one task on battery and another when on AC -- you normally just do the same thing but change power state on the fly.

>- the battery *brightness*slider* should be *brought*back*

*why?*

>- dimming on idle should *really*dim*or*do*nothing*

Right, if it dims up then that's a bug.

>- using the *brightness* *keys* should be a clear adjustment of the default

Yup, and that's what svn has been doing for a few months now.

>- DO NOT DIM WHILE ACTIVE

That's a gnome-screensaver bug, or a bug in the application you are using as it needs to be using the Inhibit API if the session is idle. PLEASE DO NOT SHOUT.

Richard.

(In reply to comment #25)
> Eddy, you've made 2 identical comments on 7 bugs, causing my inbox to be filled

Well, I am sorry that I can't merge the obvious duplicated/related bugs since I don't have the rights to do so. I just did the next best thing (or at least I thought so) trying to point out that there are parallel discussions going on on different BRs which should actually be on a single discution thread, IMHO.

> with bugzilla spam this morning. Please don't do that in the future, it's not
> polite. I would also appreciate you not telling me what I MUST do.

Still, I fail to see when I told you what *you* must do, I was speaking about some application you happen to maintain (AFAICT). So, please, you MUST :-P stop confusing yourself with an application, is bad for your health :-) .

> >- AC and battery settings MUST not interact with each other
> 
> Wrong. You're never doing one task on battery and another when on AC -- you
> normally just do the same thing but change power state on the fly.

<sarcasm>Good you're trying to see my point or understand why I said what I did </sarcasm>

Actually, as an abnormal person that I am, I already described a scenario when I explicitly described how I *am* doing different things on battery and on AC (for me, usually, the brightness is higher on battery than on AC).

Please read the link I posted:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=467264#33


OTOH, if you don't feel convinced, please tell me the reason behind this broken behaviour (described in the Debian BR):

>>>> While on battery:
>>>> - moving the AC slider changes brightness
>>
>> no longer happens
>
>Now it does.

Note that the intermediate version that wasn't broken from this pov was (apparently) 2.22.0.


> >- the battery *brightness*slider* should be *brought*back*
> 
> *why?*

Because the automatic computation of the default brightness level is broken. No preconfigured hardcoded ratio of the maximum brightness level on AC will be the correct value for everybody (see the broken default level of 50% used for the Asus laptop I was refering to in the Debian bug), so the only sane thing to do is to allow people to configure the default manually.

If such a slider is present for AC, there should be a similar one for battery, too. You can't expect the user to use the keyboard for one of the situations (i.e. battery) and the slider for the other (i.e. AC) to set the default brightness level.


> >- dimming on idle should *really*dim*or*do*nothing*
> 
> Right, if it dims up then that's a bug.

I glad we agree, there are at least 2 BR in the list I gave, that say this is happening.


> >- using the *brightness* *keys* should be a clear adjustment of the default
> 
> Yup, and that's what svn has been doing for a few months now.

And the latest stable release that does this is 2.22.1 by any chance? If so, then is broken, since the automatically computed values are used after returning from a dim. (i.e. reduce brightness on battery, dimming occurs, on activity the brightness is reverted to the automated level, not the one manually ajusted).


One more question, are there two default levels, one for each possible state (AC or battery)? If not, then this is broken. If it is, then when/which will be the next stable release that includes this change?


> >- DO NOT DIM WHILE ACTIVE
> 
> That's a gnome-screensaver bug, or a bug in the application you are using as 

Then, if you're so sure the bug is not in g-p-m, please, reassign the bugs that say this is happening to the appropriate application (or try to), in order to make it possible for the developers of the respective apps to fix them.

I haven't got ANY answer to my previous comment, although,imo, it raises some really valid points.


I have just installed Debian Lenny[1] and I am seeing the following issues with g-p-m:

- while on battery: changing the brightness level for AC changes current brightness
- Manual adjustments of the brightness are forgotten when transitioning from AC to battery or the other way around
- automatic dimming leads to the same loss of manually adjusted values
- the default brightness level for battery seems to be 50% of the AC level - this is broken, each one of the AC/battery setting should have its own brightness level
- dimming function doesn' always dim[2]
- brightness adjustments on AC affects default battery brightness level


[1] g-p-m is 2.22.1 + some patches that seem irrelevant for the issue at hand
[2] in spite of what Richard said, dimming seems to be in the realm of g-p-m since g-p-m contains such an option and I never touched the screensaver dimming option; am I missing something?

BTW, wrt this particular bug (#335673) I think

http://bugzilla.gnome.org/show_bug.cgi?id=335673#c18

Got it right. Please, please, please do not try to second guess user's manual choices.

Has this idea been totally abandoned now? In 2012, my laptop still starts with 100% brightness even though I use it at 50% brightness all the time.

Julien: We all know the current year so that's no helpful information. Your GNOME version and distro would have been more interesting...

André: Ubuntu Precise, but that's irrevelant since this bug has not been fixed in any version of GNOME, or has it? If it has, why is this bug still "NEW"?

So version 3.2. Thanks.

I can confirm that this is still happening in Gnome 3.2.1 and Fedora 16, and has been around since 3.0.

This seems to be behaving in 3.4.1.1