> 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.

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.


- Danny


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


On March 31, 2013, 4:07 p.m., Danny Baumann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109792/
> -----------------------------------------------------------
> 
> (Updated March 31, 2013, 4:07 p.m.)
> 
> 
> Review request for kde-workspace and Solid.
> 
> 
> 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
> 
>

_______________________________________________
Kde-hardware-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-hardware-devel

Reply via email to