<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/109792/">http://git.reviewboard.kde.org/r/109792/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 2nd, 2013, 2:58 p.m. UTC, <b>Oliver Henshaw</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/109792/diff/1/?file=125986#file125986line53" style="color: black; font-weight: bold; text-decoration: underline;">powerdevil/daemon/actions/bundled/dimdisplay.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void DimDisplay::onIdleTimeout(int msec)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">53</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">msec</span> <span class="o">==</span> <span class="n">m_dimOnIdleTime</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">53</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">msec</span> <span class="o">==</span> <span class="n">m_dimOnIdleTime</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">54</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setBrightnessHelper</span><span class="p">(</span><span class="mi">0</span><span class="p">);</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">55</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="n">msec</span> <span class="o">==</span> <span class="p">(</span><span class="n">m_dimOnIdleTime</span> <span class="o">*</span> <span class="mi">3</span> <span class="o">/</span> <span class="mi">4</span><span class="p">))</span> <span class="p">{</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">56</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">float</span> <span class="n">newBrightness</span> <span class="o">=</span> <span class="n">backend</span><span class="p">()</span><span class="o">-></span><span class="n">brightness</span><span class="p">()</span> <span class="o">/</span> <span class="mi">4</span><span class="p">;</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">57</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setBrightnessHelper</span><span class="p">(</span><span class="n">newBrightness</span><span class="p">);</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">58</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="n">msec</span> <span class="o">==</span> <span class="p">(</span><span class="n">m_dimOnIdleTime</span> <span class="o">*</span> <span class="mi">1</span> <span class="o">/</span> <span class="mi">2</span><span class="p">))</span> <span class="p">{</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">59</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_oldBrightness</span> <span class="o">=</span> <span class="n">backend</span><span class="p">()</span><span class="o">-></span><span class="n">brightness</span><span class="p">();</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">54</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_oldBrightness</span> <span class="o">=</span> <span class="n">backend</span><span class="p">()</span><span class="o">-></span><span class="n">brightness</span><span class="p">();</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">60</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">float</span> <span class="n">newBrightness</span> <span class="o">=</span> <span class="n"><span class="hl">backend</span></span><span class="p"><span class="hl">()</span></span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">b</span>rightness</span><span class="p"><span class="hl">()</span></span><span class="hl"> </span><span class="o"><span class="hl">/</span></span><span class="hl"> </span><span class="mi"><span class="hl">2</span></span><span class="p">;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">55</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">float</span> <span class="n">newBrightness</span> <span class="o">=</span> <span class="n"><span class="hl">m_oldB</span>rightness</span><span class="hl"> </span><span class="o"><span class="hl">*</span></span><span class="hl"> </span><span class="n"><span class="hl">m_dimRatio</span></span><span class="p">;</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">61</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setBrightnessHelper</span><span class="p">(</span><span class="n">newBrightness</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">56</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setBrightnessHelper</span><span class="p">(</span><span class="n">newBrightness</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">62</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">57</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On April 2nd, 2013, 3:30 p.m. UTC, <b>Danny Baumann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">- 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?</pre>
</blockquote>
<p>On April 2nd, 2013, 9:16 p.m. UTC, <b>Sebastian Kügler</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On April 4th, 2013, 3:35 p.m. UTC, <b>Danny Baumann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.)</pre>
</blockquote>
<p>On April 4th, 2013, 3:52 p.m. UTC, <b>Oliver Henshaw</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On April 13th, 2013, 4 p.m. UTC, <b>Oliver Henshaw</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well, for group 2 I see no problem. Handling group 3 would mean making the dim period too short for group 4 (people who find the current dim time too short, but didn't find the option). As the only group that really relies on the current behaviour (group 2) already found the option anyway, I IMHO making a new profile revision just for this change is worth the effort.</pre>
<br />
<p>- Danny</p>
<br />
<p>On April 13th, 2013, noon UTC, Danny Baumann wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kde-workspace, Solid, Dario Freddi, and Oliver Henshaw.</div>
<div>By Danny Baumann.</div>
<p style="color: grey;"><i>Updated April 13, 2013, noon</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=304696">304696</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>powerdevil/daemon/actions/bundled/dimdisplay.cpp <span style="color: grey">(11554e3ba5d2f67d4d1de9d61c744c6c40a32d27)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109792/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>