> On Авг. 1, 2014, 8:14 д.п., Kai Uwe Broulik wrote:
> > Thank you! I've always wanted to make powerdevil use the actual maximum 
> > brightness instead of just exporting fixed 10% steps.
> > 
> > Given that this is powerdevil (and not a part of kde-frameworks), I don't 
> > think we need to make that backwards compatible, ie. brightness can be the 
> > new absolute value, with a maximumBrightness being the maximum. We just 
> > need to make sure to adjust all the users of that: Plasma PM dataengine, 
> > Kamoso, … however we're already past the 5.0 release and breaking things 
> > for others is nasty.
> > 
> > Frankly, I currently do not know what the state of Solid Power is, and 
> > whether it will handle the brightness stuff (and if it already uses the 
> > absolute values) and whether it will be removed from powerdevil then.
> 
> Nikita Skovoroda wrote:
>     Actually, there is exactly no reason to break D-Bus api compatibility.
>     I was asking about breaking backwards compatibility for inner helpers. I 
> assume that they shouldn't be used from outside?
>     
>     If you confirm that, I'll clean this up a bit.

backlighthelper is a standalone binary, it had «brightness», «setbrightness» 
and «syspath» actions.
I added «brightnessvalue», «brightnessvaluemax», «serbrightnessvalue» actions.

If the old «brightness», «setbrightness» actions could be removed (from the 
helper executable), that would clean up things a bit.

D-Bus api is still backwards compatible, I just introduced new methods.


- Nikita


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119559/#review63610
-----------------------------------------------------------


On Авг. 1, 2014, 7:42 д.п., Nikita Skovoroda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119559/
> -----------------------------------------------------------
> 
> (Updated Авг. 1, 2014, 7:42 д.п.)
> 
> 
> Review request for Solid.
> 
> 
> Repository: powerdevil
> 
> 
> Description
> -------
> 
> Brightness fixes, part 1: 
> Make all layers aware of the actual brightness value and the maximum 
> brightness value.
> 
> Introduce brightnessValue, brightnessValueMax, brightnessValueChanged, 
> setBrightnessValue D-Bus endpoints in 
> org.kde.Solid.PowerManagement.Actions.BrightnessControl.
> Introduce keyboardBrightnessValue, keyboardBrightnessValueMax, 
> keyboardBrightnessValueChanged, setKeyboardBrightnessValue D-Bus endpoints in 
> org.kde.Solid.PowerManagement.Actions.KeyboardBrightnessControl.
> 
> Does not break backwards compatibility, even for the backlighthelper.
> This could be cleaned up a bit, if it's allowed to break backwards 
> compatibility for inner helpers: the old float-based logic can be removed 
> from the helpers completely and re-implemented on top of int-ish logic in the 
> backends.
> 
> The behaviour of cached brightness values is not optimal now, but this will 
> be addressed in the second part of the fixes (see description downwards).
> 
> 
> This patch is the first part of proposed brightness logic rework and makes 
> the basic required changes in all layers.
> 
> What's the problem?
> 
> Atm, the brightness logic is a bit messy: it doesn't work nice if the count 
> of underlying brightness steps is not equal to 10.
> Brightness steps when going upward do not correspond to brightness steps when 
> going downward, neither of them corresponds to brightness steps in the 
> plasmoid, and neither of them corresponds to the actual brightness steps.
> 
> The plasmoid steps are fixed at 10 (even if I have 3 actual keyboard steps).
> The upwards/downwards logic tries to add/subtract 10% (or 30%, depending on 
> the steps count) from the current value and just looks at what happens.
> 
> For example, let's suppose we have 15 brighthess steps (that happens 
> sometimes).
> When going upward from 0, that would be 8 steps: 0%, 13%, 27%, 40%, 53%, 67%, 
> 80%, 93%, 100%.
> When going downdard from 100%, that would also be 8 steps: 100%, 87%, 73%, 
> 60%, 47%, 33%, 20%, 7%, 0%.
> Notice how they don't correspond to the upward steps.
> In the plasmoid, that would be 10 steps: 0%, 13%, 20%, 33%, 40%, 53%, 60%, 
> 73%, 80%, 93%, 100%.
> Does it correspond to either of them?
> 
> 
> The proposed solution is to introduce fixed (you can think of them as 
> pre-calculated, but they are not) brightness steps in PowerDevil and stick to 
> those in upwards/downwards logic.
> The plasmoid could use those steps (slider from 0 to brightnessStepMax) or 
> the actual underlying brightness value (slider from 0 to brightnessValueMax) 
> — that would be in the third patch.
> 
> The second patch will introduce fixed brightness steps and export them as 
> D-Bus endpoints.
> In total, there would be three types of D-Bus endpoints that could be used by 
> apps (plasmoids etc.): brightness (percentage), brightnessValue (raw value), 
> brightnessStep (pre-calculated brightness steps).
> 
> 
> Diffs
> -----
> 
>   daemon/actions/bundled/brightnesscontrol.h 50e15d2 
>   daemon/actions/bundled/brightnesscontrol.cpp 787f256 
>   daemon/actions/bundled/keyboardbrightnesscontrol.h b63fa81 
>   daemon/actions/bundled/keyboardbrightnesscontrol.cpp 558a631 
>   
> daemon/actions/bundled/org.kde.Solid.PowerManagement.Actions.BrightnessControl.xml
>  9c4d3df 
>   
> daemon/actions/bundled/org.kde.Solid.PowerManagement.Actions.KeyboardBrightnessControl.xml
>  08b5407 
>   daemon/backends/hal/powerdevilhalbackend.h 294a6cf 
>   daemon/backends/hal/powerdevilhalbackend.cpp fc2d476 
>   daemon/backends/upower/backlight_helper_actions.actions 5cbf308 
>   daemon/backends/upower/backlighthelper.h 5ab9298 
>   daemon/backends/upower/backlighthelper.cpp 1c4d933 
>   daemon/backends/upower/powerdevilupowerbackend.h 46817bc 
>   daemon/backends/upower/powerdevilupowerbackend.cpp ea1741f 
>   daemon/backends/upower/xrandrbrightness.h e1f016c 
>   daemon/backends/upower/xrandrbrightness.cpp 27a6792 
>   daemon/powerdevilbackendinterface.h f7634d0 
>   daemon/powerdevilbackendinterface.cpp 40b0925 
> 
> Diff: https://git.reviewboard.kde.org/r/119559/diff/
> 
> 
> Testing
> -------
> 
> Works for me.
> Only the upower backend was tested.
> 
> 
> Thanks,
> 
> Nikita Skovoroda
> 
>

_______________________________________________
Kde-hardware-devel mailing list
Kde-hardware-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-hardware-devel

Reply via email to