<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/7017/">http://svn.reviewboard.kde.org/r/7017/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 9th, 2012, 10:50 p.m., <b>David Edmundson</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 can see why you've done this, but for me this doesn't really work.
However - what I think we can do is make a wrapper round KGameRenderer and turn it into an QML ImageProvider, which may (I've not thought everything through) be a much nicer solution.</pre>
</blockquote>
<p>On August 11th, 2012, 12:54 p.m., <b>Viranch Mehta</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;">That was my first approach, but the problem with it is that the image does not get updated when the theme is changed. A new pixmap is requested only when the "source" property changes, simply changing the theme does not involve changing the source, and hence the image cannot be updated. Also, there is no way to internally trigger an update so a new pixmap is requested when the theme is changed.
So this is the alternative approach I had to choose. What isn't working for you?</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;">Good point.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 9th, 2012, 10:50 p.m., <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://svn.reviewboard.kde.org/r/7017/diff/2/?file=48265#file48265line43" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdegames/libkdegames/declarativeimports/canvasitem.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">public:</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">43</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">static</span> <span class="kt">void</span> <span class="n">setRenderer</span><span class="p">(</span><span class="n">KGameRenderer</span><span class="o">*</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If you tried inserting two KgDeclarativeViews in an app, this now breaks heavily.
In the context of KBattleship (for example) that would be a perfectly sensible looking thing to do.
However, given a QML creatable item needs to have a default constructor, I can see why you've done this. </pre>
</blockquote>
<p>On August 11th, 2012, 12:54 p.m., <b>Viranch Mehta</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;">Well I could add a check before to decide whether to call this. This would be assuming, of course, that you want to use a common KGameRenderer for both the KgDeclarativeViews.
Is it possible that a single game uses two different KGameRenderers?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Then it's KGameRenderer that needs to be a singleton, not a static pointer in the classes using it.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 9th, 2012, 10:50 p.m., <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://svn.reviewboard.kde.org/r/7017/diff/2/?file=48268#file48268line28" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdegames/libkdegames/declarativeimports/corebindingsplugin.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">void CoreBindingsPlugin::initializeEngine(QDeclarativeEngine *engine, const char *uri)</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">28</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QObject</span> <span class="o">*</span><span class="n">property</span> <span class="o">=</span> <span class="n">engine</span><span class="o">-></span><span class="n">rootContext</span><span class="p">()</span><span class="o">-></span><span class="n">contextProperty</span><span class="p">(</span><span class="s">"renderer"</span><span class="p">).</span><span class="n">value</span><span class="o"><</span><span class="n">QObject</span><span class="o">*></span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You're making a proper QML plugin library (which is great!) but it's only usable when existing properties are already applied.
This defaults the point of it being a plugin/library.
You'd be better off running qmlRegisterType inside your KgDeclarativeView if you were doing it this way.</pre>
</blockquote>
<p>On August 11th, 2012, 12:54 p.m., <b>Viranch Mehta</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'm not sure if I understand the first point.
qmlRegisterType would only register a type with the declarative engine. I want to be able to access this particular instance of the KGameRenderer. Besides, the renderer object is not meant to be used from the QML, it is only for the c++ qml components in case they need it, so qmlRegisterType is not really required, though can be done.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'll try again.
The whole point of having a QML library is so that I can run
"import org.kde.games.core 0.1" and it will _just work_.
This isn't the case. It only works if you have already some certain rootContextProperties defined, at which point it isn't right for it to be a standalone plugin. Someone else who tries to use your plugin (without magically knowing this) will find it not working, or worse crashing. It works now for your code, but for it to be in a library, we need to make sure it's hard for people to "use incorrectly".
If the only way to use a CanvasItem is through a KgDeclarativeView. Then we should just called engine()->qmlRegisterType(CanvasItem...) in the constructor of KgDeclarativeView.
</pre>
<br />
<p>- David</p>
<br />
<p>On August 9th, 2012, 9:42 p.m., Viranch Mehta 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 Viranch Mehta.</div>
<p style="color: grey;"><i>Updated Aug. 9, 2012, 9:42 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;">This patch contains some new stuff for libkdegames mainly to support future porting of KDE games to QtQuick/QML. Post this patch, libkdeclarative becomes a dependency for libkdegames.
There are two new things:
1. KgDeclarativeView:
This is a QDeclarativeView with KDE-specific import paths for QML components configured and javascript functions bindings added (like i18n() methods) using the KDeclarative library. If the view is supplied with a KGameRenderer object, it is added to the underlying declarative engine so that it can be used by QML components built for use in QML ports.
2. CanvasItem QML component(the name of the component can be changed as per suggestions):
This is a QML component that simply loads specified sprite pixmap from the theme provided by the KGameRenderer. The component uses the KGameRenderer instance from the engine (as set by the KgDeclarativeView) and loads the theme-specific sprite pixmap. The sprite retrieval is asynchronous and is done by using KGameRendererClient, this has potential for performance improvements.
The documentation on how to use these is inline with the code.
The kgdeclarativeview{.h,.cpp} are in libkdegames and the QML component is in declarativeimports/ directory.</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 with my the ongoing port of KBreakout, works as expected.</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/CMakeLists.txt <span style="color: grey">(1309184)</span></li>
<li>trunk/KDE/kdegames/libkdegames/declarativeimports/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdegames/libkdegames/declarativeimports/canvasitem.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdegames/libkdegames/declarativeimports/canvasitem.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdegames/libkdegames/declarativeimports/corebindingsplugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdegames/libkdegames/declarativeimports/corebindingsplugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdegames/libkdegames/declarativeimports/qmldir <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdegames/libkdegames/includes/CMakeLists.txt <span style="color: grey">(1309184)</span></li>
<li>trunk/KDE/kdegames/libkdegames/includes/KgDeclarativeView <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdegames/libkdegames/kgdeclarativeview.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>trunk/KDE/kdegames/libkdegames/kgdeclarativeview.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/7017/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>