Review Request 123468: Add setting to adjust screen scaling

Sebastian Kügler sebas at kde.org
Wed Apr 22 22:25:47 UTC 2015


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

Ship it!


Some minor comments inline, otherwise looks good to me.


kcm/src/previewwidget.cpp (line 34)
<https://git.reviewboard.kde.org/r/123468/#comment54221>

    perhaps move this into the initializer list?



kcm/src/previewwidget.cpp (line 53)
<https://git.reviewboard.kde.org/r/123468/#comment54217>

    I think this part would benefit from a comment what you're doing here. I *can* follow it (I think), but for the lazy reader, a quick explanation would be useful.



kcm/src/scalingconfig.cpp (line 35)
<https://git.reviewboard.kde.org/r/123468/#comment54219>

    it should be enough to capture [&ui] here, since it's the only context we need.



kcm/src/scalingconfig.cpp (line 53)
<https://git.reviewboard.kde.org/r/123468/#comment54218>

    const



kcm/src/scalingconfig.cpp (line 95)
<https://git.reviewboard.kde.org/r/123468/#comment54216>

    Generally a good idea to explain these magic values.



kcm/src/widget.cpp (line 2)
<https://git.reviewboard.kde.org/r/123468/#comment54214>

    You may have screwed yo Dan's name here.



kcm/src/widget.cpp (line 122)
<https://git.reviewboard.kde.org/r/123468/#comment54220>

    capture could be reduced to [&this]


- Sebastian Kügler


On April 22, 2015, 3:44 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123468/
> -----------------------------------------------------------
> 
> (Updated April 22, 2015, 3:44 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: kscreen
> 
> 
> Description
> -------
> 
> Single dialog alters both QT_DEVICE_PIXEL_RAITO and XRDB.DPI text
> scaling factor that used to reside in fonts KCM.
> 
> A preview widget shows how this will look on the that monitor.
> 
> Changes take affect after logout/login; not ideal but we're limited by
> the Qt QPA for now. Will try and fix that after.
> 
> edit: having uploaded this I can see I have some whitespace left, please don't feel the need to tell me.
> 
> 
> Diffs
> -----
> 
>   kcm/src/previewwidget.h PRE-CREATION 
>   kcm/src/CMakeLists.txt 50bfdf037c331fe7c6763fb85c3b43857cbea5d5 
>   CMakeLists.txt e157a5e28e441a2e6dacb2d639cf6d120fb18c26 
>   kcm/src/previewwidget.cpp PRE-CREATION 
>   kcm/src/scaling.ui PRE-CREATION 
>   kcm/src/scalingconfig.h PRE-CREATION 
>   kcm/src/scalingconfig.cpp PRE-CREATION 
>   kcm/src/stylepreview.ui PRE-CREATION 
>   kcm/src/widget.cpp 7fe96c1c8b21b19424ef4993dff9eb3985bcefdb 
> 
> Diff: https://git.reviewboard.kde.org/r/123468/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> the dialog
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/04/22/a8cab37c-24bf-4fb9-b50b-39bc34596938__snapshot1.png
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150422/c479bdb9/attachment-0001.html>


More information about the Plasma-devel mailing list