Review Request 116648: Split into one KCM for Desktop Effects and one for Compositing

[David Edmundson]

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



File Attachment: New effects KCM - kcmeffects.png
<https://git.reviewboard.kde.org//r/116648/#fcomment164>
    Padding 


File Attachment: New effects KCM - kcmeffects.png
<https://git.reviewboard.kde.org//r/116648/#fcomment165>
    These don't align on the right.
    /might/ just be the missing frame on the scrollview
    
    Also the old plugin views we're trying to emulate here in QML have a spacing between input and main view


File Attachment: New effects KCM - kcmeffects.png
<https://git.reviewboard.kde.org//r/116648/#fcomment166>
    Padding


File Attachment: New effects KCM - kcmeffects.png
<https://git.reviewboard.kde.org//r/116648/#fcomment168>
    Does this use the user's font setting?


File Attachment: New effects KCM - kcmeffects.png
<https://git.reviewboard.kde.org//r/116648/#fcomment169>
    Icon?


File Attachment: New advanced compositing KCM - kcmcompositing.png
<https://git.reviewboard.kde.org//r/116648/#fcomment170>
    Double margin?


main.cpp
<https://git.reviewboard.kde.org/r/116648/#comment37050>

    This doesn't seem to be used.



qml/CompositingView.qml
<https://git.reviewboard.kde.org/r/116648/#comment37049>

    This is really liable to break.
    If someone were to add an entry in the earlier in the model and everything will break without any warning at compile time.
    
    give we have a compositingTypeForIndex can we use that to turn on/off the other options?



qml/main-compositing.qml
<https://git.reviewboard.kde.org/r/116648/#comment37053>

    if this fills the parent, you can't really anchor a button to the bottom of it. 
    
    I strong recommend using a ColumnLayout.
    
    It will add the right spacing between the text and the button which I think this is missing and allow you to easily return correct minimumSizeHints to the KCM
    



qml/main-compositing.qml
<https://git.reviewboard.kde.org/r/116648/#comment37051>

    This seems rather arbitrary.
    
    For small widths potentially risks you painting off the edge of the screen.



qml/main-compositing.qml
<https://git.reviewboard.kde.org/r/116648/#comment37052>

    Compositing View<space>{


- David Edmundson


On March 7, 2014, 9:20 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116648/
> -----------------------------------------------------------
> 
> (Updated March 7, 2014, 9:20 a.m.)
> 
> 
> Review request for kwin, Plasma and Antonis Tsiapaliokas.
> 
> 
> Repository: kwin-compositing-kcm
> 
> 
> Description
> -------
> 
> Split into one KCM for Desktop Effects and one for Compositing
> 
> Let's try getting the KCM a little bit less scary by properly
> hiding everything the user doesn't have to care about. The prominent
> desktop effects KCM only contains the list of all the effects which
> can be configured and nothing else. Only exception is the disabled
> check after failed GL to make this easier for the user.
> 
> All the "advanced" settings are moved into a new KCM called
> "Compositing" which is put under the hardware component in
> systemsettings. This contains all advanced settings including
> * whether compositing is enabled at all
> * backend
> * animation speeed
> * scale filter
> * unredirect fullscreen
> * color correction
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 90d27a02f691d500662754658518d1e99cfbc022 
>   kcmkwineffects.desktop PRE-CREATION 
>   kwincompositing.desktop 88ecf46b393df947d6b59ce53c924a82fc65756a 
>   main.cpp 5c68722961be9b8a0a48f3221df7c033ba3cf946 
>   model.h c047677fed50606b194049734afeb87cbcc385cb 
>   model.cpp 2b25fe04ccacf0fb719956b1b5fa0267611747e6 
>   qml/CompositingView.qml PRE-CREATION 
>   qml/EffectView.qml 1c9d3a6d292c589bd4ab31d4c6071f2b4ff658c3 
>   qml/main-compositing.qml PRE-CREATION 
>   qml/main.qml d22dd08da529df938d99ff9d43f52f7b33a7f2d3 
> 
> Diff: https://git.reviewboard.kde.org/r/116648/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> New advanced compositing KCM
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/03/07/e20bc5a1-c873-497a-b005-efb6b58da7c5__kcmcompositing.png
> New effects KCM
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/03/07/0d6c955e-fdd2-4a84-91b4-53959f2745f5__kcmeffects.png
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140307/5629df72/attachment.html>