> 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. > > Danny Baumann wrote: > - 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? > > Sebastian Kügler wrote: > I think the user would expect here that after the configured amount of > time, the display dims. It's probably nice to do this in a few steps, and I > think the course of 20 or 30 seconds is just fine. How well it "feels" should > of course be tried. > > I think your idea of users is a bit off though, users are usually not > going to look for options (well, some do, but let's call that proximity bias > from our side). If something doesn't work as expected, they'll usually just > end up ignoring it, and if that doesn't work (because it's frustrating, or > breaking their workflow), ask someone (if we're lucky), or move on (the > silent majority). The damage in UI affordance is done at that point. The > mechanism should just do the right thing, without the user even having to > think about it. > > Danny Baumann wrote: > Whom do you mean by 'your idea of users is a bit off'? If you mean me: > why would it be off to assume that a user who found something doesn't do the > expected thing and changed setting X to adapt would not be able to change it > back when the behaviour is changed to the correct one? > > (Also, totally off-topic: Can anyone of you guys point me to a good, > ideally up-to-date howto for running the code compiled using kdesrc-build as > either another user or with another KDE config directory? I'm aware of the > Xephyr method, which I'm using thus far, but it doesn't help here as Xephyr > doesn't pass the backlight Xrandr extension through. Thanks.) > > Oliver Henshaw wrote: > I managed to forget about group three - who don't notice a problem and > are content with the screen dimming when it does. Only people who have found > the configuration will realise something is amiss. > > Migrating the profile and halving dim-on-idle time would mean these users > would see no change in behaviour.
So revision three still works well for group one but changes behaviour for groups two and three. Are you making a decision not to migrate profiles or is this just something you haven't addressed yet? - Oliver ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109792/#review30278 ----------------------------------------------------------- On April 13, 2013, noon, Danny Baumann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109792/ > ----------------------------------------------------------- > > (Updated April 13, 2013, noon) > > > 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.cpp > 11554e3ba5d2f67d4d1de9d61c744c6c40a32d27 > > Diff: http://git.reviewboard.kde.org/r/109792/diff/ > > > Testing > ------- > > > Thanks, > > Danny Baumann > >
