<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/4988/">http://svn.reviewboard.kde.org/r/4988/</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 15th, 2010, 10:44 a.m., <b>Torsten Rahn</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/4988/diff/2/?file=33892#file33892line334" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeedu/marble/src/lib/MarbleModel.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class MARBLE_EXPORT MarbleModel : public QObject</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">334</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="n">mapTexture</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;">First and foremost I'd like to keep the internal implementation concept of texturemapping as much as possible out of the model API.
Why do you need this call anyways? Does it really have to be public?
There is paintGlobe already. And you should consider just using that. And if you add functionality then please integrate it with existing code instead of doing copy/paste. Thanks :)</pre>
</blockquote>
<p>On August 15th, 2010, 12:02 p.m., <b>hjain</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;">setupTextureMapper() maps the projection. In absence of mapTexture(), the texture will be mapped on the projection only by setMapTheme(). Since, we need to change the texture without the theme, we need to connect setupTextureMapper() to timeSet() and dateChanged() signals. But since, setupTextureMapper() requires the projection parameter, I have created mapTexture() which gets the current projection by RTTI and then call setupTextureMapper().
This function don't need to be public. I have fixed it.
paintGlobe is called very frequently. Changing the texture of projection is not required such frequently. Thus, it is better to have a different slot. :)
I don't know how to keep it out of model API :(
Thanks :)
</pre>
</blockquote>
<p>On August 17th, 2010, 10:51 a.m., <b>Torsten Rahn</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;">No. setTextureMapper doesn't "map" the projection. And setMapTheme() also doesn't have the intention of mapping.
What setMapTheme does is it sets the map theme to a new map theme. While doing so it might initialize a new texturemapper (via setupTextureMapper). This means that the texturemapper is getting fully initialized (slow long loud process that wakes up every one, so you want to avoid calling it).
So what you are doing here is _completely_ initializing a new texture mapper object every time the time is set. This doesn't make sense, since you don't create a new texture mapper but just setup the one that was setup already. So why set it up again? :-) That's insanely expensive and obviously is not what you want to do. You just want to trigger that the texturemapper will update its output. Yes, your approach also updates the screen (because d->notifyModelChanged()/modelChanged() is called) after the long process of initialization. But that's just a side product.
>From what I can see you just want to emit the signal "modelChanged()" instead of calling mapTexture. That's how it would be done properly.
</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;">setTextureMapper only initialize a new texture mapper for current texture. Since, every textureLayer has different texture, there may have multiple texture in one theme. We need to change the texture mapper every time the time is set. Thus, setTextuerMapper is not completely initializing for all texture layers in theme. Imho, setTextureMapper should we called every time the time is set. However, instead of creating a new texture mapper every time, a qhash could be created in MarbleModel which will store texture and texture mapper. setTextureMapper will check if textureMapper is already created it will load it otherwise create a new one.
mapTexture() will not be required anymore.</pre>
<br />
<p>- hjain</p>
<br />
<p>On August 17th, 2010, 9:19 a.m., hjain wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.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 marble and Bastian Holst.</div>
<div>By hjain.</div>
<p style="color: grey;"><i>Updated 2010-08-17 09:19:43</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 merges Temperature(July) and Temperature(Dec) themes into Temperature theme by using dgml time tag. Similar, for Precipitation themes.
Kindly download the earth folder from http://uploading.com/files/5e9755bd/earth.tar.gz/ and replace it with marble/data/maps/earth folder.</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/kdeedu/marble/data/CMakeLists.txt <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/data/maps/earth/precip-dec/precip-dec-preview.png <span style="color: grey">(UNKNOWN)</span></li>
<li>/trunk/KDE/kdeedu/marble/data/maps/earth/precip-dec/precip-dec.dgml <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/data/maps/earth/precip-jul/precip-jul.jpg <span style="color: grey">(UNKNOWN)</span></li>
<li>/trunk/KDE/kdeedu/marble/data/maps/earth/precip/precip-preview.png <span style="color: grey">(UNKNOWN)</span></li>
<li>/trunk/KDE/kdeedu/marble/data/maps/earth/precip/precip.dgml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/data/maps/earth/temp-dec/temp-dec-preview.png <span style="color: grey">(UNKNOWN)</span></li>
<li>/trunk/KDE/kdeedu/marble/data/maps/earth/temp-dec/temp-dec.dgml <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/data/maps/earth/temp-jul/temp-jul.jpg <span style="color: grey">(UNKNOWN)</span></li>
<li>/trunk/KDE/kdeedu/marble/data/maps/earth/temp/temp-preview.png <span style="color: grey">(UNKNOWN)</span></li>
<li>/trunk/KDE/kdeedu/marble/data/maps/earth/temp/temp.dgml <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleClock.h <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleClock.cpp <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleModel.h <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/MarbleModel.cpp <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/StackedTileLoader.cpp <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/dgml/DgmlElementDictionary.h <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/dgml/DgmlElementDictionary.cpp <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/dgml/DgmlTimeTagHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/handlers/dgml/DgmlTimeTagHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneLayer.h <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneLayer.cpp <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneMap.h <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/geodata/scene/GeoSceneMap.cpp <span style="color: grey">(1162190)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/marble_part.rc <span style="color: grey">(1162190)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/4988/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>