<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://svn.reviewboard.kde.org/r/6810/">http://svn.reviewboard.kde.org/r/6810/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 7th, 2011, 6:19 a.m., <b>Ian Wadham</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think this patch requires comments, in the code or CMakeLists.txt, to say what using (Qt) raster graphics will achieve, under what conditions the patch will become redundant and how the patch actually works. I do not think many people will understand the nuances of C++ involved in it if you are no longer around someday. IIUC it is not actually a static object, in which the keyword "static" is used throughout, but an "anonymous namespace" (whatever that implies, I am self-taught in C++).
Anyway, the effect is what matters most, so please ship it if you have not already done so. Thanks for such a good idea, Stefan!</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Agreed on comments. Will include some.
Re anonymous namespaces: Problem is that "static" actually does two different things in C++. (Plus I misused the term when I said "static object" where I actually meant to say "global object".) One use of "static" is to declare static class members (those shared by all instances). The other use (and the reason why "static" is also a keyword in C) is to suppress the function from being exported (that is, written into the object table). For example:
static int foo() { return 42; }
This is useful because "foo" does not pollute the application-global function namespace; it's only visible in the compilation unit. Now we have two different (and potentially interfering) usages of the "static" keyword. That's why C++ introduced the notion of anonymous namespaces: Any function or class placed in an anonymous namespace will be visible in the compilation unit like usual, but no globally visible symbols will be generated.
... Note to self: Stop using ReviewBoard like Stack Overflow.</pre>
<br />
<p>- Stefan</p>
<br />
<p>On October 3rd, 2011, 12:11 p.m., Stefan Majewsky wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.org/media/rb/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 Stefan Majewsky.</div>
<p style="color: grey;"><i>Updated Oct. 3, 2011, 12:11 p.m.</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;">Various games already choose the raster graphicssystem manually in their main(), because it has way better rendering performance than the older X11 graphicssystem. This change implements this choice of graphicssystem in libkdegames, as a static object. For Qt 4.7, this improves the rendering performance in all games immediately, sometimes the improvement is drastical. For example, in Kolf, when the ball moves over the Kolf banner on the intro screen, the animation is very flaky with the X11 graphicssystem, but fluent with the raster graphicssystem.
As far as I remember, Qt 4.8 will choose the raster graphicssystem by default, so this patch could be removed again when we require Qt >= 4.8. This patch is mainly interesting for users of Qt 4.7 (which is why I propose to also backport this patch to 4.7.3).</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;">Compiled Kolf against patched libkdegames and started it. Rendering performance indicates that correct graphicssystem is selected.
I have tested all of kdegames for visible regressions with "-graphicssystem raster". The only problem (in KBounce) is fixed in trunk by revision 1257154, and backported to 4.7 in revision 1257156.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=277590">277590</a>
</div>
<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/CMakeLists.txt <span style="color: grey">(1257072)</span></li>
<li>trunk/KDE/kdegames/libkdegames/chooserastergraphicssystem.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6810/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>