System tray rewrite ready for review

Fredrik Höglund fredrik at kde.org
Thu Aug 21 02:28:08 CEST 2008


Hi Jason,

I've looked at your patch, and here are some comments about the code.

The code in createBackgroundPixmap() that converts the QImage to an XImage
is not really sufficient, because there's no guarantee that the QImage format
matches the visual format just because the depth is the same. For example
the component order can be BGR instead of RGB if the X server is running on
a big-endian system. You have to look at the red, green and blue masks in
the XImage structure when you do the conversion.

Instead of using an image I would render the background to a QPixmap though,
and use XRenderComposite() to copy the pixmap contents to the pixmap you'll
be passing to XSetWindowBackgroundPixmap(). Xrender will do the format
conversion automatically in the X server if needed.

XGetWindowAttributes() involves a roundtrip to the X server, so I would only
call this function once when the window is embedded and save the data.
The visual can't change after the window has been created.

It also seems to me that if paintEvent() is called repeatedly at a fast enough
rate, the timer will just be reset over and over again, and no painting will
ever occur.

I'm also wondering if you could explain why you need to update the background
pixmap each time paintEvent() is called. Doesn't the background stay the same
except when the window is moved or resized?

Regards,
Fredrik



More information about the Plasma-devel mailing list