<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://reviewboard.kde.org/r/5062/">http://reviewboard.kde.org/r/5062/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="/r/5062/diff/1/?file=34205#file34205line410" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdegames/libkdegames/kgamerenderer.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QColor KGameRenderer::colorOfSprite(const QString& key, int frame) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">410</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">d</span><span class="o">-></span><span class="n">m_rendererPool</span><span class="p">.</span><span class="n">hasAvailableRenderers</span><span class="p">()</span> <span class="o">&&</span> <span class="p">(</span><span class="n">d</span><span class="o">-></span><span class="n">m_strategies</span> <span class="o">&</span> <span class="n">KGameRenderer</span><span class="o">::</span><span class="n">UseDiskCache</span><span class="p">))</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Looking at this again, I think the "!d->m_rendererPool.hasAvailableRenderers() &&" part should be removed. Even if there is a renderer available, I'm guessing it'd still be faster to check the disk cache than to render anything. Thoughts?</pre>
</div>
<br />
<p>- Parker</p>
<br />
<p>On August 20th, 2010, 1:52 p.m., Parker Coates wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Games.</div>
<div>By Parker Coates.</div>
<p style="color: grey;"><i>Updated 2010-08-20 13:52:48</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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. ;)</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>trunk/KDE/kdegames/libkdegames/kgamerenderer.h <span style="color: grey">(1164843)</span></li>
<li>trunk/KDE/kdegames/libkdegames/kgamerenderer.cpp <span style="color: grey">(1164843)</span></li>
<li>trunk/KDE/kdegames/libkdegames/kgamerenderer_p.h <span style="color: grey">(1164843)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/5062/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>