Review Request 109792: Update 'dim display' algorithm.

[Danny Baumann]

> On April 2, 2013, 2:58 p.m., Oliver Henshaw wrote:
> > powerdevil/daemon/actions/bundled/dimdisplay.cpp, lines 53-62
> > <http://git.reviewboard.kde.org/r/109792/diff/1/?file=125986#file125986line53>
> >
> >     Resolving bug #304696 is good, in my eyes. I'd like affirmation from Dario though.
> >     
> >     A couple of questions:
> >     
> >     - Can you say a few words why you decided to drop the multi-stage dimming and go straight to the final brightness?
> >     
> >     - What do you think should be done about migrating existing profiles, if anything? This change benefits users who are confused by powerdevil's behaviour, but may worsen things for users who have got wise to this quirk and set the dim display idle time to twice their desired value. I don't know what the relative populations of these two groups are.
> >     
> >     * Upgrading profiles in powerdevilprofilegenerator.cpp and halving the stored idle time means no change in dimming time for any user and makes the config UI accord with powerdevil's behaviour. Users in the first group will now be able to see that the idle time is set to a surprisingly low value and change it.
> >     
> >     * leaving the profile as is means dimming behaviour suddenly rights itself for users in the first group and changes unexpectedly for users in the second group. If the new effective dimming time is longer than certain other timeouts, such as screen power saving or system suspend, this means that the display will never dim.

- For the first question, I dropped it because it made the most sense from the user expectation to me. Dimming in 3 stages after 1/2, 3/4 and 1/1 of the configured time seemed a bit random to me. What could work IMHO is replacing it by fading from current to target brightness over e.g. 20 or 30 seconds - that would make the dimming time not deviate much from what the user actually configured. I think the tricky part there would be determining the number of steps to use.

- For the second question, that's a valid point, but I'm not sure how important the problem is. Users in the second group already found the option to configure the dim time, so it's likely (from my view, I'm not a UX expert though) that they'll find the option again when they stumble upon the behaviour change. The first suggested approach assumes that all users know where to find this configuration option, is that a safe assumption to make? Also, is it possible to update only non-default values?


- Danny


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109792/#review30278
-----------------------------------------------------------


On April 2, 2013, 1:52 p.m., Danny Baumann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109792/
> -----------------------------------------------------------
> 
> (Updated April 2, 2013, 1:52 p.m.)
> 
> 
> Review request for kde-workspace, Solid, Dario Freddi, and Oliver Henshaw.
> 
> 
> Description
> -------
> 
> This change does two things:
> - No longer dim display before the dim time set by the user elapses.
>   This fixes bug #304696
> - No longer assume that 0% display brightness produces a visible result.
>   This doesn't work with the intel-backlight backlight interface (as it
>   turns off the backlight at 0% brightness). According to the kernel
>   developers (see [1] and [2]), this isn't a safe assumption to make for
>   other backlight interfaces either. Instead of always dimming to 0%,
>   make the amount of dimming configurable.
> 
> [1]
> http://lists.freedesktop.org/archives/intel-gfx/2013-March/026152.html
> [2]
> http://lists.freedesktop.org/archives/intel-gfx/2013-March/026140.html
> 
> 
> This addresses bug 304696.
>     http://bugs.kde.org/show_bug.cgi?id=304696
> 
> 
> Diffs
> -----
> 
>   powerdevil/daemon/actions/bundled/dimdisplay.h 426640ca7a2a2d23044052710fdfb4b1f65617d1 
>   powerdevil/daemon/actions/bundled/dimdisplay.cpp 11554e3ba5d2f67d4d1de9d61c744c6c40a32d27 
>   powerdevil/daemon/actions/bundled/dimdisplayconfig.h 14b787937249a512cf958a31fd3bd71d6051540d 
>   powerdevil/daemon/actions/bundled/dimdisplayconfig.cpp bc116d6216c4624cbf14489d739d9d226fb70ff3 
> 
> Diff: http://git.reviewboard.kde.org/r/109792/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Danny Baumann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130402/1d67d4fa/attachment.htm>