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



daemon/backends/hal/powerdevilhalbackend.cpp
<https://git.reviewboard.kde.org/r/119559/#comment44365>

    Can you use value instead?



daemon/backends/upower/xrandrbrightness.cpp
<https://git.reviewboard.kde.org/r/119559/#comment44366>

    You should use xcb, and please move somewhere else the code that "fetches" 
the info and the code that has app logic.


- Àlex Fiestas


On ago. 1, 2014, 9:18 a.m., Nikita Skovoroda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119559/
> -----------------------------------------------------------
> 
> (Updated ago. 1, 2014, 9:18 a.m.)
> 
> 
> 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/powerdevilbackendinterface.h f7634d0 
>   daemon/powerdevilbackendinterface.cpp 40b0925 
>   daemon/backends/upower/xrandrbrightness.h e1f016c 
>   daemon/backends/upower/xrandrbrightness.cpp 27a6792 
>   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 
> 
> 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