Review Request: Add fade effect to wallpaper plugin.

[Aaron J. Seigo]

On Thursday 26 February 2009, Matthew Dawson wrote:
> Unfortunately, reviewboard is not accepting this updated diff, so I'm
> posting it here.  I'm not sure why I thought it didn't work with single
> images, but it does now.  It also works in the configuration dialog box :).

comments on patch (besides "thanks! always great to receive patches from new 
faces"):

- coding style. {s for methods start on their own line (see updateFadedImage) 
and there is a space between the keyword and the opening ( in a conditional: 
"If (" not "if(" and ") {" not "){". 

- it should be using Plasma::Animator for its timer and a CustomTimer so this 
can be turned off by policy and share the global timer for this. using your 
own QTimeLine is, in general, a no-go.

- it seems that there should be a more performant way of doing this than 
creating a third pixmap that is the size of the end result, painting into it 
and then painting another into it! i'd suggest trying something like just 
painting both images into the exposed rect as passed into the paint method 
using the current alpha. if that produces acceptable results, i'd say go for 
it.

* remember to free the other pixmap when the animation is done so it isn't 
sitting around in memory. assigning a QPixmap() to it should be enough.

regardless of how its done (other than "in hardware"), doing full screen 
repaints isn't going to be precisely brilliant for performance, but as an 
option it's pretty nice :) which reminds me to check the default caching mode 
for Plasma::Containment with regards to performance (both memory usage as well 
as speed)

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Software

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090302/dd982c3d/attachment-0001.sig