[Kde-games-devel] KGameRenderer Porting Experience

[Stefan Majewsky]

On Wednesday 28 July 2010 20:23:37 Parker Coates wrote:
> 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.

The
name "KGameRenderedObject" is problematic; it should include "Item"
somewhere to indicate that it's a QGraphicsItem.

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

Revision 1157564 introduces a renderer pool that
creates new QSvgRenderer instances as necessary to serve multiple threads.
At the very least, this introduces no regressions in the tested games
(KDiamond, Killbots, Kolf from my local work branch).

> Now I'm certainly
not saying that we should make everything virtual
> just in case someone
someday might need to customise it, but there are
> probably some methods
that are more likely to need custom code.
> Personally, I think
KGameRenderer::setTheme() and
> KGameRendererClient::setRenderSize() are
likely candidates, but there
> may be others.

What would be the usecase for
a virtualized KGRC::setRenderSize() method? The only case in which code in
libkdegames calls setRenderSize() is in
KGameRenderedItemPrivate::adjustRenderSize, and because that's a QObject, we
can add a renderSizeAdjusted() signal to it, should the need
arise.

Greetings
Stefan