> On Nov. 9, 2011, 6:48 a.m., Fredrik Höglund wrote:
> > kcontrol/input/xcursor/cursortheme.cpp, line 119
> > <http://git.reviewboard.kde.org/r/101701/diff/2/?file=33417#file33417line119>
> >
> >     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.
> >

The problem is that I can't drop createIcon(int size) because I need it on 
other places. So I would either have to copy the code to createIcon() or change 
the return type of createIcon to QImage. Maybe we can leave this for a later 
revision?


> On Nov. 9, 2011, 6:48 a.m., Fredrik Höglund wrote:
> > kcontrol/input/xcursor/cursortheme.h, line 80
> > <http://git.reviewboard.kde.org/r/101701/diff/2/?file=33416#file33416line80>
> >
> >     I'm having trouble working out if changing the default from -1 to 0 is 
> > an unrelated change or not.

The implementation now treats not only -1 as "default value" (=resolution 
dependend) but every value <= 0. This is important because 0 also means 
"resolutin dependend".

I think it would make no difference to leave the header "as is", but I changed 
the header because this makes clear that 0 is not a valid size.


- Lukas


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


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
> 
>

Reply via email to