[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-hardware-devel
Subject: Re: [Kde-hardware-devel] Review Request 109792: Update 'dim display' algorithm.
From: Sebastian_Kügler <sebas () kde ! org>
Date: 2013-04-08 15:19:11
Message-ID: 20130408151911.2749.81655 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On March 31, 2013, 4:14 p.m., Àlex Fiestas wrote:
> > 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?
>
> Danny Baumann wrote:
> 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.
> Sebastian Kügler wrote:
> Just set the minimum brightness to 1 instead of 0. We're generally not adding \
> config options for this kind of things.
> Danny Baumann wrote:
> 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).
>
> Sebastian Kügler wrote:
> 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.
> Danny Baumann wrote:
> A sane value for the default would be 'retain current behaviour' (that is: dim by \
> 100%). Is there evidence for the 'users are not going to look for such an option' \
> assertion? I know for a fact that I _did_ search for that option (until I looked up \
> in the source that it's nowhere to be found). Surely this one sample doesn't \
> represent the KDE user base - but do you have any representative data, especially \
> given that the KDE user base in general doesn't have a problem with a \
> slightly-higher-than-usual amount of settings (which is also evident in the very \
> dialog we're talking about: are there that many users who look for e.g. a highly \
> configurable 'run script on power status change' option?)
> I'm also not sure what you mean by 'Users are going to hit the brightness slider \
> when the brightness is too low', as we're talking about the dimmed-down case after \
> all, where the user was idle for the configured amount of time. But as I wrote \
> above, neither 0 nor 1 works for any user who is not using the acpi_video backlight \
> controls for whatever reason (e.g. because it's buggy, or simply not present at \
> all). I'm open for suggestions on how to fix this problem without config option \
> (even if I think the option makes sense), but just raising the hardcoded value from \
> 0 to 1 definitely does not help.
> Sebastian Kügler wrote:
> What would probably make sense it to use 10% or 1 (whichever is more) as lowest \
> value. We do know the max value, so we can make sure it's low, but not zero. This \
> should be tried.
> How does raising it from 0 to 1 not help, btw?
>
> Danny Baumann wrote:
> Raising to 10% would help in my case, but how do you suggest handling cases where \
> either
> - 10% is considered 'too bright' (given that it was darker before - people might \
> complain about 'more power usage than necessary') and
> - 10% is brighter than the brightness before dimming?
>
> The answer to 'why does raising to 1 not help' is a few comments above ;)
> "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%"
> But can you please explain where exactly you see the problem of this option \
> compared to e.g. the way more sophisticated 'run script' options?
> Sebastian Kügler wrote:
> The option does not solve the problem. We have to make sure the system is usable \
> without changing an option. qMax(5%, 1) is also fine with me.
> In general, we can't expect users to look into options to fix obvious breakage. The \
> fact that *you* would look into it doesn't say anything valuable, since we're not \
> developing a system for people familiar with power management internals, but for \
> people who want to get their job done. UI options come at certain costs, so we're \
> only adding those where they really add value. For cases of breakage, we have to \
> find solutions that are transparant to the user.
> Danny Baumann wrote:
> Well, the issue is that 5% solve the problem for me, but other people might need \
> other values (glossy panels, less bright backlight, etc.). The 5% figure (BTW, all \
> numbers dimdisplay.cpp deals with are in the 0..100 range) is an _empiric_ value \
> for _my_ system. I don't have any kind of data for other systems, so I simply don't \
> know at all whether there's any value that would work for all users.
> To throw in some random data point I just found: Gnome dims to 30% of the maximum \
> value (as opposed to 30% of the currently set value, what my patch is currently \
> doing). That value is not mapped into their GUI, but it's not hardcoded either: \
> It's configurable via dconf-editor. Maybe an approach like this (make it \
> configurable, but don't map into the UI) would work?
> Àlex Fiestas wrote:
> What would work.
>
> All we can do here (as we do with brightness) is having sensible defaults that work \
> best with for all hardware. Then we have to make it configurable (we don't have it \
> in brightness yet) so advanced users and in the future retailers can make use of \
> it.
> You should wait until Dario or Oliver review this (added them in review)
>
> Oliver Henshaw wrote:
> I don't have particularly strong opinions about the existence of an extra \
> configuration option, so I'll leave that matter to those who do, although I think \
> you would need very good justification to put it in the configuration UI[1].
> I understand from the intel-gfx links, and others in that thread, that you can't \
> rely on any guarantees of the visibility of brightness 0 in intel_backlight or even \
> the acpi backend. But have you explored whether it's possible to smooth over the \
> difference between backends (or hash out some agreed semantics) at xrandr level?
> [1] If this is added to the UI, perhaps it could be presented on another (or the) \
> brightness slider? We'd need someone who's actually an UI expert to agree that's a \
> good idea, of course.
> Danny Baumann wrote:
> I tried to avoid making this a slider because of the relationship to the current \
> brightness: Expressing the setting as a slider IMHO implies that the value to be \
> set is an absolute value, which imposes corner cases (e.g. what to do if current \
> brightness is higher than dimmed brightness?). Using a relative value avoids those \
> and makes sure the display is always dimmed as long as the hardware allows it. I \
> couldn't think of a good way of expressing the relative value with a slider, which \
> is why I chose the spin box. I'm open for suggestions, though :)
> As for the Xrandr question: No, I didn't (yet? - this already became quite a \
> work-intensive thing for me given that it's only a DSDT problem I'm dealing with ;) \
> ), because I'm not sure how it'll help: As KDE (as well as Gnome, as well as \
> probably the other DEs) ships a fallback for the non-Xrandr case anyway, doesn't it \
> make more sense to define the behaviour in the common piece of code? Otherwise, one \
> would get behavioral differences depending on what backend happens to be in use; \
> I'm not sure whether this is a desired effect?
> Sebastian Kügler wrote:
> There's no need for a slider, as there won't be any UI option for this. That's my \
> personal opinion, but you can ask anybody in the Plasma team, and the answer will \
> be the exact same. Making it configurable doesn't solve the problem, it reduces \
> clarity in the UI, and it's the kind of option we've been successfully removing \
> from our UI, with good reasons, for years. It just would be a huge step back.
> With that out of the way, I still don't see what's wrong with setting it to 5% or \
> 1, whichever is lower. The user, after all, configured the system to go to minimum \
> brightness after a grace period, as long as the display is lit, we're doing the \
> expected thing.
> I do agree that dimming is not switching off, so we should make sure that the value \
> never reaches 0, but other than "the display switched off instead of dimming", and \
> of course the "dims after half the grace time", there is no bug.
> From my experience, values for brightness are often either between 0 and 10 or \
> between 0 and 100.
> One thing that should not get lost in the discussion about implementation details: \
> I really appreciate your work on this. :)
> Danny Baumann wrote:
> Sebastian, honestly, why are you ignoring what I wrote multiple times? :(
>
> "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%"
> In other words: While with intel-backlight 1% may not be 'backlight off' \
> technically speaking, the visual appearance of 'display off' and 'display at 1%' is \
> almost exactly the same.
> Sebastian Kügler wrote:
> First of all, I'm not ignoring you. There's probably just a misunderstanding going \
> on. What I wanted to say is that you should try to limit it to 1, or x percent, and \
> see how that works out, now you've tried 1, that's not enough to be visible, so we \
> got to try a bit higher value, and there it makes sense to use a percentage.
> So, please try setting the minimum brightness level to for example 5% then, you \
> seem to have one of those machines, so you're probably among the best ones to judge \
> what a safe low setting is. You might want to experiment a little with the actual \
> value.
> Danny Baumann wrote:
> Sebastian, I've answered all this stuff already, please read it :-(
>
> - Neither 0% nor 1% work _for my machine_.
> - For _my machine_, 5% is fine.
> - I have absolutely no idea what a sane value for _other machines_ that use \
> different means of backlight control (neither acpi_video nor intel-backlight), e.g. \
> MacBooks, is.
First of all, please assume that I did read your comments. It's really not productive \
to start every comment with an accusation of the other party not listening. Let's \
just keep it to the actual point, as that leads to a more constructive discussion of \
your patch, and less frustration and name-calling for everyone involved. In general, \
it's important to assume that those that are involved in the discussion want it to be \
pleasant and constructive, so that's what we strive for.
If you'd like to discuss a UI option, from the Plasma team's point of view, \
additional UI for this is just not going to happen. It clearly falls into the \
category of UI options we've consistently been avoiding with good reasons. I've \
discussed this particular issue with Marco already after you first submitted your \
patch, proposing additional UI as a solution, and his opinion was as clear as mine. \
If you don't trust me on that, feel free to ask him, or Aaron, just to make you more \
confident about why we do not want to go this route. Moreover, this bug can be fixed \
without adding UI, so that's the way forward.
For now, we should use 5% then. It will work for all of the machines I've looked \
into. Generally, they provide either values between 0-10 (0-7 on the oldest machine I \
remember), or 0-100, all are visibly on at 0. So those machines don't fall under this \
bug, anyway. It will also work for the machines we found this bug on, and that is \
actually sufficient. As soon as we get to know (through bugreports, or for example a \
friend's machine we look at out of curiosity), we'll rethink the bottom value.
- Sebastian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109792/#review30173
-----------------------------------------------------------
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
>
>
[Attachment #5 (text/html)]
<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>
<p>On April 2nd, 2013, 10:01 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;">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> </blockquote>
<p>On April 2nd, 2013, 11:07 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;">A sane value for the \
default would be 'retain current behaviour' (that is: dim by 100%). Is there \
evidence for the 'users are not going to look for such an option' assertion? \
I know for a fact that I _did_ search for that option (until I looked up in the \
source that it's nowhere to be found). Surely this one sample doesn't \
represent the KDE user base - but do you have any representative data, especially \
given that the KDE user base in general doesn't have a problem with a \
slightly-higher-than-usual amount of settings (which is also evident in the very \
dialog we're talking about: are there that many users who look for e.g. a highly \
configurable 'run script on power status change' option?)
I'm also not sure what you mean by 'Users are going to hit the brightness \
slider when the brightness is too low', as we're talking about the \
dimmed-down case after all, where the user was idle for the configured amount of \
time. But as I wrote above, neither 0 nor 1 works for any user who is not using the \
acpi_video backlight controls for whatever reason (e.g. because it's buggy, or \
simply not present at all). I'm open for suggestions on how to fix this problem \
without config option (even if I think the option makes sense), but just raising the \
hardcoded value from 0 to 1 definitely does not help.</pre> </blockquote>
<p>On April 2nd, 2013, 11:31 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;">What would probably make \
sense it to use 10% or 1 (whichever is more) as lowest value. We do know the max \
value, so we can make sure it's low, but not zero. This should be tried.
How does raising it from 0 to 1 not help, btw?</pre>
</blockquote>
<p>On April 2nd, 2013, 11:42 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;">Raising to 10% would \
help in my case, but how do you suggest handling cases where either
- 10% is considered 'too bright' (given that it was darker before - people \
might complain about 'more power usage than necessary') and
- 10% is brighter than the brightness before dimming?
The answer to 'why does raising to 1 not help' is a few comments above ;)
"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%"
But can you please explain where exactly you see the problem of this option compared \
to e.g. the way more sophisticated 'run script' options?</pre> </blockquote>
<p>On April 2nd, 2013, 12:06 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;">The option does not \
solve the problem. We have to make sure the system is usable without changing an \
option. qMax(5%, 1) is also fine with me.
In general, we can't expect users to look into options to fix obvious breakage. \
The fact that *you* would look into it doesn't say anything valuable, since \
we're not developing a system for people familiar with power management \
internals, but for people who want to get their job done. UI options come at certain \
costs, so we're only adding those where they really add value. For cases of \
breakage, we have to find solutions that are transparant to the user.</pre> \
</blockquote>
<p>On April 2nd, 2013, 12:34 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, the issue is that \
5% solve the problem for me, but other people might need other values (glossy panels, \
less bright backlight, etc.). The 5% figure (BTW, all numbers dimdisplay.cpp deals \
with are in the 0..100 range) is an _empiric_ value for _my_ system. I don't have \
any kind of data for other systems, so I simply don't know at all whether \
there's any value that would work for all users.
To throw in some random data point I just found: Gnome dims to 30% of the maximum \
value (as opposed to 30% of the currently set value, what my patch is currently \
doing). That value is not mapped into their GUI, but it's not hardcoded either: \
It's configurable via dconf-editor. Maybe an approach like this (make it \
configurable, but don't map into the UI) would work?</pre> </blockquote>
<p>On April 2nd, 2013, 1:51 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;">What would work.
All we can do here (as we do with brightness) is having sensible defaults that work \
best with for all hardware. Then we have to make it configurable (we don't have \
it in brightness yet) so advanced users and in the future retailers can make use of \
it.
You should wait until Dario or Oliver review this (added them in review)</pre>
</blockquote>
<p>On April 2nd, 2013, 3:15 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 don't have \
particularly strong opinions about the existence of an extra configuration option, so \
I'll leave that matter to those who do, although I think you would need very good \
justification to put it in the configuration UI[1].
I understand from the intel-gfx links, and others in that thread, that you can't \
rely on any guarantees of the visibility of brightness 0 in intel_backlight or even \
the acpi backend. But have you explored whether it's possible to smooth over the \
difference between backends (or hash out some agreed semantics) at xrandr level?
[1] If this is added to the UI, perhaps it could be presented on another (or the) \
brightness slider? We'd need someone who's actually an UI expert to agree \
that's a good idea, of course.</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;">I tried to avoid making \
this a slider because of the relationship to the current brightness: Expressing the \
setting as a slider IMHO implies that the value to be set is an absolute value, which \
imposes corner cases (e.g. what to do if current brightness is higher than dimmed \
brightness?). Using a relative value avoids those and makes sure the display is \
always dimmed as long as the hardware allows it. I couldn't think of a good way \
of expressing the relative value with a slider, which is why I chose the spin box. \
I'm open for suggestions, though :)
As for the Xrandr question: No, I didn't (yet? - this already became quite a \
work-intensive thing for me given that it's only a DSDT problem I'm dealing \
with ;) ), because I'm not sure how it'll help: As KDE (as well as Gnome, as \
well as probably the other DEs) ships a fallback for the non-Xrandr case anyway, \
doesn't it make more sense to define the behaviour in the common piece of code? \
Otherwise, one would get behavioral differences depending on what backend happens to \
be in use; I'm not sure whether this is a desired effect?</pre> </blockquote>
<p>On April 2nd, 2013, 9:11 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;">There's no need for \
a slider, as there won't be any UI option for this. That's my personal \
opinion, but you can ask anybody in the Plasma team, and the answer will be the exact \
same. Making it configurable doesn't solve the problem, it reduces clarity in the \
UI, and it's the kind of option we've been successfully removing from our UI, \
with good reasons, for years. It just would be a huge step back.
With that out of the way, I still don't see what's wrong with setting it to \
5% or 1, whichever is lower. The user, after all, configured the system to go to \
minimum brightness after a grace period, as long as the display is lit, we're \
doing the expected thing.
I do agree that dimming is not switching off, so we should make sure that the value \
never reaches 0, but other than "the display switched off instead of \
dimming", and of course the "dims after half the grace time", there is \
no bug.
From my experience, values for brightness are often either between 0 and 10 or \
between 0 and 100.
One thing that should not get lost in the discussion about implementation details: I \
really appreciate your work on this. :)</pre> </blockquote>
<p>On April 4th, 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;">Sebastian, honestly, why \
are you ignoring what I wrote multiple times? :(
"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%"
In other words: While with intel-backlight 1% may not be 'backlight off' \
technically speaking, the visual appearance of 'display off' and 'display \
at 1%' is almost exactly the same.</pre> </blockquote>
<p>On April 8th, 2013, 2:07 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;">First of all, I'm \
not ignoring you. There's probably just a misunderstanding going on. What I \
wanted to say is that you should try to limit it to 1, or x percent, and see how that \
works out, now you've tried 1, that's not enough to be visible, so we got to \
try a bit higher value, and there it makes sense to use a percentage.
So, please try setting the minimum brightness level to for example 5% then, you seem \
to have one of those machines, so you're probably among the best ones to judge \
what a safe low setting is. You might want to experiment a little with the actual \
value.</pre> </blockquote>
<p>On April 8th, 2013, 2:44 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;">Sebastian, I've \
answered all this stuff already, please read it :-(
- Neither 0% nor 1% work _for my machine_.
- For _my machine_, 5% is fine.
- I have absolutely no idea what a sane value for _other machines_ that use different \
means of backlight control (neither acpi_video nor intel-backlight), e.g. MacBooks, \
is.</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;">First of all, please \
assume that I did read your comments. It's really not productive to start every \
comment with an accusation of the other party not listening. Let's just keep it \
to the actual point, as that leads to a more constructive discussion of your patch, \
and less frustration and name-calling for everyone involved. In general, it's \
important to assume that those that are involved in the discussion want it to be \
pleasant and constructive, so that's what we strive for.
If you'd like to discuss a UI option, from the Plasma team's point of view, \
additional UI for this is just not going to happen. It clearly falls into the \
category of UI options we've consistently been avoiding with good reasons. \
I've discussed this particular issue with Marco already after you first submitted \
your patch, proposing additional UI as a solution, and his opinion was as clear as \
mine. If you don't trust me on that, feel free to ask him, or Aaron, just to make \
you more confident about why we do not want to go this route. Moreover, this bug can \
be fixed without adding UI, so that's the way forward.
For now, we should use 5% then. It will work for all of the machines I've looked \
into. Generally, they provide either values between 0-10 (0-7 on the oldest machine I \
remember), or 0-100, all are visibly on at 0. So those machines don't fall under \
this bug, anyway. It will also work for the machines we found this bug on, and that \
is actually sufficient. As soon as we get to know (through bugreports, or for example \
a friend's machine we look at out of curiosity), we'll rethink the bottom \
value.</pre> <br />
<p>- Sebastian</p>
<br />
<p>On April 2nd, 2013, 1:52 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, Solid, Dario Freddi, and Oliver Henshaw.</div>
<div>By Danny Baumann.</div>
<p style="color: grey;"><i>Updated April 2, 2013, 1:52 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>
_______________________________________________
Kde-hardware-devel mailing list
Kde-hardware-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-hardware-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic