<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 />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 9th, 2011, 11:04 p.m., <b>Thibaut Gridel</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://git.reviewboard.kde.org/r/103191/diff/5/?file=42950#file42950line24" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/geodata/data/GeoDataSimpleArrayData.h</a>
<span style="font-weight: normal;">
(Diff revision 5)
</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; ">class GeoDataSimpleArrayDataPrivate;</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">24</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">GEODATA_EXPORT</span> <span class="n">GeoDataSimpleArrayData</span> <span class="o">:</span> <span class="n">public</span> <span class="n">GeoDataGeometry</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;">Why would GeoDatasimpleArrayData be a geometry type? Is it a rendering object like the polyline of a placemark?
I don't find the kml spec matching this object concept, it seems more related to SimpleData, which is rather abstract data type, so neither a feature nor a geometry.</pre>
</blockquote>
<p>On December 10th, 2011, 6:54 a.m., <b>Niko Sams</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;">Ok, GeoDataGeometry is wrong.
But what use instead? GeoDataObject?</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;">Yup, might be it, following the likes of ExtendedData.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 9th, 2011, 11:04 p.m., <b>Thibaut Gridel</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://git.reviewboard.kde.org/r/103191/diff/5/?file=42953#file42953line131" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/geodata/data/GeoDataTrack.cpp</a>
<span style="font-weight: normal;">
(Diff revision 5)
</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; ">void GeoDataTrack::setInterpolate(bool on)</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">130</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="n">d</span><span class="o">-></span><span class="n">m_when</span><span class="p">.</span><span class="n">at</span><span class="p">(</span> <span class="n">i</span> <span class="p">).</span><span class="n">isValid</span><span class="p">()</span> <span class="p">)</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;">This code basically builds a full map of all valid m_when elements just to extract an upperBound and check one point after it... Could it not consist in a QList world of finding the 2 valid times around when and keep the extrapolating code?</pre>
</blockquote>
<p>On December 10th, 2011, 6:54 a.m., <b>Niko Sams</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;">Not so easy if m_when is not sorted. But I can try to change that.</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;">discussing sorting and m_when below, this one is simply to reduce the cost of doing interpolation.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 9th, 2011, 11:04 p.m., <b>Thibaut Gridel</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://git.reviewboard.kde.org/r/103191/diff/5/?file=42953#file42953line184" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/geodata/data/GeoDataTrack.cpp</a>
<span style="font-weight: normal;">
(Diff revision 5)
</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">154</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">d</span><span class="o">-></span><span class="n">m_<span class="hl">pointMap</span></span><span class="p"><span class="hl">.</span></span><span class="n"><span class="hl">insert</span></span><span class="p">(</span> <span class="n">when</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">coord</span></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">178</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">d</span><span class="o">-></span><span class="n">m_<span class="hl">when</span></span><span class="p"><span class="hl">.</span></span><span class="n"><span class="hl">append</span></span><span class="p">(</span> <span class="n">when</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;">This implementation does not keep date sorted track, so breaks the display of for instance satellite.</pre>
</blockquote>
<p>On December 10th, 2011, 6:54 a.m., <b>Niko Sams</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;">hm, I wasn't aware of that breakage. But I think the new behavior is correct - the points are used in the same order as in the input file. (this is especially needed for tracks without time information)
Where exactly can I see the breakage?</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;">Well, this method asks to add one point with a given timestamp (unlike the appendxx which barely follow a file parsing convention).
m_when and m_coordinates in a track are supposed if not to stay sorted (in the strict list or map sense), at least to represent an actual path followed by an object through time, otherwise it's not easy or possible to draw that path.
For the special case of appending points without timestamp, it is okay because file reading still follows the idea that you are building the representation of a path, simply missing some actual timestamp but keeping the "been from here to there through that intermediate point i don't remember the time".
You can see the breakage with satellite plugin, which tries to append dated points to its track. You click on a satellite and tick the display orbit.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 9th, 2011, 11:04 p.m., <b>Thibaut Gridel</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://git.reviewboard.kde.org/r/103191/diff/5/?file=42953#file42953line224" style="color: black; font-weight: bold; text-decoration: underline;">src/lib/geodata/data/GeoDataTrack.cpp</a>
<span style="font-weight: normal;">
(Diff revision 5)
</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">185</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">213</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="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This new implementation will have the effect of removing points without time iiuc.
Previous code had a while( d->m_when.at(i) < when) instead which sounds better.</pre>
</blockquote>
<p>On December 10th, 2011, 6:54 a.m., <b>Niko Sams</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;">But that only works if m_when is sorted by time?
I will fix removing points without time.</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 think points without time can stay (but are a bit costy to handle) provided the continuity of the track is kept as described above.
Some implementation like:
while ( !d->m_when.isEmpty() && d->m_when.first() < when ) {
d->m_when.takeFirst();
d->m_coordinates.takeFirst();
}
might be all we need.</pre>
<br />
<p>- Thibaut</p>
<br />
<p>On December 9th, 2011, 12:36 p.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 Dec. 9, 2011, 12:36 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;">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>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">(de2f975)</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>