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

[Stefan Majewsky]

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


The only technical difference between renderer->colorFromSprite(key) and renderer->spritePixmap(key, QSize(3, 3)).toImage().pixel(1, 1) is that the former one is a bit more performant because you save two pixmap conversions, but given the pixmap size and the number of times you'll need to call this method, I consider these negligible.

Therefore, I see no immediate reason to add this to KGameRenderer itself.

- Stefan


On 2010-08-20 13:52:48, Parker Coates wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/5062/
> -----------------------------------------------------------
> 
> (Updated 2010-08-20 13:52:48)
> 
> 
> 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/20100821/736e3c8e/attachment.htm