[Kde-games-devel] KGameRenderer Porting Experience

[Stefan Majewsky]

On Wednesday 28 July 2010 20:23:37 Parker Coates wrote:
> Do you think it's a
good idea to add this class to the library to
> avoid it being recreated
multiple times? The name is a bit verbose,
> though. I was thinking that
maybe we could rename the current
> KGameRenderedItem to KGameRenderedObject
and rename this new class
> KGameRenderedItem, but I'm not sure whether that
makes the distinction
> between the classes clearer or more confusing.

ACK.
Please go ahead.

> While reading through the code, I noticed that KGR uses
a single
> QSvgRenderer instance from multiple threads without using a
mutex. QSR
> is reentrant, but not thread-safe so it needs to be protected
by a
> mutex or similar locking device. The current code seems to work
fine,
> but I doubt we want to trust that it will continue to.

Hm, that
explains the weird rendering errors I was getting in Kolf lately.

>
Killbots::Renderer (a KGR subclass) caches a few custom theme
> properties.
Obviously, these cached values need to be cleared when a
> new theme is
loaded, so I added code to do just that in setTheme().
> But setTheme()
isn't virtual, so if it is called with a KGameRenderer
> pointer, the
original implementation gets called and my cache isn't
> cleared. At first I
didn't think this was a big deal as I could always
> just use a
Killbots::Renderer pointer in Killbots code (there's only
> one instance
anyway), but KGR actually makes calls to setTheme
> internally under certain
situations such as when loading an invalid
> theme. In such cases, my cache
won't get cleared.

I'd rather insert a protected virtual function in KGR
which is empty by default and gets called when a theme has been changed,
like:

virtual void themeChanged(const QString& oldTheme, const QString&
newTheme) {}

> Games using KGR in a K_GLOBAL_STATIC (ex: KDiamond,
Killbots)
> currently generate a lot of "~QX11PixmapData(): QPixmap objects
must
> be destroyed before the QApplication object, otherwise the native
>
pixmap object will be leaked." warnings at shutdown. Do we care? 

No, we
don't. Look for the thread started by Ian some weeks ago.

> Excellent work,
Stefan.

Thx!

Greetings
Stefan