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

Thibaut Gridel tgridel at free.fr
Sat Dec 10 10:04:51 UTC 2011 


> 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.
> 
> Niko Sams wrote:
>     Ok, GeoDataGeometry is wrong.
>     But what use instead? GeoDataObject?

Yup, might be it, following the likes of ExtendedData.


> 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?
> 
> Niko Sams wrote:
>     Not so easy if m_when is not sorted. But I can try to change that.

discussing sorting and m_when below, this one is simply to reduce the cost of doing interpolation.


> 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.
> 
> Niko Sams wrote:
>     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?

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.


> 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.
> 
> Niko Sams wrote:
>     But that only works if m_when is sorted by time?
>     
>     I will fix removing points without time.

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.


- Thibaut


-----------------------------------------------------------
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/3653b6ac/attachment-0001.html>

More information about the Marble-devel mailing list