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
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
