<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 March 31st, 2013, 4:14 p.m. UTC, <b>Àlex Fiestas</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;">To be honest I don't like adding yet another configuration option by default, a configuration option that apparently is needed only in some systems.
There is no other alternative than this? </pre>
</blockquote>
<p>On March 31st, 2013, 4:48 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;">Well, I don't see any. I tried to approach this at the kernel level first, but the kernel people rejected the idea. But I don't see it as a big problem either: E.g. Windows (which isn't an example of overconfigurability) has a dimmed brightness setting as well. While it's required for my laptop, I think it can be useful for other people as well. When I hit the problem, at first I couldn't believe this isn't configurable either ;)
BTW, one thing I'm not yet certain about is whether the option should be 'dim to' or 'dim by'. Currently it's the former, but the latter might it make more obvious that the value is relative to the non-dimmed brightness.</pre>
</blockquote>
<p>On April 2nd, 2013, 9:16 a.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;">Just set the minimum brightness to 1 instead of 0. We're generally not adding config options for this kind of things.</pre>
</blockquote>
<p>On April 2nd, 2013, 9:44 a.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;">That doesn't help though, as with intel-backlight 1 is still way too low to be visible. I think the value corresponding to acpi value 0 is about 5%. At this point, I think, I would alter the acpi behaviour again. Also, if the user had set the brightness to 0 before for some reason (e.g. because he's working in the evening), 'dim display' would suddenly make the display more bright.
In general, the problem is the different backlight interfaces just behave differently, which is why I think this problem can't be solved with a hardcoded value as long as it's not possible to detect the used sysfs file (which isn't the case as long as the used X driver provides a backlight interface via Xrandr).
</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The "dim can brighten up" is a different problem that needs a different solution, it's unrelated here.
I don't see how a config option solves this problem. Even with a config option, we still don't know what the default should be. Also, users are not going to look for such an option (which maybe says enough about the option itself). Users are going to hit the brightness slider when the brightness is too low, so 1 actually does make sense.</pre>
<br />
<p>- Sebastian</p>
<br />
<p>On March 31st, 2013, 4:07 p.m. 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 and Solid.</div>
<div>By Danny Baumann.</div>
<p style="color: grey;"><i>Updated March 31, 2013, 4:07 p.m.</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.h <span style="color: grey">(426640ca7a2a2d23044052710fdfb4b1f65617d1)</span></li>
<li>powerdevil/daemon/actions/bundled/dimdisplay.cpp <span style="color: grey">(11554e3ba5d2f67d4d1de9d61c744c6c40a32d27)</span></li>
<li>powerdevil/daemon/actions/bundled/dimdisplayconfig.h <span style="color: grey">(14b787937249a512cf958a31fd3bd71d6051540d)</span></li>
<li>powerdevil/daemon/actions/bundled/dimdisplayconfig.cpp <span style="color: grey">(bc116d6216c4624cbf14489d739d9d226fb70ff3)</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>