<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://git.reviewboard.kde.org/r/103191/">http://git.reviewboard.kde.org/r/103191/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hi Niko,
There's quite a lot of work here, I have not checked the code in detail.
It seems ok to me, however see my (few) comments below.
Javier</pre>
<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="http://git.reviewboard.kde.org/r/103191/diff/3/?file=42102#file42102line315" style="color: black; font-weight: bold; text-decoration: underline;">CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">315</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl">#</span>add_definitions("-DDEBUG_TAGS")</pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">315</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">add_definitions("-DDEBUG_TAGS")</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;">Shouldn't this debug feature by deactivated by default?</pre>
</div>
<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="http://git.reviewboard.kde.org/r/103191/diff/3/?file=42107#file42107line30" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/geodata/data/GeoDataSimpleArrayData.h</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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">30</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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;">Missing ~GeoDataSimpleArrayData() declaration and {delete d;} implementation?</pre>
</div>
<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="http://git.reviewboard.kde.org/r/103191/diff/3/?file=42110#file42110line229" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/geodata/data/GeoDataTrack.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">GeoDataCoordinates GeoDataTrack::coordinatesAt( const QDateTime &when ) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">183</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QMap</span><span class="o"><</span><span class="n">QDateTime</span><span class="p">,</span> <span class="n">GeoDataCoordinates</span><span class="o">>::</span><span class="n">iterator</span> <span class="n">end</span> <span class="o">=</span> <span class="n">d</span><span class="o">-></span><span class="n">m_pointMap</span><span class="p">.</span><span class="n">end</span><span class="p">();</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">218</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span> <span class="kt">int</span> <span class="n">i</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span> <span class="n">i</span> <span class="o"><</span> <span class="n">qMin</span><span class="p">(</span> <span class="n">d</span><span class="o">-></span><span class="n">m_when</span><span class="p">.</span><span class="n">size</span><span class="p">(),</span> <span class="n">d</span><span class="o">-></span><span class="n">m_coordinates</span><span class="p">.</span><span class="n">size</span><span class="p">()</span> <span class="p">);</span> <span class="o">++</span><span class="n">i</span> <span class="p">)</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;">Does this work right? shouldn't you be removing elements top-down, so that every element prior to when is removed?</pre>
</div>
<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="http://git.reviewboard.kde.org/r/103191/diff/3/?file=42110#file42110line241" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/geodata/data/GeoDataTrack.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">GeoDataCoordinates GeoDataTrack::coordinatesAt( const QDateTime &when ) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">193</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QMap</span><span class="o"><</span><span class="n">QDateTime</span><span class="p">,</span> <span class="n">GeoDataCoordinates</span><span class="o">>::</span><span class="n">iterator</span> <span class="n">end</span> <span class="o">=</span> <span class="n">d</span><span class="o">-></span><span class="n">m_pointMap</span><span class="p">.</span><span class="n">end</span><span class="p">();</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">230</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span> <span class="kt">int</span> <span class="n">i</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span> <span class="n">i</span> <span class="o"><</span> <span class="n">d</span><span class="o">-></span><span class="n">m_when</span><span class="p">.</span><span class="n">size</span><span class="p">();</span> <span class="o">++</span><span class="n">i</span> <span class="p">)</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;">idem as line 218.</pre>
</div>
<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="http://git.reviewboard.kde.org/r/103191/diff/3/?file=42137#file42137line61" style="color: black; font-weight: bold; text-decoration: underline;">src/plugins/runner/gpx/handlers/GPXtrkptTagHandler.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">GeoNode* GPXtrkptTagHandler::parse(GeoParser& parser) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">60</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">linestring</span></span><span class="o">-></span><span class="n">append</span><span class="p"><span class="hl">(</span></span><span class="n">coord</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">61</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">track</span></span><span class="o">-></span><span class="n">append<span class="hl">Coordinates</span></span><span class="p"><span class="hl">(</span></span><span class="hl"> </span><span class="n">coord</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;">Not sure of where (if) you get time information for these coordinates, otherwise you may want to append also the corresponding QDateTime() so that they do not get messed with points with a valid time coordinate that you might add later to the same track? Maybe use addPoint(QDateTime(), coord)? A convenience addPoint(coord) function which sets time to QDateTime() might also be interesting for this case?</pre>
</div>
<br />
<p>- Javier</p>
<br />
<p>On November 26th, 2011, 10:53 a.m., Niko Sams wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.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 Marble.</div>
<div>By Niko Sams.</div>
<p style="color: grey;"><i>Updated Nov. 26, 2011, 10:53 a.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;">gx:Track can contain ExtendedData used for embedding eg. heartrate data to a track:
http://code.google.com/intl/de-DE/apis/kml/documentation/kmlreference.html#gxtrack
This patch implements:
- the data structure in GeoDataTrack
- the kml parser for this structure (supporting example files in kml specs)
- the gpx parser for this structure (supporting files created by garmin devices)
Gpx parser also now handles elevation data.
Gpx parser now creates a GeoDataTrack object (instead of GeoDataLineString)
(I'm not sure if this change could cause problems)</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;">Unittests included in patch.</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>CMakeLists.txt <span style="color: grey">(3857121)</span></li>
<li>src/lib/.TracksModel.cpp.kate-swp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/geodata/data/GeoDataExtendedData.h <span style="color: grey">(f095cdb)</span></li>
<li>src/lib/geodata/data/GeoDataExtendedData.cpp <span style="color: grey">(4011240)</span></li>
<li>src/lib/geodata/data/GeoDataExtendedData_p.h <span style="color: grey">(361e894)</span></li>
<li>src/lib/geodata/data/GeoDataSimpleArrayData.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/geodata/data/GeoDataSimpleArrayData.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/geodata/data/GeoDataTrack.h <span style="color: grey">(2f0e0c3)</span></li>
<li>src/lib/geodata/data/GeoDataTrack.cpp <span style="color: grey">(c8b5ccf)</span></li>
<li>src/lib/geodata/data/Serializable.h <span style="color: grey">(cfa4b74)</span></li>
<li>src/lib/geodata/handlers/kml/KmlCoordinatesTagHandler.cpp <span style="color: grey">(dce7679)</span></li>
<li>src/lib/geodata/handlers/kml/KmlElementDictionary.h <span style="color: grey">(624465a)</span></li>
<li>src/lib/geodata/handlers/kml/KmlElementDictionary.cpp <span style="color: grey">(07f33fd)</span></li>
<li>src/lib/geodata/handlers/kml/KmlExtendedDataTagHandler.cpp <span style="color: grey">(669fb5e)</span></li>
<li>src/lib/geodata/handlers/kml/KmlSchemaDataTagHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/geodata/handlers/kml/KmlSchemaDataTagHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/geodata/handlers/kml/KmlSimpleArrayDataTagHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/geodata/handlers/kml/KmlSimpleArrayDataTagHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/lib/geodata/handlers/kml/KmlValueTagHandler.cpp <span style="color: grey">(a2e5441)</span></li>
<li>src/lib/geodata/parser/GeoDataTypes.h <span style="color: grey">(fee81bd)</span></li>
<li>src/lib/geodata/parser/GeoDataTypes.cpp <span style="color: grey">(4443896)</span></li>
<li>src/plugins/runner/gpx/CMakeLists.txt <span style="color: grey">(dae7719)</span></li>
<li>src/plugins/runner/gpx/GpxParser.cpp <span style="color: grey">(61a749f)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXElementDictionary.h <span style="color: grey">(37152bd)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXElementDictionary.cpp <span style="color: grey">(471ad64)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXTrackPointExtensionTagHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXTrackPointExtensionTagHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXeleTagHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXeleTagHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXextensionsTagHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXextensionsTagHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXhrTagHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXhrTagHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXtimeTagHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXtimeTagHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXtrkptTagHandler.cpp <span style="color: grey">(455e8e1)</span></li>
<li>src/plugins/runner/gpx/handlers/GPXtrksegTagHandler.cpp <span style="color: grey">(9b1a24e)</span></li>
<li>src/plugins/runner/gpx/tests/TestTrack.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/TestGeoDataTrack.cpp <span style="color: grey">(c9f347d)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103191/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>