[Marble-devel] Review Request: GeoData KML and GPX Parser: parse ExtendedData in gx:Track

Sat Dec 10 06:54:57 UTC 2011

> On Dec. 9, 2011, 11:04 p.m., Thibaut Gridel wrote:
> > Some breakage in GeoDataTrack as discussed:

thanks for your review :D

> On Dec. 9, 2011, 11:04 p.m., Thibaut Gridel wrote:
> > src/lib/geodata/data/GeoDataSimpleArrayData.h, line 24
> > <http://git.reviewboard.kde.org/r/103191/diff/5/?file=42950#file42950line24>
> >
> >     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.

Ok, GeoDataGeometry is wrong.
But what use instead? GeoDataObject?

> On Dec. 9, 2011, 11:04 p.m., Thibaut Gridel wrote:
> > src/lib/geodata/data/GeoDataTrack.cpp, line 131
> > <http://git.reviewboard.kde.org/r/103191/diff/5/?file=42953#file42953line131>
> >
> >     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?

Not so easy if m_when is not sorted. But I can try to change that.

> On Dec. 9, 2011, 11:04 p.m., Thibaut Gridel wrote:
> > src/lib/geodata/data/GeoDataTrack.cpp, line 184
> > <http://git.reviewboard.kde.org/r/103191/diff/5/?file=42953#file42953line184>
> >
> >     This implementation does not keep date sorted track, so breaks the display of for instance satellite.

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?

> On Dec. 9, 2011, 11:04 p.m., Thibaut Gridel wrote:
> > src/lib/geodata/data/GeoDataTrack.cpp, line 224
> > <http://git.reviewboard.kde.org/r/103191/diff/5/?file=42953#file42953line224>
> >
> >     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.

But that only works if m_when is sorted by time?

I will fix removing points without time.

- Niko

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103191/#review8827
-----------------------------------------------------------

On Dec. 9, 2011, 12:36 p.m., Niko Sams wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103191/
> -----------------------------------------------------------
> 
> (Updated Dec. 9, 2011, 12:36 p.m.)
> 
> 
> Review request for Marble.
> 
> 
> Description
> -------
> 
> 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)
> 
> 
> Diffs
> -----
> 
>   src/lib/geodata/data/GeoDataExtendedData.h f095cdb 
>   src/lib/geodata/data/GeoDataExtendedData.cpp 4011240 
>   src/lib/geodata/data/GeoDataExtendedData_p.h 361e894 
>   src/lib/geodata/data/GeoDataSimpleArrayData.h PRE-CREATION 
>   src/lib/geodata/data/GeoDataSimpleArrayData.cpp PRE-CREATION 
>   src/lib/geodata/data/GeoDataTrack.h 2f0e0c3 
>   src/lib/geodata/data/GeoDataTrack.cpp de2f975 
>   src/lib/geodata/data/Serializable.h cfa4b74 
>   src/lib/geodata/handlers/kml/KmlCoordinatesTagHandler.cpp dce7679 
>   src/lib/geodata/handlers/kml/KmlElementDictionary.h 624465a 
>   src/lib/geodata/handlers/kml/KmlElementDictionary.cpp 07f33fd 
>   src/lib/geodata/handlers/kml/KmlExtendedDataTagHandler.cpp 669fb5e 
>   src/lib/geodata/handlers/kml/KmlSchemaDataTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlSchemaDataTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlSimpleArrayDataTagHandler.h PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlSimpleArrayDataTagHandler.cpp PRE-CREATION 
>   src/lib/geodata/handlers/kml/KmlValueTagHandler.cpp a2e5441 
>   src/lib/geodata/parser/GeoDataTypes.h fee81bd 
>   src/lib/geodata/parser/GeoDataTypes.cpp 4443896 
>   src/plugins/runner/gpx/CMakeLists.txt dae7719 
>   src/plugins/runner/gpx/GpxParser.cpp 61a749f 
>   src/plugins/runner/gpx/handlers/GPXElementDictionary.h 37152bd 
>   src/plugins/runner/gpx/handlers/GPXElementDictionary.cpp 471ad64 
>   src/plugins/runner/gpx/handlers/GPXTrackPointExtensionTagHandler.h PRE-CREATION 
>   src/plugins/runner/gpx/handlers/GPXTrackPointExtensionTagHandler.cpp PRE-CREATION 
>   src/plugins/runner/gpx/handlers/GPXeleTagHandler.h PRE-CREATION 
>   src/plugins/runner/gpx/handlers/GPXeleTagHandler.cpp PRE-CREATION 
>   src/plugins/runner/gpx/handlers/GPXextensionsTagHandler.h PRE-CREATION 
>   src/plugins/runner/gpx/handlers/GPXextensionsTagHandler.cpp PRE-CREATION 
>   src/plugins/runner/gpx/handlers/GPXhrTagHandler.h PRE-CREATION 
>   src/plugins/runner/gpx/handlers/GPXhrTagHandler.cpp PRE-CREATION 
>   src/plugins/runner/gpx/handlers/GPXtimeTagHandler.h PRE-CREATION 
>   src/plugins/runner/gpx/handlers/GPXtimeTagHandler.cpp PRE-CREATION 
>   src/plugins/runner/gpx/handlers/GPXtrkptTagHandler.cpp 455e8e1 
>   src/plugins/runner/gpx/handlers/GPXtrksegTagHandler.cpp 9b1a24e 
>   src/plugins/runner/gpx/tests/TestTrack.cpp PRE-CREATION 
>   tests/TestGeoDataTrack.cpp c9f347d 
> 
> Diff: http://git.reviewboard.kde.org/r/103191/diff/diff
> 
> 
> Testing
> -------
> 
> Unittests included in patch.
> 
> 
> Thanks,
> 
> Niko Sams
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20111210/6c2e1ad5/attachment.html>