[Kde-games-devel] Review Request: Add a colorOfSprite method to KGameRenderer.

[Parker Coates]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/5062/
-----------------------------------------------------------

Review request for KDE Games.


Summary
-------

In both KPat and Killbots, text colours are stored in the SVG as a simple filled rectangle. This system came about during discussions with some of our artists who really didn't like the idea of having to manually type RGB triplets into .desktop files and preferred being able to preview colour combinations live in Inkscape.

In porting these games to KGameRenderer, I ran into several problems implementing this colour extracting code. Issues included:
 - not having external access to the/a QSvgRenderer
 - not having external access to the KImageCache caching the theme details
 - not having a reliable way to clear any process local cache immediately before or after a theme change.

Rather than fight for the inclusion of additional (protected?) API, I decided to just implement a colorOfSprite() method directly in KGameRenderer to see what that would look like. The code is almost an exact copy of the boundsOfElement() implementation, so it turned out to be chiefly an exercise in copy/paste/find/replace.

Taking the selfish view, including this method in KGR would make life easy for me, but I understand if there's objection from others. I'm mostly posting this to start a discussion about it. If we don't add this method to KGR, how can we change KGR to make it possible to implement it in a derived class.

PS: Ideally, QSvgRenderer would provide brushFromElement() and penFromElement() methods to directly extract these details from the SVG primitives without the hackish step of rendering anything.


Diffs
-----

  trunk/KDE/kdegames/libkdegames/kgamerenderer.h 1164843 
  trunk/KDE/kdegames/libkdegames/kgamerenderer.cpp 1164843 
  trunk/KDE/kdegames/libkdegames/kgamerenderer_p.h 1164843 

Diff: http://reviewboard.kde.org/r/5062/diff


Testing
-------

Tested in KPat and Killbots. Seems to work as expected.

I also double checked that the word is consistently spelt "color" and not "colour". My fingers type EN_US only begrudgingly. ;)


Thanks,

Parker

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-games-devel/attachments/20100820/33f4be37/attachment.htm