Review Request: Make mouse cursor size configurable

[Fredrik Höglund]

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

Ship it!


Sorry for not finding the time to follow up on this until now.

The new design is not quite what I had in mind, but given that the freeze is tomorrow I'm fine with including it in 4.8.

I have given the code a quick look over, and I don't see any show stoppers, although I have some nitpicks below.
You should also add yourself to the list of copyrights.



kcontrol/input/xcursor/cursortheme.h
<http://git.reviewboard.kde.org/r/101701/#comment6919>

    Recognization is not a word.



kcontrol/input/xcursor/cursortheme.h
<http://git.reviewboard.kde.org/r/101701/#comment6920>

    I'm having trouble working out if changing the default from -1 to 0 is an unrelated change or not.



kcontrol/input/xcursor/cursortheme.cpp
<http://git.reviewboard.kde.org/r/101701/#comment6921>

    Scaling a pixmap is more expensive than scaling an image. Internally it involves converting the pixmap back to an image, scaling it, and then converting the image back to a pixmap.
    
    This is not a major issue, but it's the reason the image was scaled before it was converted to a pixmap.
    



kcontrol/input/xcursor/themepage.cpp
<http://git.reviewboard.kde.org/r/101701/#comment6922>

    Stray semicolon.



kcontrol/input/xcursor/themepage.cpp
<http://git.reviewboard.kde.org/r/101701/#comment6923>

    Stray semicolon. You have them all over the place. The compiler doesn't just ignore these; they are explicit noops.



kcontrol/input/xcursor/themepage.cpp
<http://git.reviewboard.kde.org/r/101701/#comment6925>

    The indentation in this function is different from the rest of the file.



kcontrol/input/xcursor/themepage.cpp
<http://git.reviewboard.kde.org/r/101701/#comment6924>

    Whitespace at the end of the line.


- Fredrik Höglund


On Sept. 2, 2011, 4:40 p.m., Lukas Sommer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101701/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2011, 4:40 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, kdelibs, and Christoph Feck.
> 
> 
> Description
> -------
> 
> X11 mouse cursor themes can contain cursors in multiple sizes, making them pseudo-scalable.
> 
> It is yet possible in KDE to configure manually the mouse cursor size (editing kcminput.rc). However, the GUI of the corresponding KControl module didn't provide support to change this. This patch add support for changing the mouse cursor size to the GUI.
> 
> This are mostly GUI related changes. The underlying data structure XCursorTheme did yet provide support for choosing different sizes and only needed some adjustments.
> 
> 
> This addresses bug 90444.
>     http://bugs.kde.org/show_bug.cgi?id=90444
> 
> 
> Diffs
> -----
> 
>   kcontrol/input/xcursor/cursortheme.h 586ccba 
>   kcontrol/input/xcursor/cursortheme.cpp 92abea5 
>   kcontrol/input/xcursor/legacytheme.h 846bf9b 
>   kcontrol/input/xcursor/previewwidget.h f4d2c4e 
>   kcontrol/input/xcursor/previewwidget.cpp 3c264fc 
>   kcontrol/input/xcursor/themepage.h 38ca893 
>   kcontrol/input/xcursor/themepage.cpp 6c9f29a 
>   kcontrol/input/xcursor/themepage.ui 2e38054 
>   kcontrol/input/xcursor/xcursortheme.h b474086 
>   kcontrol/input/xcursor/xcursortheme.cpp 2ecb9ba 
> 
> Diff: http://git.reviewboard.kde.org/r/101701/diff/diff
> 
> 
> Testing
> -------
> 
> Tested locally. Works fine for me. Also when using non-standard font DPI values.
> 
> 
> Screenshots
> -----------
> 
> 
>   http://git.reviewboard.kde.org/r/101701/s/248/
> 
> 
> Thanks,
> 
> Lukas Sommer
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20111109/ce64b466/attachment.htm>